Need reviewer: a couple of hprof fixes for Parfait (original) (raw)

David Holmes david.holmes at oracle.com
Tue Jun 12 19:41:59 PDT 2012

On 13/06/2012 3:23 AM, Kelly O'Hair wrote:

Ok, I have to confess, I had to look at this hprof table logic more. This generic "table" is used all over the place in hprof, so I had to dust off some brain cells. The info structures are extra structures or extra "info" for each table element, and it's perfectly valid for the info structure size to be 0. In particular, string tables have infosize==0. But when dealing with those infosize==0 tables, the info data is never asked for or accessed, if it was hprof would be seriously broken with null pointer accesses all over the place. So I am not convinced we are fixing any kind of bug here, just shutting up a static analysis tool that can't follow the non-obvious code flows. Parfait is seeing a NULL pointer returned here, and assumes that all calls to getinfo could return a NULL pointer, that is in fact wrong, because if infosize==0, this call will never be made. Parfait doesn't know that. Not unexpected. And even without this check, if infosize==0, it will return a NULL pointer anyway. So I have changed my fix. The static function getinfo() in hproftable.c is only called twice in that file, once by the extern function tablegetinfo(), and once by a table walker. I have removed the "return NULL;" from getinfo() and made the checks where it is called instead. I think this will remove the Parfait errors, but may generate some new ones, so we could be playing a bit of a whack-a-mole game here.

I like this better, but I don't see any check in table_get_info(), other than the assertion.

David

New webrev was generated and I displaced the old one: http://cr.openjdk.java.net/~ohair/openjdk8/parfaithproffixes/webrev/ Generic data structures where a pointer can be NULL sometimes and not-NULL other times is not something Parfait (or perhaps any static analysis tool) can deal with very well. -kto On Jun 12, 2012, at 8:06 AM, Kelly Ohair wrote:

if size is zero it returns null thats the cascading error that parfait is finding asserts turn into nothing with product builds so i need a dead stop here to shut up parfait :(

Sent from my iPhone On Jun 11, 2012, at 22:56, David Holmes<david.holmes at oracle.com> wrote: On 12/06/2012 12:48 PM, Kelly O'Hair wrote:

Need reviewer. I was asked to look at some Parfait errors in hprof code: 7176138: Fixes for missing close() calls and possible null pointer reference instead of fatal error http://cr.openjdk.java.net/~ohair/openjdk8/parfaithproffixes/webrev/

hproftable.c 211 if ( ltable->infosize == 0 ) { 212 HPROFERROR(JNITRUE, "Table is empty and should never be."); 213 return NULL; It is not obvious this should be a fatal error. Elsewhere this is handled as an assert. David

More information about the serviceability-dev mailing list