[llvm-commits] [compiler-rt] r170429 - in /compiler-rt/trunk/lib/tsan/rtl: tsan_fd.cc tsan_fd.h tsan_interceptors.cc tsan_stat.cc tsan_stat.h

Kostya Serebryany kcc at google.com
Tue Dec 18 23:51:21 PST 2012


On Wed, Dec 19, 2012 at 11:45 AM, David Blaikie <dblaikie at gmail.com> wrote:

> On Tue, Dec 18, 2012 at 11:39 PM, Kostya Serebryany <kcc at google.com>
> wrote:
> >
> >
> > On Wed, Dec 19, 2012 at 11:31 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>
> >> On Tue, Dec 18, 2012 at 11:26 PM, Dmitry Vyukov <dvyukov at google.com>
> >> wrote:
> >> > On Wed, Dec 19, 2012 at 11:05 AM, David Blaikie <dblaikie at gmail.com>
> >> > wrote:
> >> >>
> >> >> On Tue, Dec 18, 2012 at 11:03 PM, Dmitry Vyukov <dvyukov at google.com>
> >> >> wrote:
> >> >> > Hi,
> >> >> >
> >> >> > I've fixed the -Wgnu warnings and added -Wgnu to our presubmit
> >> >> > checks:
> >> >>
> >> >> Thanks - though, I'm confused, why would you need to add -Wgnu
> >> >> anywhere? I was already seeing these warnings (as errors) with Clang
> >> >> ToT.
> >> >>
> >> >
> >> > Normal build does not do all the checks we need (e.g. check for large
> >> > stack
> >> > frames, build with both gcc and clang, build Go runtime syso file,
> check
> >> > lint warnings, check parameters of generated machine code, etc).
> >>
> >> Rather than adding things incrementally to your custom build - perhaps
> >> it'd be best to invoke the actual build from your custom build to
> >> verify that it's still clean? (granted I run the CMake + Ninja build,
> >> and as with the rest of the LLVM project, it's not expected that
> >> developers verify both kinds of builds at all times, but both the
> >> CMake and configure+make builds are kept in sync by their maintainers
> >> to the best of their ability so keeping one clean should keep the
> >> other clean assuming you're not directly touching the build files - so
> >> running at least one of these in your build validation pipeline would
> >> be rather helpful, rather than trying to replicate the specific flag
> >> set, etc, there)
> >
> >
> > Alexey is going to add a clang self-hosting build to our bots
> (post-commit
> > checking).
> > Our scripts in tsan-rt do tsan-specific pre-commit checking for tsan-rt
> > only.
>
> Sorry, I'm still a bit confused/missing something. Essentially what
> I'm trying to understand/correct is:
>
> what did I do to build compiler-rt that's different from what you guys
> are doing?
>
> My assumption is it's just that I'm selfhosting Clang and you aren't.
>

Correct. We mostly used the default build settings which imply using gcc as
the host compiler.
If we start using clang as the host compiler, we may break the gcc-based
build as easily.
If we start using both it will slowdown the local testing time. Still
possible.
Does anyone else use both gcc and clang before commits?

--kcc



