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 @@ -246,6 +246,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 @@ -269,6 +270,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 @@ -280,6 +290,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; @@ -299,11 +317,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(), Old->getLocation(), - New->getType()->getAs(), New->getLocation(), - &MissingExceptionSpecification, &MissingEmptyExceptionSpecification, - /*AllowNoexceptAllMatchWithNoSpec=*/true, IsOperatorNew)) { + *this, PDiag(DiagID), PDiag(diag::note_previous_declaration), + 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 // exception-specification, other declarations of the function shall @@ -318,14 +337,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 @@ -335,19 +367,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 @@ -487,6 +513,7 @@ const FunctionProtoType *New, SourceLocation NewLoc, bool *MissingExceptionSpecification, bool *MissingEmptyExceptionSpecification, + bool *ExtraEmptyExceptionSpecification, bool AllowNoexceptAllMatchWithNoSpec, bool IsOperatorNew) { if (MissingExceptionSpecification) *MissingExceptionSpecification = false; @@ -494,6 +521,9 @@ if (MissingEmptyExceptionSpecification) *MissingEmptyExceptionSpecification = false; + if (ExtraEmptyExceptionSpecification) + *ExtraEmptyExceptionSpecification = false; + Old = S.ResolveExceptionSpec(NewLoc, Old); if (!Old) return false; @@ -608,6 +638,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,2 @@ +// Missing throw() is allowed in this case as we are in a system header. +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); +}