)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"a67e5d1568dc7f7470d718a1b3ab9e9074536f2b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"03315b8b_6dcdec2b","updated":"2023-09-15 17:49:44.000000000","message":"Antonio pointed that the commit message claims to workaround of following problem:\n\"breakpoint temporarily removed in arc_step() will not be reactivated.\"\nSo, he analyzed arc_step() function point that described problem can only happen if arc_step() got an error somewhere in the middle and was not able to reactivate breakpoint. So, he recommend to fix actual problem, not make a workaround.\n\nYour answer \".. others do it too..\" is not something what is expected in this case.","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dcf716161e3747aa9f605c6294535311ee36152a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"71324d22_10951aab","updated":"2023-07-23 14:19:50.000000000","message":"I have doubts on this patch.\nThe code explicitly reactivate the breakpoint that has been removed on handle_breakpoints. Maybe an error occurred during arc_step() and it exits before restoring the breakpoint. I think it would be better providing an exit patch in arc_step() to prevent this case.\nIs there any other case that requires restoring the breakpoint, apart from step?\nWhich is the case that requires restoring a watchpoint?","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1002160,"name":"Artemiy Volkov","display_name":"Artemiy Volkov","email":"artemiy@synopsys.com","username":"artemiy-volkov","status":"Synopsys"},"change_message_id":"c1e209d1564acc414031c57cfdc4f2cbfcc04af2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"819f3fc9_6e8b779a","in_reply_to":"03315b8b_6dcdec2b","updated":"2023-12-01 10:20:42.000000000","message":"Hello,\n\nIn my previous comment I had a question to Antonio about why half of the\ntargets choose to unconditionally enable all BP/WPs on resume() and the other\nhalf doesn\u0027t. (I was not able to find any information on this neither in the\ncode / git history nor elsewhere.) \n\nMy impression now is that this is done for user convenience -- if the user (that\ncommunicates with openocd via telnet, not gdb) deletes some BP/WPs while the\nexecution is halted and then issues the \u0027resume\u0027 command, those would be\nautomatically re-enabled by the code on lines 1275-76. This is the best\nexplanation I currently have for why some targets choose to do this, and if I\u0027m\nright, this is up to the maintainer of the target-specific code to make this\nchoice between the two behaviors. Thus, those calls to\narc_enable_breakpoints() and arc_enable_watchpoints() can be left in place.\n\nThat said, I agree with you that this patch is not a good workaround just for \narc_step(), and I should remove all mentions of it from the commit message. It \nis a completely separate issue that could be addressed by another patch at a \nlater stage.\n\nI have changed the commit message to clarify that the intent of this patch is \nto change the user-visible behavior of resume(), not work around \narc_step(). Could you please let me know if I\u0027m understanding things correctly \nand if this makes sense to you?","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1002160,"name":"Artemiy Volkov","display_name":"Artemiy Volkov","email":"artemiy@synopsys.com","username":"artemiy-volkov","status":"Synopsys"},"change_message_id":"7d34f19f9e7a5d346bc670743889fee07ce05a56","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"208e9ff4_2378bcd9","in_reply_to":"16ee3958_20e1fa1f","updated":"2023-09-11 07:30:57.000000000","message":"Hi Antonio, could you consider giving this patch another look please?","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"6192beb26cfaea1123de8cfa9bdfeb25692eb4c5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5ef76481_3b0345e0","in_reply_to":"208e9ff4_2378bcd9","updated":"2024-01-13 14:39:58.000000000","message":"Done","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1002160,"name":"Artemiy Volkov","display_name":"Artemiy Volkov","email":"artemiy@synopsys.com","username":"artemiy-volkov","status":"Synopsys"},"change_message_id":"1a86eee2940e6f8b0f01796c4927f400569d17df","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"16ee3958_20e1fa1f","in_reply_to":"71324d22_10951aab","updated":"2023-08-04 15:02:32.000000000","message":"Hi Antonio,\n\nI must admit that it is not entirely clear to me whether breakpoints and watchpoints should be restored here, but seeing what some of the other targets (mips_m4k.c:437, xscale.c:1220, stm8.c:1003, arm7_9_common.c:1799) do, I assumed that it\u0027s default openocd behavior to restore breakpoints/watchpoints in absence of an external debugger that could override it. I decided to go this way entirely by analogy with those targets listed above.\n\nIs there something more to it than meets the eye? If this is some legacy code, and if the new implementations do not have to do this, please let me know. I\u0027ll be happy to remove those calls and move the fix directly to arc_step() instead.","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"eeb4665de1db12d429db2eefe0934c7d0da40c07","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"29f7a727_38d44b38","in_reply_to":"819f3fc9_6e8b779a","updated":"2023-12-31 17:21:07.000000000","message":"Looking quickly in the code, this ugly method of re-enabling at resume any breakpoint and watchpoint that has been incorrectly left disabled, is present at least in:\n- HLA\n- arm7_9 (common code for arm920t, arm926ejs, arm966e, arm720t, ...)\n- avr32\n- cortex_m\n- esirisc\n- mips_m4k\n- mips_mips64\n- stm8\nI have not investigated if it\u0027s only a problem due to \u0027step\u0027 or if there is any other use case.\nI think this should be addressed/fixed globally, so I\u0027m ok to merge this patch as a fix and to rework it later.\nThere is some code that should be moved in target.c, like deallocating work-area and also check if a breakpoint/watchpoint is left disabled and then log an error so it can be addressed/fixed.\nWhat do you think, Oleksij?","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"6f40086621745aa129ec48059c75b25daea2109c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"781a57cd_4f14c586","updated":"2024-01-01 15:01:16.000000000","message":"I\u0027m OK :)","commit_id":"4bda53a31a88c180d5c7a28799fb695c8406a5f5"}],"src/target/arc.c":[{"author":{"_account_id":1000410,"name":"Oleksij Rempel","email":"linux@rempel-privat.de","username":"olerem"},"change_message_id":"a67e5d1568dc7f7470d718a1b3ab9e9074536f2b","unresolved":true,"context_lines":[{"line_number":1270,"context_line":""},{"line_number":1271,"context_line":"\tif (!debug_execution) {"},{"line_number":1272,"context_line":"\t\t/* (gdb) continue \u003d execute until we hit break/watch-point */"},{"line_number":1273,"context_line":"\t\tLOG_DEBUG(\"we are in debug execution mode\");"},{"line_number":1274,"context_line":"\t\ttarget_free_all_working_areas(target);"},{"line_number":1275,"context_line":"\t\tCHECK_RETVAL(arc_enable_breakpoints(target));"},{"line_number":1276,"context_line":"\t\tCHECK_RETVAL(arc_enable_watchpoints(target));"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"e30ea17a_da376b82","line":1273,"updated":"2023-09-15 17:49:44.000000000","message":"This debug message is confusing. If debug_execution is false, we say \"we are in debug execution mode\".","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1002160,"name":"Artemiy Volkov","display_name":"Artemiy Volkov","email":"artemiy@synopsys.com","username":"artemiy-volkov","status":"Synopsys"},"change_message_id":"c1e209d1564acc414031c57cfdc4f2cbfcc04af2","unresolved":false,"context_lines":[{"line_number":1270,"context_line":""},{"line_number":1271,"context_line":"\tif (!debug_execution) {"},{"line_number":1272,"context_line":"\t\t/* (gdb) continue \u003d execute until we hit break/watch-point */"},{"line_number":1273,"context_line":"\t\tLOG_DEBUG(\"we are in debug execution mode\");"},{"line_number":1274,"context_line":"\t\ttarget_free_all_working_areas(target);"},{"line_number":1275,"context_line":"\t\tCHECK_RETVAL(arc_enable_breakpoints(target));"},{"line_number":1276,"context_line":"\t\tCHECK_RETVAL(arc_enable_watchpoints(target));"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"bfbb8088_937baa8d","line":1273,"in_reply_to":"e30ea17a_da376b82","updated":"2023-12-01 10:20:42.000000000","message":"Done","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dcf716161e3747aa9f605c6294535311ee36152a","unresolved":true,"context_lines":[{"line_number":2065,"context_line":"\tif (handle_breakpoints) {"},{"line_number":2066,"context_line":"\t\tbreakpoint \u003d breakpoint_find(target, buf_get_u32(pc-\u003evalue, 0, 32));"},{"line_number":2067,"context_line":"\t\tif (breakpoint)"},{"line_number":2068,"context_line":"\t\t\tCHECK_RETVAL(arc_unset_breakpoint(target, breakpoint));"},{"line_number":2069,"context_line":"\t}"},{"line_number":2070,"context_line":""},{"line_number":2071,"context_line":"\t/* restore context */"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d85ecc63_8d11537e","line":2068,"updated":"2023-07-23 14:19:50.000000000","message":"In single step, the breakpoint is \"temporarily\" removed here","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"6192beb26cfaea1123de8cfa9bdfeb25692eb4c5","unresolved":false,"context_lines":[{"line_number":2065,"context_line":"\tif (handle_breakpoints) {"},{"line_number":2066,"context_line":"\t\tbreakpoint \u003d breakpoint_find(target, buf_get_u32(pc-\u003evalue, 0, 32));"},{"line_number":2067,"context_line":"\t\tif (breakpoint)"},{"line_number":2068,"context_line":"\t\t\tCHECK_RETVAL(arc_unset_breakpoint(target, breakpoint));"},{"line_number":2069,"context_line":"\t}"},{"line_number":2070,"context_line":""},{"line_number":2071,"context_line":"\t/* restore context */"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"f63f1192_710d27ee","line":2068,"in_reply_to":"d85ecc63_8d11537e","updated":"2024-01-13 14:39:58.000000000","message":"Done","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"dcf716161e3747aa9f605c6294535311ee36152a","unresolved":true,"context_lines":[{"line_number":2088,"context_line":"\tregister_cache_invalidate(arc-\u003ecore_and_aux_cache);"},{"line_number":2089,"context_line":""},{"line_number":2090,"context_line":"\tif (breakpoint)"},{"line_number":2091,"context_line":"\t\tCHECK_RETVAL(arc_set_breakpoint(target, breakpoint));"},{"line_number":2092,"context_line":""},{"line_number":2093,"context_line":"\tLOG_DEBUG(\"target stepped \");"},{"line_number":2094,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"7f19b81c_0749109c","line":2091,"updated":"2023-07-23 14:19:50.000000000","message":"and here it is set again.\nThe only reason for not arriving here is because an error between unset above and set here has caused this function arc_step() to exit.\nIf the only reason for this patch is the missing set of the breakpoint, I think it would be way more clear fixing it in this function.","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"6192beb26cfaea1123de8cfa9bdfeb25692eb4c5","unresolved":false,"context_lines":[{"line_number":2088,"context_line":"\tregister_cache_invalidate(arc-\u003ecore_and_aux_cache);"},{"line_number":2089,"context_line":""},{"line_number":2090,"context_line":"\tif (breakpoint)"},{"line_number":2091,"context_line":"\t\tCHECK_RETVAL(arc_set_breakpoint(target, breakpoint));"},{"line_number":2092,"context_line":""},{"line_number":2093,"context_line":"\tLOG_DEBUG(\"target stepped \");"},{"line_number":2094,"context_line":""}],"source_content_type":"text/x-csrc","patch_set":1,"id":"498b1727_177d5839","line":2091,"in_reply_to":"7f19b81c_0749109c","updated":"2024-01-13 14:39:58.000000000","message":"Done","commit_id":"7ab25a2135c56fc42922e0e4b95e3236d357a746"}]}