Re: BUG #19612: SEGV in ParseConfigFp() in guc-file.l

Hi, Michael!

Thanks for review, and for catching the
yylex_init() warning. Sorry, I missed that one.

The non-volatile copy for yylex_init() looks right. That call writes
through a yyscan_t *, so &scanner after the volatile change is exactly
the qualifier discard the compiler reports. Casting the address would
only silence the warning. scanner_init is never read after the
longjmp, so it does not need to be volatile.

I would drop the if (scanner) guard in cleanup.

A flex fatal only comes from GUC_flex_fatal() during yylex(), after
yylex_init() has already stored a live scanner. yylex_init() itself
only allocates the scanner struct. It does not take the fatal path.
Nothing between sigsetjmp() and "scanner = scanner_init" can call
GUC_flex_fatal() either. So on the longjmp path, cleanup always sees
a real scanner.
yy_delete_buffer() already returns immediately when lex_buffer is NULL,
and lex_buffer is already volatile and initialized. yylex_destroy(NULL)
is not a no-op in flex: it dereferences the scanner. A NULL check would
matter if that case were reachable here. It is not.

Let's take your revised version without the cleanup change.

чт, 13 авг. 2026 г. в 13:27, Michael Paquier <michael(at)paquier(dot)xyz>:

> On Tue, Aug 11, 2026 at 11:30:03AM +0500, Andrey Rachitskiy wrote:
> > Commit 4b496a3583e already marked the YY_BUFFER_STATE local volatile
> > for that longjmp path. Commit d663f150b5e made the scanner reentrant
> > and added a yyscan_t local used in the same cleanup. That local is
> > written after sigsetjmp and read after siglongjmp, but was not
> > volatile, so its value is indeterminate after the jump.
>
> Asan failure reproduced, thanks. Your patch has missed the following
> piece with yylex_init():
> guc-file.l:387:17: warning: passing 'volatile yyscan_t *' (aka 'void
> *volatile *') to parameter of type 'yyscan_t *' (aka 'void **')
> discards qualifiers
> [-Wincompatible-pointer-types-discards-qualifiers] 387 | if
> (yylex_init(&scanner) != 0)
>
> I am wondering whether we should just use a non-volatile copy of
> "scanner", just for the sake of yylex_init(). The attached seems to
> work fine here with asan.
>
> Thoughts?
> --
> Michael
>

--
Regards,
Rachitskiy Andrey