papi/SOURCES/papi-rhbz1807346.patch

commit 660bfd20bc89a26629e99de958d38b031db4250d
Author: William Cohen <wcohen@redhat.com>
Date:   Thu Oct 31 15:30:00 2019 -0400

    This code is a modification of krentel_pthreads.c, to better test
    some race conditions. It is not included in the standard tests;
    it is a diagnostic that should be run with "valgrind --tool=helgrind".
    
    Signed-off-by: Anthony Castaldo <TonyCastaldo@icl.utk.edu>

diff --git a/src/ctests/krentel_pthreads_race.c b/src/ctests/krentel_pthreads_race.c
new file mode 100644
index 000000000..0ebfb5056
--- /dev/null
+++ b/src/ctests/krentel_pthreads_race.c
@@ -0,0 +1,236 @@
+/*
+ *  Test PAPI with multiple threads.
+ *  This code is a modification of krentel_pthreads.c by William Cohen
+ *  <wcohen@redhat.com>, on Sep 10 2019, to exercise and test for the race
+ *  condition in papi_internal.c involving the formerly static variables
+ *  papi_event_code and papi_event_code_changed.  This code should be run with
+ *  "valgrind --tool=helgrind" to show any data races. If run with:
+ *  "valgrind --tool=helgrind --log-file=helgrind_out.txt"
+ *  The output will be captured in helgrind_out.txt and can then be processed
+ *  with the program filter_helgrind.c; see commentary at the top of that file.
+ */
+
+#define MAX_THREADS 256
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+#include <sys/time.h>
+
+#include "papi.h"
+#include "papi_test.h"
+
+#define EVENT  PAPI_TOT_CYC
+
+static int program_time = 5;
+static int threshold = 20000000;
+static int num_threads = 3;
+
+static long count[MAX_THREADS];
+static long iter[MAX_THREADS];
+static struct timeval last[MAX_THREADS];
+
+static pthread_key_t key;
+
+static struct timeval start;
+
+static void
+my_handler( int EventSet, void *pc, long long ovec, void *context )
+{
+	( void ) EventSet;
+	( void ) pc;
+	( void ) ovec;
+	( void ) context;
+
+	long num = ( long ) pthread_getspecific( key );
+
+	if ( num < 0 || num > num_threads )
+		test_fail( __FILE__, __LINE__, "getspecific failed", 1 );
+	count[num]++;
+}
+
+static void
+print_rate( long num )
+{
+	struct timeval now;
+	long st_secs;
+	double last_secs;
+
+	gettimeofday( &now, NULL );
+	st_secs = now.tv_sec - start.tv_sec;
+	last_secs = ( double ) ( now.tv_sec - last[num].tv_sec )
+		+ ( ( double ) ( now.tv_usec - last[num].tv_usec ) ) / 1000000.0;
+	if ( last_secs <= 0.001 )
+		last_secs = 0.001;
+
+	if (!TESTS_QUIET) {
+		printf( "[%ld] time = %ld, count = %ld, iter = %ld, "
+			"rate = %.1f/Kiter\n",
+			num, st_secs, count[num], iter[num],
+			( 1000.0 * ( double ) count[num] ) / ( double ) iter[num] );
+	}
+
+	count[num] = 0;
+	iter[num] = 0;
+	last[num] = now;
+}
+
+static void
+do_cycles( long num, int len )
+{
+	struct timeval start, now;
+	double x, sum;
+
+	gettimeofday( &start, NULL );
+
+	for ( ;; ) {
+		sum = 1.0;
+		for ( x = 1.0; x < 250000.0; x += 1.0 )
+			sum += x;
+		if ( sum < 0.0 )
+			printf( "==>>  SUM IS NEGATIVE !!  <<==\n" );
+
+		iter[num]++;
+
+		gettimeofday( &now, NULL );
+		if ( now.tv_sec >= start.tv_sec + len )
+			break;
+	}
+}
+
+static void *
+my_thread( void *v )
+{
+	long num = ( long ) v;
+	int n;
+	int EventSet = PAPI_NULL;
+	int event_code;
+	long long value;
+
+	int retval;
+
+	retval = PAPI_register_thread(  );
+	if ( retval != PAPI_OK ) {
+		test_fail( __FILE__, __LINE__, "PAPI_register_thread", retval );
+	}
+	pthread_setspecific( key, v );
+
+	count[num] = 0;
+	iter[num] = 0;
+	last[num] = start;
+
+	retval = PAPI_create_eventset( &EventSet );
+	if ( retval != PAPI_OK ) {
+		test_fail( __FILE__, __LINE__, "PAPI_create_eventset failed", retval );
+	}
+
+	retval = PAPI_event_name_to_code("PAPI_TOT_CYC", &event_code);
+	if (retval != PAPI_OK ) {
+		if (!TESTS_QUIET) printf("Trouble creating event name\n");
+		test_fail( __FILE__, __LINE__, "PAPI_event_name_to_code failed", retval );
+	}
+
+	retval = PAPI_add_event( EventSet, EVENT );
+	if (retval != PAPI_OK ) {
+		if (!TESTS_QUIET) printf("Trouble adding event\n");
+		test_fail( __FILE__, __LINE__, "PAPI_add_event failed", retval );
+	}
+
+	if ( PAPI_overflow( EventSet, EVENT, threshold, 0, my_handler ) != PAPI_OK )
+		test_fail( __FILE__, __LINE__, "PAPI_overflow failed", 1 );
+
+	if ( PAPI_start( EventSet ) != PAPI_OK )
+		test_fail( __FILE__, __LINE__, "PAPI_start failed", 1 );
+
+	if (!TESTS_QUIET) printf( "launched timer in thread %ld\n", num );
+
+	for ( n = 1; n <= program_time; n++ ) {
+		do_cycles( num, 1 );
+		print_rate( num );
+	}
+
+	PAPI_stop( EventSet, &value );
+
+        retval = PAPI_overflow( EventSet, EVENT, 0, 0, my_handler);
+	if ( retval != PAPI_OK )
+            test_fail( __FILE__, __LINE__, "PAPI_overflow failed to reset the overflow handler", retval );
+
+	if ( PAPI_remove_event( EventSet, EVENT ) != PAPI_OK ) 
+	    test_fail( __FILE__, __LINE__, "PAPI_remove_event", 1 );
+
+	if ( PAPI_destroy_eventset( &EventSet ) != PAPI_OK ) 
+	    test_fail( __FILE__, __LINE__, "PAPI_destroy_eventset", 1 );
+
+	if ( PAPI_unregister_thread( ) != PAPI_OK ) 
+            test_fail( __FILE__, __LINE__, "PAPI_unregister_thread", 1 );
+
+	return ( NULL );
+}
+
+int
+main( int argc, char **argv )
+{
+	pthread_t *td = NULL;
+	long n;
+	int quiet,retval;
+
+	/* Set TESTS_QUIET variable */
+	quiet=tests_quiet( argc, argv );
+
+	if ( argc < 2 || sscanf( argv[1], "%d", &program_time ) < 1 )
+		program_time = 6;
+	if ( argc < 3 || sscanf( argv[2], "%d", &threshold ) < 1 )
+		threshold = 20000000;
+	if ( argc < 4 || sscanf( argv[3], "%d", &num_threads ) < 1 )
+		num_threads = 32;
+
+	td = malloc((num_threads+1) * sizeof(pthread_t));
+	if (!td) {
+		test_fail( __FILE__, __LINE__, "td malloc failed", 1 );
+	}
+
+	if (!quiet) {
+		printf( "program_time = %d, threshold = %d, num_threads = %d\n\n",
+			program_time, threshold, num_threads );
+	}
+
+	if ( PAPI_library_init( PAPI_VER_CURRENT ) != PAPI_VER_CURRENT )
+		test_fail( __FILE__, __LINE__, "PAPI_library_init failed", 1 );
+
+	/* Test to be sure we can add events */
+	retval = PAPI_query_event( EVENT );
+	if (retval!=PAPI_OK) {
+		if (!quiet) printf("Trouble finding event\n");
+		test_skip(__FILE__,__LINE__,"Event not available",1);
+	}
+
+	if ( PAPI_thread_init( ( unsigned long ( * )( void ) ) ( pthread_self ) ) !=
+		 PAPI_OK )
+		test_fail( __FILE__, __LINE__, "PAPI_thread_init failed", 1 );
+
+	if ( pthread_key_create( &key, NULL ) != 0 )
+		test_fail( __FILE__, __LINE__, "pthread key create failed", 1 );
+
+	gettimeofday( &start, NULL );
+
+	for ( n = 1; n <= num_threads; n++ ) {
+		if ( pthread_create( &(td[n]), NULL, my_thread, ( void * ) n ) != 0 )
+			test_fail( __FILE__, __LINE__, "pthread create failed", 1 );
+	}
+
+	my_thread( ( void * ) 0 );
+
+	/* wait for all the threads */
+	for ( n = 1; n <= num_threads; n++ ) {
+	  	if ( pthread_join( td[n], NULL))
+			test_fail( __FILE__, __LINE__, "pthread join failed", 1 );
+	}
+
+	free(td);
+
+	if (!quiet) printf( "done\n" );
+
+	test_pass( __FILE__ );
+
+	return 0;
+}
commit 979e80136fd5e0ee2fb26f7374b36a8433147a68
Author: Anthony Castaldo <TonyCastaldo@icl.utk.edu>
Date:   Thu Oct 31 15:56:55 2019 -0400

    The changes to papi.c, papi_internal.c, threads.h and threads.c
    correct a race condition that was the result of all threads using
    the same two static variables (papi_event_code and papi_event_code_changed)
    to temporarily record a state of operation. The solution was to
    make these variables unique per thread, using the ThreadInfo_t
    structure already provided in PAPI for such purposes. The file
    krentel_pthread_race.c is a stress test to produce race conditions.
    filter_helgrind.c reduces the volume of --tool-helgrind output to
    a more manageable summary. Both are added to Makefile.recipies.

