<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Feb 13, 2015 at 9:42 PM, Larisse Voufo <span dir="ltr"><<a href="mailto:lvoufo@google.com" target="_blank">lvoufo@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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" target="_blank">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" target="_blank">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></blockquote><div><br></div><div>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.)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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" target="_blank">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></blockquote><div><br></div><div>Likewise here, how about -fdefine-sized-deallocation, to match the existing -fsized-deallocation flag?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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" target="_blank">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" target="_blank">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" target="_blank">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" target="_blank">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" target="_blank">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" target="_blank">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></blockquote><div><br></div><div>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? Thanks.</div></div></div></div>