[vm/ffi] Fix MemoryCopyInstr on ia32

The MemoryCopyInstr destroys the value in src-TypedDataBase
location. It overwrites it by loading the data_ field.

This requires using a WritableRegister, rather than a Register.

This issue seems to have been already present before the recent
optimizations (the leaq instruction overwrites array_reg), but the
recent optimizations made it exercise this instruction in a loop where
it was assumed the src-position was not destroyed.

TEST=tests/ffi/regress_51315_test.dart

Closes: https://github.com/dart-lang/sdk/issues/51315
Change-Id: If37c6c39de8fd0cba9b8a25a1f38f025a39123d9
Cq-Include-Trybots: luci.dart.try:vm-kernel-win-debug-ia32-try,vm-kernel-nnbd-win-release-ia32-try,vm-kernel-nnbd-linux-release-ia32-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-eager-optimization-linux-release-ia32-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/283021
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
This commit is contained in:
Daco Harkes 2023-02-14 13:20:43 +00:00 committed by Commit Queue
parent 59a1e9afeb
commit 6a90c21992
4 changed files with 85 additions and 2 deletions

View file

@ -85,7 +85,7 @@ LocationSummary* MemoryCopyInstr::MakeLocationSummary(Zone* zone,
const intptr_t kNumTemps = remove_loop ? 1 : 0;
LocationSummary* locs = new (zone)
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
locs->set_in(kSrcPos, Location::RequiresRegister());
locs->set_in(kSrcPos, Location::WritableRegister());
locs->set_in(kDestPos, Location::RegisterLocation(EDI));
locs->set_in(kSrcStartPos, LocationRegisterOrConstant(src_start()));
locs->set_in(kDestStartPos, LocationRegisterOrConstant(dest_start()));

View file

@ -291,6 +291,9 @@ class Location : public ValueObject {
return UnallocatedLocation(kPrefersRegister);
}
// Blocks a CPU register for the entirety of the IL instruction.
//
// The register value _must_ be preserved by the machine code.
static Location RequiresRegister() {
return UnallocatedLocation(kRequiresRegister);
}
@ -299,6 +302,9 @@ class Location : public ValueObject {
return UnallocatedLocation(kRequiresFpuRegister);
}
// Blocks a CPU register for the entirety of the IL instruction.
//
// The register value does not have to be preserved by the machine code.
static Location WritableRegister() {
return UnallocatedLocation(kWritableRegister);
}
@ -317,7 +323,10 @@ class Location : public ValueObject {
return PolicyField::decode(payload());
}
// Register locations.
// Blocks `reg` for the entirety of the IL instruction.
//
// The register value does not have to be preserved by the machine code.
// TODO(https://dartbug.com/51409): Rename to WritableRegisterLocation.
static Location RegisterLocation(Register reg) {
return Location(kRegister, reg);
}

View file

@ -0,0 +1,36 @@
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// VMOptions=--deterministic --optimization-counter-threshold=100
import 'dart:ffi';
import "package:ffi/ffi.dart";
main(List<String> arguments) {
for (int i = 0; i < 1000; i++) {
testCompoundLoadAndStore();
}
print('done');
}
const count = 10;
testCompoundLoadAndStore() {
final foos = calloc<Foo>(count);
final reference = foos.ref;
reference.a = count;
for (var j = 1; j < count; j++) {
final foo = foos.elementAt(j);
foo.ref = reference;
}
calloc.free(foos);
}
class Foo extends Struct {
@Int8()
external int a;
}

View file

@ -0,0 +1,38 @@
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// @dart=2.9
// VMOptions=--deterministic --optimization-counter-threshold=100
import 'dart:ffi';
import "package:ffi/ffi.dart";
main(List<String> arguments) {
for (int i = 0; i < 1000; i++) {
testCompoundLoadAndStore();
}
print('done');
}
const count = 10;
testCompoundLoadAndStore() {
final foos = calloc<Foo>(count);
final reference = foos.ref;
reference.a = count;
for (var j = 1; j < count; j++) {
final foo = foos.elementAt(j);
foo.ref = reference;
}
calloc.free(foos);
}
class Foo extends Struct {
@Int8()
int a;
}