[LSDA] Take ttype_index into account when taking unwind action #106446

bzEq · 2023-01-04T10:10:58Z

If cs_action != 0, we should check the ttype_index field in action record. If ttype_index == 0, a clean up action is taken; otherwise catch action is taken.

This can fix unwind failure on AIX which uses LLVM's libunwind by default. IIUC, rust's LSDA is borrowed from c++ and I'm assuming itanium-cxx-abi https://itanium-cxx-abi.github.io/cxx-abi/exceptions.pdf should be followed, so the fix follows what libcxxabi does. See https://github.com/llvm/llvm-project/blob/ec48682ce9f61d056361c5095f21e930b8365661/libcxxabi/src/cxa_personality.cpp#L152 for use of ttype_index.

If cs_action != 0, we should check the ttype_index field in action record. If ttype_index == 0, a clean up action is taken.

rustbot · 2023-01-04T10:11:07Z

r? @cuviper

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-01-04T10:11:09Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

cuviper · 2023-01-06T19:40:48Z

I don't know these details about unwinding -- maybe r? @Amanieu

Amanieu · 2023-01-07T15:27:59Z

I think this looks correct, based on the implementation in libcxxabi. However I think this change also needs to be applied to the SJLJ code path.

Amanieu · 2023-01-07T15:28:46Z

I'm surprised this is needed though, I didn't think LLVM emitted code like this. Could you expand on the cases where this causes problems on AIX?

bzEq · 2023-01-09T09:49:10Z

Hi @Amanieu , there are numbers of such cases when bootstrapping rust on AIX. Here's the reduced case from getopt crate

; ModuleID = 'bugpoint-reduced-simplified.bc'
source_filename = "getopts.b321dad9-cgu.8"
target datalayout = "E-m:a-i64:64-n32:64-S128-v256:256:256-v512:512:512"
target triple = "powerpc64-ibm-aix"

define hidden void @_RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts() unnamed_addr #0 personality ptr @rust_eh_personality {
  br i1 poison, label %8, label %1

1:                                                ; preds = %0
  br label %2

2:                                                ; preds = %5, %1
  br i1 poison, label %3, label %5

3:                                                ; preds = %2
  invoke void @_RINvNvMs_NtCs3Cu2L953qsK_5alloc7raw_vecINtB7_6RawVecppE7reserve21do_reserve_and_handlehNtNtB9_5alloc6GlobalECsc4UU1XxBbkb_7getopts()
          to label %4 unwind label %6

4:                                                ; preds = %3
  br label %5

5:                                                ; preds = %4, %2
  br i1 poison, label %8, label %2

6:                                                ; preds = %3
  %7 = landingpad { ptr, i32 }
          cleanup
  resume { ptr, i32 } %7

8:                                                ; preds = %5, %0
  ret void
}

declare i32 @rust_eh_personality(i32, i32, i64, ptr, ptr) unnamed_addr #0

declare hidden void @_RINvNvMs_NtCs3Cu2L953qsK_5alloc7raw_vecINtB7_6RawVecppE7reserve21do_reserve_and_handlehNtNtB9_5alloc6GlobalECsc4UU1XxBbkb_7getopts() unnamed_addr #0

attributes #0 = { "target-cpu"="pwr7" }

On AIX, we have

	.csect .text[PR],5
	.globl	_RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts[DS],hidden # -- Begin function _RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts
	.globl	._RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts,hidden
	.align	4
	.csect _RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts[DS],3
	.vbyte	8, ._RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts # @_RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts
	.vbyte	8, TOC[TC0]
	.vbyte	8, 0
	.csect .text[PR],5
._RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts:
L..func_begin0:
# %bb.0:
	mflr 0
	stdu 1, -112(1)
	std 0, 128(1)
	bc 12, 20, L..BB0_3
# %bb.1:
	bc 12, 20, L..BB0_3
# %bb.2:
L..tmp0:
	bl ._RINvNvMs_NtCs3Cu2L953qsK_5alloc7raw_vecINtB7_6RawVecppE7reserve21do_reserve_and_handlehNtNtB9_5alloc6GlobalECsc4UU1XxBbkb_7getopts[PR]
	nop
L..tmp1:
L..BB0_3:
	addi 1, 1, 112
	ld 0, 16(1)
	mtlr 0
	blr
L..BB0_4:
L..tmp2:
	bl ._Unwind_Resume[PR]
	nop
