Hi!

As discussed on IRC, this function calls htab_find_slot_with_hash with
INSERT early, but sometimes decides to return without actually making
sure *hash_slot is non-NULL.  That can result in broken hash table,
because NULL is empty entry and could break lookup for some other hash
value.  Fixed by calling htab_clear_slot.  Also, I think it is an aliasing
violation to access *hash_slot through a different type than the one
hashtab.c accesses it through (void *), so I've fixed that too.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2013-02-27  Jakub Jelinek  <ja...@redhat.com>

        * files.c (_cpp_find_file): If returning early, before storing
        something to *hash_slot and *hash_slot is NULL, call htab_clear_slot
        on it.  Access *hash_slot using void * type rather than
        struct file_hash_entry * to avoid aliasing issues.

--- gcc/files.c.jj      2013-01-15 09:04:55.000000000 +0100
+++ gcc/files.c 2013-02-27 16:31:50.000912853 +0100
@@ -492,7 +492,8 @@ _cpp_file *
 _cpp_find_file (cpp_reader *pfile, const char *fname, cpp_dir *start_dir,
                bool fake, int angle_brackets, bool implicit_preinclude)
 {
-  struct file_hash_entry *entry, **hash_slot;
+  struct file_hash_entry *entry;
+  void **hash_slot;
   _cpp_file *file;
   bool invalid_pch = false;
   bool saw_bracket_include = false;
@@ -503,13 +504,12 @@ _cpp_find_file (cpp_reader *pfile, const
   if (start_dir == NULL)
     cpp_error (pfile, CPP_DL_ICE, "NULL directory in find_file");
 
-  hash_slot = (struct file_hash_entry **)
-    htab_find_slot_with_hash (pfile->file_hash, fname,
-                             htab_hash_string (fname),
-                             INSERT);
+  hash_slot
+    = htab_find_slot_with_hash (pfile->file_hash, fname,
+                               htab_hash_string (fname), INSERT);
 
   /* First check the cache before we resort to memory allocation.  */
-  entry = search_cache (*hash_slot, start_dir);
+  entry = search_cache ((struct file_hash_entry *) *hash_slot, start_dir);
   if (entry)
     return entry->u.file;
 
@@ -533,6 +533,17 @@ _cpp_find_file (cpp_reader *pfile, const
                 the list of all files so that #import works.  */
              file->next_file = pfile->all_files;
              pfile->all_files = file;
+             if (*hash_slot == NULL)
+               {
+                 /* If *hash_slot is NULL, the above htab_find_slot_with_hash
+                    call just created the slot, but we aren't going to store
+                    there anything, so need to remove the newly created entry.
+                    htab_clear_slot requires that it is non-NULL, so store
+                    there some non-NULL pointer, htab_clear_slot will
+                    overwrite it immediately.  */
+                 *hash_slot = file;
+                 htab_clear_slot (pfile->file_hash, hash_slot);
+               }
              return file;
            }
 
@@ -548,6 +559,12 @@ _cpp_find_file (cpp_reader *pfile, const
            {
              free ((char *) file->name);
              free (file);
+             if (*hash_slot == NULL)
+               {
+                 /* See comment on the above htab_clear_slot call.  */
+                 *hash_slot = file;
+                 htab_clear_slot (pfile->file_hash, hash_slot);
+               }
              return NULL;
            }
          else
@@ -565,7 +582,7 @@ _cpp_find_file (cpp_reader *pfile, const
       else
        continue;
 
-      entry = search_cache (*hash_slot, file->dir);
+      entry = search_cache ((struct file_hash_entry *) *hash_slot, file->dir);
       if (entry)
        {
          found_in_cache = file->dir;
@@ -589,11 +606,11 @@ _cpp_find_file (cpp_reader *pfile, const
 
   /* Store this new result in the hash table.  */
   entry = new_file_hash_entry (pfile);
-  entry->next = *hash_slot;
+  entry->next = (struct file_hash_entry *) *hash_slot;
   entry->start_dir = start_dir;
   entry->location = pfile->line_table->highest_location;
   entry->u.file = file;
-  *hash_slot = entry;
+  *hash_slot = (void *) entry;
 
   /* If we passed the quote or bracket chain heads, cache them also.
      This speeds up processing if there are lots of -I options.  */
@@ -602,22 +619,22 @@ _cpp_find_file (cpp_reader *pfile, const
       && found_in_cache != pfile->bracket_include)
     {
       entry = new_file_hash_entry (pfile);
-      entry->next = *hash_slot;
+      entry->next = (struct file_hash_entry *) *hash_slot;
       entry->start_dir = pfile->bracket_include;
       entry->location = pfile->line_table->highest_location;
       entry->u.file = file;
-      *hash_slot = entry;
+      *hash_slot = (void *) entry;
     }
   if (saw_quote_include
       && pfile->quote_include != start_dir
       && found_in_cache != pfile->quote_include)
     {
       entry = new_file_hash_entry (pfile);
-      entry->next = *hash_slot;
+      entry->next = (struct file_hash_entry *) *hash_slot;
       entry->start_dir = pfile->quote_include;
       entry->location = pfile->line_table->highest_location;
       entry->u.file = file;
-      *hash_slot = entry;
+      *hash_slot = (void *) entry;
     }
 
   return file;

        Jakub

Reply via email to