[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