Patchwork asan_test.cc from llvm

login
register
mail settings
Submitter Jakub Jelinek
Date Dec. 3, 2012, 9:18 a.m.
Message ID <20121203091815.GP2315@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/203300/
State New
Headers show

Comments

Jakub Jelinek - Dec. 3, 2012, 9:18 a.m.
On Mon, Dec 03, 2012 at 11:06:48AM +0400, Konstantin Serebryany wrote:
> This patch copies the asan tests almost, but not quite, verbatim from upstream.
> Since the patch is not in attachment (and gmail messes up with inlined
> patches) I can't see the exact changes.

Sending patches inline rather than as attachments is the preferred way for
gcc-patches, you can always grab it from the gcc-patches ml archive:
http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02557.html
(then click on Raw text).

I'm attaching the diff for asan_test.cc from llvm anyway.

> I see #ifdef ASAN_AVOID_EXPENSIVE_TESTS, which I don't really like
> because I'd rather fix the test than disable it.

The test isn't disabled, just by default limited to 30 threads instead of
1000, because that really will ruin testing for everybody with ulimit -u
in 1024-ish range.  Even 500 threads would be undesirable for that.

> Can we commit the tests 100% verbatim, and then fix them as separate commits
> (preferably, by fixing the tests upstream and doing the merge with
> libsanitizer/merge.sh)?

I'd prefer delay committing the patch over causing random testsuite failures
elsewhere.

	Jakub
Konstantin Serebryany - Dec. 3, 2012, 9:51 a.m.
On Mon, Dec 3, 2012 at 1:18 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Dec 03, 2012 at 11:06:48AM +0400, Konstantin Serebryany wrote:
>> This patch copies the asan tests almost, but not quite, verbatim from upstream.
>> Since the patch is not in attachment (and gmail messes up with inlined
>> patches) I can't see the exact changes.
>
> Sending patches inline rather than as attachments is the preferred way for
> gcc-patches, you can always grab it from the gcc-patches ml archive:
> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02557.html
> (then click on Raw text).

Got it, thanks.

>
> I'm attaching the diff for asan_test.cc from llvm anyway.
>
>> I see #ifdef ASAN_AVOID_EXPENSIVE_TESTS, which I don't really like
>> because I'd rather fix the test than disable it.
>
> The test isn't disabled, just by default limited to 30 threads instead of
> 1000, because that really will ruin testing for everybody with ulimit -u
> in 1024-ish range.  Even 500 threads would be undesirable for that.

Which is the same as disabling it.
Unfortunately, we don't have a good automated way to test asan performance,
so this test is guarding us from performance degradation in the asan's
pthread wrappers.
If there is a bug there, we may not notice it on 30 threads, but may
(with a high probability)
on 1000 threads.

Anyway, does http://llvm.org/viewvc/llvm-project?rev=169118&view=rev
solve the problem?
It adds ASAN_AVOID_EXPENSIVE_TESTS and checks the results of all
pthread_{create,join} calls in tests.

--kcc

>
>> Can we commit the tests 100% verbatim, and then fix them as separate commits
>> (preferably, by fixing the tests upstream and doing the merge with
>> libsanitizer/merge.sh)?
>
> I'd prefer delay committing the patch over causing random testsuite failures
> elsewhere.
>
>         Jakub
Jakub Jelinek - Dec. 3, 2012, 11:05 a.m.
On Mon, Dec 03, 2012 at 01:51:50PM +0400, Konstantin Serebryany wrote:
> > I'm attaching the diff for asan_test.cc from llvm anyway.
> >
> >> I see #ifdef ASAN_AVOID_EXPENSIVE_TESTS, which I don't really like
> >> because I'd rather fix the test than disable it.
> >
> > The test isn't disabled, just by default limited to 30 threads instead of
> > 1000, because that really will ruin testing for everybody with ulimit -u
> > in 1024-ish range.  Even 500 threads would be undesirable for that.
> 
> Which is the same as disabling it.
> Unfortunately, we don't have a good automated way to test asan performance,
> so this test is guarding us from performance degradation in the asan's
> pthread wrappers.

I understand that, that is why the test by default, when not run as part of
dejagnu, or even in dejagnu when requested expensive tests, runs 1000
threads instead of 30.

> Anyway, does http://llvm.org/viewvc/llvm-project?rev=169118&view=rev
> solve the problem?
> It adds ASAN_AVOID_EXPENSIVE_TESTS and checks the results of all
> pthread_{create,join} calls in tests.

Yes, thanks.  So, is the patch ok to commit to GCC with the imported tests remerged
from upstream (or do I need to repost the patch for that)?

	Jakub
Konstantin Serebryany - Dec. 3, 2012, 11:41 a.m.
On Mon, Dec 3, 2012 at 3:05 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Dec 03, 2012 at 01:51:50PM +0400, Konstantin Serebryany wrote:
>> > I'm attaching the diff for asan_test.cc from llvm anyway.
>> >
>> >> I see #ifdef ASAN_AVOID_EXPENSIVE_TESTS, which I don't really like
>> >> because I'd rather fix the test than disable it.
>> >
>> > The test isn't disabled, just by default limited to 30 threads instead of
>> > 1000, because that really will ruin testing for everybody with ulimit -u
>> > in 1024-ish range.  Even 500 threads would be undesirable for that.
>>
>> Which is the same as disabling it.
>> Unfortunately, we don't have a good automated way to test asan performance,
>> so this test is guarding us from performance degradation in the asan's
>> pthread wrappers.
>
> I understand that, that is why the test by default, when not run as part of
> dejagnu, or even in dejagnu when requested expensive tests, runs 1000
> threads instead of 30.
>
>> Anyway, does http://llvm.org/viewvc/llvm-project?rev=169118&view=rev
>> solve the problem?
>> It adds ASAN_AVOID_EXPENSIVE_TESTS and checks the results of all
>> pthread_{create,join} calls in tests.
>
> Yes, thanks.  So, is the patch ok to commit to GCC with the imported tests remerged
> from upstream

Yes, ok to commit. Thanks!

--kcc

> (or do I need to repost the patch for that)?
>
>         Jakub

Patch

--- /usr/src/llvm/projects/compiler-rt/lib/asan/tests/asan_test.cc	2012-11-30 14:23:48.525229790 +0100
+++ asan_test.cc	2012-12-03 10:12:14.503366704 +0100
@@ -1,7 +1,5 @@ 
 //===-- asan_test.cc ------------------------------------------------------===//
 //
-//                     The LLVM Compiler Infrastructure
-//
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
@@ -481,10 +479,16 @@  void *ManyThreadsWorker(void *a) {
 }
 
 TEST(AddressSanitizer, ManyThreadsTest) {
-  const size_t kNumThreads = SANITIZER_WORDSIZE == 32 ? 30 : 1000;
-  pthread_t t[kNumThreads];
+#ifdef ASAN_AVOID_EXPENSIVE_TESTS
+  const size_t kMaxThreads = 30;
+#else
+  const size_t kMaxThreads = SANITIZER_WORDSIZE == 32 ? 30 : 1000;
+#endif
+  pthread_t t[kMaxThreads];
+  size_t kNumThreads = kMaxThreads;
   for (size_t i = 0; i < kNumThreads; i++) {
-    pthread_create(&t[i], 0, (void* (*)(void *x))ManyThreadsWorker, (void*)i);
+    if (pthread_create(&t[i], 0, (void* (*)(void *x))ManyThreadsWorker, (void*)i))
+      kNumThreads = i;
   }
   for (size_t i = 0; i < kNumThreads; i++) {
     pthread_join(t[i], 0);