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 @@ -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(), 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 @@ -277,14 +296,28 @@ 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()". + // 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(); - // 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::ExceptionSpecInfo ESI = OldProto->getExceptionSpecType(); if (ESI.Type == EST_Dynamic) { @@ -438,6 +466,7 @@ const FunctionProtoType *New, SourceLocation NewLoc, bool *MissingExceptionSpecification, bool *MissingEmptyExceptionSpecification, + bool *ExtraEmptyExceptionSpecification, bool AllowNoexceptAllMatchWithNoSpec, bool IsOperatorNew) { if (MissingExceptionSpecification) *MissingExceptionSpecification = false; @@ -445,6 +474,9 @@ 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: 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,33 @@ +// 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); +}