powa-team / pg_qualstats

A PostgreSQL extension for collecting statistics about predicates, helping find what indices are missing
Other
272 stars 26 forks source link

Dereference after null #59

Closed sminux closed 5 months ago

sminux commented 6 months ago

I have run static analyzer tests. Here is the warning: After having been compared to a NULL value at pg_qualstats.c:1069, pointer 'planstate->instrument' is dereferenced at pg_qualstats.c:1121. It can cause the program to crash, is it?

rjuju commented 6 months ago

Hi,

I can see how it sound confusing, but I don't think there's any problem here.

The fact that we first check for a non null instrument is because we reused the same code as in upstream ExecutorEnd() for consistency, and this code has to deal with possibly NULL instrument.

But in our case we have the guarantee to find one since we ask for it ExecutorStart (https://github.com/powa-team/pg_qualstats/blob/1a6d6472e1c7d4259f9083d34274ffaf95de550c/pg_qualstats.c#L631) and recheck it before calling the code you're pointing (https://github.com/powa-team/pg_qualstats/blob/1a6d6472e1c7d4259f9083d34274ffaf95de550c/pg_qualstats.c#L728).

sminux commented 5 months ago

Now it's clear, seems correct, thanks)