[LLVMbugs] [Bug 18710] Only generate .ARM.exidx and .ARM.extab when needed in EHABI

[chrib]

Summary

add uwtable if unwind table is needed,regardless of nothrow/nothow status. Fulfill-f[no]-unwind-tables

Depends on reviews.llvm.org/D31139

[jroelofs]

Can you clarify the logic here? It's my understanding that:

-fno-exceptions does *not* imply -fno-unwind-tables

however:

-fno-unwind-tables *does* imply that exceptions cannot be used on targets that require the tables to do unwinding.

[chrib]

In D31140#706411, @jroelofs wrote:

Can you clarify the logic here? It's my understanding that:

-fno-exceptions does *not* imply -fno-unwind-tables

however:

-fno-unwind-tables *does* imply that exceptions cannot be used on targets that require the tables to do unwinding.

Yes, (bad things might happen or (std::terminate will be called, or destructors not called.)...

But -f[no]-unwind-tables implies the UWTable attribute, not NoUwind attribute. To toggle NoUnwind, use -fno-exceptions

And this is getting worse with .canunwind which means DoesNotThrow :)

in my understanding, the logic is as follow:

Since "An exception cannot propagate through a function with a nounwind table. The exception handling runtime environment terminates the program if it encounters a nounwind table during exception processing." (ARM Information Center)

The "nounwind" LLVM attribute, which means "Function does not throw" translates as the EXIDX_CANTUNWIND value in the exception table index table which needs to be created for the purpose (for the function)

And of course without exception runtime environment (the test here) we don't need this table. So I can see 3 cases:

• nounwind set : Generate .cantunwind directive and unwind table
• nounwind set but not EH Do not generate the .cantunwind directive and do not emit the unwind table
• uwtable set Need to generate the unwind table (even without EH)

The disable-arm-cantunwind flag means: without EH support if the function does not throw, do dot generate the exception tables and the EXIDX_CANTUNWIND value.

[ahatanak]

Is it possible to enable cantunwind for one function and enable for another? If so, does this have to be a function attribute?

[chrib]

Yes we can enable cantunwind with the nothrow gcc attribute when exceptions are enabled
Forcing it when exceptions are not enabled (e.g for attribute cleanup) would require -funwind-tables at function level anyway

So the flag should work, but conceptually I agree, this semantic should be better carried thru attributes, hopefully without adding a new one

[chrib]

1. Updating D31140: [LLVMbugs] [Bug 18710] Only generate .ARM.exidx and .ARM.extab when needed in EHABI #

Set UWTable if the language requires an exception table.
Conservatively only for ARM (necessary to emit the .cantunwind directive). But could probably be enabled for all targets.

depends on D31139

[chrib]

• NeedsUnwindTable check thumb arch

[chrib]

Rebase and cleanup NeedsUnwindTable for be variants.

[efriedma]

If you want to follow what we do for x86-64 on ARM, you should be doing this in the driver, not codegen (grep for IsUnwindTablesDefault).

[chrib]

Yes, I also tried to look into this direction, but I did not find a clean way from the driver to emit the attribute based on codegen options and language. Also unwind-tables must not be the default for arm. (for C this is what I want to eliminate). In fact I haven't found an example of setting attributes in the driver. not sure it's even possible.

Also, setting this new uwtable attribute close to the existing NoUnwind attribute in CodeGen makes the orthogonality both attributes more obvious (or less obscure :-).

This said, testing for the Triple Arch here is not clean (I probably prefer a target overloads) but that was a conservative approach. to avoid that we would need to propose a follow up that simplifies the code as:

// We might need an unwind table, even if the function cannot throw.                                                                                                                                                                                                                                                         
if (hasUnwindExceptions(LangOpts) || CodeGenOpts.UnwindTables)
  B.addAttribute(llvm::Attribute::UWTable);

// If the module doesn't support exceptions the function cannot throw.                                                                                                                                                                                                                                                       
// We can have a nothrow function even if unwind tables are required.                                                                                                                                                                                                                                                        
if (!hasUnwindExceptions(LangOpts))
  B.addAttribute(llvm::Attribute::NoUnwind);

