Skip to content

Commit

Permalink
[ VM / dart:typed_data ] Fixed issue where null could be passed for s…
Browse files Browse the repository at this point in the history 
…imd types in AOT

Arguments to simd type constructors were being checked in the bootstrap
natives entrypoint but not elsewhere. Checking for null in Dart code
ensures that we don't accidentally miss these checks in AOT.

The changes to sdk_nnbd are required in order to ensure the number of
arguments passed to native code match the number of arguments from the
non-NNBD implementation (we needed to remove the implicit type args
parameter).

Fixes #39518

Change-Id: Iaf7d8790c154f1e85db613b6dc84004c8013df9a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/126905
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
  • Loading branch information
@bkonyi
bkonyi authored and commit-bot@chromium.org committed Dec 9, 2019
1 parent 8b88946 commit 16e7647
Show file tree
Hide file tree
Showing 13 changed files with 226 additions and 113 deletions.
56 changes: 22 additions & 34 deletions runtime/lib/simd128.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,20 @@ static void ThrowMaskRangeException(int64_t m) {
}
}

DEFINE_NATIVE_ENTRY(Float32x4_fromDoubles, 0, 5) {
ASSERT(
TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(0)).IsNull());
GET_NON_NULL_NATIVE_ARGUMENT(Double, x, arguments->NativeArgAt(1));
GET_NON_NULL_NATIVE_ARGUMENT(Double, y, arguments->NativeArgAt(2));
GET_NON_NULL_NATIVE_ARGUMENT(Double, z, arguments->NativeArgAt(3));
GET_NON_NULL_NATIVE_ARGUMENT(Double, w, arguments->NativeArgAt(4));
DEFINE_NATIVE_ENTRY(Float32x4_fromDoubles, 0, 4) {
GET_NON_NULL_NATIVE_ARGUMENT(Double, x, arguments->NativeArgAt(0));
GET_NON_NULL_NATIVE_ARGUMENT(Double, y, arguments->NativeArgAt(1));
GET_NON_NULL_NATIVE_ARGUMENT(Double, z, arguments->NativeArgAt(2));
GET_NON_NULL_NATIVE_ARGUMENT(Double, w, arguments->NativeArgAt(3));
float _x = static_cast<float>(x.value());
float _y = static_cast<float>(y.value());
float _z = static_cast<float>(z.value());
float _w = static_cast<float>(w.value());
return Float32x4::New(_x, _y, _z, _w);
}

DEFINE_NATIVE_ENTRY(Float32x4_splat, 0, 2) {
ASSERT(
TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(0)).IsNull());
GET_NON_NULL_NATIVE_ARGUMENT(Double, v, arguments->NativeArgAt(1));
DEFINE_NATIVE_ENTRY(Float32x4_splat, 0, 1) {
GET_NON_NULL_NATIVE_ARGUMENT(Double, v, arguments->NativeArgAt(0));
float _v = v.value();
return Float32x4::New(_v, _v, _v, _v);
}
Expand Down Expand Up @@ -363,27 +359,23 @@ DEFINE_NATIVE_ENTRY(Float32x4_reciprocalSqrt, 0, 1) {
return Float32x4::New(_x, _y, _z, _w);
}

DEFINE_NATIVE_ENTRY(Int32x4_fromInts, 0, 5) {
ASSERT(
TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(0)).IsNull());
GET_NON_NULL_NATIVE_ARGUMENT(Integer, x, arguments->NativeArgAt(1));
GET_NON_NULL_NATIVE_ARGUMENT(Integer, y, arguments->NativeArgAt(2));
GET_NON_NULL_NATIVE_ARGUMENT(Integer, z, arguments->NativeArgAt(3));
GET_NON_NULL_NATIVE_ARGUMENT(Integer, w, arguments->NativeArgAt(4));
DEFINE_NATIVE_ENTRY(Int32x4_fromInts, 0, 4) {
GET_NON_NULL_NATIVE_ARGUMENT(Integer, x, arguments->NativeArgAt(0));
GET_NON_NULL_NATIVE_ARGUMENT(Integer, y, arguments->NativeArgAt(1));
GET_NON_NULL_NATIVE_ARGUMENT(Integer, z, arguments->NativeArgAt(2));
GET_NON_NULL_NATIVE_ARGUMENT(Integer, w, arguments->NativeArgAt(3));
int32_t _x = static_cast<int32_t>(x.AsTruncatedUint32Value());
int32_t _y = static_cast<int32_t>(y.AsTruncatedUint32Value());
int32_t _z = static_cast<int32_t>(z.AsTruncatedUint32Value());
int32_t _w = static_cast<int32_t>(w.AsTruncatedUint32Value());
return Int32x4::New(_x, _y, _z, _w);
}

