Thank you Erhan for submitting the fixes, and for Antonio for not immediately backing out these patches. It sounds like both Erhan and I are able to build now, him with gcc12 and me with gcc10.
Going forward, what parts of the review flow did I miss so I can avoid causing this trouble in the future? I should definitely have rerun static analysis on a more recent patchset of 7055. Is upgrading my GCC required also? Apologies for the trouble and thank you again @Erhan Kurubas<mailto:erhan.kuru...@espressif.com> for the very timely fixes. Best, --ian From: Antonio Borneo <borneo.anto...@gmail.com> Sent: Saturday, August 20, 2022 12:27 PM To: Ian Thompson <ia...@cadence.com>; Erhan Kurubas <erhan.kuru...@espressif.com> Cc: OpenOCD <OpenOCD-devel@lists.sourceforge.net> Subject: Re: Build error: target: add generic Xtensa LX support EXTERNAL MAIL Plus 5 new clang warnings https://build.openocd.org/job/openocd-clang/1106/<https://urldefense.com/v3/__https:/build.openocd.org/job/openocd-clang/1106/__;!!EHscmS1ygiU1lA!AyCLvbIRr3KNYJM1gTPOMzF8ZJHOrJ13vSfhWvBCDOzayhMXI9sfgdJFpzA2jhiRKacnt3fNowSa2BjkgJZyHQ$> Actually 3 are surely from xtensa code. The other 2 are not clear. Antonio On Sat, Aug 20, 2022, 19:52 Antonio Borneo <borneo.anto...@gmail.com<mailto:borneo.anto...@gmail.com>> wrote: I have made the mistake to merge the patch https://review.openocd.org/c/openocd/+/7055<https://urldefense.com/v3/__https:/review.openocd.org/c/openocd/*/7055__;Kw!!EHscmS1ygiU1lA!AyCLvbIRr3KNYJM1gTPOMzF8ZJHOrJ13vSfhWvBCDOzayhMXI9sfgdJFpzA2jhiRKacnt3fNowSa2BiwPmqxOQ$> without pre-building it locally on my PC for test, which is what I usually do (but not today!). BOOM. Master branch does not compile anymore I tried to fix it, but I ended up with one issue! Could you please investigate and provide a patch asap? For the moment I prefer not reverting this and 7083. Analysis below. Antonio Compile errors with GCC 12.1.0 src/target/xtensa/xtensa.c: In function 'xtensa_target_init': src/target/xtensa/xtensa.c:2471:65: error: '%04x' directive writing between 4 and 8 bytes into a region of size 5 [-Werror=format-overflow=] 2471 | sprintf((char *)xtensa->empty_regs[i].name, "?0x%04x", i); | ^~~~ In function 'xtensa_build_reg_cache', inlined from 'xtensa_target_init' at src/target/xtensa/xtensa.c:2899:9: src/target/xtensa/xtensa.c:2471:61: note: directive argument in the range [0, 4294967294] 2471 | sprintf((char *)xtensa->empty_regs[i].name, "?0x%04x", i); | ^~~~~~~~~ src/target/xtensa/xtensa.c:2471:17: note: 'sprintf' output between 8 and 12 bytes into a destination of size 8 2471 | sprintf((char *)xtensa->empty_regs[i].name, "?0x%04x", i); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This can be easily fixed adding a mask - sprintf((char *)xtensa->empty_regs[i].name, "?0x%04x", i); + sprintf((char *)xtensa->empty_regs[i].name, "?0x%04x", i & 0x0000ffff); Still GCC 12.1.0 In file included from src/target/xtensa/xtensa_chip.h:12, from src/target/xtensa/xtensa.c:20: In function 'xtensa_queue_dbg_reg_write', inlined from 'xtensa_write_dirty_registers' at src/target/xtensa/xtensa.c:758:3: src/target/xtensa/xtensa.h:297:16: error: 'a3' may be used uninitialized [-Werror=maybe-uninitialized] 297 | return dm->dbg_ops->queue_reg_write(dm, reg, data); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/target/xtensa/xtensa.c: In function 'xtensa_write_dirty_registers': src/target/xtensa/xtensa.c:594:26: note: 'a3' was declared here 594 | xtensa_reg_val_t a3, woe; | ^~ Here looks that GCC is dumb and fails to detect that 'preserve_a3' is not changed. Initializing a3=0 is a good workaround - xtensa_reg_val_t a3, woe; + xtensa_reg_val_t a3 = 0, woe; Then, on MacOS CLANG, looks like enum are considered as unsigned int (which surprises me, while GCC consider them as signed) src/target/xtensa/xtensa.c:2850:51: error: comparison of unsigned enum expression >= 0 is always true [-Werror,-Wtautological-compare] for (enum xtensa_ar_scratch_set_e f = s - 1; s >= 0; s--) ~ ^ ~ But here the code is broken! 2850: for (enum xtensa_ar_scratch_set_e f = s - 1; s >= 0; s--) 2851: free(xtensa->scratch_ars[f].chrval); the variable f does not change in the loop, so you free the same area 's' times. Here valgrind should scream loudly. You report valgrind-clean in the commit message; probably the execution flow never passed through this code. Is fix below ok? - for (enum xtensa_ar_scratch_set_e f = s - 1; s >= 0; s--) + for (enum xtensa_ar_scratch_set_e f = 0; f < s; f++)
