From: Adrian Ratiu Date: Fri, 09 Jan 2026 12:33:18 GMT Subject: Re: [Bug] hook: -Wanalyzer-deref-before-check warning in run_hooks_opt Message-ID: <87jyxrvucx.fsf@gentoo.mail-host-address-is-not-set> In-Reply-To: On Fri, 09 Jan 2026, Patrick Steinhardt wrote: > On Thu, Jan 08, 2026 at 08:24:01PM -0500, correctmost wrote: >> Hi, >> >> GCC 15.2.1 warns about a potential NULL pointer dereference in >> run_hooks_opt on the master branch: >> >> --- >> >> ../hook.c: In function ‘run_hooks_opt’: >> ../hook.c:167:12: error: check of ‘options’ for NULL after already dereferencing it [-Werror=analyzer-deref-before-check] >> 167 | if (!options) >> | ^ >> >> [...snip...] >> >> │ 156 | .ungroup = options->ungroup, >> │ | ~~~~~~~~~~~~~~~~ >> │ | | >> │ | (7) pointer ‘options’ is dereferenced here >> │...... >> │ 167 | if (!options) >> │ | ~ >> │ | | >> │ | (8) pointer ‘options’ is checked for NULL here but it was already dereferenced at (7) >> │ >> >> --- >> >> This does seem like a real bug, though I'm not sure how likely it is >> to occur. It looks like the warning was introduced in merge commit >> f406b89552 ("Use hook API to replace ad-hoc invocation of hook scripts >> with the run_command() API."). >> >> I noticed the warning while compiling commit d529f3a19736 on Arch >> Linux. > > It's not a real bug. If you take a look at the the `if (!options)` > check, you'll see: > > if (!options) > BUG("a struct run_hooks_opt must be provided to run_hooks"); > > So we'd abort immediatly with an error message in case the pointer was > `NULL`. Which clarifies that this is a case that shouldn't ever happen > in the first place. > > That being said, it's of course a bit careless to dereference the > pointer before we have the opportunity to call `BUG()`. I see two ways > to fix this: > > - We can either move all derefs of `options` after the call to > `BUG()`. > > - Or we can drop the call to `BUG()` altogether. > > Out of those two I think I slightly lean towards the latter, mostly > because the resulting code structure is simpler. And we'd reliably > segfault anyway if we dereference the pointer, even though we would not > get a clean error message. Not sure whether that really is worth the > hassle though. Your diagnosis is correct: options is never NULL in practice. I'd like to keep that BUG() and move it before dereferencing, just in case some future code change accidentally calls run_hooks_opt() with a NULL options, so we get a clean error and not trigger the compiler check. Will see if I can make the code structure nice and send a patch.