> My suggestion is that you selfhost (with -Werror) so you get the same
> errors sooner & it'd be easy not to break the build in this way -
> rather than waiting for bots or pre-commit checks, etc. (this advice
> would apply to anyone on any LLVM project, it's not specific to
> compiler-rt's unique build features or scripts)
>
> Am I missing something?
>
> - David
>
> >
> > --kcc
> >
> >>
> >>
> >> >
> >> >
> >> >>
> >> >> > http://llvm.org/viewvc/llvm-project?view=rev&revision=170499
> >> >> >
> >> >> > Thanks!
> >> >> >
> >> >> >
> >> >> >
> >> >> > On Wed, Dec 19, 2012 at 7:46 AM, David Blaikie <dblaikie at gmail.com
> >
> >> >> > wrote:
> >> >> >>
> >> >> >> On Tue, Dec 18, 2012 at 7:30 PM, Kostya Serebryany <
> kcc at google.com>
> >> >> >> wrote:
> >> >> >> > That's because by default we are building the run-time with the
> >> >> >> > host
> >> >> >> > compiler (gcc) and don't see this.
> >> >> >> > Alexey, what's the story about using the fresh clang for
> >> >> >> > compiler-rt?
> >> >> >>
> >> >> >> While that would/will be nice, at the very least we try to keep
> the
> >> >> >> build clean even for normal components (like Clang & LLVM, that
> >> >> >> can't
> >> >> >> be built with the fresh clang, obviously) building clean with
> Clang
> >> >> >> ToT regardless. (obviously some version skew occurs rarely, but
> >> >> >> generally most people develop with a release build of Clang from
> ToT
> >> >> >> that they refresh from time to time)
> >> >> >>
> >> >> >> Since your work goes beyond compiler-rt & involves LLVM changes as
> >> >> >> well, I'd suggest you'll probably want to address the workflow &
> >> >> >> prefer building/developing with Clang to help ensure this
> invariant
> >> >> >> both for compiler-rt (until you get the fresh-built clang usage in
> >> >> >> the
> >> >> >> build system) and for LLVM/Clang/etc.
> >> >> >>
> >> >> >> - David
> >> >> >>
> >> >> >> >
> >> >> >> > --kcc
> >> >> >> >
> >> >> >> >
> >> >> >> > On Wed, Dec 19, 2012 at 3:50 AM, David Blaikie
> >> >> >> > <dblaikie at gmail.com>
> >> >> >> > wrote:
> >> >> >> >>
> >> >> >> >> On Tue, Dec 18, 2012 at 4:35 AM, Dmitry Vyukov
> >> >> >> >> <dvyukov at google.com>
> >> >> >> >> wrote:
> >> >> >> >> > Author: dvyukov
> >> >> >> >> > Date: Tue Dec 18 06:35:31 2012
> >> >> >> >> > New Revision: 170429
> >> >> >> >> >
> >> >> >> >> > URL: http://llvm.org/viewvc/llvm-project?rev=170429&view=rev
> >> >> >> >> > Log:
> >> >> >> >> > tsan: add signalfd() and inotify_init() interceptors
> >> >> >> >> >
> >> >> >> >> > Modified:
> >> >> >> >> >     compiler-rt/trunk/lib/tsan/rtl/tsan_fd.cc
> >> >> >> >> >     compiler-rt/trunk/lib/tsan/rtl/tsan_fd.h
> >> >> >> >> >     compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
> >> >> >> >> >     compiler-rt/trunk/lib/tsan/rtl/tsan_stat.cc
> >> >> >> >> >     compiler-rt/trunk/lib/tsan/rtl/tsan_stat.h
> >> >> >> >> >
> >> >> >> >> > Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_fd.cc
> >> >> >> >> > URL:
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_fd.cc?rev=170429&r1=170428&r2=170429&view=diff
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> ==============================================================================
> >> >> >> >> > --- compiler-rt/trunk/lib/tsan/rtl/tsan_fd.cc (original)
> >> >> >> >> > +++ compiler-rt/trunk/lib/tsan/rtl/tsan_fd.cc Tue Dec 18
> >> >> >> >> > 06:35:31
> >> >> >> >> > 2012
> >> >> >> >> > @@ -90,11 +90,12 @@
> >> >> >> >> >    FdDesc *d = fddesc(thr, pc, fd);
> >> >> >> >> >    // As a matter of fact, we don't intercept all close
> calls.
> >> >> >> >> >    // See e.g. libc __res_iclose().
> >> >> >> >> > -  if (d->sync)
> >> >> >> >> > +  if (d->sync) {
> >> >> >> >> >      unref(thr, pc, d->sync);
> >> >> >> >> > +    d->sync = 0;
> >> >> >> >> > +  }
> >> >> >> >> >    if (flags()->io_sync == 0) {
> >> >> >> >> >      unref(thr, pc, s);
> >> >> >> >> > -    d->sync = 0;
> >> >> >> >> >    } else if (flags()->io_sync == 1) {
> >> >> >> >> >      d->sync = s;
> >> >> >> >> >    } else if (flags()->io_sync == 2) {
> >> >> >> >> > @@ -189,6 +190,16 @@
> >> >> >> >> >    init(thr, pc, fd, allocsync());
> >> >> >> >> >  }
> >> >> >> >> >
> >> >> >> >> > +void FdSignalCreate(ThreadState *thr, uptr pc, int fd) {
> >> >> >> >> > +  DPrintf("#%d: FdSignalCreate(%d)\n", thr->tid, fd);
> >> >> >> >> > +  init(thr, pc, fd, 0);
> >> >> >> >> > +}
> >> >> >> >> > +
> >> >> >> >> > +void FdInotifyCreate(ThreadState *thr, uptr pc, int fd) {
> >> >> >> >> > +  DPrintf("#%d: FdInotifyCreate(%d)\n", thr->tid, fd);
> >> >> >> >> > +  init(thr, pc, fd, 0);
> >> >> >> >> > +}
> >> >> >> >> > +
> >> >> >> >> >  void FdPollCreate(ThreadState *thr, uptr pc, int fd) {
> >> >> >> >> >    DPrintf("#%d: FdPollCreate(%d)\n", thr->tid, fd);
> >> >> >> >> >    init(thr, pc, fd, allocsync());
> >> >> >> >> >
> >> >> >> >> > Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_fd.h
> >> >> >> >> > URL:
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_fd.h?rev=170429&r1=170428&r2=170429&view=diff
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> ==============================================================================
> >> >> >> >> > --- compiler-rt/trunk/lib/tsan/rtl/tsan_fd.h (original)
> >> >> >> >> > +++ compiler-rt/trunk/lib/tsan/rtl/tsan_fd.h Tue Dec 18
> >> >> >> >> > 06:35:31
> >> >> >> >> > 2012
> >> >> >> >> > @@ -46,6 +46,8 @@
> >> >> >> >> >  void FdDup(ThreadState *thr, uptr pc, int oldfd, int newfd);
> >> >> >> >> >  void FdPipeCreate(ThreadState *thr, uptr pc, int rfd, int
> >> >> >> >> > wfd);
> >> >> >> >> >  void FdEventCreate(ThreadState *thr, uptr pc, int fd);
> >> >> >> >> > +void FdSignalCreate(ThreadState *thr, uptr pc, int fd);
> >> >> >> >> > +void FdInotifyCreate(ThreadState *thr, uptr pc, int fd);
> >> >> >> >> >  void FdPollCreate(ThreadState *thr, uptr pc, int fd);
> >> >> >> >> >  void FdSocketCreate(ThreadState *thr, uptr pc, int fd);
> >> >> >> >> >  void FdSocketAccept(ThreadState *thr, uptr pc, int fd, int
> >> >> >> >> > newfd);
> >> >> >> >> >
> >> >> >> >> > Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
> >> >> >> >> > URL:
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc?rev=170429&r1=170428&r2=170429&view=diff
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> ==============================================================================
> >> >> >> >> > --- compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
> >> >> >> >> > (original)
> >> >> >> >> > +++ compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc Tue
> Dec
> >> >> >> >> > 18
> >> >> >> >> > 06:35:31 2012
> >> >> >> >> > @@ -1128,6 +1128,31 @@
> >> >> >> >> >    return fd;
> >> >> >> >> >  }
> >> >> >> >> >
> >> >> >> >> > +TSAN_INTERCEPTOR(int, signalfd, int fd, void *mask, int
> flags)
> >> >> >> >> > {
> >> >> >> >> > +  SCOPED_TSAN_INTERCEPTOR(signalfd, fd, mask, flags);
> >> >> >> >> > +  FdClose(thr, pc, fd);
> >> >> >> >> > +  fd = REAL(signalfd)(fd, mask, flags);
> >> >> >> >> > +  if (fd >= 0)
> >> >> >> >> > +    FdSignalCreate(thr, pc, fd);
> >> >> >> >> > +  return fd;
> >> >> >> >> > +}
> >> >> >> >> > +
> >> >> >> >> > +TSAN_INTERCEPTOR(int, inotify_init) {
> >> >> >> >>
> >> >> >> >> This (& some other recent commits) seems to be breaking a clang
> >> >> >> >> self-hosting -Werror build:
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> llvm/src/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:1140:35:
> >> >> >> >> error: must specify at least one argument for '...' parameter
> of
> >> >> >> >> variadic macro [-Werror,-Wgnu]
> >> >> >> >> TSAN_INTERCEPTOR(int, inotify_init) {
> >> >> >> >>                                   ^
> >> >> >> >>
> >> >> >> >> Could you fix/revert ASAP, please?
> >> >> >> >>
> >> >> >> >> Thanks,
> >> >> >> >> - David
> >> >> >> >>
> >> >> >> >> > +  SCOPED_TSAN_INTERCEPTOR(inotify_init);
> >> >> >> >> > +  int fd = REAL(inotify_init)();
> >> >> >> >> > +  if (fd >= 0)
> >> >> >> >> > +    FdInotifyCreate(thr, pc, fd);
> >> >> >> >> > +  return fd;
> >> >> >> >> > +}
> >> >> >> >> > +
> >> >> >> >> > +TSAN_INTERCEPTOR(int, inotify_init1, int flags) {
> >> >> >> >> > +  SCOPED_TSAN_INTERCEPTOR(inotify_init1, flags);
> >> >> >> >> > +  int fd = REAL(inotify_init1)(flags);
> >> >> >> >> > +  if (fd >= 0)
> >> >> >> >> > +    FdInotifyCreate(thr, pc, fd);
> >> >> >> >> > +  return fd;
> >> >> >> >> > +}
> >> >> >> >> > +
> >> >> >> >> >  TSAN_INTERCEPTOR(int, socket, int domain, int type, int
> >> >> >> >> > protocol)
> >> >> >> >> > {
> >> >> >> >> >    SCOPED_TSAN_INTERCEPTOR(socket, domain, type, protocol);
> >> >> >> >> >    int fd = REAL(socket)(domain, type, protocol);
> >> >> >> >> > @@ -1743,6 +1768,9 @@
> >> >> >> >> >    TSAN_INTERCEPT(dup2);
> >> >> >> >> >    TSAN_INTERCEPT(dup3);
> >> >> >> >> >    TSAN_INTERCEPT(eventfd);
> >> >> >> >> > +  TSAN_INTERCEPT(signalfd);
> >> >> >> >> > +  TSAN_INTERCEPT(inotify_init);
> >> >> >> >> > +  TSAN_INTERCEPT(inotify_init1);
> >> >> >> >> >    TSAN_INTERCEPT(socket);
> >> >> >> >> >    TSAN_INTERCEPT(socketpair);
> >> >> >> >> >    TSAN_INTERCEPT(connect);
> >> >> >> >> >
> >> >> >> >> > Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_stat.cc
> >> >> >> >> > URL:
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_stat.cc?rev=170429&r1=170428&r2=170429&view=diff
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> ==============================================================================
> >> >> >> >> > --- compiler-rt/trunk/lib/tsan/rtl/tsan_stat.cc (original)
> >> >> >> >> > +++ compiler-rt/trunk/lib/tsan/rtl/tsan_stat.cc Tue Dec 18
> >> >> >> >> > 06:35:31
> >> >> >> >> > 2012
> >> >> >> >> > @@ -189,6 +189,9 @@
> >> >> >> >> >    name[StatInt_dup2]                     = "  dup2
> >> >> >> >> > ";
> >> >> >> >> >    name[StatInt_dup3]                     = "  dup3
> >> >> >> >> > ";
> >> >> >> >> >    name[StatInt_eventfd]                  = "  eventfd
> >> >> >> >> > ";
> >> >> >> >> > +  name[StatInt_signalfd]                 = "  signalfd
> >> >> >> >> > ";
> >> >> >> >> > +  name[StatInt_inotify_init]             = "  inotify_init
> >> >> >> >> > ";
> >> >> >> >> > +  name[StatInt_inotify_init1]            = "  inotify_init1
> >> >> >> >> > ";
> >> >> >> >> >    name[StatInt_socket]                   = "  socket
> >> >> >> >> > ";
> >> >> >> >> >    name[StatInt_socketpair]               = "  socketpair
> >> >> >> >> > ";
> >> >> >> >> >    name[StatInt_connect]                  = "  connect
> >> >> >> >> > ";
> >> >> >> >> >
> >> >> >> >> > Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_stat.h
> >> >> >> >> > URL:
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_stat.h?rev=170429&r1=170428&r2=170429&view=diff
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> ==============================================================================
> >> >> >> >> > --- compiler-rt/trunk/lib/tsan/rtl/tsan_stat.h (original)
> >> >> >> >> > +++ compiler-rt/trunk/lib/tsan/rtl/tsan_stat.h Tue Dec 18
> >> >> >> >> > 06:35:31
> >> >> >> >> > 2012
> >> >> >> >> > @@ -184,6 +184,9 @@
> >> >> >> >> >    StatInt_dup2,
> >> >> >> >> >    StatInt_dup3,
> >> >> >> >> >    StatInt_eventfd,
> >> >> >> >> > +  StatInt_signalfd,
> >> >> >> >> > +  StatInt_inotify_init,
> >> >> >> >> > +  StatInt_inotify_init1,
> >> >> >> >> >    StatInt_socket,
> >> >> >> >> >    StatInt_socketpair,
> >> >> >> >> >    StatInt_connect,
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> > _______________________________________________
> >> >> >> >> > llvm-commits mailing list
> >> >> >> >> > llvm-commits at cs.uiuc.edu
> >> >> >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >> >> >> >> _______________________________________________
> >> >> >> >> llvm-commits mailing list
> >> >> >> >> llvm-commits at cs.uiuc.edu
> >> >> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >> >> >> >
> >> >> >> >
> >> >> >
> >> >> >
> >> >
> >> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121219/753cf1c7/attachment.html>


More information about the llvm-commits mailing list