RFR(XS) 8009026: [parfait] Null pointer deference in hotspot/src/share/vm/code/nmethod.cpp

[Vladimir Kozlov]

On 4/10/13 4:45 AM, Nils Eliasson wrote:
> Hi,
>
> I know I am a bit late to the game, but I think I should raise the issue
> with false positives in Parfait. By fixing non-bugs we risk introducing
> new bugs into otherwise perfectly fine code by introducing unnecessary
> complexity.

That is why we have review process. I always asked everyone to do reviews.

>
> In this bug (8009026) Parfait complains about 'this' being null in the
> constructor of nmethod. For non-throwing new that isn't a problem. If
> the allocation fails the constructor isn't run (Section 5.3.4 (13) of
> the C++03 standard). The fix unfortunately introduces a dependence
> between heap::allocate and codecache::has_space. If the allocate code is

Yes, it is unfortunate.

> changed (as I was just about to do) without also fixing the
> codecache::has_space(), it may succeed while heap::allocate still
> returns null - and then we will fail the guarantee(), aborting the VM,
> instead of just skipping the compile as we used to. Also in the current

We can add assert into CodeCache::allocate to check if has_space() 
returns the same result.

> implementation has_space() is more conservative than allocate() - so we
> will fail some compiles that would have worked before the fix.

To be conservative is not bad and you see the difference only in edge 
case. The remaining space is not lost it will be used for adapters.

>
> If something should have been fixed, it should possibly been to have
> added an empty throw() to the new overloading, making it explicit that
> we want the empty exception-specification variant of new().

We don't use C++ exceptions.

Thanks,
Vladimir

>
> //Nils
>
>
>