diff --git a/src/ctests/Makefile.recipies b/src/ctests/Makefile.recipies
index 87340831d..b7c1963d7 100644
--- a/src/ctests/Makefile.recipies
+++ b/src/ctests/Makefile.recipies
@@ -161,6 +161,12 @@ locks_pthreads: locks_pthreads.c $(TESTLIB) $(PAPILIB)
 krentel_pthreads: krentel_pthreads.c $(TESTLIB) $(PAPILIB)
 	$(CC_R) $(INCLUDE) $(CFLAGS) $(TOPTFLAGS) krentel_pthreads.c $(TESTLIB) $(PAPILIB) $(LDFLAGS) -o krentel_pthreads -lpthread
 
+# krentel_pthreads_race is not included with the standard tests;
+# it is a modification of krentel_pthreads intended to be run with 
+# "valgrind --tool=helgrind" to test for race conditions.
+krentel_pthreads_race: krentel_pthreads_race.c $(TESTLIB) $(PAPILIB)
+	$(CC_R) $(INCLUDE) $(CFLAGS) $(TOPTFLAGS) krentel_pthreads_race.c $(TESTLIB) $(PAPILIB) $(LDFLAGS) -o krentel_pthreads_race -lpthread
+
 overflow_pthreads: overflow_pthreads.c $(TESTLIB) $(DOLOOPS) $(PAPILIB)
 	$(CC_R) $(INCLUDE) $(CFLAGS) $(TOPTFLAGS) overflow_pthreads.c $(TESTLIB) $(DOLOOPS) $(PAPILIB) $(LDFLAGS) -o overflow_pthreads -lpthread
 
