Skip to content

Commit 84fed14

Browse files
mralephCommit Queue
mraleph
authored and
Commit Queue
committed
[vm] Fix vm:align-loops pragma
Use aligned text offset for instructions when resolving static calls during relocation. The vm/dart/align_loops_test is adjusted to cover this issue: previous we only verified alignment but did not actually run the generated code, so the bug was undetected. Issue #55522 TEST=vm/dart/align_loops_test [email protected] Change-Id: I56521c104f91b85150584539f58191cf4592f4f5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/365144 Reviewed-by: Alexander Markov <[email protected]> Commit-Queue: Slava Egorov <[email protected]>
1 parent ed5aeab commit 84fed14

File tree

5 files changed

+154
-147
lines changed

5 files changed

+154
-147
lines changed

‎runtime/tests/vm/dart/align_loops_test.dart

Lines changed: 57 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -2,90 +2,72 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
import 'dart:convert';
6-
import 'dart:io';
7-
import 'dart:async';
5+
// This test verifies that vm:align-loops pragma works as expected.
6+
//
7+
// * This test should run without crashing in AOT mode.
8+
// * `align_loops_verify_alignment_test.dart` will AOT compile this test
9+
// and then verify that [alignedFunction1] and [alignedFunction2] are
10+
// aligned.
811

9-
import 'package:expect/expect.dart';
10-
import 'package:native_stack_traces/elf.dart';
11-
import 'package:path/path.dart' as path;
12+
import 'dart:typed_data';
1213

13-
import 'use_flag_test_helper.dart';
14+
// Having a static call to this function verifies that relocation works
15+
// as expected even when caller needs to be aligned.
16+
@pragma('vm:never-inline')
17+
void printOk() {
18+
print("ok");
19+
}
1420