but this will impact all targets and can cause other changes in the backend, so it's probably better to do this separately. Do you have an advice on that ?

thanks,

[chrib]

forgot to give the motivating figure. this brings a code size reduction of 9.5 % (geomean of text sections sizes) on coremarkpro -Oz for cortex-m3 eabi

[efriedma]

Oh, you don't want to emit them by default. :)

I'm not sure what you're trying to do here... there are three possibilities:

1. The function could have an exception thrown through it, so we need an unwind table.
2. The function can't have an exception thrown through it, and we don't need an unwind table.
3. The function can't have an exception thrown through it, but we need an unwind table anyway for ABI reasons.

We don't really care whether "uwtable" is set in the first scenario; it's obvious we need the table anyway. So I'm not sure what you're trying to accomplish here. (This is why I suggested using needsUnwindTableEntry() on the LLVM patch.)

[chrib]

Hello Eli,

You were right, using IsUnwindTablesDefault is the correct way to abstract the need for UnwindTables. I missed the relationship between the driver munwind-table and GodeGenOpts.UnwindTables use in Codegen.

Here is a new patch just setting IsUnwindTablesDefault for EHABI. No Changes in the testsuite results with D31139

[efriedma]

lib/Driver/ToolChains/BareMetal.cpp

61

This still seems weird. In most situations, I would expect you want the same behavior for C code and C++ with -fno-exceptions. On other platforms, we get that behavior by just returning false (the default).

[chrib]

OK, we can refine so that unwind-table is also not generated for C++ -fno-exceptions.

To summarize this gives or C++ :

	Unwind (table)	Exceptions (can throw)
default	Y	Y
fno-unwind	N	Y (1)
fno-exceptions	N	N

(1) without unwind, no handler can be found so the default is to terminate.

for C :

	Unwind (table)	Exceptions (can throw)
default	N	N
funwind	Y	N
fexceptions	Y	N

Cheers,

[chrib]

Update IsUnwindTablesDefault to test fno-exceptions. (see Darwin.cpp)

Add comments.

[efriedma]

I would rather not make ARM baremetal do something different from every other target...

On Linux, for most targets, we don't add the uwtable attribute by default; without the uwtable attribute, non-ARM backends (e.g. aarch64) only emit tables for functions which might unwind. Following this precedent is probably the most straightforward.

MachO::IsUnwindTablesDefault does something similar to what you're doing here (except it doesn't depend on CCCIsCXX). But Apple platforms have to deal with ObjC exception handling, so I'm not sure that's the best precedent to follow.

[chrib]

oops yes of course. I forgot some bits while switching from testing the arch to testing the platform.

indeed --target=arm-none-linux-gnueabihf lost its unwind info in c++. Surprisingly not caught by the llvm tests, will add some.

thanks

[chrib]

Hello,

I didn't find an easy way factorize the change in IsUnwindTableDefault to support the multiple ARM toolchains. After a quick check in Triple::arm in Driver/Toochains, many seem impacted (excepted NetBSD and Darwin that uses DwarfCFI or SJLG). So here is an attempt to move this into a small arm hook and use.

Also, I realized that the cxx tests was not correctly checked because most of them are invoked as "clang" instead if "clang++" So another proposal is to check the input type in IsUnwindTablesDefault.

Finally I've added the clang tests to check for this.

thanks !

[chrib]

Changes since last revision:

• Check IsUnwindTablesDefault in ARM Toolchains that support ExceptionHandling::ARM (not Darwin, NetBSD)
• Check input type with driver mode for C++ mode.
• Add Tests

[chrib]

Hello,

I realized that testing CCCisCXX for IsUnwindTablesDefault was not really good and useless here, because we cannot treat C as C++ mode. In fact only the file type matters, as seen in the tests.

So I'd like to amend my previous proposal to

• Add the InputType parameter to IsUnwindTablesDefault and use in the new function ARMNeedUnwindTable
• Fix tests thinko (wrong triplet)
• Rebase

