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

[SEH] Add test to track EHa register liveness verification #76921

Merged
merged 3 commits into from
Jan 4, 2024

Conversation

HaohaiWen
Copy link
Contributor

This test tracks bug of MachineVerifier to check live range segment for
EHa. Async exception can happen at any place within seh scope, not only
the call instruction. Need to teach MachineVerifier to know that.

This test tracks bug of MachineVerifier to check live range segment for
EHa. Async exception can happen at any place within seh scope, not only
the call instruction. Need to teach MachineVerifier to know that.
@llvmbot
Copy link
Member

llvmbot commented Jan 4, 2024

@llvm/pr-subscribers-backend-x86

Author: None (HaohaiWen)

Changes

This test tracks bug of MachineVerifier to check live range segment for
EHa. Async exception can happen at any place within seh scope, not only
the call instruction. Need to teach MachineVerifier to know that.


Full diff: https://github.com/llvm/llvm-project/pull/76921.diff

1 Files Affected:

  • (added) llvm/test/CodeGen/X86/windows-seh-EHa-RegisterLiveness.ll (+69)
diff --git a/llvm/test/CodeGen/X86/windows-seh-EHa-RegisterLiveness.ll b/llvm/test/CodeGen/X86/windows-seh-EHa-RegisterLiveness.ll
new file mode 100644
index 00000000000000..d23318c6e16a11
--- /dev/null
+++ b/llvm/test/CodeGen/X86/windows-seh-EHa-RegisterLiveness.ll
@@ -0,0 +1,69 @@
+; XFAIL: *
+; RUN: llc --verify-machineinstrs < %s | FileCheck %s
+source_filename = "test.cpp"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.12.0"
+
+$"?test@Test@@Plugin@@Host@@@Z" = comdat any
+
+declare dso_local i32 @__CxxFrameHandler3(...)
+
+; Function Attrs: nounwind memory(none)
+declare dso_local void @llvm.seh.scope.begin() #1
+
+; Function Attrs: nobuiltin allocsize(0)
+declare dso_local noundef nonnull ptr @"??2@Test@Z"(i64 noundef) #1
+
+; Function Attrs: nounwind memory(none)
+declare dso_local void @llvm.seh.scope.end() #0
+
+; Function Attrs: nobuiltin nounwind
+declare dso_local void @"??3@YAXPEAX@Z"(ptr noundef) #2
+
+; Function Attrs: mustprogress uwtable
+define weak_odr dso_local noundef ptr @"?test@Test@@Plugin@@Host@@@Z"(ptr noundef nonnull align 8 dereferenceable(48) %this, ptr noundef %host) unnamed_addr #3 comdat align 2 personality ptr @__CxxFrameHandler3 {
+entry:
+  %host.addr = alloca ptr, align 8
+  %this.addr = alloca ptr, align 8
+  store ptr %host, ptr %host.addr, align 8
+  store ptr %this, ptr %this.addr, align 8
+  %this1 = load ptr, ptr %this.addr, align 8
+  %call = call noalias noundef nonnull ptr @"??2@Test@Z"(i64 noundef 152) #5
+  invoke void @llvm.seh.scope.begin()
+          to label %invoke.cont unwind label %ehcleanup
+
+invoke.cont:                                      ; preds = %entry
+  %call3 = invoke noundef ptr @"??Test@?A0x2749C4FD@@QEAA@Test@Test@@@Z"(ptr noundef nonnull align 8 dereferenceable(152) %call, ptr noundef %this1)
+          to label %invoke.cont2 unwind label %ehcleanup
+
+invoke.cont2:                                     ; preds = %invoke.cont
+  invoke void @llvm.seh.scope.end()
+          to label %invoke.cont4 unwind label %ehcleanup
+
+invoke.cont4:                                     ; preds = %invoke.cont2
+  ret ptr %call
+
+ehcleanup:                                        ; preds = %invoke.cont2, %invoke.cont, %entry
+  %0 = cleanuppad within none []
+  call void @"??3@YAXPEAX@Z"(ptr noundef %call) #6 [ "funclet"(token %0) ]
+  cleanupret from %0 unwind to caller
+}
+
+; Function Attrs: uwtable
+declare hidden noundef ptr @"??Test@?A0x2749C4FD@@QEAA@Test@Test@@@Z"(ptr noundef nonnull returned align 8 dereferenceable(152), ptr noundef) unnamed_addr #4 align 2
+
+attributes #0 = { nounwind memory(none) }
+attributes #1 = { nobuiltin allocsize(0) "target-cpu"="x86-64" "target-features"="+cmov,+crc32,+cx8,+fxsr,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87" "tune-cpu"="generic" }
+attributes #2 = { nobuiltin nounwind "target-cpu"="x86-64" "target-features"="+cmov,+crc32,+cx8,+fxsr,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87" "tune-cpu"="generic" }
+attributes #3 = { mustprogress uwtable "target-cpu"="x86-64" "target-features"="+cmov,+crc32,+cx8,+fxsr,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87" "tune-cpu"="generic" }
+attributes #4 = { uwtable "target-cpu"="x86-64" "target-features"="+cmov,+crc32,+cx8,+fxsr,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87" "tune-cpu"="generic" }
+attributes #5 = { builtin allocsize(0) }
+attributes #6 = { builtin nounwind }
+
+!llvm.module.flags = !{!1, !2, !3, !4, !5}
+
+!1 = !{i32 1, !"wchar_size", i32 2}
+!2 = !{i32 2, !"eh-asynch", i32 1}
+!3 = !{i32 8, !"PIC Level", i32 2}
+!4 = !{i32 7, !"uwtable", i32 2}
+!5 = !{i32 1, !"MaxTLSAlign", i32 65536}

