[PATCH] D17362: [Sema] PR23090 Crash when return type or parameter types for extern "C" functions don't have external linkage
hintonda created this revision.
hintonda added reviewers: majnemer, rsmith.
hintonda added a subscriber: cfe-commits.
Added checks to make sure the return type and parameter types for functions
declared as extern "C" have external linkage.
http://reviews.llvm.org/D17362
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
test/Sema/pr23090-crash-on-invalid.cpp
Index: test/Sema/pr23090-crash-on-invalid.cpp
===
--- /dev/null
+++ test/Sema/pr23090-crash-on-invalid.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash (PR23090).
+
+namespace {
+
+// check return type
+struct A;
+extern "C" A *foo(); // expected-error {{'foo' has C-linkage specified, but
return type '(anonymous namespace)::A *' has internal linkage}}
+A *foo();
+
+// check parameter
+struct B;
+extern "C" void bar(B*); // expected-error {{'bar' has C-linkage specified,
but parameter type '(anonymous namespace)::B *' has internal linkage}}
+void bar(B*);
+
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8206,6 +8206,25 @@
Diag(NewFD->getLocation(), diag::ext_out_of_line_declaration)
<< D.getCXXScopeSpec().getRange();
}
+
+if (NewFD->isExternC()) {
+ // Check return type linkage
+ QualType R = NewFD->getReturnType();
+ if (R.getTypePtr()->getLinkage() != Linkage::ExternalLinkage) {
+Diag(NewFD->getLocation(), diag::err_return_value_linkage)
+ << NewFD << R;
+NewFD->setInvalidDecl();
+ }
+ // Check parameter type linkage
+ for (auto param : NewFD->parameters()) {
+QualType P = param->getOriginalType();
+if (P.getTypePtr()->getLinkage() != Linkage::ExternalLinkage) {
+ Diag(NewFD->getLocation(), diag::err_parameter_value_linkage)
+<< NewFD << P;
+ NewFD->setInvalidDecl();
+}
+ }
+}
}
ProcessPragmaWeak(S, NewFD);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -319,6 +319,11 @@
def warn_unused_private_field: Warning<"private field %0 is not used">,
InGroup, DefaultIgnore;
+def err_return_value_linkage: Error<
+ "%0 has C-linkage specified, but return type %1 has internal linkage">;
+def err_parameter_value_linkage: Error<
+ "%0 has C-linkage specified, but parameter type %1 has internal linkage">;
+
def warn_parameter_size: Warning<
"%0 is a large (%1 bytes) pass-by-value argument; "
"pass it by reference instead ?">, InGroup;
Index: test/Sema/pr23090-crash-on-invalid.cpp
===
--- /dev/null
+++ test/Sema/pr23090-crash-on-invalid.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash (PR23090).
+
+namespace {
+
+// check return type
+struct A;
+extern "C" A *foo(); // expected-error {{'foo' has C-linkage specified, but return type '(anonymous namespace)::A *' has internal linkage}}
+A *foo();
+
+// check parameter
+struct B;
+extern "C" void bar(B*); // expected-error {{'bar' has C-linkage specified, but parameter type '(anonymous namespace)::B *' has internal linkage}}
+void bar(B*);
+
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8206,6 +8206,25 @@
Diag(NewFD->getLocation(), diag::ext_out_of_line_declaration)
<< D.getCXXScopeSpec().getRange();
}
+
+if (NewFD->isExternC()) {
+ // Check return type linkage
+ QualType R = NewFD->getReturnType();
+ if (R.getTypePtr()->getLinkage() != Linkage::ExternalLinkage) {
+Diag(NewFD->getLocation(), diag::err_return_value_linkage)
+ << NewFD << R;
+NewFD->setInvalidDecl();
+ }
+ // Check parameter type linkage
+ for (auto param : NewFD->parameters()) {
+QualType P = param->getOriginalType();
+if (P.getTypePtr()->getLinkage() != Linkage::ExternalLinkage) {
+ Diag(NewFD->getLocation(), diag::err_parameter_value_linkage)
+<< NewFD << P;
+ NewFD->setInvalidDecl();
+}
+ }
+}
}
ProcessPragmaWeak(S, NewFD);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -319,6 +319,11 @@
def warn_unused_private_field: Warning<"private field %0 is not used">,
InGroup, DefaultIgnore;
+def err_return_value_linkage: Error<
+ "%0 has C-linkage specified, but return type %1 has internal linkage">;
+def err_parameter_value_linkage: Error<
+ "%0 has C-linkage specified, bu
Re: [PATCH] D17362: [Sema] PR23090 Crash when return type or parameter types for extern "C" functions don't have external linkage
hintonda added a comment.
Here's an example of when this will cause a crash:
namespace {
Struct G;
extern "C" G *f(); // has external linkage
G *f();// will assert since it has internal linkage because of G
and doesn't match previous declaration
}
http://reviews.llvm.org/D17362
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D17362: [Sema] PR23090 Crash when return type or parameter types for extern "C" functions don't have external linkage
hintonda added a comment. This also causes a few other test failures (~8) which will need to be fixed if this patch is accepted. http://reviews.llvm.org/D17362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D17362: [Sema] PR23090 Crash when return type or parameter types for extern "C" functions don't have external linkage
hintonda added a comment. This change breaks a couple tests in test/SemaCXX/linkage.cpp where we no longer allow mixing internal and external linkage, i.e., extern "C" functions have external linkage no matter where they are declared, but return types and parameters declared in anonymous namespaces have internal linkage. Not yet sure how to go about fixing those, but one was based on PR9316, however, the test is more complicated and is invalid -- so we don't generate code. http://reviews.llvm.org/D17362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D17362: [Sema] PR23090 Crash when return type or parameter types for extern "C" functions don't have external linkage
hintonda updated this revision to Diff 48601.
hintonda added a comment.
Fixed additional tests.
http://reviews.llvm.org/D17362
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
test/CXX/drs/dr3xx.cpp
test/Sema/pr23090-crash-on-invalid.cpp
test/SemaCXX/warn-unused-filescoped.cpp
Index: test/SemaCXX/warn-unused-filescoped.cpp
===
--- test/SemaCXX/warn-unused-filescoped.cpp
+++ test/SemaCXX/warn-unused-filescoped.cpp
@@ -121,7 +121,7 @@
namespace { struct A {}; }
void test(A a); // expected-warning {{unused function}}
- extern "C" void test4(A a);
+ extern "C" void test4(A a); // expected-error {{'test4' has C-linkage
specified, but parameter type 'test4::(anonymous namespace)::A' has internal
linkage}}
}
namespace rdar8733476 {
Index: test/Sema/pr23090-crash-on-invalid.cpp
===
--- /dev/null
+++ test/Sema/pr23090-crash-on-invalid.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash (PR23090).
+
+namespace {
+
+// check return type
+struct A;
+extern "C" A *foo(); // expected-error {{'foo' has C-linkage specified, but
return type '(anonymous namespace)::A *' has internal linkage}}
+A *foo();
+
+// check parameter
+struct B;
+extern "C" void bar(B*); // expected-error {{'bar' has C-linkage specified,
but parameter type '(anonymous namespace)::B *' has internal linkage}}
+void bar(B*);
+
+}
Index: test/CXX/drs/dr3xx.cpp
===
--- test/CXX/drs/dr3xx.cpp
+++ test/CXX/drs/dr3xx.cpp
@@ -232,8 +232,8 @@
typedef struct {
int i;
} *ps;
- extern "C" void f(ps);
- void g(ps); // FIXME: ill-formed, type 'ps' has no linkage
+ extern "C" void f(ps); // expected-error-re {{'f' has C-linkage specified,
but parameter type 'ps' (aka 'dr319::(anonymous struct {{.*}} *') has internal
linkage}}
+ void g(ps);
static enum { e } a1;
enum { e2 } a2; // FIXME: ill-formed, enum type has no linkage
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8218,6 +8218,25 @@
Diag(NewFD->getLocation(), diag::ext_out_of_line_declaration)
<< D.getCXXScopeSpec().getRange();
}
+
+if (NewFD->isExternC()) {
+ // Check return type linkage
+ QualType R = NewFD->getReturnType();
+ if (R.getTypePtr()->getLinkage() != Linkage::ExternalLinkage) {
+Diag(NewFD->getLocation(), diag::err_return_value_linkage)
+ << NewFD << R;
+NewFD->setInvalidDecl();
+ }
+ // Check parameter type linkage
+ for (auto param : NewFD->parameters()) {
+QualType P = param->getOriginalType();
+if (P.getTypePtr()->getLinkage() != Linkage::ExternalLinkage) {
+ Diag(NewFD->getLocation(), diag::err_parameter_value_linkage)
+<< NewFD << P;
+ NewFD->setInvalidDecl();
+}
+ }
+}
}
ProcessPragmaWeak(S, NewFD);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -325,6 +325,11 @@
def warn_unused_private_field: Warning<"private field %0 is not used">,
InGroup, DefaultIgnore;
+def err_return_value_linkage: Error<
+ "%0 has C-linkage specified, but return type %1 has internal linkage">;
+def err_parameter_value_linkage: Error<
+ "%0 has C-linkage specified, but parameter type %1 has internal linkage">;
+
def warn_parameter_size: Warning<
"%0 is a large (%1 bytes) pass-by-value argument; "
"pass it by reference instead ?">, InGroup;
Index: test/SemaCXX/warn-unused-filescoped.cpp
===
--- test/SemaCXX/warn-unused-filescoped.cpp
+++ test/SemaCXX/warn-unused-filescoped.cpp
@@ -121,7 +121,7 @@
namespace { struct A {}; }
void test(A a); // expected-warning {{unused function}}
- extern "C" void test4(A a);
+ extern "C" void test4(A a); // expected-error {{'test4' has C-linkage specified, but parameter type 'test4::(anonymous namespace)::A' has internal linkage}}
}
namespace rdar8733476 {
Index: test/Sema/pr23090-crash-on-invalid.cpp
===
--- /dev/null
+++ test/Sema/pr23090-crash-on-invalid.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash (PR23090).
+
+namespace {
+
+// check return type
+struct A;
+extern "C" A *foo(); // expected-error {{'foo' has C-linkage specified, but return type '(anonymous namespace)::A *' has internal linkage}}
+A *foo();
+
+// check parameter
+struct B;
+extern "C" void bar(B*); // expected-error {{'bar' has C-linkage specified, but parameter type '(anonymous namespace)::B *' has internal li
Re: [PATCH] D17362: [Sema] PR23090 Crash when return type or parameter types for extern "C" functions don't have external linkage
hintonda updated this revision to Diff 48617.
hintonda added a comment.
- Fix remaining test failures caused by linkage errors.
http://reviews.llvm.org/D17362
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
test/CXX/drs/dr3xx.cpp
test/Sema/pr23090-crash-on-invalid.cpp
test/SemaCXX/linkage.cpp
test/SemaCXX/warn-unused-filescoped.cpp
Index: test/SemaCXX/warn-unused-filescoped.cpp
===
--- test/SemaCXX/warn-unused-filescoped.cpp
+++ test/SemaCXX/warn-unused-filescoped.cpp
@@ -121,7 +121,6 @@
namespace { struct A {}; }
void test(A a); // expected-warning {{unused function}}
- extern "C" void test4(A a);
}
namespace rdar8733476 {
Index: test/SemaCXX/linkage.cpp
===
--- test/SemaCXX/linkage.cpp
+++ test/SemaCXX/linkage.cpp
@@ -57,43 +57,21 @@
namespace test3 {
namespace { struct A {}; }
+ struct B {};
// CHECK: define internal void @_ZN5test34testENS_12_GLOBAL__N_11AE(
void test(A a) {}
void force() { test(A()); }
// CHECK: define void @test3(
- extern "C" void test3(A a) {}
+ extern "C" void test3(B b) {}
}
namespace {
// CHECK: define void @test4(
extern "C" void test4(void) {}
}
-// PR9316: Ensure that even non-namespace-scope function declarations in
-// a C declaration context respect that over the anonymous namespace.
-extern "C" {
- namespace {
-struct X {
- int f() {
-extern int g();
-extern int a;
-
-// Test both for mangling in the code generation and warnings from use
-// of internal, undefined names via -Werror.
-// CHECK: call i32 @g(
-// CHECK: load i32, i32* @a,
-return g() + a;
- }
-};
- }
- // Force the above function to be emitted by codegen.
- int test(X& x) {
-return x.f();
- }
-}
-
// CHECK: define linkonce_odr i8* @_ZN5test11A3fooILj0EEEPvv(
// CHECK: define linkonce_odr i8* @_ZN5test21A1BILj0EE3fooEv(
Index: test/Sema/pr23090-crash-on-invalid.cpp
===
--- /dev/null
+++ test/Sema/pr23090-crash-on-invalid.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// Don't crash (PR23090).
+
+namespace {
+
+// check return type
+struct A;
+extern "C" A *foo(); // expected-error {{'foo' has C-linkage specified, but return type '(anonymous namespace)::A *' has internal linkage}}
+A *foo();
+
+// check parameter
+struct B;
+extern "C" void bar(B*); // expected-error {{'bar' has C-linkage specified, but parameter type '(anonymous namespace)::B *' has internal linkage}}
+void bar(B*);
+
+}
Index: test/CXX/drs/dr3xx.cpp
===
--- test/CXX/drs/dr3xx.cpp
+++ test/CXX/drs/dr3xx.cpp
@@ -232,8 +232,8 @@
typedef struct {
int i;
} *ps;
- extern "C" void f(ps);
- void g(ps); // FIXME: ill-formed, type 'ps' has no linkage
+ extern "C" void f(ps); // expected-error-re {{'f' has C-linkage specified, but parameter type 'ps' (aka 'dr319::(anonymous struct {{.*}} *') has internal linkage}}
+ void g(ps);
static enum { e } a1;
enum { e2 } a2; // FIXME: ill-formed, enum type has no linkage
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8218,6 +8218,25 @@
Diag(NewFD->getLocation(), diag::ext_out_of_line_declaration)
<< D.getCXXScopeSpec().getRange();
}
+
+if (NewFD->isExternC()) {
+ // Check return type linkage
+ QualType R = NewFD->getReturnType();
+ if (R.getTypePtr()->getLinkage() != Linkage::ExternalLinkage) {
+Diag(NewFD->getLocation(), diag::err_return_value_linkage)
+ << NewFD << R;
+NewFD->setInvalidDecl();
+ }
+ // Check parameter type linkage
+ for (auto param : NewFD->parameters()) {
+QualType P = param->getOriginalType();
+if (P.getTypePtr()->getLinkage() != Linkage::ExternalLinkage) {
+ Diag(NewFD->getLocation(), diag::err_parameter_value_linkage)
+<< NewFD << P;
+ NewFD->setInvalidDecl();
+}
+ }
+}
}
ProcessPragmaWeak(S, NewFD);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -325,6 +325,11 @@
def warn_unused_private_field: Warning<"private field %0 is not used">,
InGroup, DefaultIgnore;
+def err_return_value_linkage: Error<
+ "%0 has C-linkage specified, but return type %1 has internal linkage">;
+def err_parameter_value_linkage: Error<
+ "%0 has C-linkage specified, but parameter type %1 has internal linkage">;
+
def warn_parameter_size: Warning<
"%0 is a large (%1 bytes) pass-by-value argument; "
"pass it by referen
Re: [PATCH] D17362: [Sema] PR23090 Crash when return type or parameter types for extern "C" functions don't have external linkage
rsmith requested changes to this revision. rsmith added a comment. This revision now requires changes to proceed. Why do you think this is ill-formed? Using a type with internal linkage as the type of an extern "C" function is explicitly permitted by the C++ standard. [basic.link]p8 says: "A type without linkage shall not be used as the type of a variable or function with external linkage unless the entity has C language linkage [...]." http://reviews.llvm.org/D17362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
