)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002047,"name":"Evgeniy Naydanov","email":"eugnay@gmail.com","username":"en-sc"},"change_message_id":"f8fe1e67d106c7c0bd963e79f039e582f7539ee6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"de845fac_6efa71ac","updated":"2024-04-09 15:33:05.000000000","message":"Antonio, can you please take a look at this one?\nThis causes use of an undefined value in RISC-V OpenOCD under certain circumstances. I suspect the situation may be similar for other target types.","commit_id":"c75ffa60f4a04f8822bad29cdf100e9c6ea0e7b0"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"da6d73e0eb20f1cd99d8ffc3fa77f3b6dc30e810","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"32725869_1f596148","updated":"2024-04-20 11:47:16.000000000","message":"It seems to me that there is an inconsistency. In the current HW/WP interface. And I don\u0027t think that commit tackles it correctly. Let\u0027s take a look at breakpoints.\n\nThis `breakpoint-\u003enumber` field is presented to the user (as a result of breakpoint listing command). So there is an expectation that target-specific implementation will initialize it properly. Moreover, there are these functions:\n\n```\nwatchpoint_set\nbreakpoint_hw_set\n``` \n\nThat are expected to be called by the low-level implementation to set and initialize this. RISC-V implementation never calls these, so user is presented to an uninitialized values.\n\nIt seems that RISC-V implementation is not the only one affected. For example I don\u0027t see xtensa.c target does not call that.\n\n\nI guess that the affected target should be fixed, eventually (we should fix riscv-openocd fork). However, till this happens I would like to propose the following changes to this MR:\n\n1. Introduce a dedicated value that would represent that number fields is irrelevant for the target (something like #define BP_IDX_IRRELEVANT -1)\n2. After the callloc initilize the number fields of breakpoint descriptor to this value.\n3. Adjust bpreakpoint listing functions and these debug prints.\n\nAntonio, do you have an opinion or objections regarding the above proposal?","commit_id":"c75ffa60f4a04f8822bad29cdf100e9c6ea0e7b0"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"db23d92af6879915496084e4866a0fb4dc9447a5","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"867c521c_e8615000","in_reply_to":"32725869_1f596148","updated":"2024-04-20 11:52:21.000000000","message":"When I write \"so user is presented to an uninitialized values.\" - I mean when a scenario when user lists pending breakpoints using `bp` command. Example:\n\n```\n\u003e bp 0x80000008 4 hw\n[riscv.cpu0] Found 4 triggers\nbreakpoint set at 0x80000008\n\u003e bp                \nHardware breakpoint(IVA): addr\u003d0x80000008, len\u003d0x4, num\u003d1651340654\n```\n\nnum is not initialized.","commit_id":"c75ffa60f4a04f8822bad29cdf100e9c6ea0e7b0"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"de6d1b910834bb958274f02a0622ea21e2a78391","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f746ab92_2f7a126d","in_reply_to":"7bc82f24_342169bf","updated":"2025-09-11 10:38:22.000000000","message":"Hi Anatoly, was there any further discussion on this topic? I like your idea with the default `breakpoint-\u003enumber \u003d BP_IDX_IRRELEVANT` value. Looks like a nice generic solution for all targets. And it can be later followed by setting the field for both riscv and xtensa.\n\nPlease note, for SW breakpoints, it seems we print the original instruction instead of the number field, so they are not relevant here.","commit_id":"c75ffa60f4a04f8822bad29cdf100e9c6ea0e7b0"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"c3f0a55386c808ee403172ce969a13d4a654fbde","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7bc82f24_342169bf","in_reply_to":"867c521c_e8615000","updated":"2024-05-08 08:04:35.000000000","message":"Another thing to note:\n\nIt looks like breakpoint-\u003enumber is not set for SW breakpoints. So maybe printing of unique_id is the right thing","commit_id":"c75ffa60f4a04f8822bad29cdf100e9c6ea0e7b0"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"0634d1bce82e2d000c427eadd2198a2d6d6c7f65","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"dc9d4e7e_1c1c9e77","in_reply_to":"a9c52306_443240cd","updated":"2025-10-18 08:33:04.000000000","message":"Agree with Tomas, we could simply initialize it to zero to fix the uninitialized issue, but adding at the same time a comment `/* FIXME: ... */` beside.\nReal fix should follow.\n\nAt least `src/target/mips_mips64.c` seams using it incorrectly as it never set it but use it to remove the BP.\nTo highlight such cases, we should drop `breakpoint_hw_set` and inline its code in the caller points. This would make more clear which target initializes it and in which cases.\n\nProbably renaming it as `hw_index` to match the current use cases.","commit_id":"c75ffa60f4a04f8822bad29cdf100e9c6ea0e7b0"},{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"de59dc811bf929edb69e0ee2671f7b517c03a261","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"a9c52306_443240cd","in_reply_to":"d83979f3_ad53a84a","updated":"2025-10-17 18:40:46.000000000","message":"Hi Samuel and Tomas!\n\nSorry for taking long time to react. \n\n\u003e Hi Anatoly, was there any further discussion on this topic? \n\nWell actually there was no subsequent discussion :( .\n\nI posted this proposal in a hope to get some response contributors (I tagged only Antonio, though). So if you guys think that this is reasonable suggestion I can go ahead and try to implement it (if this was not addressed already)","commit_id":"c75ffa60f4a04f8822bad29cdf100e9c6ea0e7b0"},{"author":{"_account_id":1000687,"name":"Tomas Vanek","display_name":"Tomas Vanek","email":"vanekt@fbl.cz","username":"vanekt"},"change_message_id":"7ca053bf1ebddcb21a066d7832b35efbf3e21619","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"d83979f3_ad53a84a","in_reply_to":"f746ab92_2f7a126d","updated":"2025-10-13 10:07:52.000000000","message":"Yes, seems reasonable, just the identifier should better reference to `(hw) number`, like BPWP_HW_NUM_IRRELEVANT. However the initialisation to 0 could be sufficient for the time until we fix the targets which do not call `breakpoint_hw_set` and `watchpoint_set`","commit_id":"c75ffa60f4a04f8822bad29cdf100e9c6ea0e7b0"}],"src/target/breakpoints.c":[{"author":{"_account_id":1002161,"name":"Anatoly P","email":"kupokupokupopo@gmail.com","username":"ecco_the_dolphin"},"change_message_id":"565badc1f6f8667fe60ccd6f4e677a4131fe7938","unresolved":true,"context_lines":[{"line_number":60,"context_line":"\t\tbreakpoint \u003d breakpoint-\u003enext;"},{"line_number":61,"context_line":"\t}"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"\t(*breakpoint_p) \u003d calloc(1, sizeof(struct breakpoint));"},{"line_number":64,"context_line":"\t(*breakpoint_p)-\u003eaddress \u003d address;"},{"line_number":65,"context_line":"\t(*breakpoint_p)-\u003easid \u003d 0;"},{"line_number":66,"context_line":"\t(*breakpoint_p)-\u003elength \u003d length;"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"42f36e1a_0644bea4","line":63,"updated":"2024-05-08 05:06:03.000000000","message":"In addition, this is not the only place where breakpoint descriptor is allocated. I believe you should adjust all such places.","commit_id":"c75ffa60f4a04f8822bad29cdf100e9c6ea0e7b0"}]}
