On 2/11/20 5:29 PM, Tom Rini wrote:
On Tue, Feb 11, 2020 at 05:15:39PM -0700, Stephen Warren wrote:
On 2/11/20 3:06 PM, Tom Rini wrote:
On Tue, Feb 11, 2020 at 03:02:12PM -0700, Stephen Warren wrote:
On 2/11/20 11:27 AM, Tom Rini wrote:
On Tue, Feb 11, 2020 at 11:20:36AM -0700, Simon Glass wrote:
Hi Tom,

On Sat, 8 Feb 2020 at 07:51, Simon Glass <[email protected]> wrote:

Hi Stephen,

On Thu, 6 Feb 2020 at 15:38, Simon Glass <[email protected]> wrote:

Hi Stephen,

On Thu, 6 Feb 2020 at 15:32, Stephen Warren <[email protected]> wrote:

On 2/6/20 2:55 PM, Simon Glass wrote:
Hi Tom,

This cannot be pulled yet since we need to update gitlab's docker
image to include SDL2. But gitlab seems to be having various problems
this week and today i won't work at all: ...

I see the following build error in u-boot-dm/master via Jenkins:

drivers/misc/p2sb_emul.c: In function ‘sandbox_p2sb_emul_map_physmem’:
drivers/misc/p2sb_emul.c:237:6: warning: ‘child’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
     ret = axi_read(child, offset, priv->regs, AXI_SIZE_32);
         ^
     CC      common

Hmmm that's odd. I don't see that, but there are so many device-tree
and libfdt warnings at present I may have missed it. Will take a look.

That doesn't happen with my gcc 7.3 toolchain. I'll send a patch to
set child to NULL at the start, but a look at the code shows that it
is correct.

Also this is in master and was not introduced by this series.

A fix for this is going in via the x86 tree.

Tom, is there anything else you need from me for this pull request?

Testing it now, thanks.

Now that this has been merged, the build break has come along with it. Is
there any chance of picking up the fix from the x86 tree quickly?

I thought we agreed that it seemed like a compiler problem for throwing
up a false-positive?  But are you unable to move up to something a bit
newer?  Thanks!

It might be possible to upgrade the compiler yet again. I'd rather not keep
changing compilers all the time, which entails extra disk space etc. It's
trivial to solve this issue (a patch has already been posted). I don't think
forcing everyone to change compilers just because some easily-fixable
warning appears is a good idea.

The code structure in question legitimately warns; it's only by analysis of
additional possibly-inlined functions that happen to be in the same source
file that the compiler could tell if the warning is false. If those called
functions just happened to be implemented in a different source file, the
same warning would be 100% legitimately emitted and probably is even with
newer compilers, and the same fix would need to be applied. I'm not sure I'd
go so far as to call this a compiler bug.

So, we enforce using gcc-6.0 or newer (in reality 6.1, there is no 6.0
release).  That's 4 years old this April.  In CI, we use gcc 7.3 (which
is 2 years old now) and have accepted that if you aren't using at least
that version some platforms may not optimize-for-size sufficiently.  The
flip side of all of this is the problems we keep getting reported and
fixes for using gcc-9.x which we can't easily globally switch to,
unfortunately.

For a one compiler to use everywhere I would suggest the 7.3 that
buildman fetches from kernel.org and has for a long while.

That said, since Bin is the one who was unhappy with the warning-fix to
start with but has since taken a patch for it, I'm not going to overrule
him.  I expect a PR shortly.

Great!

I apologize; I'd actually mis-diagnosed the compile failure I was seeing, so the warning fix isn't strictly necessary to make compilation work; it's just a warning, not a warning-treated-as-error like I thought it was.

What happened was that I started seeing U-Boot compilation failures, and looked back from the end of the log to find the error. I found this warning first, and assumed the failure was due to a warnings-are-errors setting and didn't look any further. The actual issue turned out to be that U-Boot sandbox build has a new dependency on libsdl2, and I didn't have that installed on my Jenkins system. I've now installed it, and the builds pass even with this warning still present.

Fixing the warning is probably a good thing to do in order to reduce noise back to 0, but wasn't my issue.

Reply via email to