Plus 5 new clang warnings https://build.openocd.org/job/openocd-clang/1106/ 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> wrote: > I have made the mistake to merge the patch > https://review.openocd.org/c/openocd/+/7055 > 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++) >