[libc++abi] Fix lpStart adjustment for exceptions table #72727
Conversation
When lpStartEncoding is different from DW_EH_PE_omit, lpStart can be set to zero which is a valid base address for landing pads. Such base value is useful when landing pads are placed in different sections. Fixes llvm#72582.
|
@llvm/pr-subscribers-libcxxabi Author: Maksim Panchenko (maksfb) ChangesWhen lpStartEncoding is different from DW_EH_PE_omit, lpStart can be set to zero which is a valid base address for landing pads. Such base value is useful when landing pads are placed in different sections. Fixes #72582. Full diff: https://github.com/llvm/llvm-project/pull/72727.diff 3 Files Affected:
diff --git a/libcxxabi/src/cxa_personality.cpp b/libcxxabi/src/cxa_personality.cpp
index 4570d0b5beb2e2e..4b6c4edbc266985 100644
--- a/libcxxabi/src/cxa_personality.cpp
+++ b/libcxxabi/src/cxa_personality.cpp
@@ -660,10 +660,9 @@ static void scan_eh_tab(scan_results &results, _Unwind_Action actions,
// dwarf emission
// Parse LSDA header.
uint8_t lpStartEncoding = *lsda++;
- const uint8_t* lpStart =
- (const uint8_t*)readEncodedPointer(&lsda, lpStartEncoding, base);
- if (lpStart == 0)
- lpStart = (const uint8_t*)funcStart;
+ const uint8_t* lpStart = lpStartEncoding == DW_EH_PE_omit
+ ? (const uint8_t*)funcStart
+ : (const uint8_t*)readEncodedPointer(&lsda, lpStartEncoding, base);
uint8_t ttypeEncoding = *lsda++;
if (ttypeEncoding != DW_EH_PE_omit)
{
diff --git a/libcxxabi/test/native/x86_64-unknown-linux/lit.local.cfg b/libcxxabi/test/native/x86_64-unknown-linux/lit.local.cfg
new file mode 100644
index 000000000000000..bf3abe0d4881d98
--- /dev/null
+++ b/libcxxabi/test/native/x86_64-unknown-linux/lit.local.cfg
@@ -0,0 +1,9 @@
+def is_x86_64_linux(triple):
+ return ("x86_64" in triple) and ("linux" in triple)
+
+
+is_native = hasattr(config.root, "target_triple") and (
+ config.root.host_triple == config.root.target_triple
+)
+if not is_native or not is_x86_64_linux(config.root.host_triple):
+ config.unsupported = True
diff --git a/libcxxabi/test/native/x86_64-unknown-linux/lpstart-zero.pass.sh.s b/libcxxabi/test/native/x86_64-unknown-linux/lpstart-zero.pass.sh.s
new file mode 100644
index 000000000000000..3fd9102b340a13b
--- /dev/null
+++ b/libcxxabi/test/native/x86_64-unknown-linux/lpstart-zero.pass.sh.s
@@ -0,0 +1,107 @@
+# RUN: %{cxx} %{flags} %{link_flags} -no-pie %s -o %t.exe
+# RUN: %t.exe
+# UNSUPPORTED: no-exceptions
+
+# PURPOSE: Check that libc++abi works correctly when LPStart address is
+# explicitly set to zero.
+
+# This file is generated from the following C++ source code.
+#
+# ```
+# int main() {
+# try {
+# throw 42;
+# } catch (...) {
+# return 0;
+# }
+# return 1;
+# }
+# ```
+# The exception table is modified to use udata4 encoding for LPStart and
+# sdata4 encoding for call sites.
+
+ .text
+ .globl main # -- Begin function main
+ .p2align 4, 0x90
+ .type main,@function
+main: # @main
+.Lfunc_begin0:
+ .cfi_startproc
+ .globl __gxx_personality_v0
+ .cfi_personality 3, __gxx_personality_v0
+ .cfi_lsda 27, .Lexception0
+# %bb.0: # %entry
+ pushq %rbp
+ .cfi_def_cfa_offset 16
+ .cfi_offset %rbp, -16
+ movq %rsp, %rbp
+ .cfi_def_cfa_register %rbp
+ subq $32, %rsp
+ movl $0, -4(%rbp)
+ movl $4, %edi
+ callq __cxa_allocate_exception@PLT
+ movq %rax, %rdi
+ movl $42, (%rdi)
+.Ltmp0:
+ movq _ZTIi@GOTPCREL(%rip), %rsi
+ xorl %eax, %eax
+ movl %eax, %edx
+ callq __cxa_throw@PLT
+.Ltmp1:
+ jmp .LBB0_4
+.LBB0_1: # %lpad
+.Ltmp2:
+ movq %rax, %rcx
+ movl %edx, %eax
+ movq %rcx, -16(%rbp)
+ movl %eax, -20(%rbp)
+# %bb.2: # %catch
+ movq -16(%rbp), %rdi
+ callq __cxa_begin_catch@PLT
+ movl $0, -4(%rbp)
+ callq __cxa_end_catch@PLT
+# %bb.3: # %return
+ movl -4(%rbp), %eax
+ addq $32, %rsp
+ popq %rbp
+ .cfi_def_cfa %rsp, 8
+ retq
+.LBB0_4: # %unreachable
+.Lfunc_end0:
+ .size main, .Lfunc_end0-main
+ .cfi_endproc
+
+ .section .gcc_except_table,"a",@progbits
+ .p2align 2, 0x0
+GCC_except_table0:
+.Lexception0:
+ .byte 3 # @LPStart Encoding = udata4
+ .long 0
+ .byte 155 # @TType Encoding = indirect pcrel sdata4
+ .uleb128 .Lttbase0-.Lttbaseref0
+.Lttbaseref0:
+ .byte 11 # Call site Encoding = udata4
+ .uleb128 .Lcst_end0-.Lcst_begin0
+.Lcst_begin0:
+ .long .Lfunc_begin0-.Lfunc_begin0 # >> Call Site 1 <<
+ .long .Ltmp0-.Lfunc_begin0 # Call between .Lfunc_begin0 and .Ltmp0
+ .long 0 # has no landing pad
+ .byte 0 # On action: cleanup
+ .long .Ltmp0-.Lfunc_begin0 # >> Call Site 2 <<
+ .long .Ltmp1-.Ltmp0 # Call between .Ltmp0 and .Ltmp1
+ .long .Ltmp2
+ .byte 1 # On action: 1
+ .long .Ltmp1-.Lfunc_begin0 # >> Call Site 3 <<
+ .long .Lfunc_end0-.Ltmp1 # Call between .Ltmp1 and .Lfunc_end0
+ .long 0 # has no landing pad
+ .byte 0 # On action: cleanup
+.Lcst_end0:
+ .byte 1 # >> Action Record 1 <<
+ # Catch TypeInfo 1
+ .byte 0 # No further actions
+ .p2align 2, 0x0
+ # >> Catch TypeInfos <<
+ .long 0 # TypeInfo 1
+.Lttbase0:
+ .p2align 2, 0x0
+ # -- End function
|
|
There's one msan test that keeps timing out, but everything else passes. |
|
Ping. |
|
@ojhunt Do you feel qualified to review this? I know you've been playing around this part of the code lately. |
|
@MaskRay is probably most familiar with this, but as far as I can tell from the DWARF spec and https://www.airs.com/blog/archives/464, this is correct (and matches libsupc++, as the issue mentions). It seems like a general deficiency of |
|
@MaskRay, does the fix look reasonable to you? |
libcxxabi/test/native/x86_64-unknown-linux/lpstart-zero.pass.sh.s
Outdated
Show resolved
Hide resolved
libcxxabi/test/native/x86_64-unknown-linux/lpstart-zero.pass.sh.s
Outdated
Show resolved
Hide resolved
|
I think this patch broke our Android CI: https://buildkite.com/llvm-project/libcxx-ci/builds/32080#018c4427-1b79-4812-be06-26e96121f5d8 Can you confirm and take a look? If it looks complicated to fix, we should probably consider reverting until we understand the problem since this was just introduced. |
|
I am not sure why the BuildKite pre-commit CI doesn't seem to have been triggered by this patch so I think there's no way you could have seen this. |
|
Let me take a look. |
|
The binary created by clang cannot be located when executed. I'm not familiar with Android test system and don't have an access to x86_64 Android system to repro. Going to disable this test whenever the target triple contains "android". |
Follow up to llvm#72727. The added test could not be executed on Android.
Follow up to #72727. The added test could not be executed on Android.
When lpStartEncoding is different from DW_EH_PE_omit, lpStart can be set to zero which is a valid base address for landing pads. Such base value is useful when landing pads are placed in different sections.
Fixes #72582.