[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:59:42 PST 2012
On Wed, Dec 19, 2012 at 11:57 AM, Chandler Carruth <chandlerc at google.com>wrote:
> On Tue, Dec 18, 2012 at 11:54 PM, Alexey Samsonov <samsonov at google.com>wrote:
>
>> On Wed, Dec 19, 2012 at 11:51 AM, Kostya Serebryany <kcc at google.com>wrote:
>>
>>>
>>>
>>> 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?
>>>
>>
>> As a first step, we need to buildbots that use gcc and just-build clang
>> with -Werror - I'll handle this.
>> We may also switch our own workflow to use "relatively" new Clang as a
>> host compiler (e.g. I do this),
>> but that's probably optional.
>>
>
> Just to be clear, this is also what I and many (if not all) developers do.
>
Ok. The simplest solution then is for all of us (tsan/asan folks) to switch
to clang builds.
We'll do this.
--kcc
>
> We're hoping to make it even more common with reasonably automated
> building and release of debian packages within Google and on the LLVM site.
> Hopefully Q1 for that.
>
>
>>
>>
>>>
>>> --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
>>>> >> >> >> >
>>>> >> >> >> >
>>>> >> >> >
>>>> >> >> >
>>>> >> >
>>>> >> >
>>>> >
>>>> >
>>>>
>>>
>>>
>>
>>
>> --
>> Alexey Samsonov, MSK
>>
>>
>> _______________________________________________
>> 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/824c228f/attachment.html>
More information about the llvm-commits
mailing list