[Driver][Darwin] Pass -munwind-table when !UseSjLjExceptions
ClosedPublic

Authored by ahatanak on Jul 20 2017, 11:40 AM.

Details

Summary

When I compile the following code targeting arm64 and execute it, it terminates with an uncaught exception: "libc++abi.dylib: terminating with uncaught exception of type int":

int foo1() noexcept {

try {
  throw 0;
} catch (int i) {
  return 0;
}
return 1;

}

int main() {

return foo1();

}

This happens because function foo1 has attribute nounwind but doesn't have attribute uwtable on it, in which case Funcion::needsUnwindTableEntry, which is the function that determines whether an unwind table is needed, returns false.

bool needsUnwindTableEntry() const {
  return hasUWTable() || !doesNotThrow();
}

This patch changes MachO::IsUnwindTablesDefault to return true when !UseSjLjExceptions. When the function returns true, -munwind-table is passed to the frontend, which causes IRGen to annotate functions with attribute uwtable.

One question: instead of adding uwtable in SetLLVMFunctionAttributesForDefinition whenever CodeGenOpts.UnwindTables is true, is it possible to limit it to functions that actually need unwind tables (for example, in TryMarkNoThrow in CodeGenFunction.cpp)?

rdar://problem/32411865

Diff Detail

Repository
rL LLVM
ahatanak created this revision.Jul 20 2017, 11:40 AM

Does the ARM64 ABI call for unwind tables to be emitted for all functions, like the x86-64 ABI does?

Anyway, it seems pretty unfortunate that the default behavior breaks exceptions.

Does the ARM64 ABI call for unwind tables to be emitted for all functions, like the x86-64 ABI does?

Anyway, it seems pretty unfortunate that the default behavior breaks exceptions.

According to Nick Kledzik, the ARM64 ABI doesn't require unwind tables to be emitted for all functions, but the compiler has emitted the tables for all functions anyway. If I understood his explanation correctly, people often ran into problems when mixing C and C++ code because they forgot to use -fexceptions. Also, the compact unwind table is already small, so omitting it would most likely not make much difference in the size of the executable.

I'm also looking at r291172, which is the commit that directly caused this regression.

rjmccall accepted this revision.Jul 28 2017, 6:21 PM

Okay. Sounds fine to me.

This revision is now accepted and ready to land.Jul 28 2017, 6:21 PM
This revision was automatically updated to reflect the committed changes.