Build failure using REQUIRES_EH=1 after r205915

[Delesley Hutchins]

If gcc is indeed attempting to call the destructor anyway, wouldn't
declaring it public, but without a definition, just result in a link
error? The only option I can think of is to define delete, but throw a
hard error, thus turning a compile-time error into a run-time error.

This sounds like a bug in gcc.  After all, the purpose here is to
ensure that the destructor is never called, so I don't understand why
gcc would be trying to call it.

  -DeLesley

On Wed, Apr 23, 2014 at 9:54 AM, Abramo Bagnara
<abramo.bagnara at bugseng.com> wrote:
> Il 23/04/2014 18:35, Aaron Ballman ha scritto:
>> On Wed, Apr 23, 2014 at 12:26 PM, Abramo Bagnara
>> <abramo.bagnara at bugseng.com> wrote:
>>> In r205915 you've added to ThreadSafetyTIL.h a mechanism to avoid
>>> explicit delete for SExpr, but this break compilation with REQUIRES_EH=1
>>> as shown below:
>>>
>>> $ make VERBOSE=1 REQUIRES_EH=1
>>> llvm[0]: Compiling ThreadSafety.cpp for Debug+Asserts build
>>> if  g++ -Illvm-r206978-build/include
>>> -Illvm-r206978-build/tools/clang/lib/Analysis -Illvm-r206978/include
>>> -Illvm-r206978/tools/clang/lib/Analysis  -D_DEBUG -D_GNU_SOURCE
>>> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
>>> -Illvm-r206978/tools/clang/lib/Analysis/../../include
>>> -Illvm-r206978-build/tools/clang/lib/Analysis/../../include -g
>>> -std=c++11 -fvisibility-inlines-hidden -fPIC -Woverloaded-virtual
>>> -ffunction-sections -fdata-sections -Wcast-qual -fno-strict-aliasing
>>> -pedantic -Wno-long-long -Wall -W -Wno-unused-parameter -Wwrite-strings
>>> -I/home/abramo/eclair_cplusplus/deps/traps/include
>>> -Wno-maybe-uninitialized -Wno-missing-field-initializers -c -MMD -MP -MF
>>> "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.d.tmp"
>>> -MT
>>> "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.o"
>>> -MT
>>> "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.d"
>>> llvm-r206978/tools/clang/lib/Analysis/ThreadSafety.cpp -o
>>> llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.o ; \
>>>                 then /bin/mv -f
>>> "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.d.tmp"
>>> "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.d";
>>> else /bin/rm
>>> "llvm-r206978-build/tools/clang/lib/Analysis/Debug+Asserts/ThreadSafety.d.tmp";
>>> exit 1; fi
>>> In file included from
>>> llvm-r206978/tools/clang/lib/Analysis/ThreadSafety.cpp:25:0:
>>> llvm-r206978/tools/clang/lib/Analysis/../../include/clang/Analysis/Analyses/ThreadSafetyTIL.h:
>>> In member function ‘clang::threadSafety::til::SExpr*
>>> clang::threadSafety::til::CopyReducer::reduceUndefined(clang::threadSafety::til::Undefined&)’:
>>> llvm-r206978/tools/clang/lib/Analysis/../../include/clang/Analysis/Analyses/ThreadSafetyTIL.h:120:8:
>>> error: ‘static void clang::threadSafety::til::SExpr::operator
>>> delete(void*)’ is private
>>>    void operator delete(void *) LLVM_DELETED_FUNCTION;
>>>         ^
>>> In file included from
>>> llvm-r206978/tools/clang/lib/Analysis/ThreadSafety.cpp:26:0:
>>> llvm-r206978/tools/clang/lib/Analysis/../../include/clang/Analysis/Analyses/ThreadSafetyTraverse.h:127:38:
>>> error: within this context
>>>      return new (Arena) Undefined(Orig);
>>>                                       ^
>>> ... and many others.
>>>
>>> I don't think this is expected: what do you think about to remove the
>>> private deleted operator delete? Are there better way to fix that?
>>
>> The operator delete was deleted purposefully as it is not meant to be
>> called (all SExpr objects must be created within an arena using a
>> placement new operator, and so they should never be deleted
>> explicitly). The fact that operator delete is attempting to be used is
>> the true problem; were it to be called successfully, then a
>> double-free would occur when the arena was destroyed. So adding the
>> operator delete back simply trades a compile error for a subtle (but
>> unlikely to be triggered) bug.
>
> I've also tried to declare with throw() all constructors for SExpr and
> its derived classes, but this is not enough to appease gcc (the standard
> specify that delete operator might be called from compiler when an
> exception happens in object initialization).
>
> What about to declare it public but without a definition?
>
> Do you have a better idea to not break the build?
>
> I'm not sure this is a behaviour mandated by the standard or a subtle
> bug in gcc, however I'd say that to find a sensible way to not break
> compilation under gcc is needed.
>
> --
> Abramo Bagnara
>
> BUGSENG srl - http://bugseng.com
> mailto:abramo.bagnara at bugseng.com



-- 
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315