)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1001992,"name":"Peter Collingbourne","email":"pcc@google.com","username":"pcc"},"change_message_id":"c46add0669fe739dc5ebeaa098fe587d515eeb28","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1006eca3_828184ac","updated":"2023-03-02 06:11:30.000000000","message":"(This needs a fix for the case where p_filesz \u003d\u003d 0.)","commit_id":"28ff2b12fb20577476f23ad23dc83001b42f39ef"},{"author":{"_account_id":1001992,"name":"Peter Collingbourne","email":"pcc@google.com","username":"pcc"},"change_message_id":"6fb1830660dd04af420eb57d445cce0dba9d6590","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"83ac2235_70987913","in_reply_to":"1006eca3_828184ac","updated":"2023-03-02 06:22:40.000000000","message":"Fixed","commit_id":"28ff2b12fb20577476f23ad23dc83001b42f39ef"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c1ff3b472111cec08ee1340020c3e5f9beb632af","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c49f3acd_9b71a051","updated":"2023-03-02 12:54:19.000000000","message":"Thanks for the patch.\nI\u0027m ok with it, only a minor comment below","commit_id":"0ec78e94bcecc88584d5188659adb566ce670ce7"},{"author":{"_account_id":1001992,"name":"Peter Collingbourne","email":"pcc@google.com","username":"pcc"},"change_message_id":"5ed5db3e97a8fe79296269ae7ca62a7dc1587f24","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"3c075703_c8308f43","updated":"2023-03-17 22:08:03.000000000","message":"Can we land this now?","commit_id":"a3d11ecb653694a9c049f87b06563d34e8af210a"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"cee9cef3910773ec36e917d5847af5bfc6c1d66b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a222837a_f05a2e06","updated":"2023-03-03 22:20:18.000000000","message":"Thanks!","commit_id":"a3d11ecb653694a9c049f87b06563d34e8af210a"},{"author":{"_account_id":1002022,"name":"Marian Buschsieweke","display_name":"maribu","email":"marian.buschsieweke@ovgu.de","username":"maribu"},"change_message_id":"2d1bdb865e61a3a257aa30b3259ae30b2f12e444","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4b8f1338_141405cd","updated":"2023-05-04 08:44:22.000000000","message":"\u003e My first instinct is that verify_image is doing the right thing.\n\nThis at least breaks the work flow of many OpenOCD users: A typical pattern is to use `flash write_image` followed by a `verify_image` and a reboot. With the new behavior, also RAM would need to be loaded for the verification to succeed, but why wasting time on writing to RAM just prior to a reboot.\n\nMaybe verify_image could get an additional parameter to specify if RAM, flash, or both should be verified. With the default being flash only (if unspecified), it would not break existing setups.\n\nFor us fixing the flashing script is not as easy as just adding a `flash-only` parameter to `verify_image` if also verifying RAM would be the default. We have users with various versions of OpenOCD and intent to at least be backward compatible with whatever the latest Ubuntu LTS ships. Gracefully handling different behavior of OpenOCD depending on the version is always a big pain in the ass.\n\n\u003e What does readelf -lW show for this file?\n\narm-none-eabi-readelf -lW hello-world.elf \n\nElf file type is EXEC (Executable file)\nEntry point 0x80007ed\nThere are 6 program headers, starting at offset 52\n\nProgram Headers:\n  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align\n  EXIDX          0x003480 0x08002480 0x08002480 0x00008 0x00008 R   0x4\n  LOAD           0x001000 0x08000000 0x08000000 0x024d8 0x024d8 R E 0x1000\n  LOAD           0x004200 0x20000200 0x080024d8 0x0006c 0x008b4 RW  0x1000\n  LOAD           0x005000 0x40024000 0x08002544 0x00004 0x00004 R   0x1000\n  LOAD           0x000000 0x20000000 0x20000000 0x00000 0x00200 RW  0x1000\n  GNU_STACK      0x000000 0x00000000 0x00000000 0x00000 0x00000 RW  0x10\n\n Section to Segment mapping:\n  Segment Sections...\n   00     .ARM.exidx \n   01     .text .ARM.exidx .eh_frame \n   02     .relocate .bss \n   03     .backup.data \n   04     .stack \n   05     \n   \nNote that the segment at 0x20000200 is loaded from flash at 0x080024d8. The FileSiz is 0x0006c, so it ends at 0x8002544 in flash and does not overlap with the segment at 0x40024000 stored in flash at 0x8002544. In RAM it ends at 0x20000200 + 0x008b4 \u003d 0x20000ab4 (due to MemSiz being larger than FileSiz), but that is not overlapping either.","commit_id":"047b1a8fc237af480e3bab66a9827a358afd7547"},{"author":{"_account_id":1002022,"name":"Marian Buschsieweke","display_name":"maribu","email":"marian.buschsieweke@ovgu.de","username":"maribu"},"change_message_id":"cbe959e19738f5cc5028e95e23111122e797f718","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f4fabfc5_b88b64bb","updated":"2023-05-03 09:00:41.000000000","message":"Hi,\n\nthis sadly causes a number of regressions when flashing. E.g. I cannot flash the firmware of `examples/hello-world` of https://github.com/RIOT-OS/RIOT for the nucleo-f767zi board anymore with OpenOCD\u0027s current master:\n\nError: Section at 0x08002558 overlaps section ending at 0x08002da0\nError: Flash write aborted.\n\nReverting this comment fixes the issue. (And there seems to be no section ending at 0x08002da0 according to readelf.) Similar, `verify_image` for https://review.openocd.org/c/openocd/+/5584 now starts to verify against SRAM as well, not only flash sections.\n\nWould you mind taking another look at this?\n\nKind regards,\nMarian","commit_id":"047b1a8fc237af480e3bab66a9827a358afd7547"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"3bbb6e7c572d6e5dfd737afb45e73cf6ea6b3d0f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":6,"id":"08a3284f_c6423db9","updated":"2023-05-06 09:42:46.000000000","message":"I have checked few elf files generated from different linker scripts.\nAgree with Marian, this patch breaks the previous behavior; MUST be fixed.\n\nI also have another issue with an elf file that has bss (from objdump):\nIdx Name              Size      VMA       LMA       File off  Algn  Flags\n  4 .bss              00010038  20000108  0801fc7c  00040008  2**2  ALLOC\nwhose size exceeds the flash boundary, but it\u0027s ok when allocated in SRAM!\nPlease note that physical address is in flash (0801fc7c) not in RAM (20000108), as is the case for Marian; openocd will write the bss in flash!\nThis elf will generate write error while trying to write the filled bss in flash.\nBut even without errors, why should I zero the flash for bss, when it is not needed? It reduces the longevity time of the flash and prevents using the rest of flash for other purposes.\n\nI think a good solution would be to add a command line flag (e.g. -fill-empty-sections) to all the commands that loads elf images, so without the flag the old behavior is preserved.\nBut, if this activity takes to long time, we should first revert this commit and then quietly work at the new code.","commit_id":"047b1a8fc237af480e3bab66a9827a358afd7547"},{"author":{"_account_id":1001992,"name":"Peter Collingbourne","email":"pcc@google.com","username":"pcc"},"change_message_id":"ad16ac55c5a32fd9359656ac80f40751408809d4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"2ed7e8cb_8ab2ac30","in_reply_to":"f4fabfc5_b88b64bb","updated":"2023-05-03 18:11:01.000000000","message":"What does `readelf -lW` show for this file? The error is a bit confusing because it mentions sections but it\u0027s really talking about segments (program headers).\n\n\u003e Similar, verify_image for https://review.openocd.org/c/openocd/+/5584 now starts to verify against SRAM as well, not only flash sections.\n\nMy first instinct is that `verify_image` is doing the right thing. `load_image` can be used to load images into SRAM as well, and at the ELF level there isn\u0027t meant to be a difference between trailing zeroes represented via `p_filesz !\u003d p_memsz` and zeros physically in the file.","commit_id":"047b1a8fc237af480e3bab66a9827a358afd7547"}],"src/target/image.c":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"c1ff3b472111cec08ee1340020c3e5f9beb632af","unresolved":true,"context_lines":[{"line_number":669,"context_line":"\t\t\tLOG_ERROR(\"cannot read ELF segment content, read failed\");"},{"line_number":670,"context_line":"\t\t\treturn retval;"},{"line_number":671,"context_line":"\t\t}"},{"line_number":672,"context_line":"\t\toffset +\u003d read_size;"},{"line_number":673,"context_line":"\t\tsize -\u003d read_size;"},{"line_number":674,"context_line":"\t\t*size_read +\u003d read_size;"},{"line_number":675,"context_line":"\t\t/* need more data ? */"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"62e300b2_e1f922d3","line":672,"updated":"2023-03-02 12:54:19.000000000","message":"if you add here\nbuffer +\u003d read_size;\nyou wouldn\u0027t need to initialize read_size above, and below\nmemset(buffer, 0, memset_size);","commit_id":"0ec78e94bcecc88584d5188659adb566ce670ce7"},{"author":{"_account_id":1001992,"name":"Peter Collingbourne","email":"pcc@google.com","username":"pcc"},"change_message_id":"4ddeaa353d62061226b52798aa6bfc0f0b5180a5","unresolved":false,"context_lines":[{"line_number":669,"context_line":"\t\t\tLOG_ERROR(\"cannot read ELF segment content, read failed\");"},{"line_number":670,"context_line":"\t\t\treturn retval;"},{"line_number":671,"context_line":"\t\t}"},{"line_number":672,"context_line":"\t\toffset +\u003d read_size;"},{"line_number":673,"context_line":"\t\tsize -\u003d read_size;"},{"line_number":674,"context_line":"\t\t*size_read +\u003d read_size;"},{"line_number":675,"context_line":"\t\t/* need more data ? */"}],"source_content_type":"text/x-csrc","patch_set":4,"id":"0b05b028_5d99d133","line":672,"in_reply_to":"62e300b2_e1f922d3","updated":"2023-03-03 02:08:21.000000000","message":"Done","commit_id":"0ec78e94bcecc88584d5188659adb566ce670ce7"}]}
