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

[Alexey Samsonov]

On Mon, Mar 25, 2013 at 8:30 PM, David Blaikie <dblaikie at gmail.com> wrote:

> 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?)
>

That's not true either :(
In CMake build we use host compiler to build runtimes (of course, during a
development
we use "fresh enough clang" as a host, but that's not the general case).
The runtime code
is used in gcc as well.


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



-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130325/032bfb6e/attachment.html>