[cfe-dev] __attribute__((used)) with -mconstructor-aliases fails to emit Ctor_Complete (C1)

[John McCall via cfe-dev]

On 21 May 2020, at 5:54, John Edwards via cfe-dev wrote:
> Hi all, wanted to share some behavior I'm seeing that seems unintentional,
> and I'm wondering if there's a simple fix.
>
> As the title suggests, not all versions of an __attribute__((used)) tagged
> constructor get emitted when compiling with -mconstructor-aliases (which is
> the default when using the "clang" driver). Same with destructors. I mostly
> tested with the triple "x86_64-unknown-linux-gnu", but WebAssembly, at
> least, appears to behave the same way. Ctor_Base (the one using C2 in the
> mangled name), always get emitted, but C1 does not. There's a test at
> llvm/clang/test/CodeGenCXX/attr-used.cpp that specifically checks to make
> sure C1 gets emitted, but it doesn't use the -mconstructor-aliases flag
> (and, in fact, fails if you try). I did most of my testing on the master
> branch, but it appeared the same on several versions I sampled back to 5.0.
>
> I traced compilation on the following minimal test case:
>
> class X0 {
> public:
>     __attribute__((used)) X0() {}
> };
>
> When CodeGenModule::EmitTopLevelDecl() was called for X0(), it called
> ItaniumCXXABI::EmitCXXConstructors(), which called
> CodeGenModule::EmitGlobal() for first Ctor_Base and then Ctor_Complete.
> Normally, EmitGlobal() would try to defer emitting the inline functions
> until they were used somewhere else, but ASTContext::DeclMustBeEmitted()
> checks for __attribute__((used)) and forces
> CodeGenModule::EmitGlobalDefinition() to be called for both of them. So far
> so good, but then EmitGlobalDefinition() calls into
> ItaniumCXXABI::emitCXXStructor(), and this is where things start to go
> wrong. If you compile without the -mconstructor-aliases flag,
> emitCXXStructor() checks getCodegenToUse(), which immediately returns Emit,
> both ctor versions get emitted, and you pass your tests. With the
> -mconstructor-aliases flag, however, getCodegenToUse() wends its way down
> to eventually calling GlobalValue::isDiscardableIfUnused(), which returns
> true, and getCodegenToUse() ends up returning RAUW (Replace All Uses With).
> Ctor_Base gets emitted to comdat, but Ctor_Complete gets
> CodeGenModule::addReplacement'd with Ctor_Base, and doesn't make it to the
> object file.
>
> Intuitively, a function tagged as __attribute__((used)) seems like it
> shouldn't be isDiscardableIfUnused, though I can also imagine that argued
> the other way, but RAUW is even harder to justify. Adding a check for
> hasAttr<UsedAttr>() on the line that checks for hasAttr<DLLExportAttr>() in
> ASTContext::adjustGVALinkageForAttributes() results in RAUW becoming
> COMDAT, and C1 being emitted as an alias for C2 (along with a simple C5
> comdat), which seems like the right outcome, though I'm less sure if it's
> the right way of getting there. I'm a novice to the Clang internals, but
> I'm up for looking into this deeper with some guidance.

Well, `isDiscardableIfUnused` is just a query about IR linkages and
can’t consider the source declaration holistically.

I think (1) we should be skipping the RAUW if the declaration is used
and (2) adding the alias to the used list, and probably neither of those
is happening.

> In the meantime, is there a way to suppress clang frontend flags via the
> clang driver?

I don’t think so, no.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200602/9c8a82d1/attachment.html>