[PATCH] Ensure __va_list_tag has default visibility

[Stephan Bergmann]

Is there any update on this?

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.