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.

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

[vm/aot] co19/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01 Crash #52422

Closed
dcharkes opened this issue May 17, 2023 · 3 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. crash Process exits with SIGSEGV, SIGABRT, etc. An unhandled exception is not a crash. gardening

Comments

@dcharkes
Copy link
Contributor

dcharkes commented May 17, 2023

There are new test failures on Issue 52409. Reading a Never typed getter makes the flow unreachable....[parser] Fix issue 52365 about parsing parenthesised functions followed by >> etc.

The tests

co19/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01 Crash (expected Pass)
co19/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t02 Crash (expected Pass)

are failing on configurations

vm-aot-linux-debug-simriscv64
vm-aot-linux-debug-x64
vm-aot-linux-debug-x64c
vm-aot-win-debug-x64c
--- Command "vm_compile_to_kernel" (took 02.000717s):
DART_CONFIGURATION=DebugX64 /b/s/w/ir/pkg/vm/tool/gen_kernel --aot --platform=out/DebugX64/vm_platform_strong.dill -o /b/s/w/ir/out/DebugX64/generated_compilations/vm-aot-linux-debug-x64/tests_co19_src_LanguageFeatures_Patterns_Exhaustiveness_exhaustiveness_enum_A01_t01/out.dill /b/s/w/ir/tests/co19/src/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01.dart -Dtest_runner.configuration=vm-aot-linux-debug-x64 --packages=/b/s/w/ir/.dart_tool/package_config.json -Ddart.vm.product=false --sound-null-safety

exit code:
0

--- Command "precompiler" (took 08.000460s):
DART_CONFIGURATION=DebugX64 out/DebugX64/gen_snapshot --snapshot-kind=app-aot-assembly --assembly=/b/s/w/ir/out/DebugX64/generated_compilations/vm-aot-linux-debug-x64/tests_co19_src_LanguageFeatures_Patterns_Exhaustiveness_exhaustiveness_enum_A01_t01/out.S --sound-null-safety -Dtest_runner.configuration=vm-aot-linux-debug-x64 --ignore-unrecognized-flags --packages=/b/s/w/ir/.dart_tool/package_config.json /b/s/w/ir/out/DebugX64/generated_compilations/vm-aot-linux-debug-x64/tests_co19_src_LanguageFeatures_Patterns_Exhaustiveness_exhaustiveness_enum_A01_t01/out.dill

exit code:
-6

