Fixed last know issue with multithreading.

git-svn-id: https://gcc.gnu.org/svn/gcc/branches/google/gcc-4_6-mobile@189696 138bc75d-0d04-0410-961f-82ee72b054a4

5 files changed, 52 insertions, 28 deletions

diff --git a/vtable-security/ChangeLog.vtable-security b/vtable-security/ChangeLog.vtable-security
index 021afa50179..78adedb6c6a 100644
--- a/vtable-security/ChangeLog.vtable-security
+++ b/vtable-security/ChangeLog.vtable-security
@@ -1,3 +1,19 @@
+2012-07-19  Luis Lozano  <llozano@google.com>
+
+	* libstdc++-v3/libsupc++/vtv_malloc.cc: moved VTV_assert to header
+	file
+	* libstdc++-v3/libsupc++/vtv_malloc.cc: ditto.
+	* libstdc++-v3/libsupc++/vtv_rts.cc (log_register_pairs): Changed to 
+	use snprintf insted of sprintf for better protection.
+	(__VLTRegisterPair): Be consistent about the use of volatile variable
+	* libstdc++-v3/libsupc++/vtv_threaded_hash.cc: Added some assert around
+	the code to help with multi-threaded triaging.
+	(grow_table): make sure that the update of the "data" field is the last 
+	thing we do. This is important for consistency in the multithreaded case.
+	(vlt_hash_insert): fix the code to depend only on one field that could have
+	a race condition (table->data). The code is supposed to handle the race
+	condition. This fixes the last know issue regarding multi-threading.
+	
 2012-07-18  Caroline Tice  <cmtice@google.com>
 
 	* libstdc++-v3/libsupc++/vtv_rts.cc (__VLTRegisterPair):  Add
diff --git a/vtable-security/libstdc++-v3/libsupc++/vtv_malloc.cc b/vtable-security/libstdc++-v3/libsupc++/vtv_malloc.cc
index d1a8647ae34..8053e52e81f 100644
--- a/vtable-security/libstdc++-v3/libsupc++/vtv_malloc.cc
+++ b/vtable-security/libstdc++-v3/libsupc++/vtv_malloc.cc
@@ -51,8 +51,6 @@ VTV_error (void)
   abort();
 }
 
