[PATCH] Ensure __va_list_tag has default visibility

Richard Smith richard at metafoo.co.uk
Thu Dec 18 20:13:01 PST 2014


On Tue, Dec 16, 2014 at 1:47 AM, Stephan Bergmann <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.


> 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/20141218/cbda357a/attachment.html>


More information about the cfe-commits mailing list