Skip to content

IL tests checking DART_CONFIGURATION at runtime (e.g., using isSimulator) fail on android #53774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sstrickl opened this issue Oct 17, 2023 · 5 comments
Assignees
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). gardening

Comments

@sstrickl
Copy link
Contributor

There are new test failures on Reland "[vm/compiler] Change MemoryCopy to also take untagged addresses.".

The tests

vm/dart/pointer_as_typed_list_il_test RuntimeError (expected Pass)

are failing on configurations

vm-aot-android-release-arm64c
vm-aot-android-release-arm_x64

Example log:

--- Command "vm_compile_to_kernel" (took 01.000330s):
DART_CONFIGURATION=ReleaseAndroidARM64C /b/swarming/w/ir/pkg/vm/tool/gen_kernel --aot --platform=out/ReleaseAndroidARM64C/vm_platform_strong.dill -o /b/swarming/w/ir/out/ReleaseAndroidARM64C/generated_compilations/vm-aot-android-release-arm64c/runtime_tests_vm_dart_pointer_as_typed_list_il_test/out.dill /b/swarming/w/ir/runtime/tests/vm/dart/pointer_as_typed_list_il_test.dart -Dtest_runner.configuration=vm-aot-android-release-arm64c --packages=/b/swarming/w/ir/.dart_tool/package_config.json -Ddart.vm.product=false --sound-null-safety

exit code:
0

...

Executing adb -s HT7BG1A04039 shell export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/data/local/tmp/precompilation-testing/test;export TEST_COMPILATION_DIR=/data/local/tmp/precompilation-testing/test;/data/local/tmp/precompilation-testing/dart_precompiled_runtime --android-log-to-stderr --sound-null-safety -Dtest_runner.configuration=vm-aot-android-release-arm64c --ignore-unrecognized-flags --packages=/b/swarming/w/ir/.dart_tool/package_config.json /data/local/tmp/precompilation-testing/test/out.aotsnapshot ; echo AdbShellExitCode:  $?
Stdout:
AdbShellExitCode: 255
Stderr:
Unhandled exception:
Expected DART_CONFIGURATION to be defined
#0      isSimulator.<anonymous closure> (package:vm/testing/il_matchers.dart:571)
#1      isSimulator (package:vm/testing/il_matchers.dart:574)
#2      main (file:///b/swarming/w/ir/runtime/tests/vm/dart/pointer_as_typed_list_il_test.dart)
#3      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:295)
#4      _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:184)
ExitCode: 255
Time: 0:00:00.086481

--- Re-run this test:
python3 tools/test.py -n vm-aot-android-release-arm64c vm/dart/pointer_as_typed_list_il_test

So the checking part of the IL tests pass, but it fails when trying to run the actual program. As shown above, the DART_CONFIGURATION runtime variable isn't passed to tests running on Android. However, these tests are also compiled to kernel with -Dtest_runner.configuration, so perhaps we should use that as either the primary source of truth, falling back to DART_CONFIGURATION when not provided, or vice versa.

/cc @alexmarkov @mraleph

@sstrickl sstrickl added area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). gardening labels Oct 17, 2023
@sstrickl sstrickl self-assigned this Oct 17, 2023
@dcharkes
Copy link
Contributor

So, DART_CONFIGURATION is defined as the directory the build is performed in

'DART_CONFIGURATION': configuration.configurationDirectory,

And then test_runner.configuration is defined as the configuration name. The one that is passed into test.py as -n. (Which is not 1:1 with "builder"...)

"-Dtest_runner.configuration=${innerConfiguration.name}"

I think we should avoid special casing this test. Rather, we should ensure that both env vars are defined always in how all tests are run. Unless there's some reason why they aren't.

I believe some tests use artefacts from multiple build folders. E.g. with using a gensnapshot from the non-sim build folder and then running with the runtime from the sim build folder.

So that probably means the tests should solely rely on test_runner.configuration and that env var should be always set when all tests are run.

@sstrickl
Copy link
Contributor Author

Only checking test_runner.configuration is fine by me, I can easily adjust https://dart-review.googlesource.com/c/sdk/+/33074 accordingly.

@sstrickl
Copy link
Contributor Author

Turns out this won't work, because when the compare_il utility is run, it only gets DART_CONFIGURATION, not -Dtest_runner.configuration, whereas whether DART_CONFIGURATION is passed when running the compiled test itself varies on the target platform. So for now, going back to checking both.

@dcharkes
Copy link
Contributor

Shouldn't we fix that in pkg/test_runner so that compare_il does get a -Dtest_runner.configuration?

@sstrickl
Copy link
Contributor Author

Would like to fix the test ASAP, I'll not close the bug and look into this after landing the immediate fix.

copybara-service bot pushed a commit that referenced this issue Oct 17, 2023
Add RISCV32 to the list of architectures checked for
is32BitConfiguration.

TEST=vm/dart/pointer_as_typed_list_il_test on android trybots

Issue: #53774

Change-Id: I0c8297dc863b6121f9b1eafb99ae273ea2d0b34e
Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm_x64-try,vm-aot-android-release-arm64c-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330740
Reviewed-by: Daco Harkes <[email protected]>
Commit-Queue: Tess Strickland <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). gardening
Projects
None yet
Development

No branches or pull requests

2 participants