Skip to content

Commit f592a11

Browse files
dcharkesCommit Bot
dcharkes
authored and
Commit Bot
committed
[vm/ffi] NativeFieldWrapper FfiNative check receivers for nullptr
Changes `FfiNative` instance methods on `NativeFieldWrapperClass`es that have `Pointer<Void>` conversion to check for `nullptr`. If the native field has `0`/`nullptr` a `StateError` is thrown: "Bad state: Native field is nullptr." This only makes sense if the first native field is used to point to the C++ object corresponding to the Dart object. As far as we know all current use cases do so. TEST=pkg/vm/testcases/transformations/ffi/ffinative.dart.expect TEST=tests/ffi/vmspecific_ffi_native_test.dart Closes: #49620 Change-Id: I92f760c33d391476010722358f9713fa4491ab61 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254200 Reviewed-by: Martin Kustermann <kustermann@google.com> Commit-Queue: Daco Harkes <dacoharkes@google.com>
1 parent 5c59f08 commit f592a11

File tree

5 files changed

+137
-31
lines changed

5 files changed

+137
-31
lines changed

‎pkg/vm/lib/transformations/ffi/common.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ class FfiTransformer extends Transformer {
163163
final Procedure listElementAt;
164164
final Procedure numAddition;
165165
final Procedure numMultiplication;
166+
final Procedure objectEquals;
167+
final Procedure stateErrorThrowNewFunction;
166168

167169
final Library ffiLibrary;
168170
final Class allocatorClass;
@@ -304,6 +306,10 @@ class FfiTransformer extends Transformer {
304306
numAddition = coreTypes.index.getProcedure('dart:core', 'num', '+'),
305307
numMultiplication =
306308
coreTypes.index.getProcedure('dart:core', 'num', '*'),
309+
objectEquals =
310+
coreTypes.index.getProcedure('dart:core', 'Object', '=='),
311+
stateErrorThrowNewFunction = coreTypes.index
312+
.getProcedure('dart:core', 'StateError', '_throwNew'),
307313
ffiLibrary = index.getLibrary('dart:ffi'),
308314
allocatorClass = index.getClass('dart:ffi', 'Allocator'),
309315
nativeFunctionClass = index.getClass('dart:ffi', 'NativeFunction'),

‎pkg/vm/lib/transformations/ffi/native.dart

Lines changed: 85 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -230,18 +230,46 @@ class FfiNativeTransformer extends FfiTransformer {
230230
initializer: initializer, type: wrappedType, isFinal: true);
231231
}
232232

233-
Expression _getTemporary(VariableDeclaration temporary,
234-
DartType dartParameterType, DartType ffiParameterType) {
233+
Expression _getTemporary(
234+
VariableDeclaration temporary,
235+
DartType dartParameterType,
236+
DartType ffiParameterType, {
237+
required bool checkForNullptr,
238+
}) {
235239
if (_requiresPointerConversion(dartParameterType, ffiParameterType)) {
236-
// Pointer.fromAddress(_getNativeField(#t0))
237-
return StaticInvocation(
238-
fromAddressInternal,
239-
Arguments(<Expression>[
240-
StaticInvocation(getNativeFieldFunction,
241-
Arguments(<Expression>[VariableGet(temporary)]))
242-
], types: <DartType>[
243-
voidType
244-
]));
240+
Expression pointerAddress = StaticInvocation(getNativeFieldFunction,
241+
Arguments(<Expression>[VariableGet(temporary)]));
242+
243+
if (checkForNullptr) {
244+
final pointerAddressVar = VariableDeclaration("#pointerAddress",
245+
initializer: pointerAddress, type: coreTypes.intNonNullableRawType);
246+
pointerAddress = BlockExpression(
247+
Block([
248+
pointerAddressVar,
249+
IfStatement(
250+
InstanceInvocation(
251+
InstanceAccessKind.Instance,
252+
VariableGet(pointerAddressVar),
253+
objectEquals.name,
254+
Arguments([ConstantExpression(IntConstant(0))]),
255+
interfaceTarget: objectEquals,
256+
functionType: objectEquals.getterType as FunctionType,
257+
),
258+
ExpressionStatement(StaticInvocation(
259+
stateErrorThrowNewFunction,
260+
Arguments([
261+
ConstantExpression(StringConstant('Native field is nullptr.'))
262+
]),
263+
)),
264+
EmptyStatement(),
265+
)
266+
]),
267+
VariableGet(pointerAddressVar),
268+
);
269+
}
270+
271+
return StaticInvocation(fromAddressInternal,
272+
Arguments(<Expression>[pointerAddress], types: <DartType>[voidType]));
245273
}
246274
return VariableGet(temporary);
247275
}
@@ -262,8 +290,12 @@ class FfiNativeTransformer extends FfiTransformer {
262290
// final #t1 = passAsPointer(Pointer.fromAddress(_getNativeField(#t0)));
263291
// reachabilityFence(#t0);
264292
// } => #t1
265-
Expression _wrapArgumentsAndReturn(FunctionInvocation invocation,
266-
FunctionType dartFunctionType, FunctionType ffiFunctionType) {
293+
Expression _wrapArgumentsAndReturn({
294+
required FunctionInvocation invocation,
295+
required FunctionType dartFunctionType,
296+
required FunctionType ffiFunctionType,
297+
bool checkReceiverForNullptr = false,
298+
}) {
267299
List<DartType> ffiParameters = ffiFunctionType.positionalParameters;
268300
List<DartType> dartParameters = dartFunctionType.positionalParameters;
269301
// Create lists of temporary variables for arguments potentially being
@@ -277,8 +309,12 @@ class FfiNativeTransformer extends FfiTransformer {
277309
// Note: We also evaluate, and assign temporaries for, non-wrapped
278310
// arguments as we need to preserve the original evaluation order.
279311
temporariesForArguments.add(temporary);
280-
callArguments
281-
.add(_getTemporary(temporary, dartParameters[i], ffiParameters[i]));
312+
callArguments.add(_getTemporary(
313+
temporary,
314+
dartParameters[i],
315+
ffiParameters[i],
316+
checkForNullptr: checkReceiverForNullptr && i == 0,
317+
));
282318
if (_requiresPointerConversion(dartParameters[i], ffiParameters[i])) {
283319
fencedArguments.add(temporary);
284320
continue;
@@ -392,13 +428,15 @@ class FfiNativeTransformer extends FfiTransformer {
392428
}
393429

394430
Procedure _transformProcedure(
395-
Procedure node,
396-
StringConstant nativeFunctionName,
397-
bool isLeaf,
398-
int annotationOffset,
399-
FunctionType dartFunctionType,
400-
FunctionType ffiFunctionType,
401-
List<Expression> argumentList) {
431+
Procedure node,
432+
StringConstant nativeFunctionName,
433+
bool isLeaf,
434+
int annotationOffset,
435+
FunctionType dartFunctionType,
436+
FunctionType ffiFunctionType,
437+
List<Expression> argumentList, {
438+
required bool checkReceiverForNullptr,
439+
}) {
402440
if (!_verifySignatures(
403441
node, dartFunctionType, ffiFunctionType, annotationOffset)) {
404442
return node;
@@ -462,7 +500,11 @@ class FfiNativeTransformer extends FfiTransformer {
462500
Expression result = (wrappedDartFunctionType == dartFunctionType
463501
? functionPointerInvocation
464502
: _wrapArgumentsAndReturn(
465-
functionPointerInvocation, dartFunctionType, ffiFunctionType));
503+
invocation: functionPointerInvocation,
504+
dartFunctionType: dartFunctionType,
505+
ffiFunctionType: ffiFunctionType,
506+
checkReceiverForNullptr: checkReceiverForNullptr,
507+
));
466508

467509
// => _myFunction$FfiNative$Ptr(
468510
// Pointer<Void>.fromAddress(_getNativeField(obj)), x)
@@ -509,8 +551,16 @@ class FfiNativeTransformer extends FfiTransformer {
509551
VariableGet(parameter)
510552
];
511553

512-
return _transformProcedure(node, nativeFunctionName, isLeaf,
513-
annotationOffset, dartFunctionType, ffiFunctionType, argumentList);
554+
return _transformProcedure(
555+
node,
556+
nativeFunctionName,
557+
isLeaf,
558+
annotationOffset,
559+
dartFunctionType,
560+
ffiFunctionType,
561+
argumentList,
562+
checkReceiverForNullptr: true,
563+
);
514564
}
515565

516566
// Transform FfiNative static functions.
@@ -536,8 +586,16 @@ class FfiNativeTransformer extends FfiTransformer {
536586
VariableGet(parameter)
537587
];
538588

539-
return _transformProcedure(node, nativeFunctionName, isLeaf,
540-
annotationOffset, dartFunctionType, ffiFunctionType, argumentList);
589+
return _transformProcedure(
590+
node,
591+
nativeFunctionName,
592+
isLeaf,
593+
annotationOffset,
594+
dartFunctionType,
595+
ffiFunctionType,
596+
argumentList,
597+
checkReceiverForNullptr: false,
598+
);
541599
}
542600

543601
@override

‎pkg/vm/testcases/transformations/ffi/ffinative.dart.expect

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,13 @@ class NativeClassy extends nat::NativeFieldWrapperClass1 {
3131
return block {
3232
final nat::NativeFieldWrapperClass1 #t1 = this;
3333
final core::int #t2 = v;
34-
final void #t3 = self::NativeClassy::_goodHasReceiverPointer$FfiNative$Ptr(ffi::_fromAddress<ffi::Void>(nat::_getNativeField(#t1)), #t2){(ffi::Pointer<ffi::Void>, core::int) → void};
34+
final void #t3 = self::NativeClassy::_goodHasReceiverPointer$FfiNative$Ptr(ffi::_fromAddress<ffi::Void>( block {
35+
core::int #pointerAddress = nat::_getNativeField(#t1);
36+
if(#pointerAddress.{core::Object::==}(#C6){(core::Object) → core::bool})
37+
core::StateError::_throwNew(#C7);
38+
else
39+
;
40+
} =>#pointerAddress), #t2){(ffi::Pointer<ffi::Void>, core::int) → void};
3541
_in::reachabilityFence(#t1);
3642
} =>#t3;
3743
method goodHasReceiverHandle(core::int v) → void
@@ -49,20 +55,38 @@ class NativeClassy extends nat::NativeFieldWrapperClass1 {
4955
return block {
5056
final nat::NativeFieldWrapperClass1 #t7 = this;
5157
final self::NativeClassy #t8 = v;
52-
final void #t9 = self::NativeClassy::_goodHasReceiverPtrAndHandle$FfiNative$Ptr(ffi::_fromAddress<ffi::Void>(nat::_getNativeField(#t7)), #t8){(ffi::Pointer<ffi::Void>, self::NativeClassy) → void};
58+
final void #t9 = self::NativeClassy::_goodHasReceiverPtrAndHandle$FfiNative$Ptr(ffi::_fromAddress<ffi::Void>( block {
59+
core::int #pointerAddress = nat::_getNativeField(#t7);
60+
if(#pointerAddress.{core::Object::==}(#C6){(core::Object) → core::bool})
61+
core::StateError::_throwNew(#C7);
62+
else
63+
;
64+
} =>#pointerAddress), #t8){(ffi::Pointer<ffi::Void>, self::NativeClassy) → void};
5365
_in::reachabilityFence(#t7);
5466
} =>#t9;
5567
method meh(core::bool blah) → core::String?
5668
return block {
5769
final nat::NativeFieldWrapperClass1 #t10 = this;
5870
final core::bool #t11 = blah;
59-
final core::String? #t12 = _in::unsafeCast<core::String?>(self::NativeClassy::_meh$FfiNative$Ptr(ffi::_fromAddress<ffi::Void>(nat::_getNativeField(#t10)), #t11){(ffi::Pointer<ffi::Void>, core::bool) → core::Object?});
71+
final core::String? #t12 = _in::unsafeCast<core::String?>(self::NativeClassy::_meh$FfiNative$Ptr(ffi::_fromAddress<ffi::Void>( block {
72+
core::int #pointerAddress = nat::_getNativeField(#t10);
73+
if(#pointerAddress.{core::Object::==}(#C6){(core::Object) → core::bool})
74+
core::StateError::_throwNew(#C7);
75+
else
76+
;
77+
} =>#pointerAddress), #t11){(ffi::Pointer<ffi::Void>, core::bool) → core::Object?});
6078
_in::reachabilityFence(#t10);
6179
} =>#t12;
6280
method blah() → core::bool
6381
return block {
6482
final nat::NativeFieldWrapperClass1 #t13 = this;
65-
final core::bool #t14 = self::NativeClassy::_blah$FfiNative$Ptr(ffi::_fromAddress<ffi::Void>(nat::_getNativeField(#t13))){(ffi::Pointer<ffi::Void>) → core::bool};
83+
final core::bool #t14 = self::NativeClassy::_blah$FfiNative$Ptr(ffi::_fromAddress<ffi::Void>( block {
84+
core::int #pointerAddress = nat::_getNativeField(#t13);
85+
if(#pointerAddress.{core::Object::==}(#C6){(core::Object) → core::bool})
86+
core::StateError::_throwNew(#C7);
87+
else
88+
;
89+
} =>#pointerAddress)){(ffi::Pointer<ffi::Void>) → core::bool};
6690
_in::reachabilityFence(#t13);
6791
} =>#t14;
6892
}
@@ -90,4 +114,6 @@ constants {
90114
#C3 = 1
91115
#C4 = "doesntmatter"
92116
#C5 = 2
117+
#C6 = 0
118+
#C7 = "Native field is nullptr."
93119
}

‎sdk/lib/_internal/vm/lib/errors_patch.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,15 @@ class UnsupportedError {
160160
}
161161
}
162162

163+
@patch
164+
@pragma("vm:entry-point")
165+
class StateError {
166+
@pragma("vm:entry-point")
167+
static _throwNew(String msg) {
168+
throw new StateError(msg);
169+
}
170+
}
171+
163172
@patch
164173
class CyclicInitializationError {
165174
static _throwNew(String variableName) {

‎tests/ffi/vmspecific_ffi_native_test.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,4 +163,11 @@ void main() {
163163
6061,
164164
ClassWithNativeField(61)
165165
.addselfPtrAndHandleFieldMethod(ClassWithNativeField(6000)));
166+
Expect.throws(() {
167+
ClassWithNativeField(nullptr.address)
168+
.addSelfPtrAndPtrMethod(ClassWithNativeField(7000));
169+
});
170+
// Does not throw.
171+
ClassWithNativeField(8000)
172+
.addSelfPtrAndPtrMethod(ClassWithNativeField(nullptr.address));
166173
}

0 commit comments

Comments
 (0)