L.._RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts0:
	.vbyte	4, 0x00000000                   # Traceback table begin
	.byte	0x00                            # Version = 0
	.byte	0x09                            # Language = CPlusPlus
	.byte	0x20                            # -IsGlobaLinkage, -IsOutOfLineEpilogOrPrologue
                                        # +HasTraceBackTableOffset, -IsInternalProcedure
                                        # -HasControlledStorage, -IsTOCless
                                        # -IsFloatingPointPresent
                                        # -IsFloatingPointOperationLogOrAbortEnabled
	.byte	0x41                            # -IsInterruptHandler, +IsFunctionNamePresent, -IsAllocaUsed
                                        # OnConditionDirective = 0, -IsCRSaved, +IsLRSaved
	.byte	0x80                            # +IsBackChainStored, -IsFixup, NumOfFPRsSaved = 0
	.byte	0x80                            # +HasExtensionTable, -HasVectorInfo, NumOfGPRsSaved = 0
	.byte	0x00                            # NumberOfFixedParms = 0
	.byte	0x01                            # NumberOfFPParms = 0, +HasParmsOnStack
	.vbyte	4, L.._RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts0-._RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts # Function size
	.vbyte	2, 0x00d7                       # Function name len = 215
	.byte	"_RINvXs5_NtCs3Cu2L953qsK_5alloc6stringNtB6_6StringINtNtNtNtCs7eLrLyzl9su_4core4iter6traits7collect12FromIteratorReE9from_iterINtNtNtBS_8adapters4take4TakeINtNtNtBS_7sources6repeat6RepeatB1L_EEECsc4UU1XxBbkb_7getopts" # Function Name
	.byte	0x08                            # ExtensionTableFlag = TB_EH_INFO
	.align	2
	.vbyte	8, L..C0-TOC[TC0]               # EHInfo Table
L..func_end0:
	.csect .gcc_except_table[RO],2
	.align	2
GCC_except_table0:
L..exception0:
	.byte	255                             # @LPStart Encoding = omit
	.byte	255                             # @TType Encoding = omit
	.byte	3                               # Call site Encoding = udata4
	.byte	26
	.vbyte	4, L..tmp0-L..func_begin0       # >> Call Site 1 <<
	.vbyte	4, L..tmp1-L..tmp0              #   Call between L..tmp0 and L..tmp1
	.vbyte	4, L..tmp2-L..func_begin0       #     jumps to L..tmp2
	.byte	1                               #   On action: 1
	.vbyte	4, L..tmp1-L..func_begin0       # >> Call Site 2 <<
	.vbyte	4, L..func_end0-L..tmp1         #   Call between L..tmp1 and L..func_end0
	.vbyte	4, 0                            #     has no landing pad
	.byte	0                               #   On action: cleanup
L..cst_end0:
	.byte	0                               # >> Action Record 1 <<
                                        #   Cleanup
	.byte	0                               #   No further actions
	.align	2
	.csect .eh_info_table[RW],2
__ehinfo.0:
	.vbyte	4, 0
	.align	3
	.vbyte	8, GCC_except_table0
	.vbyte	8, rust_eh_personality[DS]
                                        # -- End function
	.extern	.rust_eh_personality[PR]
	.extern	rust_eh_personality[DS]
	.extern	._RINvNvMs_NtCs3Cu2L953qsK_5alloc7raw_vecINtB7_6RawVecppE7reserve21do_reserve_and_handlehNtNtB9_5alloc6GlobalECsc4UU1XxBbkb_7getopts[PR],hidden
	.extern	_RINvNvMs_NtCs3Cu2L953qsK_5alloc7raw_vecINtB7_6RawVecppE7reserve21do_reserve_and_handlehNtNtB9_5alloc6GlobalECsc4UU1XxBbkb_7getopts[DS],hidden
	.extern	._Unwind_Resume[PR]
	.extern	_Unwind_Resume[DS]
	.toc
L..C0:
	.tc __ehinfo.0[TC],__ehinfo.0

You can check the action at

L..cst_end0:
	.byte	0                               # >> Action Record 1 <<
                                        #   Cleanup
	.byte	0                               #   No further actions

which is required by

	.vbyte	4, L..tmp0-L..func_begin0       # >> Call Site 1 <<
	.vbyte	4, L..tmp1-L..tmp0              #   Call between L..tmp0 and L..tmp1
	.vbyte	4, L..tmp2-L..func_begin0       #     jumps to L..tmp2
	.byte	1                               #   On action: 1

