r229241 - 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.

Larisse Voufo lvoufo at google.com
Thu Feb 19 12:29:09 PST 2015


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

Hi Richard -- did you mean -fno-sized-deallocation here?

Thanks.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150219/f0f42d9a/attachment.html>


More information about the cfe-commits mailing list