[vm] Revert 3fcf660e with the correct fix for --no-sound-null-safety.

Insert CheckNull instructions if in AOT mode prior to checking the
index for being within the bounds in inlined indexing operations.
If we're in --sound-null-safety mode, they'll be canonicalized away
since the indices won't be nullable anyway.

Fixes https://github.com/dart-lang/sdk/issues/52910.

TEST=corelib{,_2}/list_removeat_test and
  co19_2/LibTest/core/String/codeUnitAt_A03_t01 on
  vm-kernel-precomp-linux-release-x64-try

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try
Change-Id: I37212e94c7c5032c75709e3f0bb91734cbc705cd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313507
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
This commit is contained in:
Tess Strickland 2023-07-14 07:06:41 +00:00 committed by Commit Queue
parent e4f1a10431
commit 0a1c797b5a
2 changed files with 22 additions and 16 deletions

View file

@ -2773,6 +2773,16 @@ static intptr_t PrepareInlineIndexedOp(FlowGraph* flow_graph,
new (Z) Value(*array), Slot::GetLengthFieldForArrayCid(array_cid),
call->source());
*cursor = flow_graph->AppendTo(*cursor, length, nullptr, FlowGraph::kValue);
if (CompilerState::Current().is_aot()) {
// Add a null-check in case the index argument is known to be compatible
// but possibly nullable. By inserting the null-check, we can allow the
// unbox instruction later inserted to be non-speculative.
auto* const null_check = new (Z) CheckNullInstr(
new (Z) Value(*index), Symbols::Index(), call->deopt_id(),
call->source(), CheckNullInstr::kArgumentError);
*cursor = flow_graph->AppendTo(*cursor, null_check, call->env(),
FlowGraph::kEffect);
}
*index = flow_graph->CreateCheckBound(length, *index, call->deopt_id());
*cursor =
flow_graph->AppendTo(*cursor, *index, call->env(), FlowGraph::kValue);
@ -3483,6 +3493,16 @@ static Definition* PrepareInlineStringIndexOp(FlowGraph* flow_graph,
cursor = flow_graph->AppendTo(cursor, length, nullptr, FlowGraph::kValue);
// Bounds check.
if (CompilerState::Current().is_aot()) {
// Add a null-check in case the index argument is known to be compatible
// but possibly nullable. By inserting the null-check, we can allow the
// unbox instruction later inserted to be non-speculative.
auto* const null_check = new (Z)
CheckNullInstr(new (Z) Value(index), Symbols::Index(), call->deopt_id(),
call->source(), CheckNullInstr::kArgumentError);
cursor = flow_graph->AppendTo(cursor, null_check, call->env(),
FlowGraph::kEffect);
}
index = flow_graph->CreateCheckBound(length, index, call->deopt_id());
cursor = flow_graph->AppendTo(cursor, index, call->env(), FlowGraph::kValue);

View file

@ -560,26 +560,12 @@ void Object::InitNullAndBool(IsolateGroup* isolate_group) {
// Allocate and initialize the null instance.
// 'null_' must be the first object allocated as it is used in allocation to
// clear the object.
// clear the pointer fields of objects.
{
uword address =
heap->Allocate(thread, Instance::InstanceSize(), Heap::kOld);
null_ = static_cast<InstancePtr>(address + kHeapObjectTag);
// The call below is using 'null_' to initialize itself.
//
// TODO(52910): Change the below to
// InitializeObjectVariant<Instance>(address, kNullCid);
// after we've fixed the unboxing of the null object without checking for
// null first when --no-sound-null-safety is on. (This is a stopgap so that
// those bad unboxings pull out really large values that almost certainly
// will fail, which was the old status quo.)
const intptr_t ptr_field_end_offset =
Instance::InstanceSize() - (Instance::ContainsCompressedPointers()
? kCompressedWordSize
: kWordSize);
InitializeObject(address, kNullCid, Instance::InstanceSize(),
Instance::ContainsCompressedPointers(),
sizeof(UntaggedObject), ptr_field_end_offset);
InitializeObjectVariant<Instance>(address, kNullCid);
null_->untag()->SetCanonical();
}