[PATCH] Ensure __va_list_tag has default visibility

Stephan Bergmann sbergman at redhat.com
Thu Jan 15 00:07:09 PST 2015


On 01/15/2015 03:31 AM, Richard Smith wrote:
> LGTM, thanks!

Can somebody please commit this, as I don't have commit access.  Thanks.

[Is there some general expectation that people w/o commit rights mark 
their patch mails in a certain way that makes it obvious they ask for 
review + commit?]

> On Tue, Jan 6, 2015 at 12:03 PM, Stephan Bergmann <sbergman at redhat.com
> <mailto:sbergman at redhat.com>> wrote:
>
>     On 12/19/2014 05:13 AM, Richard Smith wrote:
>
>         On Tue, Dec 16, 2014 at 1:47 AM, Stephan Bergmann
>         <sbergman at redhat.com <mailto:sbergman at redhat.com>
>         <mailto:sbergman at redhat.com <mailto:sbergman at redhat.com>>> wrote:
>
>              Is there any update on this?
>
>
>         Please move the attribute to buildImplicitRecord; this attribute
>         should
>         be applied to all implicitly-declared record types. Also, please
>         reformat the patch to 80 columns and add a testcase.
>
>
>     Done, see attached va_list_tag_default___visibility.patch.  (I added
>     the test to test/CodeGenCXX/ so that it can #include <typeinfo>.)
>
>              On 07/09/2014 10:37 AM, Alp Toker wrote:
>
>                  On 09/07/2014 11:29, Alp Toker wrote:
>
>                      On 09/07/2014 09:55, Stephan Bergmann wrote:
>
>                          ping
>
>                          On 06/25/2014 05:45 PM, Stephan Bergmann wrote:
>
>                              I stumbled across this on Linux with
>                              -fvisibility=hidden when
>                              -fsanitize=function reported a false
>         positive for an
>                              indirect call to a
>                              function with a va_list (aka
>         __builtin_va_list, aka
>                              __va_list_tag
>                              (*)[1]) parameter.  Because __va_list_tag was
>                              considered hidden, the
>                              RTTI for the function's type was not
>         exported from
>                              the two shared
>                              libraries involved, so the equality check
>         failed.
>
>                              This would apparently also need to be fixed
>         in the other
>                              Create*BuiltinVaListDecl variants, but I'm
>         not sure
>                              (a) whether the
>                              added const_cast is really deemed
>         appropriate here
>                              (though I guess so,
>                              given the large number of existing similar
>                              const_casts across
>                              ASTContext.cpp), and (b) what the preferred way
>                              would be to break that
>                              long line. ;)
>
>
>                      Hi Stephan,
>
>                      I wrote the buildImplicitRecord() utility, and in
>         principle the
>                      attribute could be added centrally to that function
>         -- if
>                      these are
>                      the semantics we want for most built-in types, as you
>                      suggest, it
>                      makes sense to centralize the decision.
>
>                      Another option would be to calculate the special
>         linkage in
>                      computeLVForDecl() instead of using an attribute,
>         but it doesn't
>                      really matter either way I think as the result is
>         the same.
>
>                      The bigger question is whether (1) default
>         visibility is
>                      correct for
>                      all built-in types currently being generated by
>                      buildImplicitRecord()
>                      and (2) whether this is a valid choice for all
>         targets we
>                      support
>                      including MSVC drop-in compatibility mode.
>
>                      To help answer question (1), the affected built-in
>         types
>                      would be:
>
>                      lib/AST/ASTContext.cpp:    Float128StubDecl =
>                      buildImplicitRecord("______float128");
>                      lib/AST/ASTContext.cpp:    CFConstantStringTypeDecl =
>                      buildImplicitRecord("____NSConstantString");
>                      lib/AST/ASTContext.cpp:    RecordDecl
>         *ObjCSuperTypeDecl =
>                      buildImplicitRecord("objc_____super");
>                      lib/AST/ASTContext.cpp:  RD =
>                      buildImplicitRecord("__block_____descriptor");
>                      lib/AST/ASTContext.cpp:  RD =
>
>         buildImplicitRecord("__block_____descriptor_withcopydispose");
>                      lib/AST/ASTContext.cpp:  RecordDecl *VaListTagDecl =
>                      Context->buildImplicitRecord("______va_list");
>                      lib/AST/ASTContext.cpp:  VaListTagDecl =
>                      Context->buildImplicitRecord("______va_list_tag");
>                      lib/AST/ASTContext.cpp:  VaListTagDecl =
>                      Context->buildImplicitRecord("______va_list_tag");
>                      lib/AST/ASTContext.cpp:  RecordDecl *VaListDecl =
>                      Context->buildImplicitRecord("______va_list");
>                      lib/AST/ASTContext.cpp:  VaListTagDecl =
>                      Context->buildImplicitRecord("______va_list_tag");
>                      lib/CodeGen/CodeGenModule.cpp:    RecordDecl *D =
>                      Context.buildImplicitRecord("______builtin_NSString");
>                      lib/CodeGen/CodeGenModule.cpp:    RecordDecl *D =
>
>         Context.buildImplicitRecord("______objcFastEnumerationState");
>                      lib/Sema/Sema.cpp:
>
>         PushOnScopeChains(Context.____buildImplicitRecord("type_____info",
>                      TTK_Class),
>
>
>                  There are also the implicit typedefs -- I don't know
>         what the
>                  visibility/export deal is for those:
>
>
>                  lib/AST/ASTContext.cpp:    Int128Decl =
>                  buildImplicitTypedef(Int128Ty,
>                  "__int128_t");
>                  lib/AST/ASTContext.cpp:    UInt128Decl =
>                  buildImplicitTypedef(____UnsignedInt128Ty, "__uint128_t");
>                  lib/AST/ASTContext.cpp:
>                  buildImplicitTypedef(____getObjCIdType(),
>                  "instancetype");
>                  lib/AST/ASTContext.cpp:    ObjCIdDecl =
>         buildImplicitTypedef(T,
>                  "id");
>                  lib/AST/ASTContext.cpp:    ObjCSelDecl =
>         buildImplicitTypedef(T,
>                  "SEL");
>                  lib/AST/ASTContext.cpp:    ObjCClassDecl =
>         buildImplicitTypedef(T,
>                  "Class");
>                  lib/AST/ASTContext.cpp:  return
>         Context->buildImplicitTypedef(____T,
>                  "__builtin_va_list");
>                  lib/AST/ASTContext.cpp:  return
>         Context->buildImplicitTypedef(____T,
>                  "__builtin_va_list");
>                  lib/AST/ASTContext.cpp:  return
>                  Context->buildImplicitTypedef(____VaListTagType,
>         "__builtin_va_list");
>                  lib/AST/ASTContext.cpp:
>                  Context->buildImplicitTypedef(____VaListTagType,
>                  "__va_list_tag");
>                  lib/AST/ASTContext.cpp:  return
>                  Context->buildImplicitTypedef(____VaListTagArrayType,
>                  "__builtin_va_list");
>                  lib/AST/ASTContext.cpp:
>                  Context->buildImplicitTypedef(____VaListTagType,
>                  "__va_list_tag");
>                  lib/AST/ASTContext.cpp:  return
>                  Context->buildImplicitTypedef(____VaListTagArrayType,
>                  "__builtin_va_list");
>                  lib/AST/ASTContext.cpp:  return
>                  Context->buildImplicitTypedef(____IntArrayType,
>         "__builtin_va_list");
>                  lib/AST/ASTContext.cpp:  return
>         Context->buildImplicitTypedef(____T,
>                  "__builtin_va_list");
>                  lib/AST/ASTContext.cpp:
>                  Context->buildImplicitTypedef(____VaListTagType,
>                  "__va_list_tag");
>                  lib/AST/ASTContext.cpp:  return
>                  Context->buildImplicitTypedef(____VaListTagArrayType,
>                  "__builtin_va_list");
>                  lib/Sema/Sema.cpp:
>                  PushOnScopeChains(Context.____buildImplicitTypedef(T,
>                  Name), TUScope);
>
>                  Alp.
>
>
>                      I'll need help answering those questions from
>                      linkage/visibility folks
>                      on the list before going forward with a change like
>         this.
>
>                      Thanks
>                      Alp.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: va_list_tag_default_visibility.patch
Type: text/x-patch
Size: 1597 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150115/3752c1a0/attachment.bin>


More information about the cfe-commits mailing list