DEFINE_NATIVE_ENTRY(Int32x4_fromBools, 0, 5) {
ASSERT(
TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(0)).IsNull());
GET_NON_NULL_NATIVE_ARGUMENT(Bool, x, arguments->NativeArgAt(1));
GET_NON_NULL_NATIVE_ARGUMENT(Bool, y, arguments->NativeArgAt(2));
GET_NON_NULL_NATIVE_ARGUMENT(Bool, z, arguments->NativeArgAt(3));
GET_NON_NULL_NATIVE_ARGUMENT(Bool, w, arguments->NativeArgAt(4));
DEFINE_NATIVE_ENTRY(Int32x4_fromBools, 0, 4) {
GET_NON_NULL_NATIVE_ARGUMENT(Bool, x, arguments->NativeArgAt(0));
GET_NON_NULL_NATIVE_ARGUMENT(Bool, y, arguments->NativeArgAt(1));
GET_NON_NULL_NATIVE_ARGUMENT(Bool, z, arguments->NativeArgAt(2));
GET_NON_NULL_NATIVE_ARGUMENT(Bool, w, arguments->NativeArgAt(3));
int32_t _x = x.value() ? 0xFFFFFFFF : 0x0;
int32_t _y = y.value() ? 0xFFFFFFFF : 0x0;
int32_t _z = z.value() ? 0xFFFFFFFF : 0x0;
Expand Down Expand Up @@ -640,18 +632,14 @@ DEFINE_NATIVE_ENTRY(Int32x4_select, 0, 3) {
return Float32x4::New(tempX.f, tempY.f, tempZ.f, tempW.f);
}

DEFINE_NATIVE_ENTRY(Float64x2_fromDoubles, 0, 3) {
ASSERT(
TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(0)).IsNull());
GET_NON_NULL_NATIVE_ARGUMENT(Double, x, arguments->NativeArgAt(1));
GET_NON_NULL_NATIVE_ARGUMENT(Double, y, arguments->NativeArgAt(2));
DEFINE_NATIVE_ENTRY(Float64x2_fromDoubles, 0, 2) {
GET_NON_NULL_NATIVE_ARGUMENT(Double, x, arguments->NativeArgAt(0));
GET_NON_NULL_NATIVE_ARGUMENT(Double, y, arguments->NativeArgAt(1));
return Float64x2::New(x.value(), y.value());
}

DEFINE_NATIVE_ENTRY(Float64x2_splat, 0, 2) {
ASSERT(
TypeArguments::CheckedHandle(zone, arguments->NativeArgAt(0)).IsNull());
GET_NON_NULL_NATIVE_ARGUMENT(Double, v, arguments->NativeArgAt(1));
DEFINE_NATIVE_ENTRY(Float64x2_splat, 0, 1) {
GET_NON_NULL_NATIVE_ARGUMENT(Double, v, arguments->NativeArgAt(0));
return Float64x2::New(v.value(), v.value());
}

Expand Down
12 changes: 6 additions & 6 deletions runtime/vm/bootstrap_natives.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,8 @@ namespace dart {
V(TypedDataView_length, 1) \
V(TypedDataView_offsetInBytes, 1) \
V(TypedDataView_typedData, 1) \
V(Float32x4_fromDoubles, 5) \
V(Float32x4_splat, 2) \
V(Float32x4_fromDoubles, 4) \
V(Float32x4_splat, 1) \
V(Float32x4_fromInt32x4Bits, 2) \
V(Float32x4_fromFloat64x2, 2) \
V(Float32x4_zero, 1) \
Expand Down Expand Up @@ -268,8 +268,8 @@ namespace dart {
V(Float32x4_sqrt, 1) \
V(Float32x4_reciprocal, 1) \
V(Float32x4_reciprocalSqrt, 1) \
V(Float64x2_fromDoubles, 3) \
V(Float64x2_splat, 2) \
V(Float64x2_fromDoubles, 2) \
V(Float64x2_splat, 1) \
V(Float64x2_zero, 1) \
V(Float64x2_fromFloat32x4, 2) \
V(Float64x2_add, 2) \
Expand All @@ -288,8 +288,8 @@ namespace dart {
V(Float64x2_min, 2) \
V(Float64x2_max, 2) \
V(Float64x2_sqrt, 1) \
V(Int32x4_fromInts, 5) \
V(Int32x4_fromBools, 5) \
V(Int32x4_fromInts, 4) \
V(Int32x4_fromBools, 4) \
V(Int32x4_fromFloat32x4Bits, 2) \
V(Int32x4_or, 2) \
V(Int32x4_and, 2) \
Expand Down
7 changes: 4 additions & 3 deletions runtime/vm/compiler/backend/il.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5465,9 +5465,10 @@ SimdOpInstr* SimdOpInstr::CreateFromCall(Zone* zone,
intptr_t mask /* = 0 */) {
SimdOpInstr* op =
new (zone) SimdOpInstr(KindForMethod(kind), call->deopt_id());
op->SetInputAt(0, new (zone) Value(receiver));
// Note: we are skipping receiver.
for (intptr_t i = 1; i < op->InputCount(); i++) {
if (receiver != nullptr) {
op->SetInputAt(0, new (zone) Value(receiver));
}
for (intptr_t i = (receiver != nullptr ? 1 : 0); i < op->InputCount(); i++) {
op->SetInputAt(i, call->PushArgumentAt(i)->value()->CopyWithType(zone));
}
if (op->HasMask()) {
Expand Down
8 changes: 4 additions & 4 deletions runtime/vm/compiler/backend/il.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8305,10 +8305,10 @@ class UnboxedWidthExtenderInstr : public TemplateDefinition<1, NoThrow, Pure> {
M(2, _, Float32x4LessThan, (Float32x4, Float32x4), Int32x4) \
M(2, _, Float32x4LessThanOrEqual, (Float32x4, Float32x4), Int32x4) \
M(2, _, Float32x4NotEqual, (Float32x4, Float32x4), Int32x4) \
M(4, _, Int32x4Constructor, (Int32, Int32, Int32, Int32), Int32x4) \
M(4, _, Int32x4BoolConstructor, (Bool, Bool, Bool, Bool), Int32x4) \
M(4, _, Float32x4Constructor, (Double, Double, Double, Double), Float32x4) \
M(2, _, Float64x2Constructor, (Double, Double), Float64x2) \
M(4, _, Int32x4FromInts, (Int32, Int32, Int32, Int32), Int32x4) \
M(4, _, Int32x4FromBools, (Bool, Bool, Bool, Bool), Int32x4) \
M(4, _, Float32x4FromDoubles, (Double, Double, Double, Double), Float32x4) \
M(2, _, Float64x2FromDoubles, (Double, Double), Float64x2) \
M(0, _, Float32x4Zero, (), Float32x4) \
M(0, _, Float64x2Zero, (), Float64x2) \
M(1, _, Float32x4Splat, (Double), Float32x4) \
Expand Down
16 changes: 8 additions & 8 deletions runtime/vm/compiler/backend/il_arm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4952,7 +4952,7 @@ DEFINE_EMIT(Simd32x4GetSignMask,

// Low (< 7) Q registers are needed for the vcvtsd instruction.
// TODO(dartbug.com/30953) support register range constraints in the regalloc.
DEFINE_EMIT(Float32x4Constructor,
DEFINE_EMIT(Float32x4FromDoubles,
(FixedQRegisterView<Q6> out,
QRegisterView q0,
QRegisterView q1,
Expand Down Expand Up @@ -5062,7 +5062,7 @@ DEFINE_EMIT(Float64x2Splat, (QRegisterView result, QRegisterView value)) {
__ vmovd(result.d(1), value.d(0));
}

DEFINE_EMIT(Float64x2Constructor,
DEFINE_EMIT(Float64x2FromDoubles,
(QRegisterView r, QRegisterView q0, QRegisterView q1)) {
__ vmovd(r.d(0), q0.d(0));
__ vmovd(r.d(1), q1.d(0));
Expand Down Expand Up @@ -5159,7 +5159,7 @@ DEFINE_EMIT(Float64x2Binary,
}
}

DEFINE_EMIT(Int32x4Constructor,
DEFINE_EMIT(Int32x4FromInts,
(QRegisterView result,
Register v0,
Register v1,
Expand All @@ -5170,7 +5170,7 @@ DEFINE_EMIT(Int32x4Constructor,
__ vmovdrr(result.d(1), v2, v3);
}

DEFINE_EMIT(Int32x4BoolConstructor,
DEFINE_EMIT(Int32x4FromBools,
(QRegisterView result,
Register v0,
Register v1,
Expand Down Expand Up @@ -5305,7 +5305,7 @@ DEFINE_EMIT(Int32x4WithFlag,
CASE(Float32x4GetSignMask) \
CASE(Int32x4GetSignMask) \
____(Simd32x4GetSignMask) \
SIMPLE(Float32x4Constructor) \
SIMPLE(Float32x4FromDoubles) \
SIMPLE(Float32x4Zero) \
SIMPLE(Float32x4Splat) \
SIMPLE(Float32x4Sqrt) \
Expand All @@ -5328,7 +5328,7 @@ DEFINE_EMIT(Int32x4WithFlag,
____(Simd64x2Shuffle) \
SIMPLE(Float64x2Zero) \
SIMPLE(Float64x2Splat) \
SIMPLE(Float64x2Constructor) \
SIMPLE(Float64x2FromDoubles) \
SIMPLE(Float64x2ToFloat32x4) \
SIMPLE(Float32x4ToFloat64x2) \
SIMPLE(Float64x2GetSignMask) \
Expand All @@ -5342,8 +5342,8 @@ DEFINE_EMIT(Int32x4WithFlag,
CASE(Float64x2Min) \
CASE(Float64x2Max) \
____(Float64x2Binary) \
SIMPLE(Int32x4Constructor) \
SIMPLE(Int32x4BoolConstructor) \
SIMPLE(Int32x4FromInts) \
SIMPLE(Int32x4FromBools) \
CASE(Int32x4GetFlagX) \
CASE(Int32x4GetFlagY) \
CASE(Int32x4GetFlagZ) \
Expand Down
22 changes: 11 additions & 11 deletions runtime/vm/compiler/backend/il_arm64.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4065,7 +4065,7 @@ DEFINE_EMIT(SimdBinaryOp, (VRegister result, VRegister left, VRegister right)) {
__ vdups(result, VTMP, 0);
__ vmuls(result, result, right);
break;
case SimdOpInstr::kFloat64x2Constructor:
case SimdOpInstr::kFloat64x2FromDoubles:
__ vinsd(result, 0, left, 0);
__ vinsd(result, 1, right, 0);
break;
Expand Down Expand Up @@ -4189,7 +4189,7 @@ DEFINE_EMIT(Simd32x4GetSignMask,
}

DEFINE_EMIT(
Float32x4Constructor,
Float32x4FromDoubles,
(VRegister r, VRegister v0, VRegister v1, VRegister v2, VRegister v3)) {
__ fcvtsd(VTMP, v0);
__ vinss(r, 0, VTMP, 0);
Expand Down Expand Up @@ -4265,7 +4265,7 @@ DEFINE_EMIT(Float64x2With,
}

DEFINE_EMIT(
Int32x4Constructor,
Int32x4FromInts,
(VRegister result, Register v0, Register v1, Register v2, Register v3)) {
__ veor(result, result, result);
__ vinsw(result, 0, v0);
Expand All @@ -4274,7 +4274,7 @@ DEFINE_EMIT(
__ vinsw(result, 3, v3);
}

DEFINE_EMIT(Int32x4BoolConstructor,
DEFINE_EMIT(Int32x4FromBools,
(VRegister result,
Register v0,
Register v1,
Expand Down Expand Up @@ -4373,7 +4373,7 @@ DEFINE_EMIT(Int32x4WithFlag,
CASE(Float32x4LessThan) \
CASE(Float32x4LessThanOrEqual) \
CASE(Float32x4Scale) \
CASE(Float64x2Constructor) \
CASE(Float64x2FromDoubles) \
CASE(Float64x2Scale) \
____(SimdBinaryOp) \
SIMD_OP_SIMPLE_UNARY(CASE) \
Expand All @@ -4393,8 +4393,8 @@ DEFINE_EMIT(Int32x4WithFlag,
CASE(Float32x4GetSignMask) \
CASE(Int32x4GetSignMask) \
____(Simd32x4GetSignMask) \
CASE(Float32x4Constructor) \
____(Float32x4Constructor) \
CASE(Float32x4FromDoubles) \
____(Float32x4FromDoubles) \
CASE(Float32x4Zero) \
CASE(Float64x2Zero) \
____(SimdZero) \
Expand All @@ -4413,10 +4413,10 @@ DEFINE_EMIT(Int32x4WithFlag,
CASE(Float64x2WithX) \
CASE(Float64x2WithY) \
____(Float64x2With) \
CASE(Int32x4Constructor) \
____(Int32x4Constructor) \
CASE(Int32x4BoolConstructor) \
____(Int32x4BoolConstructor) \
CASE(Int32x4FromInts) \
____(Int32x4FromInts) \
CASE(Int32x4FromBools) \
____(Int32x4FromBools) \
CASE(Int32x4GetFlagX) \
CASE(Int32x4GetFlagY) \
CASE(Int32x4GetFlagZ) \
Expand Down
16 changes: 8 additions & 8 deletions runtime/vm/compiler/backend/il_ia32.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4264,7 +4264,7 @@ DEFINE_EMIT(SimdBinaryOp,
case SimdOpInstr::kInt32x4ShuffleMix:
__ shufps(left, right, compiler::Immediate(instr->mask()));
break;
case SimdOpInstr::kFloat64x2Constructor:
case SimdOpInstr::kFloat64x2FromDoubles:
// shufpd mask 0x0 results in:
// Lower 64-bits of left = Lower 64-bits of left.
// Upper 64-bits of left = Lower 64-bits of right.
Expand Down Expand Up @@ -4400,7 +4400,7 @@ DEFINE_EMIT(SimdGetSignMask, (Register out, XmmRegister value)) {
}

DEFINE_EMIT(
Float32x4Constructor,
Float32x4FromDoubles,
(SameAsFirstInput, XmmRegister v0, XmmRegister, XmmRegister, XmmRegister)) {
// TODO(dartbug.com/30949) avoid transfer through memory. SSE4.1 has
// insertps, with SSE2 this instruction can be implemented through unpcklps.
Expand Down Expand Up @@ -4431,7 +4431,7 @@ DEFINE_EMIT(Float32x4Clamp,
__ maxps(left, lower);
}

DEFINE_EMIT(Int32x4Constructor,
DEFINE_EMIT(Int32x4FromInts,
(XmmRegister result, Register, Register, Register, Register)) {
// TODO(dartbug.com/30949) avoid transfer through memory.
__ SubImmediate(ESP, compiler::Immediate(kSimd128Size));
Expand All @@ -4442,7 +4442,7 @@ DEFINE_EMIT(Int32x4Constructor,
__ AddImmediate(ESP, compiler::Immediate(kSimd128Size));
}

DEFINE_EMIT(Int32x4BoolConstructor,
DEFINE_EMIT(Int32x4FromBools,
(XmmRegister result, Register, Register, Register, Register)) {
// TODO(dartbug.com/30949) avoid transfer through memory and branches.
__ SubImmediate(ESP, compiler::Immediate(kSimd128Size));
Expand Down Expand Up @@ -4545,7 +4545,7 @@ DEFINE_EMIT(Int32x4Select,
CASE(Float32x4Scale) \
CASE(Float32x4ShuffleMix) \
CASE(Int32x4ShuffleMix) \
CASE(Float64x2Constructor) \
CASE(Float64x2FromDoubles) \
CASE(Float64x2Scale) \
CASE(Float64x2WithX) \
CASE(Float64x2WithY) \
Expand Down Expand Up @@ -4574,9 +4574,9 @@ DEFINE_EMIT(Int32x4Select,
CASE(Int32x4GetSignMask) \
CASE(Float64x2GetSignMask) \
____(SimdGetSignMask) \
SIMPLE(Float32x4Constructor) \
SIMPLE(Int32x4Constructor) \
SIMPLE(Int32x4BoolConstructor) \
SIMPLE(Float32x4FromDoubles) \
SIMPLE(Int32x4FromInts) \
SIMPLE(Int32x4FromBools) \
SIMPLE(Float32x4Zero) \
SIMPLE(Float64x2Zero) \
SIMPLE(Float32x4Clamp) \
Expand Down
Loading

0 comments on commit 16e7647

Please sign in to comment.