)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1002438,"name":"Ian Thompson","email":"ianst+cdns@cadence.com","username":"ianst_cdns"},"change_message_id":"88aeb306a2d4ec7dfdd677bc3aab8beba5328aaf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"47191d8c_0264d210","updated":"2025-09-08 14:23:33.000000000","message":"Good find.  Thanks for fixing; looks good to me.","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d239c3a9de9396edc06fbdccc5e1336c5534aba6","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"281d5b51_a61a8e2f","updated":"2025-09-10 20:52:45.000000000","message":"Thanks for this fix.\nApart for a comment on the patch itself, I have checked further the code and I see some other issue.\nI cannot test this code because I have no xtensa targets.\nWould you mind checking my comments and eventually fix that too? Either in a separate patch or inside this one, as you prefer.","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"6ac6f777af79d2e924825c44ba6573e6432ead07","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fadd34c6_dfa8e866","in_reply_to":"08d793c1_1a59b032","updated":"2025-09-17 07:31:15.000000000","message":"Done","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"558f2f4331d2fd170c70e43bfe3274d4a3797f1c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"08d793c1_1a59b032","in_reply_to":"281d5b51_a61a8e2f","updated":"2025-09-11 10:15:38.000000000","message":"Thanks for checking. Seems calling `xtregs` multiple times really was not considered before. Its all related issues, I will just fix it here.","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1002438,"name":"Ian Thompson","email":"ianst+cdns@cadence.com","username":"ianst_cdns"},"change_message_id":"04b3345da8c60aabec27cf0e567b38f412e8d980","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"34ab8e93_c5ce60c1","updated":"2025-09-16 23:08:52.000000000","message":"Confirmed patchset 2 is working on my generic Xtensa target, although it does not use the contiguous register map that Espressif chips typically configure.  Presumably that is what @samuel is using to validate.\n\nFYI, Xtensa targets expect the \"xtregs\" command to only be issued once, during configuration time, and before attaching to a core.  These extra free() calls should be safe, and I would not expect them to impact targets under normal operation.\n\n@samuel I recommend you resolve the comments as you\u0027re able.  Antonio may wait to come back to it until everything is resolved...","commit_id":"851aaaa4eb60acf8b4f349769603e9a6b3cc2dcd"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"d8fadf4fe0ca75ad5d3e6c99703b132acfd34683","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f7c16912_f8fa9531","updated":"2025-12-11 14:31:14.000000000","message":"Hi @antonio could you please re-visit? I believe the everything was addressed here","commit_id":"851aaaa4eb60acf8b4f349769603e9a6b3cc2dcd"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"6ac6f777af79d2e924825c44ba6573e6432ead07","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5750d8de_62cff7c0","in_reply_to":"34ab8e93_c5ce60c1","updated":"2025-09-17 07:31:15.000000000","message":"Thanks for the suggestion Ian, I resolved everything now (hopefully). And yes, I tested with the contiguous registers","commit_id":"851aaaa4eb60acf8b4f349769603e9a6b3cc2dcd"}],"src/target/xtensa/xtensa.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d239c3a9de9396edc06fbdccc5e1336c5534aba6","unresolved":true,"context_lines":[{"line_number":3493,"context_line":"\t\tfree(xtensa-\u003eoptregs);"},{"line_number":3494,"context_line":"\t}"},{"line_number":3495,"context_line":"\txtensa-\u003eoptregs \u003d NULL;"},{"line_number":3496,"context_line":"\tif (xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3497,"context_line":"\t\tfree(xtensa-\u003econtiguous_regs_desc);"},{"line_number":3498,"context_line":"\t\txtensa-\u003econtiguous_regs_desc \u003d NULL;"},{"line_number":3499,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"a9003573_48c24061","line":3496,"updated":"2025-09-10 20:52:45.000000000","message":"There is no need for this test because running `free(NULL)` is ok!\nPlease remove it","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"6ac6f777af79d2e924825c44ba6573e6432ead07","unresolved":false,"context_lines":[{"line_number":3493,"context_line":"\t\tfree(xtensa-\u003eoptregs);"},{"line_number":3494,"context_line":"\t}"},{"line_number":3495,"context_line":"\txtensa-\u003eoptregs \u003d NULL;"},{"line_number":3496,"context_line":"\tif (xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3497,"context_line":"\t\tfree(xtensa-\u003econtiguous_regs_desc);"},{"line_number":3498,"context_line":"\t\txtensa-\u003econtiguous_regs_desc \u003d NULL;"},{"line_number":3499,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c60f313d_693a11dd","line":3496,"in_reply_to":"a65222ae_8b2bdf26","updated":"2025-09-17 07:31:15.000000000","message":"Done","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"558f2f4331d2fd170c70e43bfe3274d4a3797f1c","unresolved":true,"context_lines":[{"line_number":3493,"context_line":"\t\tfree(xtensa-\u003eoptregs);"},{"line_number":3494,"context_line":"\t}"},{"line_number":3495,"context_line":"\txtensa-\u003eoptregs \u003d NULL;"},{"line_number":3496,"context_line":"\tif (xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3497,"context_line":"\t\tfree(xtensa-\u003econtiguous_regs_desc);"},{"line_number":3498,"context_line":"\t\txtensa-\u003econtiguous_regs_desc \u003d NULL;"},{"line_number":3499,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"a65222ae_8b2bdf26","line":3496,"in_reply_to":"a9003573_48c24061","updated":"2025-09-11 10:15:38.000000000","message":"done.","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d239c3a9de9396edc06fbdccc5e1336c5534aba6","unresolved":true,"context_lines":[{"line_number":3497,"context_line":"\t\tfree(xtensa-\u003econtiguous_regs_desc);"},{"line_number":3498,"context_line":"\t\txtensa-\u003econtiguous_regs_desc \u003d NULL;"},{"line_number":3499,"context_line":"\t}"},{"line_number":3500,"context_line":"\tif (xtensa-\u003econtiguous_regs_list) {"},{"line_number":3501,"context_line":"\t\tfree(xtensa-\u003econtiguous_regs_list);"},{"line_number":3502,"context_line":"\t\txtensa-\u003econtiguous_regs_list \u003d NULL;"},{"line_number":3503,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"5d264bdc_c716d59e","line":3500,"updated":"2025-09-10 20:52:45.000000000","message":"Same here","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"f5acac0791ac05f5873fbe588aa541c0a94f984a","unresolved":false,"context_lines":[{"line_number":3497,"context_line":"\t\tfree(xtensa-\u003econtiguous_regs_desc);"},{"line_number":3498,"context_line":"\t\txtensa-\u003econtiguous_regs_desc \u003d NULL;"},{"line_number":3499,"context_line":"\t}"},{"line_number":3500,"context_line":"\tif (xtensa-\u003econtiguous_regs_list) {"},{"line_number":3501,"context_line":"\t\tfree(xtensa-\u003econtiguous_regs_list);"},{"line_number":3502,"context_line":"\t\txtensa-\u003econtiguous_regs_list \u003d NULL;"},{"line_number":3503,"context_line":"\t}"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"26116a6d_deeb9aee","line":3500,"in_reply_to":"5d264bdc_c716d59e","updated":"2025-09-17 07:31:43.000000000","message":"Done","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d239c3a9de9396edc06fbdccc5e1336c5534aba6","unresolved":true,"context_lines":[{"line_number":3905,"context_line":"\t\txtensa-\u003ecore_regs_num \u003d 0;"},{"line_number":3906,"context_line":"\t\txtensa-\u003enum_optregs \u003d 0;"},{"line_number":3907,"context_line":"\t\t/* A little more memory than required, but saves a second initialization pass */"},{"line_number":3908,"context_line":"\t\txtensa-\u003eoptregs \u003d calloc(xtensa-\u003etotal_regs_num, sizeof(struct xtensa_reg_desc));"},{"line_number":3909,"context_line":"\t\tif (!xtensa-\u003eoptregs) {"},{"line_number":3910,"context_line":"\t\t\tLOG_ERROR(\"Failed to allocate xtensa-\u003eoptregs!\");"},{"line_number":3911,"context_line":"\t\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"3c2e752b_a0d20197","line":3908,"updated":"2025-09-10 20:52:45.000000000","message":"If the command `xtregs` is run twice, the first time this is allocated, the second time we overwrite the old value and we have a memory leak.\nI think we need to run\n`free(xtensa-\u003eoptregs);`\nbefore this `calloc()`","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"558f2f4331d2fd170c70e43bfe3274d4a3797f1c","unresolved":true,"context_lines":[{"line_number":3905,"context_line":"\t\txtensa-\u003ecore_regs_num \u003d 0;"},{"line_number":3906,"context_line":"\t\txtensa-\u003enum_optregs \u003d 0;"},{"line_number":3907,"context_line":"\t\t/* A little more memory than required, but saves a second initialization pass */"},{"line_number":3908,"context_line":"\t\txtensa-\u003eoptregs \u003d calloc(xtensa-\u003etotal_regs_num, sizeof(struct xtensa_reg_desc));"},{"line_number":3909,"context_line":"\t\tif (!xtensa-\u003eoptregs) {"},{"line_number":3910,"context_line":"\t\t\tLOG_ERROR(\"Failed to allocate xtensa-\u003eoptregs!\");"},{"line_number":3911,"context_line":"\t\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"f160b3bd_684a131b","line":3908,"in_reply_to":"3c2e752b_a0d20197","updated":"2025-09-11 10:15:38.000000000","message":"good catch. I checked, and the suggested fix is ok","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"6ac6f777af79d2e924825c44ba6573e6432ead07","unresolved":false,"context_lines":[{"line_number":3905,"context_line":"\t\txtensa-\u003ecore_regs_num \u003d 0;"},{"line_number":3906,"context_line":"\t\txtensa-\u003enum_optregs \u003d 0;"},{"line_number":3907,"context_line":"\t\t/* A little more memory than required, but saves a second initialization pass */"},{"line_number":3908,"context_line":"\t\txtensa-\u003eoptregs \u003d calloc(xtensa-\u003etotal_regs_num, sizeof(struct xtensa_reg_desc));"},{"line_number":3909,"context_line":"\t\tif (!xtensa-\u003eoptregs) {"},{"line_number":3910,"context_line":"\t\t\tLOG_ERROR(\"Failed to allocate xtensa-\u003eoptregs!\");"},{"line_number":3911,"context_line":"\t\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"0bb573f9_0a71e324","line":3908,"in_reply_to":"f160b3bd_684a131b","updated":"2025-09-17 07:31:15.000000000","message":"Done","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"d239c3a9de9396edc06fbdccc5e1336c5534aba6","unresolved":true,"context_lines":[{"line_number":3918,"context_line":"\t/* \"xtregfmt contiguous\" must be specified prior to the first \"xtreg\" definition"},{"line_number":3919,"context_line":"\t * if general register (g-packet) requests or contiguous register maps are supported */"},{"line_number":3920,"context_line":"\tif (xtensa-\u003eregmap_contiguous \u0026\u0026 !xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3921,"context_line":"\t\txtensa-\u003econtiguous_regs_desc \u003d calloc(xtensa-\u003etotal_regs_num, sizeof(struct xtensa_reg_desc *));"},{"line_number":3922,"context_line":"\t\tif (!xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3923,"context_line":"\t\t\tLOG_ERROR(\"Failed to allocate xtensa-\u003econtiguous_regs_desc!\");"},{"line_number":3924,"context_line":"\t\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"787137a8_30a0f78e","line":3921,"updated":"2025-09-10 20:52:45.000000000","message":"Multiple issues:\n1) Here there is the allocation.\nBut this function can fail later, returning without freeing this.\n\n2) if we run `xtregs 1` we set `total_regs_num\u003d1`. Then run `xtreg \u003cregname\u003e \u003cregnum\u003e`, this allocates space for 1 register. OK!\nNow we run `xtregs 2`! The `total_regs_num` is now changed, but we have no way to update the size of `contiguous_regs_desc`.\nEven worse, we can address the second register, as allowed by `total_regs_num`, but we access beyond the boundary of the allocated memory!\n\nI think this `contiguous_regs_desc` should be `free()` and assigned to NULL when we assign the new value of `xtensa-\u003etotal_regs_num`.","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"6ac6f777af79d2e924825c44ba6573e6432ead07","unresolved":false,"context_lines":[{"line_number":3918,"context_line":"\t/* \"xtregfmt contiguous\" must be specified prior to the first \"xtreg\" definition"},{"line_number":3919,"context_line":"\t * if general register (g-packet) requests or contiguous register maps are supported */"},{"line_number":3920,"context_line":"\tif (xtensa-\u003eregmap_contiguous \u0026\u0026 !xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3921,"context_line":"\t\txtensa-\u003econtiguous_regs_desc \u003d calloc(xtensa-\u003etotal_regs_num, sizeof(struct xtensa_reg_desc *));"},{"line_number":3922,"context_line":"\t\tif (!xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3923,"context_line":"\t\t\tLOG_ERROR(\"Failed to allocate xtensa-\u003econtiguous_regs_desc!\");"},{"line_number":3924,"context_line":"\t\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"d2f2649f_4c40f5b9","line":3921,"in_reply_to":"10ee6a74_55c2155b","updated":"2025-09-17 07:31:15.000000000","message":"Done","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"2e0de99b3ea0bd2d0c4323fcbcc1b946b4b48c53","unresolved":false,"context_lines":[{"line_number":3918,"context_line":"\t/* \"xtregfmt contiguous\" must be specified prior to the first \"xtreg\" definition"},{"line_number":3919,"context_line":"\t * if general register (g-packet) requests or contiguous register maps are supported */"},{"line_number":3920,"context_line":"\tif (xtensa-\u003eregmap_contiguous \u0026\u0026 !xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3921,"context_line":"\t\txtensa-\u003econtiguous_regs_desc \u003d calloc(xtensa-\u003etotal_regs_num, sizeof(struct xtensa_reg_desc *));"},{"line_number":3922,"context_line":"\t\tif (!xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3923,"context_line":"\t\t\tLOG_ERROR(\"Failed to allocate xtensa-\u003econtiguous_regs_desc!\");"},{"line_number":3924,"context_line":"\t\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"33a98f19_aa263963","line":3921,"in_reply_to":"5f807c6e_1f6aae06","updated":"2026-01-05 15:09:13.000000000","message":"yes I think this is already the case here, as `xtensa_target_deinit()` calls `xtensa_free_reg_cache()`","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1001803,"name":"Samuel Obuch","email":"samuel.obuch@espressif.com","username":"sobuch"},"change_message_id":"558f2f4331d2fd170c70e43bfe3274d4a3797f1c","unresolved":true,"context_lines":[{"line_number":3918,"context_line":"\t/* \"xtregfmt contiguous\" must be specified prior to the first \"xtreg\" definition"},{"line_number":3919,"context_line":"\t * if general register (g-packet) requests or contiguous register maps are supported */"},{"line_number":3920,"context_line":"\tif (xtensa-\u003eregmap_contiguous \u0026\u0026 !xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3921,"context_line":"\t\txtensa-\u003econtiguous_regs_desc \u003d calloc(xtensa-\u003etotal_regs_num, sizeof(struct xtensa_reg_desc *));"},{"line_number":3922,"context_line":"\t\tif (!xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3923,"context_line":"\t\t\tLOG_ERROR(\"Failed to allocate xtensa-\u003econtiguous_regs_desc!\");"},{"line_number":3924,"context_line":"\t\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"10ee6a74_55c2155b","line":3921,"in_reply_to":"787137a8_30a0f78e","updated":"2025-09-11 10:15:38.000000000","message":"1) it should still be freed in `target_destroy` (`xtensa_free_reg_cache`), right? At least I didnt manage to invoke memory leak in this case.\n\n2) Freeing `contiguous_regs_desc` does prevent the issue, though any registers previously specified with `xtreg` will of course be lost or need to be specified again.","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"faab18cee22baa6a2e0fdb20a1fe886759184b6c","unresolved":false,"context_lines":[{"line_number":3918,"context_line":"\t/* \"xtregfmt contiguous\" must be specified prior to the first \"xtreg\" definition"},{"line_number":3919,"context_line":"\t * if general register (g-packet) requests or contiguous register maps are supported */"},{"line_number":3920,"context_line":"\tif (xtensa-\u003eregmap_contiguous \u0026\u0026 !xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3921,"context_line":"\t\txtensa-\u003econtiguous_regs_desc \u003d calloc(xtensa-\u003etotal_regs_num, sizeof(struct xtensa_reg_desc *));"},{"line_number":3922,"context_line":"\t\tif (!xtensa-\u003econtiguous_regs_desc) {"},{"line_number":3923,"context_line":"\t\t\tLOG_ERROR(\"Failed to allocate xtensa-\u003econtiguous_regs_desc!\");"},{"line_number":3924,"context_line":"\t\t\treturn ERROR_FAIL;"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"5f807c6e_1f6aae06","line":3921,"in_reply_to":"d2f2649f_4c40f5b9","updated":"2025-12-25 10:08:54.000000000","message":"For 1), yes, either `xtensa_chip_target_deinit()` or `xtensa_target_deinit()` should `free()` all these allocated memories.\nMaybe in a separate patch","commit_id":"048e5e0367bd3e716341cdcaa1b051484a0962d6"}]}