stderr:
../../runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc: 5084: error: expected: dart::SimpleInstanceOfType(expression_type)
version=3.1.0-edge.bd0e77c4fcd773037ec93b0ac9bc14de277fa97d (be) (Wed May 17 03:28:07 2023 +0000) on "linux_x64"
pid=11482, thread=11482, isolate_group=isolate(0x558b62659930), isolate=(nil)((nil))
os=linux, arch=x64, comp=no, sim=no
isolate_instructions=0, vm_instructions=0
fp=7ffe219f2400, sp=7ffe219f22c8, pc=558b6153120c
  pc 0x0000558b6153120c fp 0x00007ffe219f2400 dart::Profiler::DumpStackTrace(void*)+0x7c
  pc 0x0000558b6126e0c4 fp 0x00007ffe219f24e0 dart::Assert::Fail(char const*, ...) const+0x84
  pc 0x0000558b61a62648 fp 0x00007ffe219f2570 out/DebugX64/gen_snapshot+0xe70648
  pc 0x0000558b61a60a94 fp 0x00007ffe219f26c0 dart::kernel::StreamingFlowGraphBuilder::BuildBinarySearchSwitch(dart::kernel::SwitchHelper*)+0x124
  pc 0x0000558b61a58aab fp 0x00007ffe219f2850 dart::kernel::StreamingFlowGraphBuilder::BuildSwitchStatement(dart::TokenPosition*)+0x2db
  pc 0x0000558b61a576f9 fp 0x00007ffe219f28c0 dart::kernel::StreamingFlowGraphBuilder::BuildLabeledStatement(dart::TokenPosition*)+0x69
  pc 0x0000558b61a5713a fp 0x00007ffe219f2920 dart::kernel::StreamingFlowGraphBuilder::BuildBlock(dart::TokenPosition*)+0x8a
  pc 0x0000558b61a4e3dc fp 0x00007ffe219f29c0 dart::kernel::StreamingFlowGraphBuilder::BuildFunctionBody(dart::Function const&, dart::LocalVariable*, bool)+0x1bc
  pc 0x0000558b61a4ec79 fp 0x00007ffe219f2b40 dart::kernel::StreamingFlowGraphBuilder::BuildGraphOfFunction(bool)+0x259
  pc 0x0000558b61a4f0bf fp 0x00007ffe219f2be0 dart::kernel::StreamingFlowGraphBuilder::BuildGraph()+0x17f
  pc 0x0000558b61a69811 fp 0x00007ffe219f2eb0 dart::kernel::FlowGraphBuilder::BuildGraph()+0x91
  pc 0x0000558b619ca71a fp 0x00007ffe219f3400 out/DebugX64/gen_snapshot+0xdd871a
  pc 0x0000558b619bf688 fp 0x00007ffe219f3490 out/DebugX64/gen_snapshot+0xdcd688
  pc 0x0000558b619cfc70 fp 0x00007ffe219f3580 out/DebugX64/gen_snapshot+0xdddc70
  pc 0x0000558b619c47bc fp 0x00007ffe219f36d0 out/DebugX64/gen_snapshot+0xdd27bc
  pc 0x0000558b619c42cc fp 0x00007ffe219f37c0 dart::FlowGraphInliner::Inline()+0x20c
  pc 0x0000558b61a2b6b4 fp 0x00007ffe219f3820 out/DebugX64/gen_snapshot+0xe396b4
  pc 0x0000558b61a2ad95 fp 0x00007ffe219f38f0 dart::CompilerPass::Run(dart::CompilerPassState*) const+0x125
  pc 0x0000558b61a2b247 fp 0x00007ffe219f3910 dart::CompilerPass::RunPipeline(dart::CompilerPass::PipelineMode, dart::CompilerPassState*)+0x97
  pc 0x0000558b618dd67b fp 0x00007ffe219f3ff0 dart::PrecompileParsedFunctionHelper::Compile(dart::CompilationPipeline*)+0x4cb
  pc 0x0000558b618de467 fp 0x00007ffe219f48b0 out/DebugX64/gen_snapshot+0xcec467
  pc 0x0000558b618d899c fp 0x00007ffe219f49d0 dart::Precompiler::CompileFunction(dart::Precompiler*, dart::Thread*, dart::Zone*, dart::Function const&)+0x1ac
  pc 0x0000558b618d738e fp 0x00007ffe219f4a70 dart::Precompiler::ProcessFunction(dart::Function const&)+0x1ae
  pc 0x0000558b618d19f4 fp 0x00007ffe219f4ac0 dart::Precompiler::Iterate()+0x94
  pc 0x0000558b618ce22f fp 0x00007ffe219f53d0 dart::Precompiler::DoCompileAll()+0x18ef
  pc 0x0000558b618cc896 fp 0x00007ffe219f5860 dart::Precompiler::CompileAll()+0xb6
  pc 0x0000558b61b0091f fp 0x00007ffe219f59c0 Dart_Precompile+0x1bf
  pc 0x0000558b612401db fp 0x00007ffe219f5b50 dart::bin::main(int, char**)+0x8eb
