[PATCH] Ensure __va_list_tag has default visibility

[Stephan Bergmann]

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>> 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/20150106/e0f79aa1/attachment.bin>