Here's the LLVM's view of landingpad, https://github.com/llvm/llvm-project/blob/2d32a01b9fe2249b1a7dc57f7da1fadf804fbd8c/llvm/lib/CodeGen/MachineFunction.cpp#L766

Amanieu · 2023-01-09T10:22:52Z

LGTM. Could you also update the SJLJ code path?

bzEq · 2023-01-09T15:03:43Z

Also check ttype_index when using SJLJ.

Amanieu · 2023-01-09T15:05:08Z

That's quite a lot of duplicated code, this could be moved into interpret_cs_action.

bzEq · 2023-01-09T15:24:08Z

Is it necessary to assert lpad != 0 inside interpret_cs_action?

Amanieu · 2023-01-11T18:59:47Z

I don't think it is necessary.

@bors r+

bors · 2023-01-11T18:59:49Z

📌 Commit 93ef4b3 has been approved by Amanieu

It is now in the queue for this repository.

Rollup of 8 pull requests Successful merges: - rust-lang#105795 (Stabilize `abi_efiapi` feature) - rust-lang#106446 ([LSDA] Take ttype_index into account when taking unwind action) - rust-lang#106675 (Mark ZST as FFI-safe if all its fields are PhantomData) - rust-lang#106740 (Adding a hint on iterator type errors) - rust-lang#106741 (Fix reexport of `doc(hidden)` item) - rust-lang#106759 (Revert "Make nested RPITIT inherit the parent opaque's generics.") - rust-lang#106772 (Re-add mw to review rotation) - rust-lang#106778 (Exclude formatting commit from blame) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

[LSDA] Take ttype_index into account when taking action

3453415

If cs_action != 0, we should check the ttype_index field in action record. If ttype_index == 0, a clean up action is taken.

rustbot assigned cuviper Jan 4, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 4, 2023

bzEq changed the title ~~[LSDA] Take ttype_index into account when taking action~~ [LSDA] Take ttype_index into account when taking unwind action Jan 4, 2023

rustbot assigned Amanieu and unassigned cuviper Jan 6, 2023

Also check ttype_index when using SJLJ

04a6975

bzEq added 2 commits January 9, 2023 23:09

Add comments

8333e5c

Move to intepret_cs_action

d4926eb

Add comment

93ef4b3

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 11, 2023

JohnTitor mentioned this pull request Jan 12, 2023

Rollup of 8 pull requests #106786

Merged

bors merged commit 3f21b81 into rust-lang:master Jan 13, 2023

rustbot added this to the 1.68.0 milestone Jan 13, 2023

workingjubilee added the O-aix OS: Big Blue's Advanced Interactive eXecutive.. label Feb 5, 2023

bzEq deleted the fix-unwind-lsda branch March 16, 2023 12:42

bzEq restored the fix-unwind-lsda branch March 16, 2023 12:43

bzEq deleted the fix-unwind-lsda branch March 16, 2023 12:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LSDA] Take ttype_index into account when taking unwind action #106446

[LSDA] Take ttype_index into account when taking unwind action #106446

bzEq commented Jan 4, 2023 •

edited

Loading

rustbot commented Jan 4, 2023

rustbot commented Jan 4, 2023

cuviper commented Jan 6, 2023

Amanieu commented Jan 7, 2023

Amanieu commented Jan 7, 2023

bzEq commented Jan 9, 2023 •

edited

Loading

Amanieu commented Jan 9, 2023

bzEq commented Jan 9, 2023

Amanieu commented Jan 9, 2023

bzEq commented Jan 9, 2023

Amanieu commented Jan 11, 2023

bors commented Jan 11, 2023

[LSDA] Take ttype_index into account when taking unwind action #106446

[LSDA] Take ttype_index into account when taking unwind action #106446

Conversation

bzEq commented Jan 4, 2023 • edited Loading

rustbot commented Jan 4, 2023

rustbot commented Jan 4, 2023

cuviper commented Jan 6, 2023

Amanieu commented Jan 7, 2023

Amanieu commented Jan 7, 2023

bzEq commented Jan 9, 2023 • edited Loading

Amanieu commented Jan 9, 2023

bzEq commented Jan 9, 2023

Amanieu commented Jan 9, 2023

bzEq commented Jan 9, 2023

Amanieu commented Jan 11, 2023

bors commented Jan 11, 2023

bzEq commented Jan 4, 2023 •

edited

Loading

bzEq commented Jan 9, 2023 •

edited

Loading