[compiler-rt] r177860 - [Sanitizer] Compile sanitizer runtimes with -Wno-non-virtual-dtor. Virtual dtors may be a problem for us, as sanitizer runtime should not generally assume libstdc++ presence.

[David Blaikie]

On Mon, Mar 25, 2013 at 8:55 AM, Alexey Samsonov <samsonov at google.com> wrote:
>
> On Mon, Mar 25, 2013 at 7:14 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> On Mon, Mar 25, 2013 at 3:31 AM, Alexey Samsonov <samsonov at google.com>
>> wrote:
>> > Author: samsonov
>> > Date: Mon Mar 25 05:31:49 2013
>> > New Revision: 177860
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=177860&view=rev
>> > Log:
>> > [Sanitizer] Compile sanitizer runtimes with -Wno-non-virtual-dtor.
>> > Virtual dtors may be a problem for us, as sanitizer runtime should not
>> > generally assume libstdc++ presence.
>>
>> I'm not sure I understand the correlation between virtual dtors and
>> the use (or not) of libstdc++. Could you explain it?
>
>
> The compiler emits the call to "operator delete(void*)" while generating a
> destructor.

I know very little about this stuff - but I sort of would've expected
that sort of thing to be in the ABI runtime like libc++abi or
equivalent - but perhaps I'm wrong or you don't link with that either.

>> > Modified:
>> >     compiler-rt/trunk/CMakeLists.txt
>> >     compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc
>> >     compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h
>> >
>> > Modified: compiler-rt/trunk/CMakeLists.txt
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/CMakeLists.txt?rev=177860&r1=177859&r2=177860&view=diff
>> >
>> > ==============================================================================
>> > --- compiler-rt/trunk/CMakeLists.txt (original)
>> > +++ compiler-rt/trunk/CMakeLists.txt Mon Mar 25 05:31:49 2013
>> > @@ -147,6 +147,12 @@ check_cxx_compiler_flag(-Wno-c99-extensi
>> >  if(SUPPORTS_NO_C99_EXTENSIONS_FLAG)
>> >    list(APPEND SANITIZER_COMMON_CFLAGS -Wno-c99-extensions)
>> >  endif()
>> > +# Sanitizer may not have libstdc++, so we can have problems with
>> > virtual
>> > +# destructors.
>> > +check_cxx_compiler_flag(-Wno-non-virtual-dtor
>> > SUPPORTS_NO_NON_VIRTUAL_DTOR_FLAG)
>> > +if (SUPPORTS_NO_NON_VIRTUAL_DTOR_FLAG)
>> > +  list(APPEND SANITIZER_COMMON_CFLAGS -Wno-non-virtual-dtor)
>> > +endif()
>> >
>> >  # Setup min Mac OS X version.
>> >  if(APPLE)
>> >
>> > Modified:
>> > compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc?rev=177860&r1=177859&r2=177860&view=diff
>> >
>> > ==============================================================================
>> > --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc
>> > (original)
>> > +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc
>> > Mon Mar 25 05:31:49 2013
>> > @@ -22,11 +22,10 @@ ThreadContextBase::ThreadContextBase(u32
>> >    name[0] = '\0';
>> >  }
>> >
>> > -#ifndef SANITIZER_GO
>> >  ThreadContextBase::~ThreadContextBase() {
>> > +  // ThreadContextBase should never be deleted.
>> >    CHECK(0);
>> >  }
>> > -#endif
>> >
>> >  void ThreadContextBase::SetName(const char *new_name) {
>> >    name[0] = '\0';
>> >
>> > Modified:
>> > compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h?rev=177860&r1=177859&r2=177860&view=diff
>> >
>> > ==============================================================================
>> > --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h
>> > (original)
>> > +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.h
>> > Mon Mar 25 05:31:49 2013
>> > @@ -34,10 +34,7 @@ enum ThreadStatus {
>> >  class ThreadContextBase {
>> >   public:
>> >    explicit ThreadContextBase(u32 tid);
>> > -#ifndef SANITIZER_GO  // Go does not have libstdc++
>> > -  virtual
>> > -#endif
>> > -  ~ThreadContextBase();
>> > +  ~ThreadContextBase();  // Should never be called.
>>
>> You could replace this comment with a use of the LLVM_DELETED_FUNCTION
>> macro that will help the compiler provide better diagnostics when this
>> code is compiled as C++11
>
>
> We don't include any LLVM headers in compiler-rt libs. Do you think there is
> a value in porting this macro?

Up to you - it's not a big deal, but it's perhaps convenient.

(given that you're building the runtimes with the just-built Clang
anyway, why not just build it as C++11 always & then you wouldn't need
the compatibility macro & you could just use "= delete" directly?)

>
>>
>> >
>> >    const u32 tid;  // Thread ID. Main thread should have tid = 0.
>> >    u64 unique_id;  // Unique thread ID.
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
>
> --
> Alexey Samsonov, MSK