-#define VTV_assert(EXPR) ((void)(!(EXPR) ? VTV_error() : (void) 0))
-
 void
 VTV_malloc_protect (void)
 {
diff --git a/vtable-security/libstdc++-v3/libsupc++/vtv_malloc.h b/vtable-security/libstdc++-v3/libsupc++/vtv_malloc.h
index e5a33b1257c..ade1951ed6e 100644
--- a/vtable-security/libstdc++-v3/libsupc++/vtv_malloc.h
+++ b/vtable-security/libstdc++-v3/libsupc++/vtv_malloc.h
@@ -49,6 +49,7 @@ extern void VTV_malloc_protect (void);
 extern void VTV_malloc_unprotect (void);
 
 extern void VTV_error (void);
+#define VTV_assert(EXPR) ((void)(!(EXPR) ? VTV_error() : (void) 0))
 
 /* Name of the section where we put general VTV variables for protection */
 #define VTV_PROTECTED_VARS_SECTION ".data.rel.ro.vtable_vars"
diff --git a/vtable-security/libstdc++-v3/libsupc++/vtv_rts.cc b/vtable-security/libstdc++-v3/libsupc++/vtv_rts.cc
index 9d4955a89f2..d4a4fa579cb 100644
--- a/vtable-security/libstdc++-v3/libsupc++/vtv_rts.cc
+++ b/vtable-security/libstdc++-v3/libsupc++/vtv_rts.cc
@@ -55,7 +55,7 @@ static const int debug_register_pairs = 0;
    They are explicitly unprotected and protected again by calls to VTV_unprotect
    and VTV_protect */
 
-static FILE *log_file_fp VTV_PROTECTED_VAR = NULL;
+static FILE * log_file_fp VTV_PROTECTED_VAR = NULL;
 
 struct mprotect_data {
   int prot_mode;
@@ -196,16 +196,16 @@ log_register_pairs (FILE *fp, const char *format_string_dummy, int format_arg1,
      us to write out the string that is missing the '\0' at it's
      end. */
 
-  sprintf (format_string, format_string_dummy, format_arg1, format_arg2);
+  snprintf (format_string, sizeof(format_string), format_string_dummy, format_arg1, format_arg2);
 
   fprintf (fp, format_string, base_var_name, vtable_name, vtbl_ptr);
 }
 
-  /* For some reason, when the char * names get passed into these
-     functions, they are missing the '\0' at the end; therefore we
-     also pass in the length of the string and make sure, when writing
-     out the names, that we only write out the correct number of
-     bytes. */
+/* For some reason, when the char * names get passed into these
+   functions, they are missing the '\0' at the end; therefore we
+   also pass in the length of the string and make sure, when writing
+   out the names, that we only write out the correct number of
+   bytes. */
 void
 print_debugging_message (const char *format_string_dummy, int format_arg1,
 			 int format_arg2,
@@ -228,26 +228,26 @@ print_debugging_message (const char *format_string_dummy, int format_arg1,
   fprintf (stdout, format_string, str_arg1, str_arg2);
 }
 
-/* TODO: Why is this returning anything */
+/* TODO: Why is this returning anything
+   remove unnecessary arguments */
 void *
 __VLTRegisterPair (void **data_pointer, void *test_value, int size_hint,
 		   char *base_ptr_var_name, int len1, char *vtable_name,
 		   int len2)
 {
-  struct vlt_hashtable **base_vtbl_row_ptr =
-                                       (struct vlt_hashtable **) data_pointer;
   vptr vtbl_ptr = (vptr) test_value;
-
-  struct vlt_hashtable * volatile *tmp_volatile_ptr = base_vtbl_row_ptr;
+  struct vlt_hashtable * volatile *tmp_volatile_ptr = 
+    (struct vlt_hashtable **) data_pointer;
   static pthread_mutex_t map_var_mutex VTV_PROTECTED_VAR
                                                   = PTHREAD_MUTEX_INITIALIZER;
 
-
   if ((*tmp_volatile_ptr) == NULL)
     {
       pthread_mutex_lock (&map_var_mutex);
+
       if ((*tmp_volatile_ptr) == NULL)
         *tmp_volatile_ptr = vlt_hash_init_table (size_hint);
+
       pthread_mutex_unlock (&map_var_mutex);
     }
 
@@ -261,11 +261,11 @@ __VLTRegisterPair (void **data_pointer, void *test_value, int size_hint,
       log_register_pairs (log_file_fp, "Registering %%.%ds : %%.%ds (%%p)\n",
 			  len1, len2,
 			  base_ptr_var_name, vtable_name, vtbl_ptr);
-      if (*base_vtbl_row_ptr == NULL)
+      if (*tmp_volatile_ptr == NULL)
 	fprintf (log_file_fp, "  vtable map variable is NULL.\n");
     }
 
-  vlt_hash_insert (*base_vtbl_row_ptr, test_value);
+  vlt_hash_insert (*tmp_volatile_ptr, test_value);
   return NULL;
 }
 
diff --git a/vtable-security/libstdc++-v3/libsupc++/vtv_threaded_hash.cc b/vtable-security/libstdc++-v3/libsupc++/vtv_threaded_hash.cc
index c3e9fb15964..d05b4c9ed07 100644
--- a/vtable-security/libstdc++-v3/libsupc++/vtv_threaded_hash.cc
+++ b/vtable-security/libstdc++-v3/libsupc++/vtv_threaded_hash.cc
@@ -26,6 +26,7 @@
 #include <math.h>
 #include <stdio.h>
 #include <string.h>
+#include <assert.h>
 
 #include "vtv_threaded_hash.h"
 #include "vtv_malloc.h"
@@ -128,7 +129,6 @@ grow_table (struct vlt_hashtable *table)
   table->num_elts = rehash_elements (old_data, new_data, old_size,
                                      new_power_size);
 
-  table->data = new_data;
   table->data_size = new_size;
   table->power_of_2 = new_power_size;
 
@@ -150,8 +150,11 @@ grow_table (struct vlt_hashtable *table)
      before the action is taken.
   */
 
-  table->data[-1] = (vlt_hash_bucket *)
+  new_data[-1] = (vlt_hash_bucket *)
                                       (0xffffffffUL >> (32 - new_power_size));
+  assert(new_data[-1] != 0);
+
+  table->data = new_data;
 
   /* TODO:  Need to 'my_free' each allocated bucket in old_data... */
   my_free (old_data);
@@ -211,6 +214,7 @@ vlt_hash_init_table (int initial_size_hint)
 
   new_table->data[0] = (vlt_hash_bucket *)
                                       (0xffffffffUL >> (32 - initial_power));
+  assert(new_table->data[0] != 0);
 
   /* The very first element/bucket is reserved for the hash mask, so make
      the main table start at the second element. */
@@ -224,13 +228,15 @@ void
 vlt_hash_insert (struct vlt_hashtable *table, void *value)
 {
   uint32_t hash = vlt_hash_pointer (value);
-  uint32_t table_size = table->data_size;
+  struct vlt_hash_bucket **data = table->data;
 
   /* Reminder:  table->data[-1] contains the hash mask.   See comments in
      vlt_hash_init_table for details.  */
-  uint64_t hash_mask = (uint64_t) (table->data[-1]);
+  uint64_t hash_mask = (uint64_t) data[-1];
+  assert(hash_mask != 0);
+
   uint32_t index = hash & (uint32_t) hash_mask;
-  vlt_hash_bucket *first_bucket = table->data[index];
+  vlt_hash_bucket *first_bucket = data[index];
   void *slot = vlt_bucket_find (first_bucket, value, NULL);
 
   /* Only do the insert if the value is not already in the table.  */
@@ -245,20 +251,21 @@ vlt_hash_insert (struct vlt_hashtable *table, void *value)
 	 inserting the element, see if anybody else inserted the
 	 element since the last check (before we grabbed the
 	 lock.)  */
-      uint64_t hash_mask = (uint64_t) table->data[-1];
-      uint32_t new_size = table->data_size;
+      uint64_t new_hash_mask = (uint64_t) table->data[-1];
       uint32_t new_index;
       bool should_insert;
 
-      if (table_size == new_size)
+      if (data == table->data) // hash table was not resized
         {
+	  assert(new_hash_mask == hash_mask);
           new_index = index;
           should_insert = !vlt_bucket_find (table->data[new_index], value,
                                             first_bucket);
         }
       else
         {
-          new_index = hash & (uint32_t) hash_mask;
+	  assert(new_hash_mask != hash_mask);
+          new_index = hash & (uint32_t) new_hash_mask;
           should_insert = !vlt_bucket_find (table->data[new_index], value,
                                             NULL);
         }
@@ -268,13 +275,15 @@ vlt_hash_insert (struct vlt_hashtable *table, void *value)
           if (table->num_elts >= (REHASH_LIMIT * table->data_size))
             {
               grow_table (table);
-              hash_mask = (uint64_t) table->data[-1];
-              new_index = hash & (uint32_t) hash_mask;
+              new_hash_mask = (uint64_t) table->data[-1];
+	      assert(new_hash_mask != 0);
+              new_index = hash & (uint32_t) new_hash_mask;
             }
 
           bucket_insert (&(table->data[new_index]), value);
           table->num_elts++;
         }
+
       pthread_mutex_unlock (&(table->mutex));
     }
 }