FYI: Please pull u-boot-dm

[Stephen Warren]

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 <sjg at google.com> wrote:
>>>>>>>
>>>>>>> Hi Stephen,
>>>>>>>
>>>>>>> On Thu, 6 Feb 2020 at 15:38, Simon Glass <sjg at google.com> wrote:
>>>>>>>>
>>>>>>>> Hi Stephen,
>>>>>>>>
>>>>>>>> On Thu, 6 Feb 2020 at 15:32, Stephen Warren <swarren at wwwdotorg.org> 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.