<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>