Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Failures new nnbd VM builders on ia32 #43613

Closed
a-siva opened this issue Sep 30, 2020 · 9 comments
Closed

Failures new nnbd VM builders on ia32 #43613

a-siva opened this issue Sep 30, 2020 · 9 comments
Assignees
@alexmarkov
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. NNBD Issues related to NNBD Release
Projects
Dart VM support for Null Safe feature

Comments

@a-siva
Copy link
Contributor

a-siva commented Sep 30, 2020

There are new test failures on [infra] Add additional nnbd VM builders.

vm/dart/type_casts_with_null_safety_autodetection_test
vm/dart/appjit_determinism_test
vm/dart/appjit_cha_deopt_test
vm/dart/appjit_load_static_licm_test
vm/dart/product_aot_kernel_test
lib/isolate/nnbd_spawn_autodetect_1_test
lib/isolate/detect_nullsafety_1_test
lib/isolate/nnbd_spawn_autodetect_2_test
lib/isolate/detect_nullsafety_2_test
@a-siva a-siva added area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. NNBD Issues related to NNBD Release labels Sep 30, 2020
@a-siva a-siva added this to Needs triage in Dart VM support for Null Safe feature via automation Sep 30, 2020
@a-siva a-siva mentioned this issue Sep 30, 2020
Closed
@alexmarkov alexmarkov self-assigned this Sep 30, 2020
@alexmarkov
Copy link
Contributor

Triaging vm/dart failures:

vm/dart/type_casts_with_null_safety_autodetection_test is just slow in debug mode as it runs with --optimization-counter-threshold=10 and on IA32 we don't use kernel service snapshot.

Failures

vm/dart/appjit_determinism_test
vm/dart/appjit_cha_deopt_test
vm/dart/appjit_load_static_licm_test

are extracted to a separate issue #43626.

vm/dart/product_aot_kernel_test fails in strong mode as it depends on package:kernel, package:vm which are not migrated yet. The same compile-time error happens on x64 and tracked in #42146.

@alexmarkov
Copy link
Contributor

lib/isolate/nnbd_spawn_autodetect_1_test and lib/isolate/detect_nullsafety_1_test are also failing due to #43626.

lib/isolate/nnbd_spawn_autodetect_2_test and lib/isolate/detect_nullsafety_2_test are timing out. These tests also create app-jit snapshots, but they dodged #43626 and were able to create snapshots. On IA32 those snapshots have Snapshot::kFull kind and do not contain the code. When running from those snapshots, null safety mode auto-detection doesn't query mode from the snapshot as snapshot doesn't contain code:

sdk/runtime/vm/dart.cc

Lines 780 to 788 in f54c0a3

// If snapshot is an appJIT/AOT snapshot we will figure out the mode by
// sniffing the feature string in the snapshot.
if (snapshot_data != nullptr) {
// Read the snapshot and check for null safety option.
const Snapshot* snapshot = Snapshot::SetupFromBuffer(snapshot_data);
if (Snapshot::IncludesCode(snapshot->kind())) {
return SnapshotHeaderReader::NullSafetyFromSnapshot(snapshot);
}
}

After that, auto-detection logic falls back to kernel service for detection and passes snapshot as the main script URI, and CFE hangs while trying to parse snapshot file as a main .dart file.

It seems like we need to embed null safety mode into Snapshot::kFull application snapshots on IA32, and detect mode from such snapshots even if they do not contain code. We shouldn't really try to call kernel service if running from snapshot.

@a-siva
Copy link
Contributor Author

a-siva commented Oct 1, 2020

Another ia32 specific failure co19/LibTest/isolate/Isolate/spawn_A04_t04

@a-siva
Copy link
Contributor Author

a-siva commented Oct 1, 2020

Another ia32 specific failure co19/Language/Libraries_and_Scripts/Scripts/top_level_main_t01

@alexmarkov
Copy link
Contributor

