Build failure using REQUIRES_EH=1 after r205915

[Delesley Hutchins]

BTW, the code compiles under gcc just fine, it just doesn't work with
REQUIRES_EH=1.


On Wed, Apr 23, 2014 at 10:15 AM, Delesley Hutchins <delesley at google.com> wrote:
> 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



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