* [PATCH] Fix up _cpp_find_file
@ 2013-02-27 20:18 Jakub Jelinek
2013-02-28 19:18 ` Tom Tromey
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2013-02-27 20:18 UTC (permalink / raw)
To: Tom Tromey; +Cc: gcc-patches
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.
--- 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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Fix up _cpp_find_file
2013-02-27 20:18 [PATCH] Fix up _cpp_find_file Jakub Jelinek
@ 2013-02-28 19:18 ` Tom Tromey
0 siblings, 0 replies; 2+ messages in thread
From: Tom Tromey @ 2013-02-28 19:18 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
>>>>> "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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-02-28 19:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-27 20:18 [PATCH] Fix up _cpp_find_file Jakub Jelinek
2013-02-28 19:18 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).