teemperor updated this revision to Diff 162940.
teemperor marked an inline comment as done.
teemperor added a comment.
- Reverted unintended change to test case that slipped into the latest diff.
https://reviews.llvm.org/D43871
Files:
lib/Headers/mm_malloc.h
lib/Sema/SemaExceptionSpec.cpp
test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
test/CXX/except/except.spec/Inputs/clang/module.modulemap
test/CXX/except/except.spec/Inputs/libc/module.modulemap
test/CXX/except/except.spec/Inputs/libc/stdlib.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,34 @@
+// RUN: %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// One of the headers is in a user include, so our redeclaration should fail.
+// RUN: not %clang_cc1 -std=c++11 -I %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -std=c++11 -isystem %S/Inputs/clang -I %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// The same test cases again with enabled modules.
+// The modules cases *all* pass because we marked both as [system].
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN: -std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN: -std=c++11 -isystem %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN: -std=c++11 -I %S/Inputs/clang -internal-externc-isystem %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fimplicit-module-maps -fmodules-cache-path=%t \
+// RUN: -std=c++11 -isystem %S/Inputs/clang -I %S/Inputs/libc -fexceptions -fcxx-exceptions -fsyntax-only -verify -DREVERSE %s
+
+// expected-no-diagnostics
+#ifdef REVERSE
+#include "mm_malloc.h"
+#include "stdlib.h"
+#else
+#include "mm_malloc.h"
+#include "stdlib.h"
+#endif
+
+void f() {
+ free(nullptr);
+}
Index: test/CXX/except/except.spec/Inputs/libc/stdlib.h
===================================================================
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/libc/stdlib.h
@@ -0,0 +1,2 @@
+// Declare 'free' like glibc with a empty exception specifier.
+extern "C" void free(void *ptr) throw();
Index: test/CXX/except/except.spec/Inputs/libc/module.modulemap
===================================================================
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/libc/module.modulemap
@@ -0,0 +1,4 @@
+module libc [system] {
+ header "stdlib.h"
+ export *
+}
Index: test/CXX/except/except.spec/Inputs/clang/module.modulemap
===================================================================
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/clang/module.modulemap
@@ -0,0 +1,4 @@
+module builtin [system] {
+ header "mm_malloc.h"
+ export *
+}
Index: test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
===================================================================
--- /dev/null
+++ test/CXX/except/except.spec/Inputs/clang/mm_malloc.h
@@ -0,0 +1,3 @@
+// missing throw() is allowed in this case as we are in a system header.
+// This is a redeclaration possibly from glibc.
+extern "C" void free(void *ptr);
Index: lib/Sema/SemaExceptionSpec.cpp
===================================================================
--- lib/Sema/SemaExceptionSpec.cpp
+++ lib/Sema/SemaExceptionSpec.cpp
@@ -236,6 +236,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
@@ -259,6 +260,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
@@ -270,6 +280,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;
@@ -285,6 +303,7 @@
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
@@ -300,14 +319,27 @@
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()". In
+ // this case we don't emit an error but instead update the previous
+ // declaration with the exception specification from the "new" decl. This is
+ // usually not permitted, but it's a necessary hack required for the
+ // redeclarations of free/malloc in mm_malloc.h.
+ if (ExtraEmptyExceptionSpecification && OldProto && NewProto) {
+ UpdateExceptionSpec(Old, NewProto->getExtProtoInfo().ExceptionSpec);
+ 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
@@ -317,19 +349,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) {
// FIXME: What if the exceptions are described in terms of the old
@@ -469,13 +495,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;
@@ -590,6 +620,12 @@
}
}
+ if (ExtraEmptyExceptionSpecification && !Old->hasExceptionSpec() &&
+ New->hasExceptionSpec()) {
+ *ExtraEmptyExceptionSpecification = true;
+ return true;
+ }
+
// If the caller wants to handle the case that the new function is
// incompatible due to a missing exception specification, let it.
if (MissingExceptionSpecification && OldEST != EST_None &&
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