@HaohaiWen
Copy link
Contributor Author


# After Register Coalescer
********** INTERVALS **********
CH [0B,32r:0)[112r,128r:3)[304r,336r:2)[544r,560r:1) 0@0B-phi 1@544r 2@304r 3@112r
CL [0B,32r:0)[112r,128r:3)[304r,336r:2)[544r,560r:1) 0@0B-phi 1@544r 2@304r 3@112r
DH [0B,16r:0)[320r,336r:1) 0@0B-phi 1@320r
DL [0B,16r:0)[320r,336r:1) 0@0B-phi 1@320r
HCX [0B,32r:0)[112r,128r:3)[304r,336r:2)[544r,560r:1) 0@0B-phi 1@544r 2@304r 3@112r
HDX [0B,16r:0)[320r,336r:1) 0@0B-phi 1@320r
%0 [32r,320r:0) 0@32r  weight:0.000000e+00
%1 [160r,464r:0)[496B,544r:0) 0@160r  weight:0.000000e+00
%3 [16r,64r:0) 0@16r  weight:0.000000e+00
RegMasks: 128r 336r 496B 560r
********** MACHINEINSTRS **********
# Machine code for function ?test@Test@@Plugin@@Host@@@Z: NoPHIs, TracksLiveness, TiedOpsRewritten
Frame Objects:
  fi#0: size=8, align=8, at location [SP+8]
  fi#1: size=8, align=8, at location [SP+8]
Function Live Ins: $rcx in %2, $rdx in %3

0B	bb.0.entry:
	  successors: %bb.1(0x7ffff800), %bb.4(0x00000800); %bb.1(100.00%), %bb.4(0.00%)
	  liveins: $rcx, $rdx
