)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002143,"name":"Marek Vrbka","email":"marek.vrbka@codasip.com","username":"MarekVCodasip"},"change_message_id":"33396c5c9fe3ed5fff9e849027eb5e692a3cd890","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"21e7c22f_b1890aae","updated":"2023-08-29 05:34:41.000000000","message":"I like the change, but there is one think that I think needs to be addressed.","commit_id":"25e4a4a665d7e88ffde3428fbba004de0682b58a"},{"author":{"_account_id":1001242,"name":"Tim Newsome","email":"tim@sifive.com","username":"timsifive"},"change_message_id":"6668bbf2ff17ed65de19fd1fec09897def02c3e1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"dbb0168d_a90a06a9","updated":"2023-08-28 16:57:34.000000000","message":"This is great. I didn\u0027t realize gdb supports this. That\u0027s much better than the existing behavior.","commit_id":"25e4a4a665d7e88ffde3428fbba004de0682b58a"},{"author":{"_account_id":1002143,"name":"Marek Vrbka","email":"marek.vrbka@codasip.com","username":"MarekVCodasip"},"change_message_id":"5402f4502c23187366c4b529c1e086e7684e7a83","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b5fe0851_122a418e","in_reply_to":"21e7c22f_b1890aae","updated":"2023-08-29 05:52:50.000000000","message":"thing*","commit_id":"25e4a4a665d7e88ffde3428fbba004de0682b58a"}],"src/server/gdb_server.c":[{"author":{"_account_id":1002143,"name":"Marek Vrbka","email":"marek.vrbka@codasip.com","username":"MarekVCodasip"},"change_message_id":"33396c5c9fe3ed5fff9e849027eb5e692a3cd890","unresolved":true,"context_lines":[{"line_number":1220,"context_line":"\t\t\tgdb_str_to_target(target, tstr, reg);"},{"line_number":1221,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1222,"context_line":"\t\tcase ERROR_TARGET_RESOURCE_NOT_AVAILABLE:"},{"line_number":1223,"context_line":"\t\t\tmemset(tstr, \u0027x\u0027, DIV_ROUND_UP(reg-\u003esize, 8) * 2);"},{"line_number":1224,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1225,"context_line":"\t}"},{"line_number":1226,"context_line":"\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"6e661a7a_8424b9fa","line":1223,"updated":"2023-08-29 05:34:41.000000000","message":"I think the null termination string is missing here.","commit_id":"25e4a4a665d7e88ffde3428fbba004de0682b58a"},{"author":{"_account_id":1002143,"name":"Marek Vrbka","email":"marek.vrbka@codasip.com","username":"MarekVCodasip"},"change_message_id":"ee0a8262286477f07209ec3a6e403b3f6158a09e","unresolved":false,"context_lines":[{"line_number":1220,"context_line":"\t\t\tgdb_str_to_target(target, tstr, reg);"},{"line_number":1221,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1222,"context_line":"\t\tcase ERROR_TARGET_RESOURCE_NOT_AVAILABLE:"},{"line_number":1223,"context_line":"\t\t\tmemset(tstr, \u0027x\u0027, DIV_ROUND_UP(reg-\u003esize, 8) * 2);"},{"line_number":1224,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1225,"context_line":"\t}"},{"line_number":1226,"context_line":"\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"ff165c44_c23808bd","line":1223,"in_reply_to":"0da18da4_9f1305d3","updated":"2023-08-31 12:03:38.000000000","message":"Thanks","commit_id":"25e4a4a665d7e88ffde3428fbba004de0682b58a"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"83c291da56c202617c787b94e579694cfdee9312","unresolved":true,"context_lines":[{"line_number":1220,"context_line":"\t\t\tgdb_str_to_target(target, tstr, reg);"},{"line_number":1221,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1222,"context_line":"\t\tcase ERROR_TARGET_RESOURCE_NOT_AVAILABLE:"},{"line_number":1223,"context_line":"\t\t\tmemset(tstr, \u0027x\u0027, DIV_ROUND_UP(reg-\u003esize, 8) * 2);"},{"line_number":1224,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1225,"context_line":"\t}"},{"line_number":1226,"context_line":"\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"805c2194_74b68426","line":1223,"in_reply_to":"6e661a7a_8424b9fa","updated":"2023-08-29 14:45:59.000000000","message":"Great catch! The issue does not cause an error, since the string is processed via `gdb_put_packet()` and `strndup()`, which both take into account the size of the buffer, however, the absence of \u0027\\0\u0027 complicates debugging and makes the code less secure. Addressed.","commit_id":"25e4a4a665d7e88ffde3428fbba004de0682b58a"},{"author":{"_account_id":1002143,"name":"Marek Vrbka","email":"marek.vrbka@codasip.com","username":"MarekVCodasip"},"change_message_id":"69012fc5dfafb381a342bc8784d10e3204aa9059","unresolved":true,"context_lines":[{"line_number":1220,"context_line":"\t\t\tgdb_str_to_target(target, tstr, reg);"},{"line_number":1221,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1222,"context_line":"\t\tcase ERROR_TARGET_RESOURCE_NOT_AVAILABLE:"},{"line_number":1223,"context_line":"\t\t\tmemset(tstr, \u0027x\u0027, DIV_ROUND_UP(reg-\u003esize, 8) * 2);"},{"line_number":1224,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1225,"context_line":"\t}"},{"line_number":1226,"context_line":"\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"a9ae610e_9f510c6e","line":1223,"in_reply_to":"805c2194_74b68426","updated":"2023-08-30 06:26:59.000000000","message":"I am a ambivalent on this solution. It was addressed, but I feel like the terminator should be added right after the memset. As gdb_str_to_target returns a terminated string and the function name (gdb_get_reg_value_as_str) suggests it returns a valid string.","commit_id":"25e4a4a665d7e88ffde3428fbba004de0682b58a"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"c7efcdffc5816e912203b7a7bf309f37820640e3","unresolved":true,"context_lines":[{"line_number":1220,"context_line":"\t\t\tgdb_str_to_target(target, tstr, reg);"},{"line_number":1221,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1222,"context_line":"\t\tcase ERROR_TARGET_RESOURCE_NOT_AVAILABLE:"},{"line_number":1223,"context_line":"\t\t\tmemset(tstr, \u0027x\u0027, DIV_ROUND_UP(reg-\u003esize, 8) * 2);"},{"line_number":1224,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1225,"context_line":"\t}"},{"line_number":1226,"context_line":"\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"0da18da4_9f1305d3","line":1223,"in_reply_to":"a9ae610e_9f510c6e","updated":"2023-08-31 07:49:55.000000000","message":"Addressed.","commit_id":"25e4a4a665d7e88ffde3428fbba004de0682b58a"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"7767fe2dd099d09691d4a01e7c827e8f7e6cad38","unresolved":true,"context_lines":[{"line_number":1197,"context_line":"\tint i;"},{"line_number":1198,"context_line":"\tfor (i \u003d 0; i \u003c str_len; i +\u003d 2) {"},{"line_number":1199,"context_line":"\t\tunsigned t;"},{"line_number":1200,"context_line":"\t\tif (sscanf(tstr + i, \"%02x\", \u0026t) !\u003d 1) {"},{"line_number":1201,"context_line":"\t\t\tLOG_ERROR(\"BUG: unable to convert register value\");"},{"line_number":1202,"context_line":"\t\t\texit(-1);"},{"line_number":1203,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"c6f1a91f_8970f88f","line":1200,"updated":"2023-08-29 15:49:49.000000000","message":"Interesting...\nIf I understand correctly from gdb documentation, also register write \u0027G\u0027 and \u0027P\u0027 can use the \u0027xx\u0027 format.\nToday OpenOCD doesn\u0027t handle it. Command \u0027G\u0027 and \u0027P\u0027 use this function for the conversion, but \u0027xx\u0027 here causes a LOG_ERROR() and an exit(-1)!!!\nAlso this should be fixed, but as a separate patch.\n\nI\u0027m guessing how to force gdb to send such packets. Any idea?","commit_id":"f95cb28766ea66e1b286147a06e70f7650cf5485"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"1a84c8e77a378deaea50051cf6f1c8350b94da61","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"\tint i;"},{"line_number":1198,"context_line":"\tfor (i \u003d 0; i \u003c str_len; i +\u003d 2) {"},{"line_number":1199,"context_line":"\t\tunsigned t;"},{"line_number":1200,"context_line":"\t\tif (sscanf(tstr + i, \"%02x\", \u0026t) !\u003d 1) {"},{"line_number":1201,"context_line":"\t\t\tLOG_ERROR(\"BUG: unable to convert register value\");"},{"line_number":1202,"context_line":"\t\t\texit(-1);"},{"line_number":1203,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"d91fea97_14f56b93","line":1200,"in_reply_to":"18b8fabc_4a2a60e9","updated":"2023-08-31 20:11:24.000000000","message":"@Antonio Borneo.\n\nRegarding this one:\n\n\u003e Humm... apparently \u0027G\u0027 and \u0027P\u0027 do not implement this in GDB 13.2, nor in later master branch.\n\nActually it does. This patch is about `g` and `p` packet (not `G` and `P`, these are for setting values - just in case). In fact we\u0027ve tested these changes with gdb 13.2 and gdb 12.1.9 . We used a patched version of riscv-openocd (that returns ERROR_TARGET_RESOURCE_NOT_AVAILABLE when FPU is turned off) and spike simulator. I can share a patch if you are interested (the patch is trivial). \n\nAs for the code in GDB  - see `process_g_packet` code for example (there is processing of `x` values there). And this code is old, like 10+ years old if I\u0027m not mistaken.","commit_id":"f95cb28766ea66e1b286147a06e70f7650cf5485"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"d0e71cfe77ab3229d09a9241b79674f23a7918f9","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"\tint i;"},{"line_number":1198,"context_line":"\tfor (i \u003d 0; i \u003c str_len; i +\u003d 2) {"},{"line_number":1199,"context_line":"\t\tunsigned t;"},{"line_number":1200,"context_line":"\t\tif (sscanf(tstr + i, \"%02x\", \u0026t) !\u003d 1) {"},{"line_number":1201,"context_line":"\t\t\tLOG_ERROR(\"BUG: unable to convert register value\");"},{"line_number":1202,"context_line":"\t\t\texit(-1);"},{"line_number":1203,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"0422c357_a643eb8a","line":1200,"in_reply_to":"729e72d1_4149a1b3","updated":"2023-09-01 13:16:51.000000000","message":"If I\u0027m correct there is a misunderstanding:\nFrom the beginning of this thread, it was only about \u0027G\u0027 and \u0027P\u0027 packets and was initiated to clarify how to force GDB to send those packets with register value set to \u0027xx\u0027. And, as was clarified by @Antonio Borneo, for now there is no way to do so, at least in GDB.","commit_id":"f95cb28766ea66e1b286147a06e70f7650cf5485"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c4af2ba32f46ee7f08e88c25510b8c14bfc0223c","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"\tint i;"},{"line_number":1198,"context_line":"\tfor (i \u003d 0; i \u003c str_len; i +\u003d 2) {"},{"line_number":1199,"context_line":"\t\tunsigned t;"},{"line_number":1200,"context_line":"\t\tif (sscanf(tstr + i, \"%02x\", \u0026t) !\u003d 1) {"},{"line_number":1201,"context_line":"\t\t\tLOG_ERROR(\"BUG: unable to convert register value\");"},{"line_number":1202,"context_line":"\t\t\texit(-1);"},{"line_number":1203,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"18b8fabc_4a2a60e9","line":1200,"in_reply_to":"c6f1a91f_8970f88f","updated":"2023-08-29 22:05:19.000000000","message":"Humm... apparently \u0027G\u0027 and \u0027P\u0027 do not implement this in GDB 13.2, nor in later master branch.\nThe function store_registers_using_G simply concatenates the binary content of the registers and converts the resulting buffer in hex.\nhttps://sourceware.org/git/?p\u003dbinutils-gdb.git;a\u003dblob;f\u003dgdb/remote.c;hb\u003d662243de0e14#l8724\nNo special handling for unavailable registers, so no need to change anything in OpenOCD until we get a debugger that needs it.\nTo be checked on lldb too!","commit_id":"f95cb28766ea66e1b286147a06e70f7650cf5485"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"0df154cfcb37f5e93c1e38c8ad5bf085cf4e63c9","unresolved":false,"context_lines":[{"line_number":1197,"context_line":"\tint i;"},{"line_number":1198,"context_line":"\tfor (i \u003d 0; i \u003c str_len; i +\u003d 2) {"},{"line_number":1199,"context_line":"\t\tunsigned t;"},{"line_number":1200,"context_line":"\t\tif (sscanf(tstr + i, \"%02x\", \u0026t) !\u003d 1) {"},{"line_number":1201,"context_line":"\t\t\tLOG_ERROR(\"BUG: unable to convert register value\");"},{"line_number":1202,"context_line":"\t\t\texit(-1);"},{"line_number":1203,"context_line":"\t\t}"}],"source_content_type":"text/x-csrc","patch_set":3,"id":"729e72d1_4149a1b3","line":1200,"in_reply_to":"d91fea97_14f56b93","updated":"2023-08-31 20:24:39.000000000","message":"And for the sake of completeness - just tested on GDB 8.2 . It works there too.","commit_id":"f95cb28766ea66e1b286147a06e70f7650cf5485"}]}
