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 +#include #ifdef _WIN32 #include #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 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(), Old->getLocation(), New->getType()->getAs(), 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(); + const FunctionProtoType *OldProto = + Old->getType()->castAs(); + + // 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(); - // 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::ExceptionSpecInfo ESI = OldProto->getExceptionSpecType(); if (ESI.Type == EST_Dynamic) { // FIXME: What if the exceptions are described in terms of the old @@ -469,6 +495,7 @@ const FunctionProtoType *New, SourceLocation NewLoc, bool *MissingExceptionSpecification, bool *MissingEmptyExceptionSpecification, + bool *ExtraEmptyExceptionSpecification, bool AllowNoexceptAllMatchWithNoSpec, bool IsOperatorNew) { if (MissingExceptionSpecification) *MissingExceptionSpecification = false; @@ -476,6 +503,9 @@ 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: 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: 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/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/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/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); +}