16B	  %3:gr64 = COPY $rdx
32B	  %0:gr64 = COPY $rcx
48B	  EH_LABEL <mcsymbol .Ltmp2>
64B	  MOV64mr %stack.0.host.addr, 1, $noreg, 0, $noreg, %3:gr64 :: (store (s64) into %ir.host.addr)
80B	  MOV64mr %stack.1.this.addr, 1, $noreg, 0, $noreg, %0:gr64 :: (store (s64) into %ir.this.addr)
96B	  ADJCALLSTACKDOWN64 32, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
112B	  $rcx = MOV32ri64 152
128B	  CALL64pcrel32 @"??2@Test@Z", <regmask $bh $bl $bp $bph $bpl $bx $di $dih $dil $ebp $ebx $edi $esi $hbp $hbx $hdi $hsi $rbp $rbx $rdi $rsi $si $sih $sil $r12 $r13 $r14 $r15 $xmm6 $xmm7 $xmm8 $xmm9 $xmm10 and 25 more...>, implicit $rsp, implicit $ssp, implicit $rcx, implicit-def $rsp, implicit-def $ssp, implicit-def $rax
144B	  ADJCALLSTACKUP64 32, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
160B	  %1:gr64 = COPY killed $rax
192B	  EH_LABEL <mcsymbol .Ltmp3>
208B	  JMP_1 %bb.1

224B	bb.1.invoke.cont:
	; predecessors: %bb.0
	  successors: %bb.2(0x7ffff800), %bb.4(0x00000800); %bb.2(100.00%), %bb.4(0.00%)

240B	  EH_LABEL <mcsymbol .Ltmp4>
256B	  EH_LABEL <mcsymbol .Ltmp0>
288B	  ADJCALLSTACKDOWN64 32, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
304B	  $rcx = COPY %1:gr64
320B	  $rdx = COPY %0:gr64
336B	  CALL64pcrel32 @"??Test@?A0x2749C4FD@@QEAA@Test@Test@@@Z", <regmask $bh $bl $bp $bph $bpl $bx $di $dih $dil $ebp $ebx $edi $esi $hbp $hbx $hdi $hsi $rbp $rbx $rdi $rsi $si $sih $sil $r12 $r13 $r14 $r15 $xmm6 $xmm7 $xmm8 $xmm9 $xmm10 and 25 more...>, implicit $rsp, implicit $ssp, implicit $rcx, implicit $rdx, implicit-def $rsp, implicit-def $ssp, implicit-def dead $rax
352B	  ADJCALLSTACKUP64 32, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
368B	  EH_LABEL <mcsymbol .Ltmp1>
384B	  EH_LABEL <mcsymbol .Ltmp5>
400B	  JMP_1 %bb.2

416B	bb.2.invoke.cont2:
	; predecessors: %bb.1
	  successors: %bb.3(0x7ffff800), %bb.4(0x00000800); %bb.3(100.00%), %bb.4(0.00%)

432B	  JMP_1 %bb.3

448B	bb.3.invoke.cont4:
	; predecessors: %bb.2

464B	  $rax = COPY %1:gr64
480B	  RET 0, killed $rax

496B	bb.4.ehcleanup (machine-block-address-taken, landing-pad, ehfunclet-entry):
	; predecessors: %bb.0, %bb.1, %bb.2

512B	  EH_LABEL <mcsymbol .Ltmp6>
528B	  ADJCALLSTACKDOWN64 32, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
544B	  $rcx = COPY %1:gr64
560B	  CALL64pcrel32 @"??3@YAXPEAX@Z", <regmask $bh $bl $bp $bph $bpl $bx $di $dih $dil $ebp $ebx $edi $esi $hbp $hbx $hdi $hsi $rbp $rbx $rdi $rsi $si $sih $sil $r12 $r13 $r14 $r15 $xmm6 $xmm7 $xmm8 $xmm9 $xmm10 and 25 more...>, implicit $rsp, implicit $ssp, implicit $rcx, implicit-def $rsp, implicit-def $ssp
576B	  ADJCALLSTACKUP64 32, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
592B	  EH_LABEL <mcsymbol .Ltmp7>
608B	  CLEANUPRET

# End machine code for function ?test@Test@@Plugin@@Host@@@Z.

*** Bad machine code: Register not marked live out of predecessor ***
- function:    ?test@Test@@Plugin@@Host@@@Z
- basic block: %bb.0 entry (0x363f3c8) [0B;224B)
- liverange:   [160r,464r:0)[496B,544r:0) 0@160r
- v. register: %1
- ValNo:       0 (def 160r)
 live into %bb.4@496B, not live before 128d
