[PATCH] Ensure __va_list_tag has default visibility

David Majnemer david.majnemer at gmail.com
Thu Jan 15 00:42:51 PST 2015


Committed in r226148.

On Thu, Jan 15, 2015 at 12:07 AM, Stephan Bergmann <sbergman at redhat.com>
wrote:

> 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.
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150115/3e3104fe/attachment.html>


More information about the cfe-commits mailing list