Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vm/ffi] Null exceptions on arguments are reported as null receiver in FFI calls #47094

Closed
dcharkes opened this issue Sep 3, 2021 · 2 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Sep 3, 2021

Null-checks on arguments are reported as null-checks on the receiver in trampolines. (The slow path reporting the error assumes it is checking the receiver for null.)

Originally posted by @dcharkes in #36780 (comment)

If there is a selector, it will be reported as a NoSuchMethodError:

static void NullErrorHelper(Zone* zone, const String& selector) {
// 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()) {
const Array& args = Array::Handle(zone, Array::New(4));
args.SetAt(
3, String::Handle(
zone, String::New("Null check operator used on a null value")));
Exceptions::ThrowByType(Exceptions::kCast, args);
return;
}
InvocationMirror::Kind kind = InvocationMirror::kMethod;
if (Field::IsGetterName(selector)) {
kind = InvocationMirror::kGetter;
} else if (Field::IsSetterName(selector)) {
kind = InvocationMirror::kSetter;
}
const Smi& invocation_type = Smi::Handle(
zone,
Smi::New(InvocationMirror::EncodeType(InvocationMirror::kDynamic, kind)));
const Array& args = Array::Handle(zone, Array::New(7));
args.SetAt(0, /* instance */ Object::null_object());
args.SetAt(1, selector);
args.SetAt(2, invocation_type);
args.SetAt(3, /* func_type_args_length */ Object::smi_zero());
args.SetAt(4, /* func_type_args */ Object::null_object());
args.SetAt(5, /* func_args */ Object::null_object());
args.SetAt(6, /* func_arg_names */ Object::null_object());
Exceptions::ThrowByType(Exceptions::kNoSuchMethod, args);
}

We need to encode in the code source map that this is a param name rather than a method name.

@dcharkes
Copy link
Contributor Author

dcharkes commented Sep 3, 2021

Note we generate argument names, even if the signature would contain names:

    final sumPlus42 = ffiTestFunctions.lookupFunction<
        Int32 Function(Int32, Int32), int Function(int? a, int b)>("SumPlus42");

// Create unique names for the parameters, as they are used in scope building
// and error messages.
if (num_fixed > 0) {
function.CreateNameArray();
function.SetParameterNameAt(0, Symbols::ClosureParameter());
for (intptr_t i = 1; i < num_fixed; i++) {
name = Symbols::NewFormatted(thread, ":ffi_param%" Pd, i);
function.SetParameterNameAt(i, name);
}
}

The names a and b are ignored. We should check if we can do something about that as well.

@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi labels Sep 3, 2021
@dcharkes
Copy link
Contributor Author

dcharkes commented Sep 3, 2021

What Error type should it be?

  1. When doing dynamic invocations on closures it's a TypeError: type 'Null' is not a subtype of type 'int' of 'b'.
  2. When doing a ! it's a CastError: Null check operator used on a null value (The location inside the expression is indicated with the stack trace.). CastError is deprecated, use TypeError instead.
  3. We have an ArgumentError: Invalid argument(s): argument value is null. (Does not report the name of the argument currently, and does also not report the source location of the parameter, so that leaves the user in the dark about which param is null.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-ffi
Projects
None yet
Development

No branches or pull requests

1 participant