<p dir="ltr"><br>
On Feb 17, 2015 5:07 PM, "Larisse Voufo" <<a href="mailto:lvoufo@google.com">lvoufo@google.com</a>> wrote:<br>
><br>
><br>
><br>
> On Sun, Feb 15, 2015 at 6:10 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
>><br>
>> On Fri, Feb 13, 2015 at 9:42 PM, Larisse Voufo <<a href="mailto:lvoufo@google.com">lvoufo@google.com</a>> wrote:<br>
>>><br>
>>> Author: lvoufo<br>
>>> Date: Fri Feb 13 23:42:57 2015<br>
>>> New Revision: 229241<br>
>>><br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=229241&view=rev">http://llvm.org/viewvc/llvm-project?rev=229241&view=rev</a><br>
>>> Log:<br>
>>> Revise the implementation logic of sized deallocation: Do not automatically generate weak definitions of the sized operator delete (in terms of unsized operator delete). Instead, provide the funcitonality via a new compiler flag, -fdef-sized-delete.<br>
>>> The current implementation causes link-time ODR violations when the delete symbols are exported into the dynamic table.<br>
>>><br>
>>> Modified:<br>
>>> cfe/trunk/include/clang/Basic/LangOptions.def<br>
>>> cfe/trunk/include/clang/Driver/Options.td<br>
>>> cfe/trunk/lib/CodeGen/CodeGenFunction.cpp<br>
>>> cfe/trunk/lib/CodeGen/CodeGenModule.cpp<br>
>>> cfe/trunk/lib/Driver/Tools.cpp<br>
>>> cfe/trunk/lib/Frontend/CompilerInvocation.cpp<br>
>>> cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp<br>
>>> cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp<br>
>>><br>
>>> Modified: cfe/trunk/include/clang/Basic/LangOptions.def<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=229241&r1=229240&r2=229241&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=229241&r1=229240&r2=229241&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/include/clang/Basic/LangOptions.def (original)<br>
>>> +++ cfe/trunk/include/clang/Basic/LangOptions.def Fri Feb 13 23:42:57 2015<br>
>>> @@ -162,6 +162,7 @@ LANGOPT(CUDAIsDevice , 1, 0, "Compi<br>
>>><br>
>>> LANGOPT(AssumeSaneOperatorNew , 1, 1, "implicit __attribute__((malloc)) for C++'s new operators")<br>
>>> LANGOPT(SizedDeallocation , 1, 0, "enable sized deallocation functions")<br>
>>> +LANGOPT(DefaultSizedDelete , 1, 0, "Generate weak definitions of sized delete")<br>
>><br>
>><br>
>> Please start this string with a lowercase letter. How about DefineSizedDeallocation, to match the existing SizedDeallocation LangOpt? ("Define" seems like a better word than "Default" here in any case.)<br>
>> <br>
>>><br>
>>> BENIGN_LANGOPT(ElideConstructors , 1, 1, "C++ copy constructor elision")<br>
>>> BENIGN_LANGOPT(DumpRecordLayouts , 1, 0, "dumping the layout of IRgen'd records")<br>
>>> BENIGN_LANGOPT(DumpRecordLayoutsSimple , 1, 0, "dumping the layout of IRgen'd records in a simple form")<br>
>>><br>
>>> Modified: cfe/trunk/include/clang/Driver/Options.td<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=229241&r1=229240&r2=229241&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=229241&r1=229240&r2=229241&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/include/clang/Driver/Options.td (original)<br>
>>> +++ cfe/trunk/include/clang/Driver/Options.td Fri Feb 13 23:42:57 2015<br>
>>> @@ -394,6 +394,8 @@ def fasm_blocks : Flag<["-"], "fasm-bloc<br>
>>> def fno_asm_blocks : Flag<["-"], "fno-asm-blocks">, Group<f_Group>;<br>
>>><br>
>>> def fassume_sane_operator_new : Flag<["-"], "fassume-sane-operator-new">, Group<f_Group>;<br>
>>> +def fdef_sized_delete: Flag<["-"], "fdef-sized-delete">, Group<f_Group>,<br>
>>> + HelpText<"Allow compiler-generated definition of sized deallocation function">, Flags<[CC1Option]>;<br>
>><br>
>><br>
>> Likewise here, how about -fdefine-sized-deallocation, to match the existing -fsized-deallocation flag?<br>
><br>
><br>
> Thanks, Richard. Please see r229597 for the above comments.<br>
> <br>
>><br>
>><br>
>>> def fastcp : Flag<["-"], "fastcp">, Group<f_Group>;<br>
>>> def fastf : Flag<["-"], "fastf">, Group<f_Group>;<br>
>>> def fast : Flag<["-"], "fast">, Group<f_Group>;<br>
>>><br>
>>> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=229241&r1=229240&r2=229241&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=229241&r1=229240&r2=229241&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)<br>
>>> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Fri Feb 13 23:42:57 2015<br>
>>> @@ -891,8 +891,11 @@ void CodeGenFunction::GenerateCode(Globa<br>
>>> } else if (FunctionDecl *UnsizedDealloc =<br>
>>> FD->getCorrespondingUnsizedGlobalDeallocationFunction()) {<br>
>>> // Global sized deallocation functions get an implicit weak definition if<br>
>>> - // they don't have an explicit definition.<br>
>>> + // they don't have an explicit definition, if allowed.<br>
>>> + assert(getLangOpts().DefaultSizedDelete &&<br>
>>> + "Can't emit unallowed definition.");<br>
>>> EmitSizedDeallocationFunction(*this, UnsizedDealloc);<br>
>>> +<br>
>>> } else<br>
>>> llvm_unreachable("no definition for emitted function");<br>
>>><br>
>>><br>
>>> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=229241&r1=229240&r2=229241&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=229241&r1=229240&r2=229241&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)<br>
>>> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Fri Feb 13 23:42:57 2015<br>
>>> @@ -1615,13 +1615,13 @@ CodeGenModule::GetOrCreateLLVMFunction(S<br>
>>> // don't need it anymore).<br>
>>> addDeferredDeclToEmit(F, DDI->second);<br>
>>> DeferredDecls.erase(DDI);<br>
>>> -<br>
>>> +<br>
>>> // Otherwise, if this is a sized deallocation function, emit a weak<br>
>>> - // definition<br>
>>> - // for it at the end of the translation unit.<br>
>>> + // definition for it at the end of the translation unit.<br>
>>> } else if (D && cast<FunctionDecl>(D)<br>
>>> ->getCorrespondingUnsizedGlobalDeallocationFunction()) {<br>
>>> - addDeferredDeclToEmit(F, GD);<br>
>>> + if (getLangOpts().DefaultSizedDelete)<br>
>>> + addDeferredDeclToEmit(F, GD);<br>
>>><br>
>>> // Otherwise, there are cases we have to worry about where we're<br>
>>> // using a declaration for which we must emit a definition but where<br>
>>><br>
>>> Modified: cfe/trunk/lib/Driver/Tools.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=229241&r1=229240&r2=229241&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=229241&r1=229240&r2=229241&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/lib/Driver/Tools.cpp (original)<br>
>>> +++ cfe/trunk/lib/Driver/Tools.cpp Fri Feb 13 23:42:57 2015<br>
>>> @@ -4243,6 +4243,11 @@ void Clang::ConstructJob(Compilation &C,<br>
>>> if (!Args.hasFlag(options::OPT_fassume_sane_operator_new,<br>
>>> options::OPT_fno_assume_sane_operator_new))<br>
>>> CmdArgs.push_back("-fno-assume-sane-operator-new");<br>
>>> +<br>
>>> + // -def-sized-delete: default implementation of sized delete as a<br>
>>> + // weak definition.<br>
>>> + if (Args.hasArg(options::OPT_fdef_sized_delete))<br>
>>> + CmdArgs.push_back("-fdef-sized-delete");<br>
>>><br>
>>> // -fconstant-cfstrings is default, and may be subject to argument translation<br>
>>> // on Darwin.<br>
>>><br>
>>> Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=229241&r1=229240&r2=229241&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=229241&r1=229240&r2=229241&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)<br>
>>> +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Feb 13 23:42:57 2015<br>
>>> @@ -1525,6 +1525,8 @@ static void ParseLangArgs(LangOptions &O<br>
>>> Opts.NoMathBuiltin = Args.hasArg(OPT_fno_math_builtin);<br>
>>> Opts.AssumeSaneOperatorNew = !Args.hasArg(OPT_fno_assume_sane_operator_new);<br>
>>> Opts.SizedDeallocation |= Args.hasArg(OPT_fsized_deallocation);<br>
>>> + Opts.DefaultSizedDelete = Opts.SizedDeallocation &&<br>
>>> + Args.hasArg(OPT_fdef_sized_delete);<br>
>>> Opts.HeinousExtensions = Args.hasArg(OPT_fheinous_gnu_extensions);<br>
>>> Opts.AccessControl = !Args.hasArg(OPT_fno_access_control);<br>
>>> Opts.ElideConstructors = !Args.hasArg(OPT_fno_elide_constructors);<br>
>>><br>
>>> Modified: cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp?rev=229241&r1=229240&r2=229241&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp?rev=229241&r1=229240&r2=229241&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp (original)<br>
>>> +++ cfe/trunk/test/CodeGenCXX/cxx1y-sized-deallocation.cpp Fri Feb 13 23:42:57 2015<br>
>>> @@ -1,5 +1,7 @@<br>
>>> -// RUN: %clang_cc1 -std=c++1y %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s<br>
>>> -// RUN: %clang_cc1 -std=c++11 -fsized-deallocation %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s<br>
>>> +// RUN: %clang_cc1 -std=c++1y %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECKUND<br>
>>> +// RUN: %clang_cc1 -std=c++1y %s -emit-llvm -triple x86_64-linux-gnu -fdef-sized-delete -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECKDEF<br>
>>> +// RUN: %clang_cc1 -std=c++11 -fsized-deallocation %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECKUND<br>
>>> +// RUN: %clang_cc1 -std=c++11 -fsized-deallocation -fdef-sized-delete %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECKDEF<br>
>>> // RUN: %clang_cc1 -std=c++11 %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=CHECK-UNSIZED<br>
>>><br>
>>> // CHECK-UNSIZED-NOT: _ZdlPvm<br>
>>> @@ -50,8 +52,9 @@ D::D() {}<br>
>>> // CHECK: call void @_ZdlPvm(i8* %{{[^ ]*}}, i64 4)<br>
>>> // CHECK: call void @_ZdaPv(i8* %{{[^ ]*}})<br>
>>><br>
>>> -// CHECK-LABEL: define linkonce void @_ZdlPvm(i8*<br>
>>> -// CHECK: call void @_ZdlPv(i8* %0)<br>
>>> +// CHECKDEF-LABEL: define linkonce void @_ZdlPvm(i8*<br>
>>> +// CHECKDEF: call void @_ZdlPv(i8* %0)<br>
>>> +// CHECKUND-LABEL: declare void @_ZdlPvm(i8*<br>
>>><br>
>>> // CHECK-LABEL: define weak_odr void @_Z3delI1BEvv()<br>
>>> // CHECK: call void @_ZdlPvm(i8* %{{[^ ]*}}, i64 4)<br>
>>> @@ -71,8 +74,9 @@ D::D() {}<br>
>>> // CHECK: add i64 %{{[^ ]*}}, 8<br>
>>> // CHECK: call void @_ZdaPvm(i8* %{{[^ ]*}}, i64 %{{[^ ]*}})<br>
>>><br>
>>> -// CHECK-LABEL: define linkonce void @_ZdaPvm(i8*<br>
>>> -// CHECK: call void @_ZdaPv(i8* %0)<br>
>>> +// CHECKDEF-LABEL: define linkonce void @_ZdaPvm(i8*<br>
>>> +// CHECKDEF: call void @_ZdaPv(i8* %0)<br>
>>> +// CHECKUND-LABEL: declare void @_ZdaPvm(i8*<br>
>>><br>
>>> // CHECK-LABEL: define weak_odr void @_Z3delI1DEvv()<br>
>>> // CHECK: call void @_ZdlPvm(i8* %{{[^ ]*}}, i64 8)<br>
>>><br>
>>> Modified: cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp?rev=229241&r1=229240&r2=229241&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp?rev=229241&r1=229240&r2=229241&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp (original)<br>
>>> +++ cfe/trunk/test/CodeGenCXX/implicit-allocation-functions.cpp Fri Feb 13 23:42:57 2015<br>
>>> @@ -1,7 +1,9 @@<br>
>>> // RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o - -std=c++11 %s 2>&1 | FileCheck %s -check-prefix=CHECKDEF -check-prefix=CHECK11<br>
>>> // RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o - -std=c++11 -fvisibility hidden %s 2>&1 | FileCheck %s -check-prefix=CHECKHID -check-prefix=CHECK11<br>
>>> -// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o - -std=c++14 %s 2>&1 | FileCheck %s -check-prefix=CHECKDEF -check-prefix=CHECK14<br>
>>> -// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o - -std=c++14 -fvisibility hidden %s 2>&1 | FileCheck %s -check-prefix=CHECKHID -check-prefix=CHECK14<br>
>>> +// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o - -std=c++14 %s 2>&1 | FileCheck %s -check-prefix=CHECKDEF -check-prefix=CHECK14 -check-prefix=CHECK14UND<br>
>>> +// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o - -std=c++14 -fvisibility hidden %s 2>&1 | FileCheck %s -check-prefix=CHECKHID -check-prefix=CHECK14 -check-prefix=CHECK14UND<br>
>>> +// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o - -std=c++14 -fdef-sized-delete %s 2>&1 | FileCheck %s -check-prefix=CHECKDEF -check-prefix=CHECK14 -check-prefix=CHECK14DEF<br>
>>> +// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-unknown -o - -std=c++14 -fdef-sized-delete -fvisibility hidden %s 2>&1 | FileCheck %s -check-prefix=CHECKHID -check-prefix=CHECK14 -check-prefix=CHECK14DEF<br>
>>><br>
>>> // PR22419: Implicit sized deallocation functions always have default visibility.<br>
>>> // Generalized to all implicit allocation functions.<br>
>>> @@ -27,8 +29,9 @@ void foo(A* is) {<br>
>>> // CHECK11-DAG: declare void @_ZdlPv(i8*)<br>
>>><br>
>>> // CHECK14-DAG: declare noalias i8* @_Znwm(i64)<br>
>>> -// CHECK14-DAG: define linkonce void @_ZdlPvm(i8*, i64)<br>
>>> -// CHECK14-DAG: declare void @_ZdlPv(i8*)<br>
>>> +// CHECK14UND-DAG: declare void @_ZdlPvm(i8*, i64)<br>
>>> +// CHECK14DEF-DAG: define linkonce void @_ZdlPvm(i8*, i64)<br>
>>> +// CHECK14DEF-DAG: declare void @_ZdlPv(i8*)<br>
>>><br>
>>> // CHECK14-DAG: %struct.B = type { i8 }<br>
>>> struct B { ~B() { }};<br>
>>> @@ -50,5 +53,6 @@ void f(B *p) {<br>
>>> // CHECK11-DAG: declare void @_ZdaPv(i8*)<br>
>>><br>
>>> // CHECK14-DAG: declare noalias i8* @_Znam(i64)<br>
>>> -// CHECK14-DAG: define linkonce void @_ZdaPvm(i8*, i64)<br>
>>> -// CHECK14-DAG: declare void @_ZdaPv(i8*)<br>
>>> +// CHECK14UND-DAG: declare void @_ZdaPvm(i8*, i64)<br>
>>> +// CHECK14DEF-DAG: define linkonce void @_ZdaPvm(i8*, i64)<br>
>>> +// CHECK14DEF-DAG: declare void @_ZdaPv(i8*)<br>
>><br>
>><br>
>> Please can you update cxx_status.html and the Clang 3.7 release notes to note that we now require one of (a) libc++ 3.7 or later, (b) libstdc++ 5 or later, (c) -fdefine-sized-deallocation, or (d) -fno-sized-deallocation to build code in C++14 mode? </p>
<p dir="ltr">Hi Richard -- did you mean -fno-sized-deallocation here?<br></p>
<p dir="ltr">Thanks.<br>
><br>
><br>
</p>