Hi! That mechanism is flawed anyway, since it appears to assume that the host is little-endian. The pointer cast is a strong indicator that something is wrong.
/Andreas On Mon, Aug 22, 2022 at 12:32 AM Antonio Borneo <borneo.anto...@gmail.com> wrote: > Thanks for the quick fixes for this build issue. > I have just merged them after some checking on gcc 12 and on MacOS. > > There are still two new scan-build issues not fixed by the patch > https://review.openocd.org/7137 > They are both triggered by command "xtensa exe" on big-endian target > when the command line argument of the command is not a multiple of 4 > bytes, e.g. "xtensa exe ff". > One warning is reported for the command line of one byte, the second > for 3 bytes. The same fix should work for both warnings. > > Trace for "xtensa exe ff": > xtensa_cmd_exe() > xtensa_cmd_exe_do() > here one byte 0xff is put in ops[], while set oplen=1 > xtensa_queue_exec_ins_wide(xtensa, ops, oplen); > 503: uint8_t oplenw = (oplen + 3) / 4; > now oplenw=1, but we don't have one word, we only have one byte. Then in > 505: buf_bswap32((uint8_t *)opsw, ops, oplenw * 4); > we try to swap 4 bytes, but three of them are uninitialized. Warning! > > I prefer you to fix the issue; I don't know if the command line must > have multiple of 4 bytes or even 1 byte is ok. > > Regards > Antonio > > On Sun, Aug 21, 2022 at 7:10 AM Ian Thompson <ia...@cadence.com> wrote: > > > > 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 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/ > > > > 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++) > >