-- End of DumpStackTrace
=== Crash occurred when compiling file:///b/s/w/ir/tests/co19/src/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01.dart_::_main in AOT mode in Inlining pass
*** BEGIN CFG
Inlining
==== file:///b/s/w/ir/tests/co19/src/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01.dart_::_main (RegularFunction)
B0[graph]:0 {
      v0 <- Constant(#null) T{Null?}
      v2 <- Constant(#ok) T{_OneByteString}
      v3 <- Constant(#Instance of 'E<int>') T{E}
      v5 <- Constant(#Instance of 'E<double>') T{E}
      v7 <- Constant(#a) T{_OneByteString}
      v9 <- Constant(#c) T{_OneByteString}
}
B1[function entry]:2
    CheckStackOverflow:8(stack=0, loop=0)
    DebugStepCheck:10()
    v4 <- StaticCall:12( test1<0> v3, result_type = T{_OneByteString?}) T{_OneByteString}
    StaticCall:14( equals<0> v2, v4)
    v6 <- StaticCall:16( test1<0> v5, result_type = T{_OneByteString?}) T{_OneByteString}
    StaticCall:18( equals<0> v2, v6)
    v8 <- StaticCall:20( test2<0> v3, result_type = T{_OneByteString}) T{_OneByteString}
    StaticCall:22( equals<0> v7, v8)
    v10 <- StaticCall:24( test2<0> v5, result_type = T{_OneByteString}) T{_OneByteString}
    StaticCall:26( equals<0> v9, v10)
    DebugStepCheck:28()
    Return:30(v0)
*** END CFG

--- Re-run this test:
python3 tools/test.py -n vm-aot-linux-debug-x64 co19/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01

log

repro:

$ tools/build.py -ax64 dart_precompiled_runtime create_platform_sdk && tools/test.py -n vm-aot-linux-debug-x64 co19/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01 -k

@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. gardening crash Process exits with SIGSEGV, SIGABRT, etc. An unhandled exception is not a crash. labels May 17, 2023
@mkustermann
Copy link
Member

Possibly related to db89fe0? @alexmarkov Could you take a look?

@dcharkes
Copy link
Contributor Author

I already have a fix: https://dart-review.googlesource.com/c/sdk/+/304002

@dcharkes
Copy link
Contributor Author

Hm, the test is an enum with type arguments:

import "../../../Utils/expect.dart";

enum E<T> {
  a<int>(),
  b<String>(),
  c<double>(),
}

String test1(E<num> e) {
  switch (e) {
    case E.a:
    case E.c:
      return "ok";
  }
}

String test2(E<num> e) =>
  switch (e) {
    E.a => "a",
    E.c => "c"
  };

main() {
  Expect.equals("ok", test1(E.a));
  Expect.equals("ok", test1(E.c));
  Expect.equals("a", test2(E.a));
  Expect.equals("c", test2(E.c));
}

TIL that we can have type arguments in enum values. 🤯

We're doing BuildBinarySearchSwitch on the switch.

I wouldn't think the type arguments are important at all here. Wouldn't you want to check instance equality rather than the type?

From reading the code&comments, it looks like enums should be jump tables instead of binary search?

copybara-service bot pushed a commit that referenced this issue May 18, 2023
…itches"

This reverts commit 43ce548.

Reason for revert: test failures on vm-aot-*-product-* bots.

Original change's description:
> [vm/compiler] Remove unnecessary type check from optimized switches
>
> Switch optimization now relies on static type of the tested value,
> so it is safe to omit the type check (with sound null safety) or
> reduce it to a null check (without sound null safety).
>
> TEST=co19/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01
> Fixes #52422
>
> Change-Id: Ic93f4f212bee9ed3bfe5035f3c8d7535274c2f63
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304102
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
> Reviewed-by: Alexander Aprelev <aam@google.com>

Change-Id: Ifa6ee75be49f7264fa4dfc08183d5ccb731c60d1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304321
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Alexander Markov <alexmarkov@google.com>
copybara-service bot pushed a commit that referenced this issue May 18, 2023
…itches"

This is a reland of commit 43ce548

In addition to removing type check, switch range checks are adjusted
in order to avoid call to 'int.operator<=', which can be removed by
tree shaker in AOT/product mode, causing NoSuchMethodError at runtime.

Original change's description:
> [vm/compiler] Remove unnecessary type check from optimized switches
>
> Switch optimization now relies on static type of the tested value,
> so it is safe to omit the type check (with sound null safety) or
> reduce it to a null check (without sound null safety).
>
> TEST=co19/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01
> Fixes #52422
>
> Change-Id: Ic93f4f212bee9ed3bfe5035f3c8d7535274c2f63
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304102
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
> Reviewed-by: Alexander Aprelev <aam@google.com>

TEST=co19/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01
TEST=language/async/switch_test

Change-Id: I70bf480bf376f946d66caa504120c8f85093ea8d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304322
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. crash Process exits with SIGSEGV, SIGABRT, etc. An unhandled exception is not a crash. gardening
Projects
None yet
Development

No branches or pull requests

3 participants