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++)
>


Reply via email to