sorry for the respin, thanks !

[chrib]

hello, realizing that this has been stuck for a while now. did I answer all the concerns ? rebased patch,

gentle ping :-)

many thanks,

Christian

[chrib]

damn it last diff was the llvm part (D31139). Here is the clang part.

sorry for the noise, still not experienced with Phabricator.

include/clang/Driver/ToolChain.h

 
   /// IsUnwindTablesDefault - Does this tool chain use -funwind-tables
   /// by default.
-  virtual bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const;
+  virtual bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args,
+             types::ID InputType) const;
 
   /// \brief Test whether this toolchain defaults to PIC.
   virtual bool isPICDefault() const = 0;

lib/Driver/ToolChain.cpp

   return Input.getFilename();
 }
 
-bool ToolChain::IsUnwindTablesDefault(const ArgList &Args) const {
+bool ToolChain::IsUnwindTablesDefault(const ArgList &Args,
+              types::ID InputType) const {
   return false;
 }
 

lib/Driver/ToolChains/Arch/ARM.h

 int getARMSubArchVersionNumber(const llvm::Triple &Triple);
 bool isARMMProfile(const llvm::Triple &Triple);
 
+bool ARMNeedUnwindTable(const llvm::opt::ArgList &Args, bool isCXX);
+
 } // end namespace arm
 } // end namespace tools
 } // end namespace driver

lib/Driver/ToolChains/Arch/ARM.cpp

   if (arm::getARMSubArchVersionNumber(Triple) >= 7 || arm::isARMMProfile(Triple))
     CmdArgs.push_back("--be8");
 }
+
+bool arm::ARMNeedUnwindTable(const ArgList &Args, bool isCXX) {
+  // Emit the uwtable attribute to generate an unwind table when exceptions are
+  // supported, even the functions that do not throw still need a .cantunwind directive.
+  return Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions, isCXX);
+}

lib/Driver/ToolChains/BareMetal.h

 
 public:
   bool useIntegratedAs() const override { return true; }
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args,
+           types::ID InputType) const override;
   bool isCrossCompiling() const override { return true; }
   bool isPICDefault() const override { return false; }
   bool isPIEDefault() const override { return false; }

lib/Driver/ToolChains/BareMetal.cpp

 #include "InputInfo.h"
 #include "Gnu.h"
 
+#include "Arch/ARM.h"
 #include "clang/Basic/VirtualFileSystem.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"

                                           Args.MakeArgString(TC.GetLinkerPath()),
                                           CmdArgs, Inputs));
 }
+
+bool BareMetal::IsUnwindTablesDefault(const ArgList &Args, types::ID InputType) const {
+  switch (getArch()) {
+  case llvm::Triple::arm:
+  case llvm::Triple::armeb:
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+    return tools::arm::ARMNeedUnwindTable(Args, types::isCXX(InputType));
+  default:
+    return false;
+  }
+}

lib/Driver/ToolChains/Clang.cpp

   bool AsynchronousUnwindTables =
       Args.hasFlag(options::OPT_fasynchronous_unwind_tables,
                    options::OPT_fno_asynchronous_unwind_tables,
-                   (getToolChain().IsUnwindTablesDefault(Args) ||
+                   (getToolChain().IsUnwindTablesDefault(Args, Input.getType()) ||
                     getToolChain().getSanitizerArgs().needsUnwindTables()) &&
                        !Freestanding);
   if (Args.hasFlag(options::OPT_funwind_tables, options::OPT_fno_unwind_tables,

lib/Driver/ToolChains/CrossWindows.h

                         const llvm::opt::ArgList &Args);
 
   bool IsIntegratedAssemblerDefault() const override { return true; }
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args, types::ID InputType) const override;
   bool isPICDefault() const override;
   bool isPIEDefault() const override;
   bool isPICDefaultForced() const override;

lib/Driver/ToolChains/CrossWindows.cpp

 //===----------------------------------------------------------------------===//
 
 #include "CrossWindows.h"
+#include "Arch/ARM.h"
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"

                                              const llvm::opt::ArgList &Args)
     : Generic_GCC(D, T, Args) {}
 
