Patchwork fix PR48299 by merging changes for thread_leak_test.c from upstream

login
register
mail settings
Submitter Jack Howarth
Date Feb. 27, 2012, 4:41 p.m.
Message ID <20120227164127.GA30684@bromo.med.uc.edu>
Download mbox | patch
Permalink /patch/143225/
State New
Headers show

Comments

Jack Howarth - Feb. 27, 2012, 4:41 p.m.
Currently the testsuite/boehm-gc.c/thread_leak_test.c executation test
will hang on several targets including x86_64-apple-darwin10/11 for -m32/-m64.
The attached patch eliminates this issue, PR48299, by merging in the
upstream changes for this testcase from...

https://raw.github.com/ivmai/bdwgc/master/tests/thread_leak_test.c

The only changes made to the upstram test case was retaining the use of...

GC_find_leak = 1;

rather than...

GC_set_find_leak(1);

and formatting style changes. Tested on x86_64-apple-darwin10 and
x86_64-apple-darwin11. Okay for gcc trunk for gcc 4.7 (as the
timeouts in the testsuite are very annoying)?
         Jack
ps I was uncertain how much detail we need in the changelog as the
upstream commit logs aren't very informative...

https://github.com/ivmai/bdwgc/commits/master/tests/thread_leak_test.c

2012-02-27  Jack Howarth  <howarth@bromo.med.uc.edu>
	    Patrick Marlier  <patrick.marlier@gmail.com>

	PR boehm-gc/48299
	testsuite/boehm-gc.c/thread_leak_test.c: Merge upstream changes.

boehm-gc/
Mike Stump - Feb. 27, 2012, 6:07 p.m.
On Feb 27, 2012, at 8:41 AM, Jack Howarth wrote:
> Currently the testsuite/boehm-gc.c/thread_leak_test.c executation test
> will hang on several targets including x86_64-apple-darwin10/11 for -m32/-m64.
> The attached patch eliminates this issue, PR48299, by merging in the
> upstream changes for this testcase from...

> Index: testsuite/boehm-gc.c/thread_leak_test.c

So, ideally, I'd like to have Bryce or Tom review this change or at least the methodology, if they are still working in this area.
Jack Howarth - Feb. 27, 2012, 9:01 p.m.
On Mon, Feb 27, 2012 at 10:07:20AM -0800, Mike Stump wrote:
> On Feb 27, 2012, at 8:41 AM, Jack Howarth wrote:
> > Currently the testsuite/boehm-gc.c/thread_leak_test.c executation test
> > will hang on several targets including x86_64-apple-darwin10/11 for -m32/-m64.
> > The attached patch eliminates this issue, PR48299, by merging in the
> > upstream changes for this testcase from...
> 
> > Index: testsuite/boehm-gc.c/thread_leak_test.c
> 
> So, ideally, I'd like to have Bryce or Tom review this change or at least the methodology, if they are still working in this area.

Mike,
   Since this is just a testsuite issue in boehm-gc, wouldn't this be Hans' call? I would definitely
like to get this fixed in gcc 4.7 so that the boehm-gc testsuite doesn't suffer timeouts. Also note
that gcc 4.7 is the first release which seems to report boehm-gc testresults.

http://gcc.gnu.org/ml/gcc-testresults/2012-02/msg02427.html

vs

http://gcc.gnu.org/ml/gcc-testresults/2012-02/msg01854.html

                  Jack
Mike Stump - Feb. 27, 2012, 9:55 p.m.
On Feb 27, 2012, at 1:01 PM, Jack Howarth wrote:
>   Since this is just a testsuite issue in boehm-gc, wouldn't this be Hans' call? I would definitely
> like to get this fixed in gcc 4.7 so that the boehm-gc testsuite doesn't suffer timeouts. Also note
> that gcc 4.7 is the first release which seems to report boehm-gc testresults.

