[PATCH] Ensure __va_list_tag has default visibility
Stephan Bergmann
sbergman at redhat.com
Thu Jan 15 00:07:09 PST 2015
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.
-------------- 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/20150115/3752c1a0/attachment.bin>
More information about the cfe-commits
mailing list