@@ -434,6 +440,9 @@ forkexec4: forkexec4.c $(TESTLIB) $(PAPILIB)
 prof_utils.o: prof_utils.c $(testlibdir)/papi_test.h prof_utils.h
 	$(CC) $(INCLUDE) $(CFLAGS) $(TOPTFLAGS) -c prof_utils.c
 
+filter_helgrind: filter_helgrind.c $(TESTLIB) $(PAPILIB)
+	-$(CC) $(INCLUDE) $(CFLAGS) $(TOPTFLAGS) filter_helgrind.c $(TESTLIB) $(PAPILIB) $(LDFLAGS) -o filter_helgrind 
+
 .PHONY : all default ctests ctest clean
 
 clean:
diff --git a/src/ctests/filter_helgrind.c b/src/ctests/filter_helgrind.c
new file mode 100644
index 000000000..d918a789e
--- /dev/null
+++ b/src/ctests/filter_helgrind.c
@@ -0,0 +1,170 @@
+/*
+ *  This code is a simple filter for the helgrind_out.txt file
+ *  produced by:
+ *  "valgrind --tool=helgrind --log-file=helgrind_out.txt someProgram"
+ *
+ * This is useful because the tool does not recognize PAPI locks,
+ * thus reports as possible race conditions reads/writes by
+ * different threads that are actually fine (surrounded by locks).
+ *
+ * This was written particularly for krentel_pthreads_race.c 
+ * when processed by the above valgrind. We produce a line per
+ * condition, in the form:
+ * OP@file:line OP@file:line
+ * where OP is R or W. The first file:line code occurred
+ * after the second file:line code, and on a different thread.
+ * 
+ * We print the results to stdout. It is useful to filter this
+ * through the standard utility 'uniq', each occurrence only 
+ * needs to be investigated once. Just insure there are
+ * MATCHING locks around each operation within the code.
+ *
+ * An example run (using uniq): The options -uc will print 
+ * only unique lines, preceeded by a count of how many times
+ * it occurs.
+ *
+ * ./filter_helgrind | uniq -uc
+ *
+ * An example output line (piped through uniq as above):
+ *       1 R@threads.c:190                    W@threads.c:206
+ * An investigation shows threads.c:190 is protected by 
+ * _papi_hwi_lock(THREADS_LOCK); and threads.c:206 is
+ * protected by the same lock. Thus no data race can 
+ * occur for this instance.
+ *
+ * Compilation within the papi/src/ctests directory:
+ * make filter_helgrind
+ * 
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int main(int argc, char** args) {
+   (void) argc;
+   (void) args;
+
+   char myLine[16384];
+   int state, size;
+   char type1, type2;
+   char fname1[256], fname2[256];
+   char *paren1, *paren2;
+
+   FILE *HELOUT = fopen("helgrind_out.txt", "r");  // Read the file.
+   if (HELOUT == NULL) {
+      fprintf(stderr, "Could not open helgrind_out.txt.\n");
+      exit(-1);
+   }
+
+   char PDRR[]="Possible data race during read";
+   char PDRW[]="Possible data race during write";
+   char TCWW[]="This conflicts with a previous write";
+   char TCWR[]="This conflicts with a previous read";
+   char atSTR[]="   at ";
+
+   // State machine:
+   // State 0: We are looking for a line with PDRR or PDRW.
+   //          We don't exit until we find it, or run out of lines.
+   //          if we find it, we remember which and go to state 1.
+   // State 1: Looking for "   at " in column 11. 
+   //          When found, we extract the string betweeen '(' and ')'
+   //          which is program name:line. go to state 2.
+   // State 2: We are searching for TCWW, TCWR, PDRW, PDRR.
+   //          If we find the first two:
+   //             Remember which, and go to state 3.
+   //          If we find either of the second two, go back to State 1.
+   // State 3: Looking for "   at " in column 11.
+   //          When found, extract the string betweeen '(' and ')',
+   //          which is program name:line.
+   //          OUTPUT LINE for an investigation.
+   //          Go to State 0.
+
+   state = 0;        // looking for PDRR, PDRW. 
+   while (fgets(myLine, 16384, HELOUT) != NULL) {
+      if (strlen(myLine) < 20) continue;
+      switch (state) {
+         case 0:  // Looking for PDRR or PRDW.
+            if (strstr(myLine, PDRR) != NULL) { 
+               type1='R';
+               state=1;
+               continue;
+            }
+
+            if (strstr(myLine, PDRW) != NULL) {
+               type1='W';
+               state=1;
+               continue;
+            }
+      
+            continue;
+            break;
+
+         case 1: // Looking for atSTR in column 11.
+            if (strncmp(myLine+10, atSTR, 6) != 0) continue;
+            paren1=strchr(myLine, '(');
+            paren2=strchr(myLine, ')');
+            if (paren1 == NULL || paren2 == NULL ||
+                paren1 > paren2) {
+               state=0;             // Abort, found something I don't understand.
+               continue;
+            }
+
+            size = paren2-paren1-1;          // compute length of name.
+            strncpy(fname1, paren1+1, size); // Copy the name.
+            fname1[size]=0;                  // install z-terminator.
+            state=2;
+            continue;
+            break;
+
+         case 2: // Looking for TCWW, TCWR, PDRR, PDRW.
+            if (strstr(myLine, TCWR) != NULL) {
+               type2='R';
+               state=3;
+               continue;
+            }
+
+            if (strstr(myLine, TCWW) != NULL) { 
+               type2='W';
+               state=3;
+               continue;
+            }
+
+            if (strstr(myLine, PDRR) != NULL) { 
+               type1='R';
+               state=1;
+               continue;
+            }
+
+            if (strstr(myLine, PDRW) != NULL) {
+               type1='W';
+               state=1;
+               continue;
+            }
+
+            continue;
+            break;
+
+         case 3: // Looking for atSTR in column 11.
+            if (strncmp(myLine+10, atSTR, 6) != 0) continue;
+            paren1=strchr(myLine, '(');
+            paren2=strchr(myLine, ')');
+            if (paren1 == NULL || paren2 == NULL ||
+                paren1 > paren2) {
+               state=0;             // Abort, found something I don't understand.
+               continue;
+            }
+
+            size = paren2-paren1-1;          // compute length of name.
+            strncpy(fname2, paren1+1, size); // Copy the name.
+            fname2[size]=0;                  // install z-terminator.
+            fprintf(stdout, "%c@%-32s %c@%-32s\n", type1, fname1, type2, fname2);
+            state=0;
+            continue;
+            break;
+      } // end switch.
+   } // end while.
+   
+   fclose(HELOUT);
+   exit(0);
+}  
diff --git a/src/papi.c b/src/papi.c
index 4e08dc840..070e3f8c6 100644
--- a/src/papi.c
+++ b/src/papi.c
@@ -608,32 +608,26 @@ PAPI_library_init( int version )
 	   papi_return( init_retval );
 	}
 
-	/* Initialize component globals */
+	/* Initialize thread globals, including the main threads  */
 