-bool CrossWindowsToolChain::IsUnwindTablesDefault(const ArgList &Args) const {
+bool CrossWindowsToolChain::IsUnwindTablesDefault(const ArgList &Args, types::ID InputType) const {
   // FIXME: all non-x86 targets need unwind tables, however, LLVM currently does
   // not know how to emit them.
-  return getArch() == llvm::Triple::x86_64;
+  switch (getArch()) {
+  case llvm::Triple::arm:
+  case llvm::Triple::armeb:
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+    return tools::arm::ARMNeedUnwindTable(Args, types::isCXX(InputType));
+  case  llvm::Triple::x86_64:
+    return true;
+  default:
+    return false;
+  }
 }
 
 bool CrossWindowsToolChain::isPICDefault() const {

lib/Driver/ToolChains/Darwin.h

 
   bool UseObjCMixedDispatch() const override { return true; }
 
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args, types::ID InputType) const override;
 
   RuntimeLibType GetDefaultRuntimeLibType() const override {
     return ToolChain::RLT_CompilerRT;

lib/Driver/ToolChains/Darwin.cpp

   return DAL;
 }
 
-bool MachO::IsUnwindTablesDefault(const ArgList &Args) const {
+bool MachO::IsUnwindTablesDefault(const ArgList &Args, types::ID InputType) const {
   // Unwind tables are not emitted if -fno-exceptions is supplied (except when
   // targeting x86_64).
   return getArch() == llvm::Triple::x86_64 ||

lib/Driver/ToolChains/Fuchsia.h

   CXXStdlibType GetDefaultCXXStdlibType() const override {
     return ToolChain::CST_Libcxx;
   }
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override {
-    return true;
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args,
+                             types::ID InputType) const override {
+      return true;
   }
   bool isPICDefault() const override { return false; }
   bool isPIEDefault() const override { return true; }

lib/Driver/ToolChains/Gnu.h

 
   void printVerboseInfo(raw_ostream &OS) const override;
 
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args,
+           types::ID InputType) const override;
   bool isPICDefault() const override;
   bool isPIEDefault() const override;
   bool isPICDefaultForced() const override;

lib/Driver/ToolChains/Gnu.cpp

   CudaInstallation.print(OS);
 }
 
-bool Generic_GCC::IsUnwindTablesDefault(const ArgList &Args) const {
-  return getArch() == llvm::Triple::x86_64;
+bool Generic_GCC::IsUnwindTablesDefault(const ArgList &Args, types::ID InputType) const {
+  switch (getArch()) {
+  case llvm::Triple::arm:
+  case llvm::Triple::armeb:
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+    return tools::arm::ARMNeedUnwindTable(Args, types::isCXX(InputType));
+  case llvm::Triple::x86_64:
+    return true;
+  default:
+    return false;
+  }
 }
 
 bool Generic_GCC::isPICDefault() const {

lib/Driver/ToolChains/Linux.h

   void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                            llvm::opt::ArgStringList &CC1Args) const override;
   bool isPIEDefault() const override;
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args,
+           types::ID InputType) const override;
   SanitizerMask getSupportedSanitizers() const override;
   void addProfileRTLibs(const llvm::opt::ArgList &Args,
                         llvm::opt::ArgStringList &CmdArgs) const override;

lib/Driver/ToolChains/Linux.cpp

         Twine("-u", llvm::getInstrProfRuntimeHookVarName())));
   ToolChain::addProfileRTLibs(Args, CmdArgs);
 }
+
+bool Linux::IsUnwindTablesDefault(const ArgList &Args, types::ID InputType) const {
+  switch (getArch()) {
+  case llvm::Triple::arm:
+  case llvm::Triple::armeb:
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+    return tools::arm::ARMNeedUnwindTable(Args, types::isCXX(InputType));
+  case llvm::Triple::x86_64:
+    return true;
+  default:
+    return false;
+  }
+}

