Skip to content

Commit f04f534

Browse files
alexmarkovCommit Queue
alexmarkov
authored and
Commit Queue
committed
[vm] Make FFI implementation less fragile wrt field order
Replace access of certain fields by hardcoded indices with lookup by name. As number of fields in those classes is small, the lookup should not take considerable time. This fix is done in anticipation for the CFE change https://dart-review.googlesource.com/c/sdk/+/412880 which may change order of declared fields if there are patches. TEST=manually tested with CFE change Change-Id: Ibf4ab6719ee263711fb7c6b21b038ea1dc5f9e7b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/412803 Reviewed-by: Daco Harkes <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
1 parent 0a7aae2 commit f04f534

File tree

4 files changed

+29
-43
lines changed

4 files changed

+29
-43
lines changed

‎runtime/vm/compiler/ffi/native_type.cc‎

Lines changed: 19 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -448,18 +448,14 @@ static const NativeType* CompoundFromPragma(Zone* zone,
448448
const auto& clazz = Class::Handle(zone, struct_layout.clazz());
449449
ASSERT(String::Handle(zone, clazz.UserVisibleName())
450450
.Equals(Symbols::FfiStructLayout()));
451-
const auto& struct_layout_fields = Array::Handle(zone, clazz.fields());
452-
ASSERT(struct_layout_fields.Length() == 2);
453-
const auto& types_field =
454-
Field::Handle(zone, Field::RawCast(struct_layout_fields.At(0)));
455-
ASSERT(String::Handle(zone, types_field.name())
456-
.Equals(Symbols::FfiFieldTypes()));
451+
const auto& types_field = Field::Handle(
452+
zone, clazz.LookupFieldAllowPrivate(Symbols::FfiFieldTypes()));
453+
ASSERT(!types_field.IsNull());
457454
const auto& field_types =
458455
Array::Handle(zone, Array::RawCast(struct_layout.GetField(types_field)));
459-
const auto& packed_field =
460-
Field::Handle(zone, Field::RawCast(struct_layout_fields.At(1)));
461-
ASSERT(String::Handle(zone, packed_field.name())
462-
.Equals(Symbols::FfiFieldPacking()));
456+
const auto& packed_field = Field::Handle(
457+
zone, clazz.LookupFieldAllowPrivate(Symbols::FfiFieldPacking()));
458+
ASSERT(!packed_field.IsNull());
463459
const auto& packed_value = Integer::Handle(
464460
zone, Integer::RawCast(struct_layout.GetField(packed_field)));
465461
const intptr_t member_packing =
@@ -486,18 +482,15 @@ static const NativeType* CompoundFromPragma(Zone* zone,
486482
Class::Handle(zone, field_instance.clazz());
487483
ASSERT(String::Handle(zone, struct_layout_array_class.UserVisibleName())
488484
.Equals(Symbols::FfiStructLayoutArray()));
489-
const auto& struct_layout_array_fields =
490-
Array::Handle(zone, struct_layout_array_class.fields());
491-
ASSERT(struct_layout_array_fields.Length() == 3);
492485
const auto& element_type_field =
493-
Field::Handle(zone, Field::RawCast(struct_layout_array_fields.At(0)));
494-
ASSERT(String::Handle(zone, element_type_field.UserVisibleName())
495-
.Equals(Symbols::FfiElementType()));
486+
Field::Handle(zone, struct_layout_array_class.LookupFieldAllowPrivate(
487+
Symbols::FfiElementType()));
488+
ASSERT(!element_type_field.IsNull());
496489
field_type ^= field_instance.GetField(element_type_field);
497-
const auto& length_field =
498-
Field::Handle(zone, Field::RawCast(struct_layout_array_fields.At(1)));
499-
ASSERT(String::Handle(zone, length_field.UserVisibleName())
500-
.Equals(Symbols::Length()));
490+
const auto& length_field = Field::Handle(
491+
zone,
492+
struct_layout_array_class.LookupFieldAllowPrivate(Symbols::Length()));
493+
ASSERT(!length_field.IsNull());
501494
const auto& length = Smi::Handle(
502495
zone, Smi::RawCast(field_instance.GetField(length_field)));
503496
const auto element_type =
@@ -508,9 +501,9 @@ static const NativeType* CompoundFromPragma(Zone* zone,
508501

509502
#if defined(DEBUG)
510503
const auto& variable_length_field =
511-
Field::Handle(zone, Field::RawCast(struct_layout_array_fields.At(2)));
512-
ASSERT(String::Handle(zone, variable_length_field.UserVisibleName())
513-
.Equals(Symbols::VariableLength()));
504+
Field::Handle(zone, struct_layout_array_class.LookupFieldAllowPrivate(
505+
Symbols::VariableLength()));
506+
ASSERT(!variable_length_field.IsNull());
514507
const auto& variable_length = Bool::Handle(
515508
zone, Bool::RawCast(field_instance.GetField(variable_length_field)));
516509
ASSERT(variable_length.value() == true ||
@@ -536,12 +529,9 @@ static const NativeType* AbiSpecificFromPragma(Zone* zone,
536529
const Class& abi_specific_int,
537530
const char** error) {
538531
const auto& clazz = Class::Handle(zone, pragma.clazz());
539-
const auto& fields = Array::Handle(zone, clazz.fields());
540-
ASSERT(fields.Length() == 1);
541-
const auto& native_types_field =
542-
Field::Handle(zone, Field::RawCast(fields.At(0)));
543-
ASSERT(String::Handle(zone, native_types_field.name())
544-
.Equals(Symbols::FfiNativeTypes()));
532+
const auto& native_types_field = Field::Handle(
533+
zone, clazz.LookupFieldAllowPrivate(Symbols::FfiNativeTypes()));
534+
ASSERT(!native_types_field.IsNull());
545535
const auto& native_types =
546536
Array::Handle(zone, Array::RawCast(pragma.GetField(native_types_field)));
547537

‎runtime/vm/compiler/frontend/kernel_to_il.cc‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5285,14 +5285,12 @@ Fragment FlowGraphBuilder::FfiNativeLookupAddress(
52855285
const auto& native_class = Class::Handle(Z, native.clazz());
52865286
ASSERT(String::Handle(Z, native_class.UserVisibleName())
52875287
.Equals(Symbols::FfiNative()));
5288-
const auto& native_class_fields = Array::Handle(Z, native_class.fields());
5289-
ASSERT(native_class_fields.Length() == 4);
5290-
const auto& symbol_field =
5291-
Field::Handle(Z, Field::RawCast(native_class_fields.At(1)));
5292-
ASSERT(!symbol_field.is_static());
5293-
const auto& asset_id_field =
5294-
Field::Handle(Z, Field::RawCast(native_class_fields.At(2)));
5295-
ASSERT(!asset_id_field.is_static());
5288+
const auto& symbol_field = Field::Handle(
5289+
Z, native_class.LookupInstanceFieldAllowPrivate(Symbols::symbol()));
5290+
ASSERT(!symbol_field.IsNull());
5291+
const auto& asset_id_field = Field::Handle(
5292+
Z, native_class.LookupInstanceFieldAllowPrivate(Symbols::assetId()));
5293+
ASSERT(!asset_id_field.IsNull());
52965294
const auto& symbol =
52975295
String::ZoneHandle(Z, String::RawCast(native.GetField(symbol_field)));
52985296
const auto& asset_id =

‎runtime/vm/object.cc‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8827,13 +8827,9 @@ bool Function::FfiIsLeaf() const {
88278827
UNREACHABLE();
88288828
}
88298829
const auto& pragma_value_class = Class::Handle(zone, pragma_value.clazz());
8830-
const auto& pragma_value_fields =
8831-
Array::Handle(zone, pragma_value_class.fields());
8832-
ASSERT(pragma_value_fields.Length() >= 1);
88338830
const auto& is_leaf_field = Field::Handle(
8834-
zone,
8835-
Field::RawCast(pragma_value_fields.At(pragma_value_fields.Length() - 1)));
8836-
ASSERT(is_leaf_field.name() == Symbols::isLeaf().ptr());
8831+
zone, pragma_value_class.LookupFieldAllowPrivate(Symbols::isLeaf()));
8832+
ASSERT(!is_leaf_field.IsNull());
88378833
return Bool::Handle(zone, Bool::RawCast(pragma_value.GetField(is_leaf_field)))
88388834
.value();
88398835
}

‎runtime/vm/symbols.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,7 @@ class ObjectPointerVisitor;
507507
V(add, "add") \
508508
V(addStream, "addStream") \
509509
V(addStreamFuture, "addStreamFuture") \
510+
V(assetId, "assetId") \
510511
V(asyncStarBody, "asyncStarBody") \
511512
V(byteOffset, "byteOffset") \
512513
V(c_result, ":result") \
@@ -552,6 +553,7 @@ class ObjectPointerVisitor;
552553
V(state, "state") \
553554
V(string_param, ":string_param") \
554555
V(string_param_length, ":string_param_length") \
556+
V(symbol, "symbol") \
555557
V(system, "system") \
556558
V(vm_always_consider_inlining, "vm:always-consider-inlining") \
557559
V(vm_awaiter_link, "vm:awaiter-link") \

0 commit comments

Comments
 (0)