standalone/regress_42092_test times out on dartk-weak-asserts-linux-release-ia32 (maybe flaky). Log: https://dart-ci.appspot.com/log/vm-kernel-nnbd-linux-release-ia32/dartk-weak-asserts-linux-release-ia32/3/standalone/regress_42092_test

@alexmarkov
Copy link
Contributor

co19/LibTest/isolate/Isolate/spawn_A04_t04 is flaky, similarly to co19_2/LibTest/isolate/Isolate/spawn_A04_t04 and not specific to ia32 nor nnbd (fails on all architectures and both legacy and nnbd bots).

co19/Language/Libraries_and_Scripts/Scripts/top_level_main_t01 fails on all architectures, corresponding legacy test co19_2/Language/Libraries_and_Scripts/Scripts/top_level_main_t01 fails similarly.

@alexmarkov
Copy link
Contributor

standalone/regress_42092_test is also flaky and failing on all architectures and both nnbd and legacy bots.

dart-bot pushed a commit that referenced this issue Oct 7, 2020
@alexmarkov
[tests/nnbd] Mark test as slow in debug mode, add diagnostics
34e8b42 
Issue: #43613
Change-Id: I12fe82b3dc2eced6e0c582ecc487ef140e5edc40
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/165365
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
dart-bot pushed a commit that referenced this issue Oct 8, 2020
@alexmarkov
[vm/nnbd] Add separate Snapshot::Kind for core snapshots
d77fff7 
Core snapshots should be agnostic to the sound null safety mode
(so they can be used both in weak and strong modes), and snapshot
writer verifies that.

Snapshot::kFull was previously used both for core snapshots and
app snapshots on ia32. However, app snapshots are not guaranteed to
be agnostic, which appeared as failures on a few test on ia32.
Also, VM should be able to detect null safety mode from app snapshots,
even if they do not contain code, but null safety mode was not
written into features string of kFull snapshots.

In order to disambiguate core snapshots, a new Snapshot::Kind is
added. Snapshot::kFullCore works exactly as Snapshot::kFull, except
for verification of agnostic null safety and snapshot features string
omitting null safety mode. All snapshots except kFullCore now have
null safety mode included into their features string.

Fixes #43626
Issue #43613

Change-Id: I8cd3b049ef4e428dd5e1ce666d4c7aa3b596d70c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166308
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
@alexmarkov
Copy link
Contributor

Summary:

vm/dart/type_casts_with_null_safety_autodetection_test is addressed in 34e8b42.

The tests

vm/dart/appjit_determinism_test
vm/dart/appjit_cha_deopt_test
vm/dart/appjit_load_static_licm_test
lib/isolate/nnbd_spawn_autodetect_1_test
lib/isolate/detect_nullsafety_1_test
lib/isolate/nnbd_spawn_autodetect_2_test
lib/isolate/detect_nullsafety_2_test

are fixed in d77fff7.

vm/dart/product_aot_kernel_test is not specific to ia32 and tracked in #42146.

Other failures mentioned above are not specific to NNBD, and not specific to ia32.

Dart VM support for Null Safe feature automation moved this from Needs triage to Done Oct 8, 2020
@sstrickl
Copy link
Contributor

It looks like vm/dart/type_casts_with_null_safety_autodetection_test has still been at least flakily timing out since Oct 28 in release mode, and finally went red today (log). Will also mark it Slow in release mode.

copybara-service bot pushed a commit that referenced this issue Feb 23, 2022
@sstrickl
[gardening] Mark test as slow on NNBD IA32 release.
cf38c6a 
Bug: #43613
Change-Id: If17ac86e9348053aee345778d46c890700f3e6bd
Cq-Include-Trybots: luci.dart.try:vm-kernel-nnbd-win-release-ia32-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/234040
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alexmarkov alexmarkov

Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. NNBD Issues related to NNBD Release
Projects
Dart VM support for Null Safe feature
  
Done
Milestone
No milestone
Development

No branches or pull requests
3 participants
@sstrickl @a-siva @alexmarkov