diff mbox

Fix up _cpp_find_file

Message ID 20130227201814.GO12913@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 27, 2013, 8:18 p.m. UTC 
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  <jakub@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.


	Jakub

Comments

Tom Tromey Feb. 28, 2013, 7:18 p.m. UTC | #1 
>>>>> "Jakub" == Jakub Jelinek <jakub@redhat.com> writes:

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

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

This is ok.  Thanks.

Tom
diff mbox

Patch

--- 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;