[PATCH 1/2] Always create unwind table entries for functions marked `uwtable`
Rafael Espíndola
rafael.espindola at gmail.com
Sun Feb 16 15:28:48 PST 2014
I am not sure this is the correct way to read uwtable.
The original intension was for it to force the output of any
information needed for unwinding only. Sometimes it is needed because
of an abi. Sometimes it is needed because the user wants to make sure
a profiler or debugger can unwind the stack past a call to it.
With that interpretation, the GCC_except_table should never be
modified based on uwtable.
What use case are we not capturing? I.E., what prevents you from just
dropping nounwind from a function you want to see in GCC_except_table?
On 16 February 2014 14:13, Björn Steinbrink <bsteinbr at gmail.com> wrote:
> As explained in Attributes.h, functions marked `uwtable` should always
> have an entry in the unwind table, even if they are additionally marked
> `nounwind`.
>
> But the check in codegen only checks if the function is marked
> `nounwind` and if so, it always omits the table entry, which is wrong.
> The Function class already provides a method to correctly check if a
> table entry is required. Using that instead of the `nounwind` check
> fixes problem.
> ---
> lib/CodeGen/AsmPrinter/DwarfException.cpp | 40 +++++++++--------
> lib/CodeGen/AsmPrinter/DwarfException.h | 6 +--
> test/CodeGen/X86/gcc_except_table.ll | 73 +++++++++++++++++++++++++++++--
> 3 files changed, 93 insertions(+), 26 deletions(-)
>
> diff --git a/lib/CodeGen/AsmPrinter/DwarfException.cpp b/lib/CodeGen/AsmPrinter/DwarfException.cpp
> index f72bc81..4ff5599 100644
> --- a/lib/CodeGen/AsmPrinter/DwarfException.cpp
> +++ b/lib/CodeGen/AsmPrinter/DwarfException.cpp
> @@ -181,12 +181,12 @@ ComputeActionsTable(const SmallVectorImpl<const LandingPadInfo*> &LandingPads,
> return SizeActions;
> }
>
> -/// CallToNoUnwindFunction - Return `true' if this is a call to a function
> -/// marked `nounwind'. Return `false' otherwise.
> -bool DwarfException::CallToNoUnwindFunction(const MachineInstr *MI) {
> + /// CallNeedsUnwindTableEntry - Return `true' if this is a call to a function
> + /// that needs an unwind table entry. Return `false' otherwise.
> +bool DwarfException::CallNeedsUnwindTableEntry(const MachineInstr *MI) {
> assert(MI->isCall() && "This should be a call instruction!");
>
> - bool MarkedNoUnwind = false;
> + bool NeedsUnwindTableEntry = true;
> bool SawFunc = false;
>
> for (unsigned I = 0, E = MI->getNumOperands(); I != E; ++I) {
> @@ -204,15 +204,15 @@ bool DwarfException::CallToNoUnwindFunction(const MachineInstr *MI) {
> //
> // FIXME: Determine if there's a way to say that `F' is the callee or
> // parameter.
> - MarkedNoUnwind = false;
> + NeedsUnwindTableEntry = true;
> break;
> }
>
> - MarkedNoUnwind = F->doesNotThrow();
> + NeedsUnwindTableEntry = F->needsUnwindTableEntry();
> SawFunc = true;
> }
>
> - return MarkedNoUnwind;
> + return NeedsUnwindTableEntry;
> }
>
> /// ComputeCallSiteTable - Compute the call-site table. The entry for an invoke
> @@ -230,9 +230,11 @@ ComputeCallSiteTable(SmallVectorImpl<CallSiteEntry> &CallSites,
> // The end label of the previous invoke or nounwind try-range.
> MCSymbol *LastLabel = 0;
>
> - // Whether there is a potentially throwing instruction (currently this means
> - // an ordinary call) between the end of the previous try-range and now.
> - bool SawPotentiallyThrowing = false;
> + // Whether there is an instruction that forces us to emit unwind table
> + // entries for calls between the end of the previous try-range and now.
> + // Currently this means function calls that potentially throw or are marked
> + // to always required an unwind table entry.
> + bool ForceCallSiteEntry = false;
>
> // Whether the last CallSite entry was for an invoke.
> bool PreviousIsInvoke = false;
> @@ -244,14 +246,14 @@ ComputeCallSiteTable(SmallVectorImpl<CallSiteEntry> &CallSites,
> MI != E; ++MI) {
> if (!MI->isLabel()) {
> if (MI->isCall())
> - SawPotentiallyThrowing |= !CallToNoUnwindFunction(MI);
> + ForceCallSiteEntry |= CallNeedsUnwindTableEntry(MI);
> continue;
> }
>
> // End of the previous try-range?
> MCSymbol *BeginLabel = MI->getOperand(0).getMCSymbol();
> if (BeginLabel == LastLabel)
> - SawPotentiallyThrowing = false;
> + ForceCallSiteEntry = false;
>
> // Beginning of a new try-range?
> RangeMapType::const_iterator L = PadMap.find(BeginLabel);
> @@ -265,10 +267,10 @@ ComputeCallSiteTable(SmallVectorImpl<CallSiteEntry> &CallSites,
> "Inconsistent landing pad map!");
>
> // For Dwarf exception handling (SjLj handling doesn't use this). If some
> - // instruction between the previous try-range and this one may throw,
> - // create a call-site entry with no landing pad for the region between the
> - // try-ranges.
> - if (SawPotentiallyThrowing && Asm->MAI->isExceptionHandlingDwarf()) {
> + // instruction between the previous try-range and this one requires an
> + // unwind table entry create a call-site entry with no landing pad for
> + // the region between the try-ranges.
> + if (ForceCallSiteEntry && Asm->MAI->isExceptionHandlingDwarf()) {
> CallSiteEntry Site = { LastLabel, BeginLabel, 0, 0 };
> CallSites.push_back(Site);
> PreviousIsInvoke = false;
> @@ -316,9 +318,9 @@ ComputeCallSiteTable(SmallVectorImpl<CallSiteEntry> &CallSites,
> }
>
> // If some instruction between the previous try-range and the end of the
> - // function may throw, create a call-site entry with no landing pad for the
> - // region following the try-range.
> - if (SawPotentiallyThrowing && Asm->MAI->isExceptionHandlingDwarf()) {
> + // function requires an unwind table entry, create a call-site entry with no
> + // landing pad for the region following the try-range.
> + if (ForceCallSiteEntry && Asm->MAI->isExceptionHandlingDwarf()) {
> CallSiteEntry Site = { LastLabel, 0, 0, 0 };
> CallSites.push_back(Site);
> }
> diff --git a/lib/CodeGen/AsmPrinter/DwarfException.h b/lib/CodeGen/AsmPrinter/DwarfException.h
> index a28eaf0..ba02f70 100644
> --- a/lib/CodeGen/AsmPrinter/DwarfException.h
> +++ b/lib/CodeGen/AsmPrinter/DwarfException.h
> @@ -85,9 +85,9 @@ protected:
> SmallVectorImpl<ActionEntry> &Actions,
> SmallVectorImpl<unsigned> &FirstActions);
>
> - /// CallToNoUnwindFunction - Return `true' if this is a call to a function
> - /// marked `nounwind'. Return `false' otherwise.
> - bool CallToNoUnwindFunction(const MachineInstr *MI);
> + /// CallNeedsUnwindTableEntry - Return `true' if this is a call to a function
> + /// that needs an unwind table entry. Return `false' otherwise.
> + bool CallNeedsUnwindTableEntry(const MachineInstr *MI);
>
> /// ComputeCallSiteTable - Compute the call-site table. The entry for an
> /// invoke has a try-range containing the call, a non-zero landing pad and an
> diff --git a/test/CodeGen/X86/gcc_except_table.ll b/test/CodeGen/X86/gcc_except_table.ll
> index fcc4e9f..b1b8490 100644
> --- a/test/CodeGen/X86/gcc_except_table.ll
> +++ b/test/CodeGen/X86/gcc_except_table.ll
> @@ -24,10 +24,75 @@ eh.resume:
> resume { i8*, i32 } %0
> }
>
> -declare void @_Z1fv() optsize
> -
> -declare i32 @__gxx_personality_v0(...)
> -
> ; CHECK: Leh_func_end0:
> ; CHECK: GCC_except_table0
> ; CHECK: = Leh_func_end0-
> +
> +define void @test_uwtable() uwtable {
> + call void @uwtable()
> + invoke void @_Z1fv() to label %ret unwind label %unwind
> +
> +ret:
> + ret void
> +
> +unwind:
> + %1 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*)
> + cleanup
> + resume { i8*, i32 } %1
> +}
> +
> +; CHECK: GCC_except_table1
> +; CHECK: Call Site 1
> +; CHECK: has no landing pad
> +; CHECK: Call Site 2
> +; CHECK: jumps to
> +; CHECK: Call Site 3
> +; CHECK: has no landing pad
> +
> +define void @test_nounwind() uwtable {
> + call void @nounwind()
> + invoke void @_Z1fv() to label %ret unwind label %unwind
> +
> +ret:
> + ret void
> +
> +unwind:
> + %1 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*)
> + cleanup
> + resume { i8*, i32 } %1
> +}
> +
> +; CHECK: GCC_except_table2
> +; CHECK: Call Site 1
> +; CHECK: jumps to
> +; CHECK: Call Site 2
> +; CHECK: has no landing pad
> +
> +define void @test_both() uwtable {
> + call void @both()
> + invoke void @_Z1fv() to label %ret unwind label %unwind
> +
> +ret:
> + ret void
> +
> +unwind:
> + %1 = landingpad { i8*, i32 } personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*)
> + cleanup
> + resume { i8*, i32 } %1
> +}
> +
> +; CHECK: GCC_except_table3
> +; CHECK: Call Site 1
> +; CHECK: has no landing pad
> +; CHECK: Call Site 2
> +; CHECK: jumps to
> +; CHECK: Call Site 3
> +; CHECK: has no landing pad
> +
> +declare void @uwtable() uwtable;
> +declare void @nounwind() nounwind;
> +declare void @both() uwtable nounwind;
> +
> +declare void @_Z1fv() optsize
> +
> +declare i32 @__gxx_personality_v0(...)
> --
> 1.9.0.rc3
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list