Re: pgsql: Change ThisTimeLineID from a global variable to a local variable

Robert Haas <robertmhaas@gmail.com> writes:
> On Sun, Nov 7, 2021 at 12:23 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah.  I've seen some older compilers complain about that code before,
>> but now there are over a dozen buildfarm members warning about it.
>> It's clearly a false positive, since checkPointLoc does get set in
>> the read_backup_label()-failure-return code path.  I think we've
>> seen a few other places where likely-to-be-inlined functions have
>> to initialize their result variables to prevent compiler warnings,
>> so I stuck an initialization step into read_backup_label in hopes
>> of silencing it.

> That makes sense, and sorry for not noticing this discussion sooner.
> But, I'm a bit confused about what's going on here. Why did we start
> getting this complaint only now? AFAICS I didn't change anything about
> how checkPointLoc is handled.

I think it's pretty clearly a compiler bug.  The triggering conditions
are obscure, but they seem to have to do with inlining a function that
is supposed to initialize some variables but skips doing so in some
code paths for which "it shouldn't matter".  You'd think that having
inlined, the compiler could see that all live code paths initialize
the variable ... but sometimes it doesn't see that.

Based on this data point, we can speculate that the number of such
variables matters, too.

Also, as I said, I've seen compilers warn about checkPointLoc before.
prairiedog's admittedly-ancient gcc has been doing so for years.

            regards, tom lane