-	tmp = _papi_hwi_init_global(  );
+	tmp = _papi_hwi_init_global_threads(  );
 	if ( tmp ) {
 		init_retval = tmp;
 		_papi_hwi_shutdown_global_internal(  );
-		_in_papi_library_init_cnt--;
+   	_in_papi_library_init_cnt--;
 		papi_return( init_retval );
 	}
-	
-	/* Initialize thread globals, including the main threads  */
 
-	tmp = _papi_hwi_init_global_threads(  );
+	/* Initialize component globals */
+
+	tmp = _papi_hwi_init_global(  );
 	if ( tmp ) {
-		int i;
 		init_retval = tmp;
 		_papi_hwi_shutdown_global_internal(  );
-		for ( i = 0; i < papi_num_components; i++ ) {
-		    if (!_papi_hwd[i]->cmp_info.disabled) {
-                       _papi_hwd[i]->shutdown_component(  );
-		    }
-		}
 		_in_papi_library_init_cnt--;
 		papi_return( init_retval );
 	}
-
+	
 	init_level = PAPI_LOW_LEVEL_INITED;
 	_in_papi_library_init_cnt--;
 
diff --git a/src/papi_internal.c b/src/papi_internal.c
index 2412eca63..f0e457bf7 100644
--- a/src/papi_internal.c
+++ b/src/papi_internal.c
@@ -111,31 +111,28 @@ _papi_hwi_free_papi_event_string() {
 	}
 	return;
 }
