[Ada] fix potential memory corruption in annotated value cache
Alexandre Oliva
aoliva@redhat.com
Fri Sep 16 07:56:00 GMT 2011
A -fcompare-debug regression in s-regexp.adb on x86_64-linux-gnu turned
out to be caused by a hashtable management error in annotate_value().
We ask for an insertion and leave the allocated slot empty while
proceeding to other computations that might (a) return without filling
it in, or (b) recurse and allocate the same slot or (c) grow and move
the table.
(a) is a performance problem because it might render cached values
invisible, should the allocated slot be formerly deleted, thus breaking
a rehash chain.
(b) is also a performance problem, because when the context that first
allocated the slot proceeds to fill it in, it may override another
cached value that happened to be assigned the same slot. This is what
caused the -fcompare-debug difference: the annotation of a value had its
cache entry overridden by an upstream caller in only one of the
compilations, so a subsequent call of annotate_value with the same value
resulted in a use of the cached value in one, and an expansion that
remapped new decls in the other. This out-of-sync decl numbering ended
up causing different symbol names to be chosen within
lhd_set_decl_assembler_name().
(c) is the scariest possibility: if the hash table that holds cached
values grows to the point of being moved during recursion, and upon
return we fill in the pointer in the slot that is in the old (possibly
reused) storage, we may be corrupting internal compiler state.
Some possible fixes I considered were:
1. inserting on entry (as is), allocating the cache entry right away,
and *always* filling it before returning
2. inserting on entry (as is), allocating the cache entry right away,
and releasing it before returning unless we're filling it in
3. not inserting on entry, and looking up again for insertion before
caching and returning, so as to get a fresh slot pointer
I implemented 3., and considered splitting the logic of annotate_value()
into one function that manages caching and calls the other to perform
the computation, so as to simplify the implementation.
Here's the patch I've tested on i686-pc-linux-gnu and x86_64-linux-gnu.
Ok to install?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ada-cache-annotated-values-hash-early.patch
Type: text/x-diff
Size: 2049 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20110916/5c4890d5/attachment.bin>
-------------- next part --------------
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist Red Hat Brazil Compiler Engineer
More information about the Gcc-patches
mailing list