teemperor created this revision.
teemperor added reviewers: rsmith, v.g.vassilev, chandlerc.
The GNU C library includes headers from the `_Builtin_intrinsics` module. As
the `_Builtin_intrinsics` module via
the `mm_malloc.h` header also includes the `stdlib.h` header from libc, we get
a cyclic dependency with
`-fmodules` enabled. The best way to solve this is seems to be removing the
`stdlib.h` include from `mm_malloc.h`
and make the redeclarations in there work without the include.
This patch is doing this in two steps:
1. It reverts some of the changes done in r119958 which re-added the include to
`mm_malloc.h` and removed the forward declarations.
2. It expands the workaround in Sema::CheckEquivalentExceptionSpec to also work
in the case where we first declare a function with a missing empty exception
specification and then redeclare it with an empty exception specification.
The second part is necessary because the current workaround only works in the
case where the redeclaration
is missing an empty exception specification and the `#include <stdlib.h>`
before our redeclaration ensured that
we always have our declarations in this expected order.
I compiled a few projects with this patch (curl, ncnn, opencv, openjpeg,
scummvm, sqlite, zlib), and it doesn't
seem to break any compilation there.
Repository:
rC Clang
https://reviews.llvm.org/D43871
Files:
lib/Headers/mm_malloc.h
lib/Sema/SemaExceptionSpec.cpp
test/CXX/except/except.spec/libc-empty-except-sys/libc-empty-except.h
test/CXX/except/except.spec/libc-empty-except.cpp
Index: test/CXX/except/except.spec/libc-empty-except.cpp
===================================================================
--- /dev/null
+++ test/CXX/except/except.spec/libc-empty-except.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++11 -isystem %S/libc-empty-except-sys -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c++11 -I %S/libc-empty-except-sys -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+#include "libc-empty-except.h"
+
+void f() {
+ free(nullptr);
+}
Index: test/CXX/except/except.spec/libc-empty-except-sys/libc-empty-except.h
===================================================================
--- /dev/null
+++ test/CXX/except/except.spec/libc-empty-except-sys/libc-empty-except.h
@@ -0,0 +1,3 @@
+extern "C" void free(void *ptr) throw();
+// missing throw() is allowed in this case as we are in a system header.
+extern "C" void free(void *ptr);
Index: lib/Sema/SemaExceptionSpec.cpp
===================================================================
--- lib/Sema/SemaExceptionSpec.cpp
+++ lib/Sema/SemaExceptionSpec.cpp
@@ -213,6 +213,7 @@
const FunctionProtoType *New, SourceLocation NewLoc,
bool *MissingExceptionSpecification = nullptr,
bool *MissingEmptyExceptionSpecification = nullptr,
+ bool *ExtraEmptyExceptionSpecification = nullptr,
bool AllowNoexceptAllMatchWithNoSpec = false, bool IsOperatorNew = false);
/// Determine whether a function has an implicitly-generated exception
@@ -236,6 +237,15 @@
return !Ty->hasExceptionSpec();
}
+/// Returns true if the given function is a function/builtin with C linkage
+/// and from a system header.
+static bool isCSystemFuncOrBuiltin(FunctionDecl *D, ASTContext &Context) {
+ return (D->getLocation().isInvalid() ||
+ Context.getSourceManager().isInSystemHeader(D->getLocation()) ||
+ D->getBuiltinID()) &&
+ D->isExternC();
+}
+
bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl *New) {
// Just completely ignore this under -fno-exceptions prior to C++17.
// In C++17 onwards, the exception specification is part of the type and
@@ -247,6 +257,14 @@
bool IsOperatorNew = OO == OO_New || OO == OO_Array_New;
bool MissingExceptionSpecification = false;
bool MissingEmptyExceptionSpecification = false;
+ bool ExtraEmptyExceptionSpecification = false;
+ bool *AllowExtraEmptyExceptionSpecification = nullptr;
+
+ // If both functions are from C functions from system headers, we want to
+ // know if the redeclaration has an additional empty exception specification.
+ if (isCSystemFuncOrBuiltin(Old, Context) &&
+ isCSystemFuncOrBuiltin(New, Context))
+ AllowExtraEmptyExceptionSpecification = &ExtraEmptyExceptionSpecification;
unsigned DiagID = diag::err_mismatched_exception_spec;
bool ReturnValueOnError = true;
@@ -258,11 +276,12 @@
// Check the types as written: they must match before any exception
// specification adjustment is applied.
if (!CheckEquivalentExceptionSpecImpl(
- *this, PDiag(DiagID), PDiag(diag::note_previous_declaration),
- Old->getType()->getAs<FunctionProtoType>(), Old->getLocation(),
- New->getType()->getAs<FunctionProtoType>(), New->getLocation(),
- &MissingExceptionSpecification, &MissingEmptyExceptionSpecification,
- /*AllowNoexceptAllMatchWithNoSpec=*/true, IsOperatorNew)) {
+ *this, PDiag(DiagID), PDiag(diag::note_previous_declaration),
+ Old->getType()->getAs<FunctionProtoType>(), Old->getLocation(),
+ New->getType()->getAs<FunctionProtoType>(), New->getLocation(),
+ &MissingExceptionSpecification, &MissingEmptyExceptionSpecification,
+ AllowExtraEmptyExceptionSpecification,
+ /*AllowNoexceptAllMatchWithNoSpec=*/true, IsOperatorNew)) {
// C++11 [except.spec]p4 [DR1492]:
// If a declaration of a function has an implicit
// exception-specification, other declarations of the function shall
@@ -277,14 +296,28 @@
return false;
}
+ const FunctionProtoType *NewProto =
+ New->getType()->castAs<FunctionProtoType>();
+ const FunctionProtoType *OldProto =
+ Old->getType()->castAs<FunctionProtoType>();
+
+ // We know that both decls have C linkage, are from a system header and that
+ // the "new" decl has an extra empty exception specification "throw()".
+ // We just add an empty exception specification to the "old" function to make
+ // the redeclaration valid. This code complements the case below that handles
+ // MissingEmptyExceptionSpecification.
+ if (ExtraEmptyExceptionSpecification && OldProto) {
+ Old->setType(Context.getFunctionType(
+ OldProto->getReturnType(), OldProto->getParamTypes(),
+ OldProto->getExtProtoInfo().withExceptionSpec(EST_DynamicNone)));
+ return false;
+ }
+
// The failure was something other than an missing exception
// specification; return an error, except in MS mode where this is a warning.
if (!MissingExceptionSpecification)
return ReturnValueOnError;
- const FunctionProtoType *NewProto =
- New->getType()->castAs<FunctionProtoType>();
-
// The new function declaration is only missing an empty exception
// specification "throw()". If the throw() specification came from a
// function in a system header that has C linkage, just add an empty
@@ -294,18 +327,13 @@
//
// Likewise if the old function is a builtin.
if (MissingEmptyExceptionSpecification && NewProto &&
- (Old->getLocation().isInvalid() ||
- Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
- Old->getBuiltinID()) &&
- Old->isExternC()) {
+ isCSystemFuncOrBuiltin(Old, Context)) {
New->setType(Context.getFunctionType(
NewProto->getReturnType(), NewProto->getParamTypes(),
NewProto->getExtProtoInfo().withExceptionSpec(EST_DynamicNone)));
return false;
}
- const FunctionProtoType *OldProto =
- Old->getType()->castAs<FunctionProtoType>();
FunctionProtoType::ExceptionSpecInfo ESI = OldProto->getExceptionSpecType();
if (ESI.Type == EST_Dynamic) {
@@ -438,13 +466,17 @@
const FunctionProtoType *New, SourceLocation NewLoc,
bool *MissingExceptionSpecification,
bool *MissingEmptyExceptionSpecification,
+ bool *ExtraEmptyExceptionSpecification,
bool AllowNoexceptAllMatchWithNoSpec, bool IsOperatorNew) {
if (MissingExceptionSpecification)
*MissingExceptionSpecification = false;
if (MissingEmptyExceptionSpecification)
*MissingEmptyExceptionSpecification = false;
+ if (ExtraEmptyExceptionSpecification)
+ *ExtraEmptyExceptionSpecification = false;
+
Old = S.ResolveExceptionSpec(NewLoc, Old);
if (!Old)
return false;
@@ -563,6 +595,11 @@
// At this point, the only remaining valid case is two matching dynamic
// specifications. We return here unless both specifications are dynamic.
if (OldEST != EST_Dynamic || NewEST != EST_Dynamic) {
+ if (ExtraEmptyExceptionSpecification && !Old->hasExceptionSpec() &&
+ New->hasExceptionSpec()) {
+ *ExtraEmptyExceptionSpecification = true;
+ return true;
+ }
if (MissingExceptionSpecification && Old->hasExceptionSpec() &&
!New->hasExceptionSpec()) {
// The old type has an exception specification of some sort, but
Index: lib/Headers/mm_malloc.h
===================================================================
--- lib/Headers/mm_malloc.h
+++ lib/Headers/mm_malloc.h
@@ -24,19 +24,23 @@
#ifndef __MM_MALLOC_H
#define __MM_MALLOC_H
-#include <stdlib.h>
+#include <stddef.h>
#ifdef _WIN32
#include <malloc.h>
#else
#ifndef __cplusplus
extern int posix_memalign(void **__memptr, size_t __alignment, size_t __size);
+extern void(free)(void *ptr);
+extern void *(malloc)(size_t size) __attribute__((__malloc__));
#else
-// Some systems (e.g. those with GNU libc) declare posix_memalign with an
-// exception specifier. Via an "egregious workaround" in
-// Sema::CheckEquivalentExceptionSpec, Clang accepts the following as a valid
-// redeclaration of glibc's declaration.
+// Some systems (e.g. those with GNU libc) declare some of these functions with
+// an exception specifier. Via an "egregious workaround" in
+// Sema::CheckEquivalentExceptionSpec, Clang accepts the following as valid
+// (re)declarations of glibc's declarations.
extern "C" int posix_memalign(void **__memptr, size_t __alignment, size_t __size);
+extern "C" void(free)(void *ptr);
+extern "C" void *(malloc)(size_t size) __attribute__((__malloc__));
#endif
#endif
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