-// A place to keep the current papi event code so some component functions can fetch its value
-// The current event code can be stored here prior to component calls and cleared after the component returns
-static unsigned int papi_event_code = -1;
-static int papi_event_code_changed = -1;
+
 void
 _papi_hwi_set_papi_event_code (unsigned int event_code, int update_flag) {
 	INTDBG("new event_code: %#x, update_flag: %d, previous event_code: %#x\n", event_code, update_flag, papi_event_code);
 
 	// if call is just to reset and start over, set both flags to show nothing saved yet
 	if (update_flag < 0) {
-		papi_event_code_changed = -1;
-		papi_event_code = -1;
+		_papi_hwi_my_thread->tls_papi_event_code_changed = -1;
+		_papi_hwi_my_thread->tls_papi_event_code = -1;
 		return;
 	}
 
 	// if 0, it is being set prior to calling a component, if >0 it is being changed by the component
-	papi_event_code_changed = update_flag;
+	_papi_hwi_my_thread->tls_papi_event_code_changed = update_flag;
 	// save the event code passed in
-	papi_event_code = event_code;
+	_papi_hwi_my_thread->tls_papi_event_code = event_code;
 	return;
 }
 unsigned int
 _papi_hwi_get_papi_event_code () {
 	INTDBG("papi_event_code: %#x\n", papi_event_code);
-	return papi_event_code;
+	return _papi_hwi_my_thread->tls_papi_event_code;
 }
 /* Get the index into the ESI->NativeInfoArray for the current PAPI event code */
 int