lib/Driver/ToolChains/MSVC.h

                 Action::OffloadKind DeviceOffloadKind) const override;
 
   bool IsIntegratedAssemblerDefault() const override;
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args,
+           types::ID InputType) const override;
   bool isPICDefault() const override;
   bool isPIEDefault() const override;
   bool isPICDefaultForced() const override;

lib/Driver/ToolChains/MSVC.cpp

 #include "MSVC.h"
 #include "CommonArgs.h"
 #include "Darwin.h"
+#include "Arch/ARM.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Version.h"
 #include "clang/Driver/Compilation.h"

   return true;
 }
 
-bool MSVCToolChain::IsUnwindTablesDefault(const ArgList &Args) const {
+bool MSVCToolChain::IsUnwindTablesDefault(const ArgList &Args, types::ID InputType) const {
   // Emit unwind tables by default on Win64. All non-x86_32 Windows platforms
   // such as ARM and PPC actually require unwind tables, but LLVM doesn't know
   // how to generate them yet.

   if (getTriple().isOSBinFormatMachO())
     return false;
 
-  return getArch() == llvm::Triple::x86_64;
+  switch (getArch()) {
+  case llvm::Triple::arm:
+  case llvm::Triple::armeb:
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+    return tools::arm::ARMNeedUnwindTable(Args, types::isCXX(InputType));
+  case llvm::Triple::x86_64:
+    return true;
+  default:
+    return false;
+  }
 }
 
 bool MSVCToolChain::isPICDefault() const {

lib/Driver/ToolChains/MinGW.h

         const llvm::opt::ArgList &Args);
 
   bool IsIntegratedAssemblerDefault() const override;
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args, types::ID InputType) const override;
   bool isPICDefault() const override;
   bool isPIEDefault() const override;
   bool isPICDefaultForced() const override;

lib/Driver/ToolChains/MinGW.cpp

 //===----------------------------------------------------------------------===//
 
 #include "MinGW.h"
+#include "Arch/ARM.h"
 #include "InputInfo.h"
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"

   return new tools::MinGW::Linker(*this);
 }
 
-bool toolchains::MinGW::IsUnwindTablesDefault(const ArgList &Args) const {
-  return getArch() == llvm::Triple::x86_64;
+bool toolchains::MinGW::IsUnwindTablesDefault(const ArgList &Args, types::ID InputType) const {
+  switch (getArch()) {
+  case llvm::Triple::arm:
+  case llvm::Triple::armeb:
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+    return tools::arm::ARMNeedUnwindTable(Args, types::isCXX(InputType));
+  case llvm::Triple::x86_64:
+    return true;
+  default:
+    return false;
+  }
 }
 
 bool toolchains::MinGW::isPICDefault() const {

lib/Driver/ToolChains/NetBSD.h

       const llvm::opt::ArgList &DriverArgs,
       llvm::opt::ArgStringList &CC1Args) const override;
 
-  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override {
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args, types::ID InputType) const override {
     return true;
   }
 

test/Driver/arm-unwind.c

+// Do not add function attribute "uwtable" for arm ehabi targets for C mode.
+
+// RUN: %clang -target arm-none-eabi  -### -S %s -o %t.s 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-EABI %s
+// CHECK-EABI-NOT: -munwind-tables
+
+// RUN: %clang -target arm-linux-gnueabihf -### -S %s -o %t.s 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-GNU %s
+// CHECK-GNU-NOT: -munwind-tables

test/Driver/arm-unwind.cpp

+// Add function attribute "uwtable" for arm ehabi targets in C++.
+
+// RUN: %clang -target arm-none-eabi  -### -S %s -o %t.s 2>&1 \
+// RUN:    | FileCheck --check-prefix=CHECK-EABI %s
+// CHECK-EABI: -munwind-tables
+
+// RUN: %clang -target arm-linux-gnueabihf  -### -S %s -o %t.s 2>&1 \
+// RUN:    | FileCheck --check-prefix=CHECK-GNU %s
+// CHECK-GNU: -munwind-tables