Make x86 __ehhandler comdat if parent function is

[kastiglione]

Summary

This change comes from using lld for i686-windows-msvc. Before this change, lld
emits an error of:

error: relocation against symbol in discarded section: .xdata

It's possible that this could be addressed in lld, but I think this change is
reasonable on its own.

At a high level, this is being generated:

A (.text comdat) -> B (.text) -> C (.xdata comdat)

Where A is a C++ inline function, which references B, an exception handler
thunk, which references C, the exception handling info.

With this structure, lld will error when applying relocations to B if the C it
references has been discarded (some other C has been selected).

This change checks if A is comdat, and if so places the exception registration
thunk (B) in the comdata group of A (and B).

It appears that MSVC makes the __ehhandler function comdat.

Is it possible that duplicate thunks are being emitted into the final binary
with other linkers, or are they stripping the unused thunks?

[kastiglione]

Here's a reduced case: http://godbolt.org/g/rDEJcV

I'm going to add a test case. What's the best way? I'm thinking that I would take the reduced C++ code and use the generated IR to create assembly and then FileCheck against that.

[compnerd]

You should be able to just do an IR level check for this. You can ensure that both get placed into a COMDAT section.

[compnerd]

BTW, I think that this is sufficient to show the behavior in MSVC:

struct S { ~S(); };
void g();

__declspec(dllexport) void f() {
  S s;
  g();
}

[kastiglione]

do an IR level check for this

running clang -target i686-windows-msvc -S -emit-llvm -o - input.cpp generates IR for the function, but not for the __ehhandler thunk. I think the __ehhandler is encapsulated by the cleanuppad, and if so are there flags to get the IR through the passes that generate the thunk?

> BTW, I think that this is sufficient to show the behavior in MSVC:

In the code you provided, f and its xdata are not comdat. I believe the problem only arises when the xdata is comdat and then dropped by the linker, leaving the __ehhandler with relocations to a discarded section. The code I tested with uses an inline member function to generate a comdat function, and a free function to call and materialize the member function. This also mirrors the real code that I first saw this happen with.

[smeenai]

You can use /Gy (or -ffunction-sections if you're not using the cl-style driver) to force each function into its own COMDAT section.

[rnk]

Thanks! Needs a test case. It should be possible to test this directly with opt.

lib/Target/X86/X86WinEHState.cpp

404–405

It's probably more correct to take the comdat of ParentFunc directly, like this:

if (auto *C = ParentFunc->getComdat())
  Trampoline->setComdat(C);

This is unlikely to matter in practice. The only way I can think of to get into a situation where it matters is perhaps:

int g();
static int f() { // will make an __ehhandler and get inlined into the initializer for x
  try { return g(); } catch (...) { return 0; }
}
int __declspec(selectany) x = f(); // will create a dynamic initializer function comdat with 'x'

[kastiglione]

Reuse parent comdat; Add test

[rnk]

lgtm

[kastiglione]

Thanks @rnk and @compnerd

lib/Target/X86/X86WinEHState.cpp

Function *WinEHStatePass::generateLSDAInEAXThunk(Function *ParentFunc) {
   FunctionType *TargetFuncTy =
       FunctionType::get(Int32Ty, makeArrayRef(&ArgTys[0], 5),
                         /*isVarArg=*/false);
   Function *Trampoline =
       Function::Create(TrampolineTy, GlobalValue::InternalLinkage,
                        Twine("__ehhandler$") + GlobalValue::dropLLVMManglingEscape(
                                                    ParentFunc->getName()),
                        TheModule);
+  if (ParentFunc->getComdat())
+    Trampoline->setComdat(TheModule->getOrInsertComdat(ParentFunc->getName()));
   BasicBlock *EntryBB = BasicBlock::Create(Context, "entry", Trampoline);
   IRBuilder<> Builder(EntryBB);
   Value *LSDA = emitEHLSDA(Builder, ParentFunc);
   Value *CastPersonality =
       Builder.CreateBitCast(PersonalityFn, TargetFuncTy->getPointerTo());
   auto AI = Trampoline->arg_begin();
   Value *Args[5] = {LSDA, &*AI++, &*AI++, &*AI++, &*AI++};
   CallInst *Call = Builder.CreateCall(CastPersonality, Args);