@@ -560,7 +557,7 @@ _papi_hwi_native_to_eventcode(int cidx, int event_code, int ntv_idx, const char
 
   int result;
 
-  if (papi_event_code_changed > 0) {
+  if (_papi_hwi_my_thread->tls_papi_event_code_changed > 0) {
 	  result = _papi_hwi_get_papi_event_code();
 	  INTDBG("EXIT: papi_event_code: %#x set by the component\n", result);
 	  return result;
diff --git a/src/threads.c b/src/threads.c
index 4dd0cf4e3..9f586c415 100644
--- a/src/threads.c
+++ b/src/threads.c
@@ -286,6 +286,10 @@ _papi_hwi_initialize_thread( ThreadInfo_t ** dest, int tid )
 		return PAPI_ENOMEM;
 	}
 
+   /* init event memory variables, used by papi_internal.c  */
+   thread->tls_papi_event_code = -1;
+   thread->tls_papi_event_code_changed = -1;
+
 	/* Call the component to fill in anything special. */
 
 	for ( i = 0; i < papi_num_components; i++ ) {
@@ -421,6 +425,11 @@ _papi_hwi_shutdown_thread( ThreadInfo_t * thread, int force_shutdown )
 	unsigned long tid;
 	int i, failure = 0;
 
+   /* Clear event memory variables */
+   thread->tls_papi_event_code = -1;
+   thread->tls_papi_event_code_changed = -1;
+
+   /* Get thread id */
 	if ( _papi_hwi_thread_id_fn )
 		tid = ( *_papi_hwi_thread_id_fn ) (  );
 	else
diff --git a/src/threads.h b/src/threads.h
index cd3369068..264d9f3a6 100644
--- a/src/threads.h
+++ b/src/threads.h
@@ -30,6 +30,11 @@ typedef struct _ThreadInfo
 	EventSetInfo_t **running_eventset;
 	EventSetInfo_t *from_esi;          /* ESI used for last update this control state */
 	int wants_signal;
+
+   // The current event code can be stored here prior to 
+   // component calls and cleared after the component returns.
+   unsigned int tls_papi_event_code;
+   int tls_papi_event_code_changed;
 } ThreadInfo_t;
 
 /** The list of threads, gets initialized to master process with TID of getpid()