diff --git a/runtime/vm/compiler/backend/il.h b/runtime/vm/compiler/backend/il.h index b4b8664d78c..f28d7419997 100644 --- a/runtime/vm/compiler/backend/il.h +++ b/runtime/vm/compiler/backend/il.h @@ -778,8 +778,7 @@ class Instruction : public ZoneAllocated { // the inlining ID to that; instead, treat it as unset. explicit Instruction(const InstructionSource& source, intptr_t deopt_id = DeoptId::kNone) - : deopt_id_(deopt_id), - inlining_id_(source.inlining_id) {} + : deopt_id_(deopt_id), inlining_id_(source.inlining_id) {} explicit Instruction(intptr_t deopt_id = DeoptId::kNone) : Instruction(InstructionSource(), deopt_id) {} @@ -1508,7 +1507,6 @@ class BlockEntryInstr : public Instruction { DEFINE_INSTRUCTION_TYPE_CHECK(BlockEntry) - protected: BlockEntryInstr(intptr_t block_id, intptr_t try_index, @@ -5301,7 +5299,6 @@ class DebugStepCheckInstr : public TemplateInstruction<0, NoThrow> { virtual bool HasUnknownSideEffects() const { return true; } virtual Instruction* Canonicalize(FlowGraph* flow_graph); - private: const TokenPosition token_pos_; const UntaggedPcDescriptors::Kind stub_kind_; @@ -8803,7 +8800,6 @@ class CheckNullInstr : public TemplateDefinition<1, Throws, Pure> { virtual Value* RedefinedValue() const; - PRINT_OPERANDS_TO_SUPPORT private: diff --git a/runtime/vm/compiler/frontend/base_flow_graph_builder.cc b/runtime/vm/compiler/frontend/base_flow_graph_builder.cc index e4666e27595..a34ae49794e 100644 --- a/runtime/vm/compiler/frontend/base_flow_graph_builder.cc +++ b/runtime/vm/compiler/frontend/base_flow_graph_builder.cc @@ -1054,11 +1054,14 @@ Fragment BaseFlowGraphBuilder::CheckNull(TokenPosition position, return instructions; } -Fragment BaseFlowGraphBuilder::CheckNullOptimized(TokenPosition position, - const String& function_name) { +Fragment BaseFlowGraphBuilder::CheckNullOptimized( + const String& function_name, + CheckNullInstr::ExceptionType exception_type, + TokenPosition position) { Value* value = Pop(); - CheckNullInstr* check_null = new (Z) CheckNullInstr( - value, function_name, GetNextDeoptId(), InstructionSource(position)); + CheckNullInstr* check_null = + new (Z) CheckNullInstr(value, function_name, GetNextDeoptId(), + InstructionSource(position), exception_type); Push(check_null); // Use the redefinition. return Fragment(check_null); } diff --git a/runtime/vm/compiler/frontend/base_flow_graph_builder.h b/runtime/vm/compiler/frontend/base_flow_graph_builder.h index 6a8b1b3cee1..93875e09200 100644 --- a/runtime/vm/compiler/frontend/base_flow_graph_builder.h +++ b/runtime/vm/compiler/frontend/base_flow_graph_builder.h @@ -384,13 +384,20 @@ class BaseFlowGraphBuilder { // Pops the top of the stack, checks it for null, and pushes the result on // the stack to create a data dependency. - // 'function_name' is a selector which is being called (reported in - // NoSuchMethod message). + // // Note that the result can currently only be used in optimized code, because // optimized code uses FlowGraph::RemoveRedefinitions to remove the // redefinitions, while unoptimized code does not. - Fragment CheckNullOptimized(TokenPosition position, - const String& function_name); + Fragment CheckNullOptimized( + const String& name, + CheckNullInstr::ExceptionType exception_type, + TokenPosition position = TokenPosition::kNoSource); + Fragment CheckNullOptimized( + const String& function_name, + TokenPosition position = TokenPosition::kNoSource) { + return CheckNullOptimized(function_name, CheckNullInstr::kNoSuchMethod, + position); + } // Records extra unchecked entry point 'unchecked_entry' in 'graph_entry'. void RecordUncheckedEntryPoint(GraphEntryInstr* graph_entry, diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc index 115caf4e1cc..ad2f1038316 100644 --- a/runtime/vm/compiler/frontend/kernel_to_il.cc +++ b/runtime/vm/compiler/frontend/kernel_to_il.cc @@ -1280,12 +1280,10 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod( body += LoadLocal(parsed_function_->RawParameterVariable(0)); // decoder body += LoadLocal(parsed_function_->RawParameterVariable(1)); // bytes body += LoadLocal(parsed_function_->RawParameterVariable(2)); // start - body += CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, function.name())); + body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); body += UnboxTruncate(kUnboxedIntPtr); body += LoadLocal(parsed_function_->RawParameterVariable(3)); // end - body += CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, function.name())); + body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); body += UnboxTruncate(kUnboxedIntPtr); body += LoadLocal(parsed_function_->RawParameterVariable(4)); // table body += Utf8Scan(); @@ -1327,13 +1325,11 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod( LocalVariable* arg_offset = parsed_function_->RawParameterVariable(1); body += LoadLocal(arg_offset); - body += CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, function.name())); + body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); LocalVariable* arg_offset_not_null = MakeTemporary(); body += LoadLocal(arg_pointer); - body += CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, function.name())); + body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); // No GC from here til LoadIndexed. body += LoadUntagged(compiler::target::PointerBase::data_field_offset()); body += LoadLocal(arg_offset_not_null); @@ -1430,8 +1426,7 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod( // Pointer> as argument, and (2) the bound on the pointer // type parameter guarantees X is an interface type. body += LoadLocal(arg_pointer); - body += CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, function.name())); + body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); body += LoadNativeField( Slot::GetTypeArgumentsSlotFor(thread_, pointer_class)); body += NullConstant(); // function_type_args. @@ -1441,17 +1436,14 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod( ASSERT_EQUAL(function.NumParameters(), 3); body += LoadLocal(arg_offset); - body += CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, function.name())); + body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); LocalVariable* arg_offset_not_null = MakeTemporary(); body += LoadLocal(arg_value); - body += CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, function.name())); + body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); LocalVariable* arg_value_not_null = MakeTemporary(); body += LoadLocal(arg_pointer); // Pointer. - body += CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, function.name())); + body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); // No GC from here til StoreIndexed. body += LoadUntagged(compiler::target::PointerBase::data_field_offset()); body += LoadLocal(arg_offset_not_null); @@ -1489,16 +1481,14 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod( body += AllocateObject(TokenPosition::kNoSource, pointer_class, 1); body += LoadLocal(MakeTemporary()); // Duplicate Pointer. body += LoadLocal(parsed_function_->RawParameterVariable(0)); // Address. - body += CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, function.name())); + body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); body += UnboxTruncate(kUnboxedFfiIntPtr); body += StoreNativeField(Slot::Pointer_data_field()); } break; case MethodRecognizer::kFfiGetAddress: { ASSERT_EQUAL(function.NumParameters(), 1); body += LoadLocal(parsed_function_->RawParameterVariable(0)); // Pointer. - body += CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, function.name())); + body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); // This can only be Pointer, so it is always safe to LoadUntagged. body += LoadUntagged(compiler::target::Pointer::data_field_offset()); body += ConvertUntaggedToUnboxed(kUnboxedFfiIntPtr); @@ -1517,9 +1507,9 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod( // Load TypedDataArray from Instance Handle implementing // NativeFieldWrapper. body += LoadLocal(parsed_function_->RawParameterVariable(0)); // Object. - body += CheckNullOptimized(TokenPosition::kNoSource, name); + body += CheckNullOptimized(name); body += LoadNativeField(Slot::Instance_native_fields_array()); // Fields. - body += CheckNullOptimized(TokenPosition::kNoSource, name); + body += CheckNullOptimized(name); // Load the native field at index. body += IntConstant(0); // Index. body += LoadIndexed(kIntPtrCid); @@ -4362,10 +4352,10 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfFfiNative(const Function& function) { function_body += LoadLocal( parsed_function_->ParameterVariable(kFirstArgumentParameterOffset + i)); // Check for 'null'. - // TODO(36780): Mention the param name instead of function reciever. - function_body += - CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, marshaller.function_name())); + function_body += CheckNullOptimized( + String::ZoneHandle( + Z, function.ParameterNameAt(kFirstArgumentParameterOffset + i)), + CheckNullInstr::kArgumentError); function_body += StoreLocal( TokenPosition::kNoSource, parsed_function_->ParameterVariable(kFirstArgumentParameterOffset + i)); @@ -4544,8 +4534,8 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfFfiCallback(const Function& function) { body += IntConstant(0); } else if (!marshaller.IsHandle(compiler::ffi::kResultIndex)) { body += - CheckNullOptimized(TokenPosition::kNoSource, - String::ZoneHandle(Z, marshaller.function_name())); + CheckNullOptimized(String::ZoneHandle(Z, String::New("return_value")), + CheckNullInstr::kArgumentError); } if (marshaller.IsCompound(compiler::ffi::kResultIndex)) { diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index fb90a6e5b6c..5ef43b9c446 100644 --- a/runtime/vm/runtime_entry.cc +++ b/runtime/vm/runtime_entry.cc @@ -144,7 +144,19 @@ DEFINE_RUNTIME_ENTRY(RangeError, 2) { Exceptions::ThrowByType(Exceptions::kRange, args); } -static void NullErrorHelper(Zone* zone, const String& selector) { +static void NullErrorHelper(Zone* zone, + const String& selector, + bool is_param_name = false) { + if (is_param_name) { + const String& error = String::Handle( + selector.IsNull() + ? String::New("argument value is null") + : String::NewFormatted("argument value for '%s' is null", + selector.ToCString())); + Exceptions::ThrowArgumentError(error); + return; + } + // If the selector is null, this must be a null check that wasn't due to a // method invocation, so was due to the null check operator. if (selector.IsNull()) { @@ -178,7 +190,10 @@ static void NullErrorHelper(Zone* zone, const String& selector) { Exceptions::ThrowByType(Exceptions::kNoSuchMethod, args); } -static void DoThrowNullError(Isolate* isolate, Thread* thread, Zone* zone) { +static void DoThrowNullError(Isolate* isolate, + Thread* thread, + Zone* zone, + bool is_param) { DartFrameIterator iterator(thread, StackFrameIterator::kNoCrossThreadIteration); const StackFrame* caller_frame = iterator.NextFrame(); @@ -205,11 +220,11 @@ static void DoThrowNullError(Isolate* isolate, Thread* thread, Zone* zone) { member_name = Symbols::OptimizedOut().ptr(); } - NullErrorHelper(zone, member_name); + NullErrorHelper(zone, member_name, is_param); } DEFINE_RUNTIME_ENTRY(NullError, 0) { - DoThrowNullError(isolate, thread, zone); + DoThrowNullError(isolate, thread, zone, /*is_param=*/false); } // Collects information about pointers within the top |kMaxSlotsCollected| @@ -258,7 +273,7 @@ DEFINE_RUNTIME_ENTRY(DispatchTableNullError, 1) { RELEASE_ASSERT(caller_frame->IsDartFrame()); ReportImpossibleNullError(cid.Value(), caller_frame, thread); } - DoThrowNullError(isolate, thread, zone); + DoThrowNullError(isolate, thread, zone, /*is_param=*/false); } DEFINE_RUNTIME_ENTRY(NullErrorWithSelector, 1) { @@ -271,8 +286,7 @@ DEFINE_RUNTIME_ENTRY(NullCastError, 0) { } DEFINE_RUNTIME_ENTRY(ArgumentNullError, 0) { - const String& error = String::Handle(String::New("argument value is null")); - Exceptions::ThrowArgumentError(error); + DoThrowNullError(isolate, thread, zone, /*is_param=*/true); } DEFINE_RUNTIME_ENTRY(ArgumentError, 1) { diff --git a/tests/ffi/function_test.dart b/tests/ffi/function_test.dart index d5fd3c0f1c5..6d4c77e9c71 100644 --- a/tests/ffi/function_test.dart +++ b/tests/ffi/function_test.dart @@ -39,6 +39,7 @@ void main() { testFloatRounding(); testVoidReturn(); testNoArgs(); + testNativeFunctionNullableInt(); } } @@ -457,3 +458,16 @@ void testNoArgs() { double result = inventFloatValue(); Expect.approxEquals(1337.0, result); } + +void testNativeFunctionNullableInt() { + final sumPlus42 = ffiTestFunctions.lookupFunction< + Int32 Function(Int32, Int32), int Function(int, int?)>("SumPlus42"); + + try { + sumPlus42(3, null); + } catch (e) { + // TODO(http://dartbug.com/47098): Save param names to dwarf. + Expect.isTrue(e.toString().contains('ffi_param2') || + e.toString().contains('')); + } +} diff --git a/tests/ffi_2/function_test.dart b/tests/ffi_2/function_test.dart index f955bbdfa0e..c0f9657159c 100644 --- a/tests/ffi_2/function_test.dart +++ b/tests/ffi_2/function_test.dart @@ -41,6 +41,7 @@ void main() { testFloatRounding(); testVoidReturn(); testNoArgs(); + testNativeFunctionNullableInt(); } } @@ -459,3 +460,16 @@ void testNoArgs() { double result = inventFloatValue(); Expect.approxEquals(1337.0, result); } + +void testNativeFunctionNullableInt() { + final sumPlus42 = ffiTestFunctions.lookupFunction< + Int32 Function(Int32, Int32), int Function(int, int)>("SumPlus42"); + + try { + sumPlus42(3, null); + } catch (e) { + // TODO(http://dartbug.com/47098): Save param names to dwarf. + Expect.isTrue(e.toString().contains('ffi_param2') || + e.toString().contains('')); + } +}