)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1001982,"name":"Ian Thompson","email":"ianst@cadence.com","username":"ianstcdns"},"change_message_id":"27caf0655d458832f0d5bb725022e16c73984ab2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5bae7a98_d68b31c9","updated":"2022-08-21 23:07:03.000000000","message":"Fixed 2 new clang sanitizer warnings from patch 7055.","commit_id":"ea5e0915d66a57c043c180722b7530993a4ad200"},{"author":{"_account_id":1001982,"name":"Ian Thompson","email":"ianst@cadence.com","username":"ianstcdns"},"change_message_id":"6434f746a2de3937fa444c618618a76c992ab6e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"bda89cfa_be507455","updated":"2022-08-21 23:12:21.000000000","message":"Sending in one byte on command-line should work.  I believe the issue is that the big-endian swap needs the other 3 bytes in the word to be initialized, even though they won\u0027t be used.  I\u0027ve addressed both issues with a memset() of the working buffer to zero.  @erhan, please let me know if you see any problems with this strategy.  Thank you both!\n\n\nFrom Antonio via email:\n\nxtensa_cmd_exe()\nxtensa_cmd_exe_do()\n  here one byte 0xff is put in ops[], while set oplen\u003d1 xtensa_queue_exec_ins_wide(xtensa, ops, oplen);\n  503: uint8_t oplenw \u003d (oplen + 3) / 4;\n  now oplenw\u003d1, but we don\u0027t have one word, we only have one byte. Then in\n  505: buf_bswap32((uint8_t *)opsw, ops, oplenw * 4);\n  we try to swap 4 bytes, but three of them are uninitialized. Warning!\n\nI prefer you to fix the issue; I don\u0027t know if the command line must have multiple of 4 bytes or even 1 byte is ok.\n","commit_id":"ea5e0915d66a57c043c180722b7530993a4ad200"},{"author":{"_account_id":1001982,"name":"Ian Thompson","email":"ianst@cadence.com","username":"ianstcdns"},"change_message_id":"fdb1a4342784066000cc3cb8542482fe71b89049","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a9a6d185_81930f1b","updated":"2022-08-22 15:18:59.000000000","message":"Thanks, Antonio.  Will fix the endian issues separately.","commit_id":"ea5e0915d66a57c043c180722b7530993a4ad200"}],"src/target/xtensa/xtensa.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"1d4f6c0a5c01c6f11a9184b4fa688825895ad334","unresolved":true,"context_lines":[{"line_number":502,"context_line":"\t\tuint32_t opsw[8] \u003d { 0, 0, 0, 0, 0, 0, 0, 0 };\t/* 8 DIRx regs: max width 64B */"},{"line_number":503,"context_line":"\t\tuint8_t oplenw \u003d (oplen + 3) / 4;"},{"line_number":504,"context_line":"\t\tif (xtensa-\u003etarget-\u003eendianness \u003d\u003d TARGET_BIG_ENDIAN)"},{"line_number":505,"context_line":"\t\t\tbuf_bswap32((uint8_t *)opsw, ops, oplenw * 4);"},{"line_number":506,"context_line":"\t\telse"},{"line_number":507,"context_line":"\t\t\tmemcpy(opsw, ops, oplen);"},{"line_number":508,"context_line":"\t\tfor (int32_t i \u003d oplenw - 1; i \u003e 0; i--)"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"60ba3ae6_530c4b9e","line":505,"updated":"2022-08-22 12:14:00.000000000","message":"As Andreas pointed out in his email, here there is something weird.\nxtensa_queue_dbg_reg_write() below takes one uint32_t value in host-endianess that xtensa_dm_queue_reg_write() converts in an array for JTAG queue.\nThis conversion should be done with target_buffer_get_u32(), not with buf_bswap32() or memcpy().\nWould you mind revisiting this code an the other places where target-\u003eendianness is checked?","commit_id":"ea5e0915d66a57c043c180722b7530993a4ad200"},{"author":{"_account_id":1001982,"name":"Ian Thompson","email":"ianst@cadence.com","username":"ianstcdns"},"change_message_id":"5a562c003ef2472808e6c0a0a2bb9075cd63e5b0","unresolved":true,"context_lines":[{"line_number":502,"context_line":"\t\tuint32_t opsw[8] \u003d { 0, 0, 0, 0, 0, 0, 0, 0 };\t/* 8 DIRx regs: max width 64B */"},{"line_number":503,"context_line":"\t\tuint8_t oplenw \u003d (oplen + 3) / 4;"},{"line_number":504,"context_line":"\t\tif (xtensa-\u003etarget-\u003eendianness \u003d\u003d TARGET_BIG_ENDIAN)"},{"line_number":505,"context_line":"\t\t\tbuf_bswap32((uint8_t *)opsw, ops, oplenw * 4);"},{"line_number":506,"context_line":"\t\telse"},{"line_number":507,"context_line":"\t\t\tmemcpy(opsw, ops, oplen);"},{"line_number":508,"context_line":"\t\tfor (int32_t i \u003d oplenw - 1; i \u003e 0; i--)"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c4aeb095_74a1e485","line":505,"in_reply_to":"60ba3ae6_530c4b9e","updated":"2022-08-22 13:38:08.000000000","message":"Sure, I can change them, although this week I\u0027m away from my FPGA setup that I use for big-endian testing.  Would it be best to, in order to resolve the clang warnings ASAP, push a fix for this instance now and fix the other endian swapping issues separately, or wait to do both as part of this patch next week?","commit_id":"ea5e0915d66a57c043c180722b7530993a4ad200"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c51e8fdd0406cf4e5183aa04f221b56cf9d441b4","unresolved":true,"context_lines":[{"line_number":502,"context_line":"\t\tuint32_t opsw[8] \u003d { 0, 0, 0, 0, 0, 0, 0, 0 };\t/* 8 DIRx regs: max width 64B */"},{"line_number":503,"context_line":"\t\tuint8_t oplenw \u003d (oplen + 3) / 4;"},{"line_number":504,"context_line":"\t\tif (xtensa-\u003etarget-\u003eendianness \u003d\u003d TARGET_BIG_ENDIAN)"},{"line_number":505,"context_line":"\t\t\tbuf_bswap32((uint8_t *)opsw, ops, oplenw * 4);"},{"line_number":506,"context_line":"\t\telse"},{"line_number":507,"context_line":"\t\t\tmemcpy(opsw, ops, oplen);"},{"line_number":508,"context_line":"\t\tfor (int32_t i \u003d oplenw - 1; i \u003e 0; i--)"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c88ec4c7_40ae9b04","line":505,"in_reply_to":"c4aeb095_74a1e485","updated":"2022-08-22 13:57:41.000000000","message":"I think it can be fixed later, thanks.","commit_id":"ea5e0915d66a57c043c180722b7530993a4ad200"},{"author":{"_account_id":1001982,"name":"Ian Thompson","email":"ianst@cadence.com","username":"ianstcdns"},"change_message_id":"fdb1a4342784066000cc3cb8542482fe71b89049","unresolved":false,"context_lines":[{"line_number":502,"context_line":"\t\tuint32_t opsw[8] \u003d { 0, 0, 0, 0, 0, 0, 0, 0 };\t/* 8 DIRx regs: max width 64B */"},{"line_number":503,"context_line":"\t\tuint8_t oplenw \u003d (oplen + 3) / 4;"},{"line_number":504,"context_line":"\t\tif (xtensa-\u003etarget-\u003eendianness \u003d\u003d TARGET_BIG_ENDIAN)"},{"line_number":505,"context_line":"\t\t\tbuf_bswap32((uint8_t *)opsw, ops, oplenw * 4);"},{"line_number":506,"context_line":"\t\telse"},{"line_number":507,"context_line":"\t\t\tmemcpy(opsw, ops, oplen);"},{"line_number":508,"context_line":"\t\tfor (int32_t i \u003d oplenw - 1; i \u003e 0; i--)"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"9ebd12e6_840b6945","line":505,"in_reply_to":"c88ec4c7_40ae9b04","updated":"2022-08-22 15:18:59.000000000","message":"Sounds good.  I\u0027ve made a note of this and will fix these with target_buffer_get_u32() next week.","commit_id":"ea5e0915d66a57c043c180722b7530993a4ad200"},{"author":{"_account_id":1001964,"name":"Erhan Kurubas","display_name":"Erhan Kurubas","email":"erhan.kurubas@espressif.com","username":"erhankur"},"change_message_id":"dfb5b832b12c484e3a2386e6203ee63493f9bb71","unresolved":true,"context_lines":[{"line_number":2608,"context_line":"\t\tgoto xtensa_gdbqc_qxtreg_fail;"},{"line_number":2609,"context_line":"\t}"},{"line_number":2610,"context_line":"\tuint8_t regbuf[XT_QUERYPKT_RESP_MAX];"},{"line_number":2611,"context_line":"\tmemset(regbuf, 0, XT_QUERYPKT_RESP_MAX);"},{"line_number":2612,"context_line":"\tLOG_DEBUG(\"TIE reg 0x%08\" PRIx32 \" %s (%d bytes)\", regnum, iswrite ? \"write\" : \"read\", reglen);"},{"line_number":2613,"context_line":"\tif (reglen * 2 + 1 \u003e XT_QUERYPKT_RESP_MAX) {"},{"line_number":2614,"context_line":"\t\tLOG_ERROR(\"TIE register too large\");"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"680be532_b3bc6326","line":2611,"updated":"2022-08-22 07:49:08.000000000","message":"@Antonio, by convention, do we prefer zero init list or memset to clear the local arrays? \n\nuint8_t regbuf[XT_QUERYPKT_RESP_MAX] \u003d {0}; is OK?","commit_id":"ea5e0915d66a57c043c180722b7530993a4ad200"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c51e8fdd0406cf4e5183aa04f221b56cf9d441b4","unresolved":false,"context_lines":[{"line_number":2608,"context_line":"\t\tgoto xtensa_gdbqc_qxtreg_fail;"},{"line_number":2609,"context_line":"\t}"},{"line_number":2610,"context_line":"\tuint8_t regbuf[XT_QUERYPKT_RESP_MAX];"},{"line_number":2611,"context_line":"\tmemset(regbuf, 0, XT_QUERYPKT_RESP_MAX);"},{"line_number":2612,"context_line":"\tLOG_DEBUG(\"TIE reg 0x%08\" PRIx32 \" %s (%d bytes)\", regnum, iswrite ? \"write\" : \"read\", reglen);"},{"line_number":2613,"context_line":"\tif (reglen * 2 + 1 \u003e XT_QUERYPKT_RESP_MAX) {"},{"line_number":2614,"context_line":"\t\tLOG_ERROR(\"TIE register too large\");"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"0da5880f_1f68d7fd","line":2611,"in_reply_to":"57aae12c_9be4ceab","updated":"2022-08-22 13:57:41.000000000","message":"Strange, my clang 13.0.1 does not highlight this.\n\nThe warnings about helper/types.h are fixed by the second part of this patch (line 2994).\nclang only shows the line (in helper/types.h) that uses the uninitialized data, but such data are coming from this code.","commit_id":"ea5e0915d66a57c043c180722b7530993a4ad200"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"1d4f6c0a5c01c6f11a9184b4fa688825895ad334","unresolved":true,"context_lines":[{"line_number":2608,"context_line":"\t\tgoto xtensa_gdbqc_qxtreg_fail;"},{"line_number":2609,"context_line":"\t}"},{"line_number":2610,"context_line":"\tuint8_t regbuf[XT_QUERYPKT_RESP_MAX];"},{"line_number":2611,"context_line":"\tmemset(regbuf, 0, XT_QUERYPKT_RESP_MAX);"},{"line_number":2612,"context_line":"\tLOG_DEBUG(\"TIE reg 0x%08\" PRIx32 \" %s (%d bytes)\", regnum, iswrite ? \"write\" : \"read\", reglen);"},{"line_number":2613,"context_line":"\tif (reglen * 2 + 1 \u003e XT_QUERYPKT_RESP_MAX) {"},{"line_number":2614,"context_line":"\t\tLOG_ERROR(\"TIE register too large\");"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"7d80eb76_7e4948e2","line":2611,"in_reply_to":"680be532_b3bc6326","updated":"2022-08-22 12:14:00.000000000","message":"It\u0027s ok for me. But I didn\u0027t get this from clang... Is this another issue?","commit_id":"ea5e0915d66a57c043c180722b7530993a4ad200"},{"author":{"_account_id":1001982,"name":"Ian Thompson","email":"ianst@cadence.com","username":"ianstcdns"},"change_message_id":"5a562c003ef2472808e6c0a0a2bb9075cd63e5b0","unresolved":true,"context_lines":[{"line_number":2608,"context_line":"\t\tgoto xtensa_gdbqc_qxtreg_fail;"},{"line_number":2609,"context_line":"\t}"},{"line_number":2610,"context_line":"\tuint8_t regbuf[XT_QUERYPKT_RESP_MAX];"},{"line_number":2611,"context_line":"\tmemset(regbuf, 0, XT_QUERYPKT_RESP_MAX);"},{"line_number":2612,"context_line":"\tLOG_DEBUG(\"TIE reg 0x%08\" PRIx32 \" %s (%d bytes)\", regnum, iswrite ? \"write\" : \"read\", reglen);"},{"line_number":2613,"context_line":"\tif (reglen * 2 + 1 \u003e XT_QUERYPKT_RESP_MAX) {"},{"line_number":2614,"context_line":"\t\tLOG_ERROR(\"TIE register too large\");"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"57aae12c_9be4ceab","line":2611,"in_reply_to":"7d80eb76_7e4948e2","updated":"2022-08-22 13:38:08.000000000","message":"This change resolves the following warning from (clang-8-based-) scan-build:\n\n../src/target/xtensa/xtensa.c:2692:5: warning: 3rd function call argument is an uninitialized value\n                                sprintf(*response_p + 2 * i, \"%02x\", regbuf[i]);\n                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\nIs my scan-build configuration somehow different to bring up this warning, or could it have just been lost in a large output log?\n\nSeparately, there are also a couple of unrelated warnings about helper/types.h that I ignored, as they don\u0027t appear to be specific to the xtensa files.  I didn\u0027t want to address them as part of this patch, but I\u0027m assuming they\u0027re legitimate warnings... have those been coming up for your output as well?","commit_id":"ea5e0915d66a57c043c180722b7530993a4ad200"}]}