LLVM ERROR: Found 1 machine code errors.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: llc --verify-machineinstrs
1.	Running pass 'Function Pass Manager' on module '<stdin>'.
2.	Running pass 'Verify generated machine code' on function '@"?test@Test@@Plugin@@Host@@@Z"'
 #0 0x0000000001b00f98 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /export/users2/haohaiwe/compilers/llvm-project/llvm/lib/Support/Unix/Signals.inc:727:3
 #1 0x0000000001afea4c llvm::sys::RunSignalHandlers() /export/users2/haohaiwe/compilers/llvm-project/llvm/lib/Support/Signals.cpp:105:20
 #2 0x0000000001afed86 SignalHandler(int) /export/users2/haohaiwe/compilers/llvm-project/llvm/lib/Support/Unix/Signals.inc:413:1
 #3 0x00007fb166bf4db0 __restore_rt (/lib64/libc.so.6+0x59db0)
 #4 0x00007fb166c4142c __pthread_kill_implementation (/lib64/libc.so.6+0xa642c)
 #5 0x00007fb166bf4d06 gsignal (/lib64/libc.so.6+0x59d06)
 #6 0x00007fb166bc77d3 abort (/lib64/libc.so.6+0x2c7d3)
 #7 0x000000000064d9ce llvm::ConvertUTF8toUTF32Impl(unsigned char const**, unsigned char const*, unsigned int**, unsigned int*, llvm::ConversionFlags, unsigned char) (.cold) /export/users2/haohaiwe/compilers/llvm-project/llvm/lib/Support/ConvertUTF.cpp:691:9
 #8 0x0000000000ff978b (/export/users2/haohaiwe/compilers/llvm-project/builds/bin/llc+0xff978b)
 #9 0x0000000000ff97bb (anonymous namespace)::MachineVerifierPass::runOnMachineFunction(llvm::MachineFunction&) /export/users2/haohaiwe/compilers/llvm-project/llvm/lib/CodeGen/MachineVerifier.cpp:328:5
#10 0x0000000000ef20c7 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (.part.0) /export/users2/haohaiwe/compilers/llvm-project/llvm/lib/CodeGen/MachineFunctionPass.cpp:93:33
#11 0x000000000142ceae llvm::FPPassManager::runOnFunction(llvm::Function&) /export/users2/haohaiwe/compilers/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1453:7
#12 0x000000000142d0d1 llvm::ilist_node_base<true>::getNext() const /export/users2/haohaiwe/compilers/llvm-project/llvm/include/llvm/ADT/ilist_node_base.h:43:45
#13 0x000000000142d0d1 llvm::ilist_node_impl<llvm::ilist_detail::node_options<llvm::Function, true, false, void, false>>::getNext() /export/users2/haohaiwe/compilers/llvm-project/llvm/include/llvm/ADT/ilist_node.h:94:66
#14 0x000000000142d0d1 llvm::ilist_iterator<llvm::ilist_detail::node_options<llvm::Function, true, false, void, false>, false, false>::operator++() /export/users2/haohaiwe/compilers/llvm-project/llvm/include/llvm/ADT/ilist_iterator.h:157:25
#15 0x000000000142d0d1 llvm::FPPassManager::runOnModule(llvm::Module&) /export/users2/haohaiwe/compilers/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1488:22
#16 0x000000000142d91e runOnModule /export/users2/haohaiwe/compilers/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1565:7
#17 0x000000000142d91e llvm::legacy::PassManagerImpl::run(llvm::Module&) /export/users2/haohaiwe/compilers/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:541:55
#18 0x00000000006c946d compileModule(char**, llvm::LLVMContext&) /export/users2/haohaiwe/compilers/llvm-project/llvm/tools/llc/llc.cpp:751:34
#19 0x0000000000652206 main /export/users2/haohaiwe/compilers/llvm-project/llvm/tools/llc/llc.cpp:425:35
#20 0x00007fb166bdfe50 __libc_start_call_main (/lib64/libc.so.6+0x44e50)
#21 0x00007fb166bdfefc __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x44efc)
#22 0x00000000006c09a5 _start (/export/users2/haohaiwe/compilers/llvm-project/builds/bin/llc+0x6c09a5)

