)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"ef0947fa6e4dd6e2c318359e7e42e9c908e5c516","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"97ee5649_a021034b","updated":"2024-05-15 13:41:23.000000000","message":"Thank you for the reply!\nThe change in the code LGTM.\nHowever, I would like to further discuss the topic of `ERROR_TARGET_RESOURCE_NOT_AVAILABLE`, please see the relevant comment.","commit_id":"b833d13491f28561872aca59a497ea65a06656a4"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"2a5fba8ceb8fad1b90286f4a33cd1bec56591792","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cae80442_f48eea52","updated":"2024-05-07 10:18:37.000000000","message":"Thanks for the catch!\nI have a few minor concerns, please take a look.","commit_id":"b833d13491f28561872aca59a497ea65a06656a4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"9b7b97bf1e6a3d61935989b90c723a7dc7a5c9f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b67f2174_4be4804c","updated":"2024-05-15 16:10:47.000000000","message":"Commit message updated","commit_id":"e67f991401c6557df65dad006d0e0b8523a2128d"}],"src/server/gdb_server.c":[{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"2a5fba8ceb8fad1b90286f4a33cd1bec56591792","unresolved":true,"context_lines":[{"line_number":1233,"context_line":"\t\t\ttstr[len] \u003d \u0027\\0\u0027;"},{"line_number":1234,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1235,"context_line":"\t}"},{"line_number":1236,"context_line":"\tmemset(tstr, \u00270\u0027, len);"},{"line_number":1237,"context_line":"\ttstr[len] \u003d \u0027\\0\u0027;"},{"line_number":1238,"context_line":"\treturn ERROR_FAIL;"},{"line_number":1239,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"3a87e2cd_5665e307","line":1236,"updated":"2024-05-07 10:18:37.000000000","message":"AFAIU, this is not the same behavior as the one before 236c54c94a53ff76537a1bf91cfe74a264a2756f. My understanding is, `reg-\u003evalue` used to be reported in such case.\nSince the behavior changes anyway, maybe it would be beneficial to return a string of `x` instead? In such case GDB will not indicate an error (`gdb_report_register_access_error` value is respected). However the user will be presented with an information that getting the register failed, not with some arbitrary value of a register.","commit_id":"b833d13491f28561872aca59a497ea65a06656a4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dcf8499c67375643cf7e476876eea7b0a2e31994","unresolved":true,"context_lines":[{"line_number":1233,"context_line":"\t\t\ttstr[len] \u003d \u0027\\0\u0027;"},{"line_number":1234,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1235,"context_line":"\t}"},{"line_number":1236,"context_line":"\tmemset(tstr, \u00270\u0027, len);"},{"line_number":1237,"context_line":"\ttstr[len] \u003d \u0027\\0\u0027;"},{"line_number":1238,"context_line":"\treturn ERROR_FAIL;"},{"line_number":1239,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"b10ec345_1deedfe0","line":1236,"in_reply_to":"10eb5df1_4ebebdd4","updated":"2024-05-15 15:51:03.000000000","message":"You are right!\nThe register is locally cached as unavailable and GDB stops updating it until it get in a situation where it has to update the cache, either: continue+halt, or register write, or command ``maintenance flush register-cache``, ...\n\nI\u0027m checking better the use case that triggered this patch.\nIn the manual we report how to attach a running target without halting it.\nhttps://review.openocd.org/c/openocd/+/8228/1/doc/openocd.texi#12494\nCurrent code returns error on Cortex-A because it cannot read register with the CPU not halted, so the GDB connection fails\n```\n  [remote] Sending packet: $g#67\n  [remote] Packet received: E0E\n[remote] start_remote_1: exit\nCould not read registers; remote failure reply \u0027E0E\u0027\n```\n\nThis patch fixes the issue.\n\nIn the case above, if I force OpenOCD to provide ``xxxx`` for the register I get another error:\n```\n  [remote] Sending packet: $g#67\n  [remote] Packet received: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n[remote] start_remote_1: exit\nPC register is not available\n```\n\nand again the GDB connection fails.\n\nSo, I now consider ``gdb_report_register_access_error`` as an hack to allow the connection as reported in the manual.\nAnd, as reported, it is the only way to go until we implement the non-stop mode.\n\nMaybe this patch should be revisited, at least I should report the info above.","commit_id":"b833d13491f28561872aca59a497ea65a06656a4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"ba26409087e6b607bc0a12bd7fb4b8281c155f45","unresolved":true,"context_lines":[{"line_number":1233,"context_line":"\t\t\ttstr[len] \u003d \u0027\\0\u0027;"},{"line_number":1234,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1235,"context_line":"\t}"},{"line_number":1236,"context_line":"\tmemset(tstr, \u00270\u0027, len);"},{"line_number":1237,"context_line":"\ttstr[len] \u003d \u0027\\0\u0027;"},{"line_number":1238,"context_line":"\treturn ERROR_FAIL;"},{"line_number":1239,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"958215e2_3629c472","line":1236,"in_reply_to":"3a87e2cd_5665e307","updated":"2024-05-11 08:22:22.000000000","message":"In case of read error, the target\u0027s register read was called because ``!reg-\u003evalid``, so the value in ``reg-\u003evalue`` is garbage. In this case sending zero to GDB is more appropriate than sending garbage.\n\nI send zero here because I have seen that if we report even only once a string of ``x``, then GDB will never again ask for that register. Somehow GDB consider the register as not existing any longer.\nSo here I discriminate between:\n- your registers that return ``ERROR_TARGET_RESOURCE_NOT_AVAILABLE`` and that it means are not available forever,\n- from registers whose read return another error and that could be possibly available later.\n\n``gdb_report_register_access_error`` gets used only in latter case and does not impact your registers that return ``ERROR_TARGET_RESOURCE_NOT_AVAILABLE``.","commit_id":"b833d13491f28561872aca59a497ea65a06656a4"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"ef0947fa6e4dd6e2c318359e7e42e9c908e5c516","unresolved":true,"context_lines":[{"line_number":1233,"context_line":"\t\t\ttstr[len] \u003d \u0027\\0\u0027;"},{"line_number":1234,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1235,"context_line":"\t}"},{"line_number":1236,"context_line":"\tmemset(tstr, \u00270\u0027, len);"},{"line_number":1237,"context_line":"\ttstr[len] \u003d \u0027\\0\u0027;"},{"line_number":1238,"context_line":"\treturn ERROR_FAIL;"},{"line_number":1239,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"10eb5df1_4ebebdd4","line":1236,"in_reply_to":"958215e2_3629c472","updated":"2024-05-15 13:41:23.000000000","message":"\u003e your registers that return ERROR_TARGET_RESOURCE_NOT_AVAILABLE and that it means are not available forever\n\nAFAIU, this is not the behavior that is expected by GDB. Looking at the documentation for `p` packet response (https://sourceware.org/gdb/current/onlinedocs/gdb.html/Packets.html#read-registers-packet):\n\n\u003e When reading registers, the stub may also return a string of literal ‘x’’s in place of the register data digits, to indicate that the corresponding register’s value is unavailable. ... Note that if a register truly does not exist on the target, then it is better to not include it in the target description in the first place.\n\nI think the part\n\u003e ... it is better to not include it in the target description ...\n\nshould be applied if a register may not be reed.\n\n\u003e if we report even only once a string of x, then GDB will never again ask for that register\n\nI\u0027ve experimented with GCC 13.2 and this is not the behavior implemented.\nIn particular:\n\n1. I\u0027ve modified OpenOCD to always return `ERROR_TARGET_RESOURCE_NOT_AVAILABLE` when `v0` (a vector register) is read.\n\n2. I\u0027ve ran the following GDB script:\n```\n\u003e cat unavailable-reg.gdb\nset trace-commands on\nset debug remote on\n\ntarget extended-remote | \\\n\topenocd \\\n\t\t...\n\t\t-c \"gdb_port pipe\"\n\ninfo registers v0\n\ninfo registers v0\n\nset $x1\u003d$x1\n\ninfo registers v0\n\nset $v0.w[0]\u003d0\n\ninfo registers v0\n```\nAnd the result was (iirelevant output ommited):\n```\u003e riscv64-unknown-elf-gdb -x unavailable-reg.gdb --batch\n+set debug remote on\n+target extended-remote | ...\n[remote] start_remote_1: enter\n...\n[remote] start_remote_1: exit\n+info registers v0\nv0             [remote] Sending packet: $p1042#37\n[remote] Packet received: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n[remote] packet_ok: Packet p (fetch-register) is supported\n{b \u003d {\u003cunavailable\u003e \u003crepeats 16 times\u003e}, s \u003d {\u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e}, w \u003d {\u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e}, l \u003d {\u003cunavailable\u003e, \u003cunavailable\u003e}, q \u003d {\u003cunavailable\u003e}}\n+info registers v0\nv0             {b \u003d {\u003cunavailable\u003e \u003crepeats 16 times\u003e}, s \u003d {\u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e}, w \u003d {\u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e}, l \u003d {\u003cunavailable\u003e, \u003cunavailable\u003e}, q \u003d {\u003cunavailable\u003e}}\n```\nHere we can see that the result of `p` packet is cached in GDB.\n```\n+set $x1\u003d$x1\n[remote] Sending packet: $g#67\n[remote] Packet received: ...\n+info registers v0\nv0             [remote] Sending packet: $p1042#37\n[remote] Packet received: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n{b \u003d {\u003cunavailable\u003e \u003crepeats 16 times\u003e}, s \u003d {\u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e}, w \u003d {\u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e, \u003cunavailable\u003e}, l \u003d {\u003cunavailable\u003e, \u003cunavailable\u003e}, q \u003d {\u003cunavailable\u003e}}\n```\nHowever, reading/writing a GPR invalidates the cached result and `v0` value is requested again.\n```\n+set $v0.w[0]\u003d0\n[remote] Sending packet: $P1042\u003d00000000000000000000000000000000#54\n[remote] Packet received: OK\n[remote] packet_ok: Packet P (set-register) is supported\n[remote] Sending packet: $g#67\n[remote] Packet received: ...\n+info registers v0\nv0             [remote] Sending packet: $p1042#37\n[remote] Packet received: 00000000000000000000000000000000\n{b \u003d {0x0 \u003crepeats 16 times\u003e}, s \u003d {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, w \u003d {0x0, 0x0, 0x0, 0x0}, l \u003d {0x0, 0x0}, q \u003d {0x0}}\n[remote] Sending packet: $D#44\n[remote] Packet received: OK\n[Inferior 1 (Remote target) detached]\n```\nSimilarly, writing `v0` itself invalidates the cached result and `v0` value is requested again. (`v0` is no longer \u003cunavailable\u003e, since I have only modified the reads and the write was cached in OpenOCD)","commit_id":"b833d13491f28561872aca59a497ea65a06656a4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c325e986192aa16625b458ba3649607b5f78e498","unresolved":false,"context_lines":[{"line_number":1233,"context_line":"\t\t\ttstr[len] \u003d \u0027\\0\u0027;"},{"line_number":1234,"context_line":"\t\t\treturn ERROR_OK;"},{"line_number":1235,"context_line":"\t}"},{"line_number":1236,"context_line":"\tmemset(tstr, \u00270\u0027, len);"},{"line_number":1237,"context_line":"\ttstr[len] \u003d \u0027\\0\u0027;"},{"line_number":1238,"context_line":"\treturn ERROR_FAIL;"},{"line_number":1239,"context_line":"}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"4d289569_592af892","line":1236,"in_reply_to":"b10ec345_1deedfe0","updated":"2024-06-08 08:38:20.000000000","message":"Commit message updated","commit_id":"b833d13491f28561872aca59a497ea65a06656a4"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"2a5fba8ceb8fad1b90286f4a33cd1bec56591792","unresolved":true,"context_lines":[{"line_number":1281,"context_line":"\t\t\tcontinue;"},{"line_number":1282,"context_line":"\t\tretval \u003d gdb_get_reg_value_as_str(target, reg_packet_p, reg_list[i]);"},{"line_number":1283,"context_line":"\t\tif (retval !\u003d ERROR_OK \u0026\u0026 gdb_report_register_access_error) {"},{"line_number":1284,"context_line":"\t\t\tLOG_DEBUG(\"Couldn\u0027t get register %s.\", reg_list[i]-\u003ename);"},{"line_number":1285,"context_line":"\t\t\tfree(reg_packet);"},{"line_number":1286,"context_line":"\t\t\tfree(reg_list);"},{"line_number":1287,"context_line":"\t\t\treturn gdb_error(connection, retval);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d9d17e98_f208ea5a","line":1284,"updated":"2024-05-07 10:18:37.000000000","message":"Wouldn\u0027t it be better to leave the debug message in case `gdb_report_register_access_error` is `false`? Without the message it will not be possible to distinguish between a valid register value of zero read from cache and an error (at least from `gdb_server.c` logs alone).","commit_id":"b833d13491f28561872aca59a497ea65a06656a4"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"ba26409087e6b607bc0a12bd7fb4b8281c155f45","unresolved":true,"context_lines":[{"line_number":1281,"context_line":"\t\t\tcontinue;"},{"line_number":1282,"context_line":"\t\tretval \u003d gdb_get_reg_value_as_str(target, reg_packet_p, reg_list[i]);"},{"line_number":1283,"context_line":"\t\tif (retval !\u003d ERROR_OK \u0026\u0026 gdb_report_register_access_error) {"},{"line_number":1284,"context_line":"\t\t\tLOG_DEBUG(\"Couldn\u0027t get register %s.\", reg_list[i]-\u003ename);"},{"line_number":1285,"context_line":"\t\t\tfree(reg_packet);"},{"line_number":1286,"context_line":"\t\t\tfree(reg_list);"},{"line_number":1287,"context_line":"\t\t\treturn gdb_error(connection, retval);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"fbd8e04d_8b6a0340","line":1284,"in_reply_to":"d9d17e98_f208ea5a","updated":"2024-05-11 08:22:22.000000000","message":"There was no message before 236c54c94a53, as ``gdb_report_register_access_error \u003d\u003d false`` means don\u0027t trigger any error and just send value ``0`` to GDB.\nThis patch keeps back the old behavior, apart for the registers that return ``ERROR_TARGET_RESOURCE_NOT_AVAILABLE``.","commit_id":"b833d13491f28561872aca59a497ea65a06656a4"},{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"ef0947fa6e4dd6e2c318359e7e42e9c908e5c516","unresolved":false,"context_lines":[{"line_number":1281,"context_line":"\t\t\tcontinue;"},{"line_number":1282,"context_line":"\t\tretval \u003d gdb_get_reg_value_as_str(target, reg_packet_p, reg_list[i]);"},{"line_number":1283,"context_line":"\t\tif (retval !\u003d ERROR_OK \u0026\u0026 gdb_report_register_access_error) {"},{"line_number":1284,"context_line":"\t\t\tLOG_DEBUG(\"Couldn\u0027t get register %s.\", reg_list[i]-\u003ename);"},{"line_number":1285,"context_line":"\t\t\tfree(reg_packet);"},{"line_number":1286,"context_line":"\t\t\tfree(reg_list);"},{"line_number":1287,"context_line":"\t\t\treturn gdb_error(connection, retval);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"ea08e980_54980969","line":1284,"in_reply_to":"fbd8e04d_8b6a0340","updated":"2024-05-15 13:41:23.000000000","message":"Understood. Thank you!","commit_id":"b833d13491f28561872aca59a497ea65a06656a4"}]}