<div dir="ltr">LGTM, thanks!</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 6, 2015 at 12:03 PM, Stephan Bergmann <span dir="ltr"><<a href="mailto:sbergman@redhat.com" target="_blank">sbergman@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 12/19/2014 05:13 AM, Richard Smith wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
On Tue, Dec 16, 2014 at 1:47 AM, Stephan Bergmann <<a href="mailto:sbergman@redhat.com" target="_blank">sbergman@redhat.com</a><br></span><span class="">
<mailto:<a href="mailto:sbergman@redhat.com" target="_blank">sbergman@redhat.com</a>>> wrote:<br>
<br>
Is there any update on this?<br>
<br>
<br>
Please move the attribute to buildImplicitRecord; this attribute should<br>
be applied to all implicitly-declared record types. Also, please<br>
reformat the patch to 80 columns and add a testcase.<br>
</span></blockquote>
<br>
Done, see attached va_list_tag_default_<u></u>visibility.patch. (I added the test to test/CodeGenCXX/ so that it can #include <typeinfo>.)<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
On 07/09/2014 10:37 AM, Alp Toker wrote:<br>
<br>
On 09/07/2014 11:29, Alp Toker wrote:<br>
<br>
On 09/07/2014 09:55, Stephan Bergmann wrote:<br>
<br>
ping<br>
<br>
On 06/25/2014 05:45 PM, Stephan Bergmann wrote:<br>
<br>
I stumbled across this on Linux with<br>
-fvisibility=hidden when<br>
-fsanitize=function reported a false positive for an<br>
indirect call to a<br>
function with a va_list (aka __builtin_va_list, aka<br>
__va_list_tag<br>
(*)[1]) parameter. Because __va_list_tag was<br>
considered hidden, the<br>
RTTI for the function's type was not exported from<br>
the two shared<br>
libraries involved, so the equality check failed.<br>
<br>
This would apparently also need to be fixed in the other<br>
Create*BuiltinVaListDecl variants, but I'm not sure<br>
(a) whether the<br>
added const_cast is really deemed appropriate here<br>
(though I guess so,<br>
given the large number of existing similar<br>
const_casts across<br>
ASTContext.cpp), and (b) what the preferred way<br>
would be to break that<br>
long line. ;)<br>
<br>
<br>
Hi Stephan,<br>
<br>
I wrote the buildImplicitRecord() utility, and in principle the<br>
attribute could be added centrally to that function -- if<br>
these are<br>
the semantics we want for most built-in types, as you<br>
suggest, it<br>
makes sense to centralize the decision.<br>
<br>
Another option would be to calculate the special linkage in<br>
computeLVForDecl() instead of using an attribute, but it doesn't<br>
really matter either way I think as the result is the same.<br>
<br>
The bigger question is whether (1) default visibility is<br>
correct for<br>
all built-in types currently being generated by<br>
buildImplicitRecord()<br>
and (2) whether this is a valid choice for all targets we<br>
support<br>
including MSVC drop-in compatibility mode.<br>
<br>
To help answer question (1), the affected built-in types<br>
would be:<br>
<br>
lib/AST/ASTContext.cpp: Float128StubDecl =<br></div></div>
buildImplicitRecord("____<u></u>float128");<br>
lib/AST/ASTContext.cpp: CFConstantStringTypeDecl =<br>
buildImplicitRecord("__<u></u>NSConstantString");<br>
lib/AST/ASTContext.cpp: RecordDecl *ObjCSuperTypeDecl =<br>
buildImplicitRecord("objc___<u></u>super");<br>
lib/AST/ASTContext.cpp: RD =<br>
buildImplicitRecord("__block__<u></u>_descriptor");<br>
lib/AST/ASTContext.cpp: RD =<br>
buildImplicitRecord("__block__<u></u>_descriptor_withcopydispose");<br>
lib/AST/ASTContext.cpp: RecordDecl *VaListTagDecl =<br>
Context->buildImplicitRecord("<u></u>____va_list");<br>
lib/AST/ASTContext.cpp: VaListTagDecl =<br>
Context->buildImplicitRecord("<u></u>____va_list_tag");<br>
lib/AST/ASTContext.cpp: VaListTagDecl =<br>
Context->buildImplicitRecord("<u></u>____va_list_tag");<br>
lib/AST/ASTContext.cpp: RecordDecl *VaListDecl =<br>
Context->buildImplicitRecord("<u></u>____va_list");<br>
lib/AST/ASTContext.cpp: VaListTagDecl =<br>
Context->buildImplicitRecord("<u></u>____va_list_tag");<br>
lib/CodeGen/CodeGenModule.cpp: RecordDecl *D =<br>
Context.buildImplicitRecord("_<u></u>___builtin_NSString");<br>
lib/CodeGen/CodeGenModule.cpp: RecordDecl *D =<br>
Context.buildImplicitRecord("_<u></u>___objcFastEnumerationState");<br>
lib/Sema/Sema.cpp:<br>
PushOnScopeChains(Context.__<u></u>buildImplicitRecord("type___<u></u>info",<span class=""><br>
TTK_Class),<br>
<br>
<br>
There are also the implicit typedefs -- I don't know what the<br>
visibility/export deal is for those:<br>
<br>
<br>
lib/AST/ASTContext.cpp: Int128Decl =<br>
buildImplicitTypedef(Int128Ty,<br>
"__int128_t");<br>
lib/AST/ASTContext.cpp: UInt128Decl =<br></span>
buildImplicitTypedef(__<u></u>UnsignedInt128Ty, "__uint128_t");<br>
lib/AST/ASTContext.cpp:<br>
buildImplicitTypedef(__<u></u>getObjCIdType(),<span class=""><br>
"instancetype");<br>
lib/AST/ASTContext.cpp: ObjCIdDecl = buildImplicitTypedef(T,<br>
"id");<br>
lib/AST/ASTContext.cpp: ObjCSelDecl = buildImplicitTypedef(T,<br>
"SEL");<br>
lib/AST/ASTContext.cpp: ObjCClassDecl = buildImplicitTypedef(T,<br>
"Class");<br></span>
lib/AST/ASTContext.cpp: return Context->buildImplicitTypedef(<u></u>__T,<br>
"__builtin_va_list");<br>
lib/AST/ASTContext.cpp: return Context->buildImplicitTypedef(<u></u>__T,<br>
"__builtin_va_list");<br>
lib/AST/ASTContext.cpp: return<br>
Context->buildImplicitTypedef(<u></u>__VaListTagType, "__builtin_va_list");<br>
lib/AST/ASTContext.cpp:<br>
Context->buildImplicitTypedef(<u></u>__VaListTagType,<br>
"__va_list_tag");<br>
lib/AST/ASTContext.cpp: return<br>
Context->buildImplicitTypedef(<u></u>__VaListTagArrayType,<br>
"__builtin_va_list");<br>
lib/AST/ASTContext.cpp:<br>
Context->buildImplicitTypedef(<u></u>__VaListTagType,<br>
"__va_list_tag");<br>
lib/AST/ASTContext.cpp: return<br>
Context->buildImplicitTypedef(<u></u>__VaListTagArrayType,<br>
"__builtin_va_list");<br>
lib/AST/ASTContext.cpp: return<br>
Context->buildImplicitTypedef(<u></u>__IntArrayType, "__builtin_va_list");<br>
lib/AST/ASTContext.cpp: return Context->buildImplicitTypedef(<u></u>__T,<br>
"__builtin_va_list");<br>
lib/AST/ASTContext.cpp:<br>
Context->buildImplicitTypedef(<u></u>__VaListTagType,<br>
"__va_list_tag");<br>
lib/AST/ASTContext.cpp: return<br>
Context->buildImplicitTypedef(<u></u>__VaListTagArrayType,<br>
"__builtin_va_list");<br>
lib/Sema/Sema.cpp:<br>
PushOnScopeChains(Context.__<u></u>buildImplicitTypedef(T,<span class=""><br>
Name), TUScope);<br>
<br>
Alp.<br>
<br>
<br>
I'll need help answering those questions from<br>
linkage/visibility folks<br>
on the list before going forward with a change like this.<br>
<br>
Thanks<br>
Alp.<br>
</span></blockquote>
</blockquote></div><br></div>