[vmkit-commits] fix ThreadAllocator regression

[nicolas geoffray]

Thanks very much Allan for finding the bug and fixing it!

About checking for allocation failure, we should definitely not check at
this place but in the allocator, and just die if the allocation fails. The
previous code is a left-over of some attempts to cope with exception
handling.

For the hash map of UTF8, you are right: we want to avoid creating an UTF8
in the map if it is not needed. The reason why we have this limitation is to
implement class loader isolation, so that one class loader can not pollute
the map of another class loader.

Nicolas

On Sun, Aug 8, 2010 at 8:59 AM, Allan Tong <actong88 at gmail.com> wrote:

> Attached patch fixes a regression caused by replacing alloca with
> ThreadAllocator.  The threadAllocator goes out of scope too early,
> causing the temporary UTF8 object to be freed prematurely before it
> has actually had a chance to be used.
>
> Note I also removed the allocation failure check, mostly because it
> doesn't seem to be possible for ThreadAllocator to return a NULL (or
> at least it'll segfault long before returning).  I'm not really sure
> what the strategy is for handling allocation failures.  It seems most
> of the code base simply ignores that possibility.  If you do choose to
> keep the allocation failure check, at the very least it should be
> moved ahead of the temp->size assignment.
>
> I had an earlier patch that simply replaced the whole thing with a
> call to asciizConstructUTF8, but I wasn't sure if that was correct
> since it would hash the name even if it couldn't find the class.  Is
> hashUTF8 only supposed to map strings of classes that are actually
> loaded?
>
>  - Allan
>
> _______________________________________________
> vmkit-commits mailing list
> vmkit-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20100810/2e97660d/attachment.html>