@HaohaiWen
Copy link
Contributor Author

%1 lives before entering seh.scope.begin. It's valid.

Copy link
Contributor

@phoebewang phoebewang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

!llvm.module.flags = !{!1, !2, !3, !4, !5}

!1 = !{i32 1, !"wchar_size", i32 2}
!2 = !{i32 2, !"eh-asynch", i32 1}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can leave it only.

declare dso_local void @llvm.seh.scope.begin() #1

; Function Attrs: nobuiltin allocsize(0)
declare dso_local noundef nonnull ptr @"??2@Test@Z"(i64 noundef) #1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the dso_local, noundef etc.

declare dso_local void @"??3@YAXPEAX@Z"(ptr noundef) #2

; Function Attrs: mustprogress uwtable
define weak_odr dso_local noundef ptr @"?test@Test@@Plugin@@Host@@@Z"(ptr noundef nonnull align 8 dereferenceable(48) %this, ptr noundef %host) unnamed_addr #3 comdat align 2 personality ptr @__CxxFrameHandler3 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.

@HaohaiWen HaohaiWen merged commit e147dcb into llvm:main Jan 4, 2024
@HaohaiWen HaohaiWen deleted the eha-bug branch January 4, 2024 12:49
aheejin added a commit to aheejin/llvm-project that referenced this pull request Mar 17, 2025
This effectively reverts
llvm@61d22f2.

llvm@61d22f2
claims that when the predecessor unwinds to an EH pad, we should take
the live-out values of not the predecessor's terminator but the last
call. But if the call has a possibility to unwind to an EH pad, it means
it was an `invoke` before and it should either be the terminating
instruction of a BB or be right before a branch, which is the
terminator. If a call is in a middle of a BB, that means the call was
not an `invoke` before and it unwinds to the caller if it throws.

This can fail in the case when there are unwinding instructions that are
not calls, such as `CLEANUPRET` or other target-specific throwing
instructions (e.g. Wasm's `throw`/`rethrow`/`throw_ref`). For example,
as in the test case attached,
```mir
bb.2 (landing-pad):
  ...
  CALL @foo
  %0 = CONST_I32 0
  CLEANUPRET %bb.3

bb.3 (landing-pad):
  call @bar, %0
  ...
```
When examining the live range of `bb.3`'s `%0`, we should check the
live-outs at `CLEANUPRET`, and not `CALL @foo`. But because of this
checking routine this PR deletes, this ends up passing `CLEANUPRET` and
`%0 = CONST_I32 0` and search for `%0`'s liveout at `CALL @foo`, which
is incorrect and results in a validation error.
```cpp
      // Predecessor of landing pad live-out on last call.
      if (MFI->isEHPad()) {
        for (const MachineInstr &MI : llvm::reverse(*Pred)) {
          if (MI.isCall()) {
            PEnd = Indexes->getInstructionIndex(MI).getBoundaryIndex();
            break;
          }
        }
      }
```

Also I'm not sure if I understand the description about
`llvm.experimental.gc.statepoint` intrinsic given in
llvm@61d22f2.
There doesn't seem to be any special thing about that intrinsic; if it
can unwind to an EH pad, it would be at the end of a BB. If it unwinds
to the caller, it shouldn't be where we take the live-outs from. Both
the test case `statepoint-invoke-ra1.ll` given in that commit, and also
https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/X86/statepoint-invoke-ra.mir
that replaced it in
llvm@2e1150d,
seem to pass even with this PR (because it was an `invoke` and at the
end of a BB anyway).

---

This also deletes `windows-seh-EHa-RegisterLiveness.ll `, a test case
added in llvm#76921. It looks that test case was added to track a
MachineVerifier bug, which I guess may be the same thing this PR is
trying to fix. After this PR, that test unexpectedly succeeds, which I
think means we can delete it. (There is also another PR (llvm#76933) that
tried to fix this bug in another way, which still has not landed.)

---

This bug is discovered from the reproducer in llvm#126916, even though this
is not the bug reported there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants