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
info_size==0.
But when dealing with those info_size==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
get_info could
return a NULL pointer, that is in fact wrong, because if info_size==0, this
call will never be made.
Parfait doesn't know that. Not unexpected. And even without this check, if
info_size==0, it
will return a NULL pointer anyway.
So I have changed my fix.
The static function get_info() in hprof_table.c is only called twice in that
file, once by the
extern function table_get_info(), and once by a table walker.
I have removed the "return NULL;" from get_info() 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/parfait_hprof_fixes/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.hol...@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/parfait_hprof_fixes/webrev/
hprof_table.c
211 if ( ltable->info_size == 0 ) {
212 HPROF_ERROR(JNI_TRUE, "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
