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 <[email protected]
<mailto:[email protected]>> wrote:
On 12/19/2014 05:13 AM, Richard Smith wrote:
On Tue, Dec 16, 2014 at 1:47 AM, Stephan Bergmann
<[email protected] <mailto:[email protected]>
<mailto:[email protected] <mailto:[email protected]>>> 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.
Index: lib/AST/ASTContext.cpp
===================================================================
diff --git a/cfe/trunk/lib/AST/ASTContext.cpp b/cfe/trunk/lib/AST/ASTContext.cpp
--- a/cfe/trunk/lib/AST/ASTContext.cpp (revision 225158)
+++ b/cfe/trunk/lib/AST/ASTContext.cpp (working copy)
@@ -876,6 +876,9 @@
NewDecl = RecordDecl::Create(*this, TK, getTranslationUnitDecl(), Loc, Loc,
&Idents.get(Name));
NewDecl->setImplicit();
+ NewDecl->addAttr(TypeVisibilityAttr::CreateImplicit(
+ *const_cast<ASTContext*>(this),
+ TypeVisibilityAttr::Default));
return NewDecl;
}
Index: test/CodeGenCXX/implicit-record-visibility.cpp
===================================================================
diff --git a/cfe/trunk/test/CodeGenCXX/implicit-record-visibility.cpp b/cfe/trunk/test/CodeGenCXX/implicit-record-visibility.cpp
new file mode 10644
--- /dev/null (revision 0)
+++ b/cfe/trunk/test/CodeGenCXX/implicit-record-visibility.cpp (working copy)
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -I%S -fvisibility hidden -triple x86_64-linux-gnu -emit-llvm -o - | FileCheck %s
+
+#include <stdarg.h>
+#include <typeinfo>
+
+// If struct __va_list_tag did not explicitly have default visibility, then
+// under -fvisibility hidden the type of function f, due to its va_list (aka
+// __builtin_va_list, aka __va_list_tag (*)[1]) parameter would be hidden:
+
+// CHECK: @_ZTSFvP13__va_list_tagE = linkonce_odr constant
+// CHECK: @_ZTIFvP13__va_list_tagE = linkonce_odr constant
+void f(va_list) { (void)typeid(f); }
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
