[PATCH] D39627: Fix vtable not receiving hidden visibility when using push(visibility)

Jake Ehrlich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 6 11:59:30 PST 2017


jakehehrlich added a comment.

In https://reviews.llvm.org/D39627#915820, @rjmccall wrote:

> Hmm.  We're calling this from getAddrOfVTable, which can be used to get a declaration for an external v-table, but setGlobalVisibility appears to set visibility as if we were emitting a definition.  Among other reasons, the difference is important because we generally don't honor implicit visibility when emitting external references.  The pragma should be treated as an explicit source of visibility — IIRC we model it by actually adding a visibility attribute — but I think your patch would cause us to use hidden visibility on an external v-table declaration even if the only reason we thought it was hidden was that we were compiling under -fvisibility=hidden.
>
> The logic for declarations is currently defined in the function setLinkageAndVisibilityForGV in CodeGenModule.cpp.  That's a static function in that file: we could just add it to CodeGenModule, but I think there's a better solution: we could add a ForDefinition_t flag to CGM::setGlobalVisibility and then unify these two implementations.


I'm missing some background lingo here. What is the meaning of "a declaration" and "a definition" here? It sounds like setLinkageAndVisibilityForGV will be called if the GlobalValue is synthetically generated (as vtables are) and doesn't have a corresponding deceleration in source? And setGlobalVisibility will be used to set visibility of GlobalValues mentioned in source?


Repository:
  rL LLVM

https://reviews.llvm.org/D39627





More information about the llvm-commits mailing list