15-
void checkAligned(Symbol sym) {
16-
// We only expect to run this test on X64 Linux.
17-
final expectedAlignment = 32;
18-
if ((sym.value & (expectedAlignment - 1)) != 0) {
19-
throw 'Symbol $sym has value ${sym.value} which is not aligned by '
20-
'$expectedAlignment';
21+
@pragma('vm:never-inline')
22+
int foo(Uint8List list) {
23+
printOk();
24+
var result = 0;
25+
for (var i = 0; i < list.length; i++) {
26+
result ^= list[i];
2127
}
28+
printOk();
29+
return result;
2230
}
2331

24-
Future<void> testAOT(String dillPath, {bool useAsm = false}) async {
25-
await withTempDir('align-loops-test-${useAsm ? 'asm' : 'elf'}',
26-
(String tempDir) async {
27-
// Generate the snapshot
28-
final snapshotPath = path.join(tempDir, 'libtest.so');
29-
final commonSnapshotArgs = [dillPath];
30-
31-
if (useAsm) {
32-
final assemblyPath = path.join(tempDir, 'test.S');
33-
34-
await run(genSnapshot, <String>[
35-
'--snapshot-kind=app-aot-assembly',
36-
'--assembly=$assemblyPath',
37-
...commonSnapshotArgs,
38-
]);
39-
40-
await assembleSnapshot(assemblyPath, snapshotPath);
41-
} else {
42-
await run(genSnapshot, <String>[
43-
'--snapshot-kind=app-aot-elf',
44-
'--elf=$snapshotPath',
45-
...commonSnapshotArgs,
46-
]);
47-
}
48-
49-
print("Snapshot generated at $snapshotPath.");
50-
51-
final elf = Elf.fromFile(snapshotPath)!;
52-
// The very first symbol should be aligned by 32 bytes because it is
53-
// the start of the instructions section.
54-
checkAligned(elf.staticSymbols.first);
55-
for (var symbol in elf.staticSymbols) {
56-
if (symbol.name.startsWith('alignedFunction')) {
57-
checkAligned(symbol);
58-
}
59-
}
60-
});
32+
@pragma('vm:never-inline')
33+
@pragma('vm:align-loops')
34+
int alignedFunction1(Uint8List list) {
35+
printOk();
36+
var result = 0;
37+
for (var i = 0; i < list.length; i++) {
38+
result ^= list[i];
39+
}
40+
printOk();
41+
return result;
6142
}
6243

63-
void main() async {
64-
// Only run this test on Linux X64 for simplicity.
65-
if (!(Platform.isLinux && buildDir.endsWith('X64'))) {
66-
return;
44+
@pragma('vm:never-inline')
45+
int baz(Uint8List list) {
46+
printOk();
47+
var result = 1;
48+
for (var i = 0; i < list.length; i++) {
49+
result ^= list[i];
6750
}
51+
printOk();
52+
return result;
53+
}
6854

69-
await withTempDir('align_loops', (String tempDir) async {
70-
final testProgram = path.join(sdkDir, 'runtime', 'tests', 'vm', 'dart',
71-
'align_loops_test_program.dart');
72-
73-
final aotDillPath = path.join(tempDir, 'aot_test.dill');
74-
await run(genKernel, <String>[
75-
'--aot',
76-
'--platform',
77-
platformDill,
78-
...Platform.executableArguments
79-
.where((arg) => arg.startsWith('--enable-experiment=')),
80-
'-o',
81-
aotDillPath,
82-
testProgram
83-
]);
55+
@pragma('vm:never-inline')
56+
@pragma('vm:align-loops')
57+
int alignedFunction2(Uint8List list) {
58+
printOk();
59+
var result = 2;
60+
for (var i = 0; i < list.length; i++) {
61+
result ^= list[i];
62+
}
63+
printOk();
64+
return result;
65+
}
8466

85-
await Future.wait([
86-
// Test unstripped ELF generation directly.
87-
testAOT(aotDillPath),
88-
testAOT(aotDillPath, useAsm: true),
89-
]);
90-
});
67+
void main(List<String> args) {
68+
final v = Uint8List(10);
69+
foo(v);
70+
alignedFunction1(v);
71+
baz(v);
72+
alignedFunction2(v);
9173
}

‎runtime/tests/vm/dart/align_loops_test_program.dart

Lines changed: 0 additions & 68 deletions
This file was deleted.

‎runtime/tests/vm/dart/align_loops_verify_alignment_test.dart

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:convert';
6+
import 'dart:io';
7+
import 'dart:async';
8+
9+
import 'package:expect/expect.dart';
10+
import 'package:native_stack_traces/elf.dart';
11+
import 'package:path/path.dart' as path;
12+
13+
import 'use_flag_test_helper.dart';
14+
15+
void checkAligned(Symbol sym) {
16+
// We only expect to run this test on X64 Linux.
17+
final expectedAlignment = 32;
18+
if ((sym.value & (expectedAlignment - 1)) != 0) {
19+
throw 'Symbol $sym has value ${sym.value} which is not aligned by '
20+
'$expectedAlignment';
21+
}
22+
}
23+
24+
Future<void> testAOT(String dillPath, {bool useAsm = false}) async {
25+
await withTempDir('align-loops-test-${useAsm ? 'asm' : 'elf'}',
26+
(String tempDir) async {
27+
// Generate the snapshot
28+
final snapshotPath = path.join(tempDir, 'libtest.so');
29+
final commonSnapshotArgs = [dillPath];
30+
31+
if (useAsm) {
32+
final assemblyPath = path.join(tempDir, 'test.S');
33+
34+
await run(genSnapshot, <String>[
35+
'--snapshot-kind=app-aot-assembly',
36+
'--assembly=$assemblyPath',
37+
...commonSnapshotArgs,
38+
]);
39+
40+
await assembleSnapshot(assemblyPath, snapshotPath);
41+
} else {
42+
await run(genSnapshot, <String>[
43+
'--snapshot-kind=app-aot-elf',
44+
'--elf=$snapshotPath',
45+
...commonSnapshotArgs,
46+
]);
47+
}
48+
49+
print("Snapshot generated at $snapshotPath.");
50+
51+
final elf = Elf.fromFile(snapshotPath)!;
52+
// The very first symbol should be aligned by 32 bytes because it is
53+
// the start of the instructions section.
54+
checkAligned(elf.staticSymbols.first);
55+
for (var symbol in elf.staticSymbols) {
56+
if (symbol.name.startsWith('alignedFunction')) {
57+
checkAligned(symbol);
58+
}
59+
}
60+
});
61+
}
62+
63+
void main() async {
64+
// Only run this test on Linux X64 for simplicity.
65+
if (!(Platform.isLinux && buildDir.endsWith('X64'))) {
66+
return;
67+
}
68+
69+
await withTempDir('align_loops', (String tempDir) async {
70+
final testProgram = path.join(
71+
sdkDir, 'runtime', 'tests', 'vm', 'dart', 'align_loops_test.dart');
72+
73+
final aotDillPath = path.join(tempDir, 'aot_test.dill');
74+
await run(genKernel, <String>[
75+
'--aot',
76+
'--platform',
77+
platformDill,
78+
...Platform.executableArguments
79+
.where((arg) => arg.startsWith('--enable-experiment=')),
80+
'-o',
81+
aotDillPath,
82+
testProgram
83+
]);
84+
85+
await Future.wait([
86+
// Test unstripped ELF generation directly.
87+
testAOT(aotDillPath),
88+
testAOT(aotDillPath, useAsm: true),
89+
]);
90+
});
91+
}

‎runtime/vm/compiler/relocation.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ void CodeRelocator::Relocate(bool is_vm_isolate) {
8888
for (intptr_t i = 0; i < code_objects_->length(); ++i) {
8989
current_caller = (*code_objects_)[i];
9090

91-
const intptr_t code_text_offset = next_text_offset_;
92-
if (!AddInstructionsToText(current_caller.ptr())) {
91+
intptr_t code_text_offset;
92+
if (!AddInstructionsToText(current_caller.ptr(), &code_text_offset)) {
9393
continue;
9494
}
9595

@@ -144,7 +144,8 @@ void CodeRelocator::Relocate(bool is_vm_isolate) {
144144
// however we might need it to write information into V8 snapshot profile.
145145
}
146146

147-
bool CodeRelocator::AddInstructionsToText(CodePtr code) {
147+
bool CodeRelocator::AddInstructionsToText(CodePtr code,
148+
intptr_t* code_text_offset) {
148149
InstructionsPtr instructions = Code::InstructionsOf(code);
149150

150151
// If two [Code] objects point to the same [Instructions] object, we'll just
@@ -163,6 +164,7 @@ bool CodeRelocator::AddInstructionsToText(CodePtr code) {
163164
next_text_offset_ += padding_size;
164165
}
165166

167+
*code_text_offset = next_text_offset_;
166168
text_offsets_.Insert({instructions, next_text_offset_});
167169
commands_->Add(ImageWriterCommand(next_text_offset_, code));
168170
next_text_offset_ += ImageWriter::SizeInSnapshot(instructions);

‎runtime/vm/compiler/relocation.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ class CodeRelocator : public StackResource {
163163

164164
void FindLargestInstruction();
165165

166-
bool AddInstructionsToText(CodePtr code);
166+
bool AddInstructionsToText(CodePtr code, intptr_t* code_text_offset);
167167
void ScanCallTargets(const Code& code,
168168
const Array& call_targets,
169169
intptr_t code_text_offset);

0 commit comments

Comments
 (0)