If you check the changelogs, the java people usually do the pulling.  Since they have a history doing it, they are the best to know about any problems associated with imports.  If they abdicate, I'll approve it, though, late timing might push it past the .0 release.
Richard Guenther - Feb. 28, 2012, 9:05 a.m.
On Mon, 27 Feb 2012, Mike Stump wrote:

> On Feb 27, 2012, at 1:01 PM, Jack Howarth wrote:
> >   Since this is just a testsuite issue in boehm-gc, wouldn't this be Hans' call? I would definitely
> > like to get this fixed in gcc 4.7 so that the boehm-gc testsuite doesn't suffer timeouts. Also note
> > that gcc 4.7 is the first release which seems to report boehm-gc testresults.
> 
> If you check the changelogs, the java people usually do the pulling.  Since they have a history doing it, they are the best to know about any problems associated with imports.  If they abdicate, I'll approve it, though, late timing might push it past the .0 release.

As it is a testsuite-only change and removes a testsuite regression
(fingers crossing) the patch is ok.  It cannot really make things worse
here.

Thanks,
Richard.

Patch

Index: testsuite/boehm-gc.c/thread_leak_test.c
===================================================================
--- testsuite/boehm-gc.c/thread_leak_test.c	(revision 184602)
+++ testsuite/boehm-gc.c/thread_leak_test.c	(working copy)
@@ -1,13 +1,22 @@ 
-#define GC_LINUX_THREADS
+#ifndef GC_THREADS
+# define GC_THREADS
+#endif
 #include "leak_detector.h"
-#include <pthread.h>
+#ifdef GC_PTHREADS
+# include <pthread.h>
+#else
+# include <windows.h>
+#endif
 #include <stdio.h>
 
-void * test(void * arg) {
+#ifdef GC_PTHREADS
+  void * test(void * arg)
+#else
+  DWORD WINAPI test(LPVOID arg)
+#endif
+{
     int *p[10];
     int i;
-    GC_find_leak = 1; /* for new collect versions not compiled  */
-    /* with -DFIND_LEAK.                                        */
     for (i = 0; i < 10; ++i) {
         p[i] = malloc(sizeof(int)+i);
     }
@@ -15,23 +24,47 @@ 
     for (i = 1; i < 10; ++i) {
         free(p[i]);
     }
-}       
+#ifdef GC_PTHREADS
+    return arg;
+#else
+    return (DWORD)(GC_word)arg;
+#endif
+}
 
 #define NTHREADS 5
 
-int main() {
+int main(void) {
     int i;
+#ifdef GC_PTHREADS
     pthread_t t[NTHREADS];
+#else
+    HANDLE t[NTHREADS];
+    DWORD thread_id;
+#endif
     int code;
 
+    GC_find_leak = 1;    /* for new collect versions not compiled       */
+    GC_INIT();
     for (i = 0; i < NTHREADS; ++i) {
-	if ((code = pthread_create(t + i, 0, test, 0)) != 0) {
-    	    printf("Thread creation failed %d\n", code);
+#ifdef GC_PTHREADS
+       code = pthread_create(t + i, 0, test, 0);
+#else
+       t[i] = CreateThread(NULL, 0, test, 0, 0, &thread_id);
+       code = t[i] != NULL ? 0 : (int)GetLastError();
+#endif
+       if (code != 0) {
+          printf("Thread creation failed %d\n", code);
         }
     }
     for (i = 0; i < NTHREADS; ++i) {
-	if ((code = pthread_join(t[i], 0)) != 0) {
-            printf("Thread join failed %lu\n", code);
+#ifdef GC_PTHREADS
+       code = pthread_join(t[i], 0);
+#else
+       code = WaitForSingleObject(t[i], INFINITE) == WAIT_OBJECT_0 ? 0 :
+                                                        (int)GetLastError();
+#endif
+       if (code != 0) {
+          printf("Thread join failed %d\n", code);
         }
     }
     CHECK_LEAKS();