
Hi Tom,
On Tuesday, 18 July 2017 02:07:59 BST Tom Rini wrote:
On Thu, Jul 13, 2017 at 10:58:47AM -0700, Paul Burton wrote:
Hi Daniel & Tom,
On Thursday, 13 July 2017 03:51:00 PDT Daniel Schwierzeck wrote:
Hi Paul,
2017-07-13 2:33 GMT+02:00 Tom Rini trini@konsulko.com:
On Wed, Jul 12, 2017 at 04:57:42PM -0400, Tom Rini wrote:
On Wed, Jul 12, 2017 at 10:32:29PM +0200, Daniel Schwierzeck wrote:
Hi Tom,
This supports dynamic relocation on MIPS without the need for building a position-independent executable. This notably reduces the code size for all MIPS boards.
The following changes since commit
d85ca029f257b53a96da6c2fb421e78a003a9943:
Prepare v2017.07 (2017-07-10 13:07:38 -0400)
are available in the git repository at: git://git.denx.de/u-boot-mips.git master
for you to fetch changes up to
f653dcd5720c4135607211f7304283d7a8ec3b8a:
MIPS: bootm: Fix broken boot_env_legacy codepath (2017-07-12 22:10:42 +0200)>>
I'm seeing: mips: + tplink_wdr4300
+(tplink_wdr4300) pfx##hdr32[idx].field = _val; \ +(tplink_wdr4300) ^ +(tplink_wdr4300) ../tools/mips-relocs.c:51:11: note: ?_val? was declared here +(tplink_wdr4300) uint64_t _val; \ +(tplink_wdr4300) ^ +(tplink_wdr4300) ../tools/mips-relocs.c:88:2: note: in expansion of macro ?set_hdr_field? +(tplink_wdr4300) set_hdr_field(p, idx, field, val) +(tplink_wdr4300) ^~~~~~~~~~~~~ +(tplink_wdr4300) ../tools/mips-relocs.c:408:3: note: in expansion of macro ?set_phdr_field? +(tplink_wdr4300) set_phdr_field(i, p_filesz, load_sz); +(tplink_wdr4300) ^~~~~~~~~~~~~~ w+(tplink_wdr4300) ../tools/mips-relocs.c: In function ?main?: w+(tplink_wdr4300) ../tools/mips-relocs.c:77:25: warning: ?_val? may be used uninitialized in this function [-Wmaybe-uninitialized]
for what I suspect is going to be all MIPS. Host tools here are gcc-6.3.
Yeah, this is all MIPS boards. Please fix, thanks!
Paul, could you send a follow-up patch to fix this? Thanks.
Sure. I'm on gcc 7.1.1 which doesn't show this issue. Is the following sufficient to fix this for you Tom? I can submit it as a proper patch if you like & it works out.
Oh? That it doesn't show up with a newer compiler is interesting...
Yeah, I imagine gcc got smarter at recognising that the path it was complaining about is never actually taken.
Thanks,
Paul
diff --git a/tools/mips-relocs.c b/tools/mips-relocs.c index b690fa53c4..75d532546b 100644 --- a/tools/mips-relocs.c +++ b/tools/mips-relocs.c @@ -69,6 +69,9 @@
case 8: \ _val = is_be ? htobe64(val) : htole64(val); \ break; \
default: \
__builtin_unreachable(); \
break; \ } \
I'm not a huge fan of adding builtin calls like this. Is there some other way to restructure the code perhaps, while still being clear? Thanks!
An alternative would be to assign _val = 0 to silence the warning, and probably call abort() or assert(0) or something similar in that path. Would that be preferrable to you?
Thanks, Paul