Created attachment 8331 [details] test data According to section 3.10.5 (title "The Location Counter"): The location counter may not be moved backwards inside an output section, and may not be moved backwards outside of an output section if so doing creates areas with overlapping LMAs. Contrary to the documentation, LD actually allows creating overlapping sections, at least if one of the overlapping sections is the ".bss" section. The reason that this has the potential to be a serious problem is that if a linker script uses fixed addresses for output sections, there is a chance that changes to the program being linked will cause one of the output sections to increase in size so that it overlaps the section that follows it. If LD doesn't treat this condition as an error, the result is likely to be malfunctions which are difficult to debug. For example, if the .bss section overlaps the .text section, then the calling one function could overwrite a portion of the code of a completely unrelated function, and no problem would be evident until the latter function was called. To reproduce the problem, unpack the attached tar file into an empty directory and run the command "sh mk". That will assemble and link an object file named "lib.out", and run objdump and nm on it. The expected behavior (if the bug has not been fixed) is that 1) The ld command will not produce any messages. 2) The objdump command will show that the .text and .bss segments overlap. 3) The nm command will show that the linker has assigned the same address to both main (a function) and var2 (a variable). The following command output describes the system I am using: $ /usr/bin/ld -version GNU ld (GNU Binutils for Ubuntu) 2.24 Copyright 2013 Free Software Foundation, Inc. This program is free software; you may redistribute it under the terms of the GNU General Public License version 3 or (at your option) a later version. This program has absolutely no warranty. $ ldd /usr/bin/ld linux-vdso.so.1 => (0x00007fff21b1e000) libbfd-2.24-system.so => /usr/lib/libbfd-2.24-system.so (0x00007fdf47641000) libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fdf4743d000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fdf47077000) libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007fdf46e5e000) /lib64/ld-linux-x86-64.so.2 (0x00007fdf47985000) $ ls -l /lib/x86_64-linux-gnu/libdl.so.2 lrwxrwxrwx 1 root root 13 Feb 25 11:56 /lib/x86_64-linux-gnu/libdl.so.2 -> libdl-2.19.so $ ls -l /lib/x86_64-linux-gnu/libc.so.6 lrwxrwxrwx 1 root root 12 Feb 25 11:56 /lib/x86_64-linux-gnu/libc.so.6 -> libc-2.19.so $ ls -l /lib/x86_64-linux-gnu/libz.so.1 lrwxrwxrwx 1 root root 13 Aug 14 2014 /lib/x86_64-linux-gnu/libz.so.1 -> libz.so.1.2.8 $ uname --kernel-name Linux $ uname --kernel-release 3.13.0-36-generic $ uname --kernel-version #63-Ubuntu SMP Wed Sep 3 21:30:07 UTC 2014 $ uname --machine x86_64 $ lsb_release -d Description: Ubuntu 14.04.2 LTS $ # CPU is Intel i5-4690K (Haswell)
Confirmed. This is where we update the region "current" location, which lacks any sort of check. if (output_section_statement == abs_output_section) { /* If we don't have an output section, then just adjust the default memory address. */ lang_memory_region_lookup (DEFAULT_MEMORY_REGION, FALSE)->current = newdot; } The behaviour goes back forever as far as I can tell. This code was introduced with commit 27f7237e.
https://sourceware.org/git/?p=binutils.git;a=commit;h=ec457111c02dd5a5e29ce7ce30e95a17ef470c78 The linker previously detected section conflicts involving the .bss section, but this commit in 2009 changed it to not check sections that's aren't loaded from the executable (I'm not clear what the reason for this was). https://sourceware.org/git/?p=binutils.git;a=commit;h=ac3f93dc025a4dcf2c7dcf12039c46555b8a7ecd It was changed shortly after that to ignore sections that aren't allocated as well. "!(s->flags & SEC_LOAD) || !(s->flags & SEC_ALLOC)" should likely be "!(s->flags & SEC_LOAD) && !(s->flags & SEC_ALLOC)". Or just "!(s->flags & SEC_ALLOC)" (can a section be SEC_LOAD but not SEC_ALLOC?).
See https://sourceware.org/ml/binutils/2009-05/msg00330.html for why lang_check_section_addresses can't check bss.
Created attachment 9132 [details] binutils LD change patch Like the link in the last comment shows, this behavior is sane because BSS sections have to be able to overlap between themselves and data sections for overlay reasons. But as one user of LD who both needs overlaps but at the same time has been bitten by longish debugging hours because of not realizing that I had incidentally created unwanted/accidental overlaps I suggest the following change: 1) Have LD inform by default if there are noload sections overlapping other sections 2) If someone saw the information, became aware of the overlap and consciously makes a choice to quiet down that information, provide a LD flag that would silence that extra verbosity The attached patch is a suggestion of such a change. Basically, in a default run, LD would just print information of the sort of: "legal section overlap of section .text loaded at [0000000000008010,000000000000801f] overlapping section .bss loaded at [0000000000008000,000000000000801f]. See https://sourceware.org/ml/binutils/2009-05/msg00330.html" And if the user decides that they have become aware and that for their case this is a legal use, they could silence the information by using a "--no-check-overlap-noload-sections" flag sent to LD. The patch is done against binutils 2.24.
(In reply to Alan Modra from comment #3) > See https://sourceware.org/ml/binutils/2009-05/msg00330.html for why > lang_check_section_addresses can't check bss. The "relaxed" checks above (suitable for LMA mapping into ROM scenario) means hours of debug for other people (in my case ALLOC .bss section silently overlapped .text section; LMA = VMA = RAM addresses). I don't think LD "can't check .bss" is an acceptable answer; I think LD should check (as before) ALLOC section overlap (by default or optionally).
Created attachment 9135 [details] binutils LD individual section selection patch suggestion I've been thinking a little bit more about this and I realized in the same application one can have both the problem of accidental overlaps but at the same time, in the same application have a need to create overlapped noload sections. A reference to this usecase was hinted I think previously when the text in the endnote of https://sourceware.org/ml/binutils/2009-05/msg00330.html . As a linker script evolves, one may have additions or changes to the linker script(s) used. If at some point one had enabled a general option to not warn anymore, one may still get later in trouble if a new section is accidentally overlapped. As a consequence I'm thinking that the previous suggestion should be changed a little bit to something of this sort: 1) By default LD should just not allow section overlapping of any sort but 2) During the checks, if a section overlapping is detected to be a NOLOAD section then LD, even if not linking, could display a message informing the user that they can consciously allow the overlap if they mark that particular NOLOAD section as possible to be overlapped 3) Provide an option to allow the overlap of a NOLOAD section as per the description at point "2)" Thinking about this problem I had thought of maybe adding an extra flag to the bfd_section structure may be one solution, but the bits in the flagword are pretty expensive I think to burn for this, so instead I thought of keeping a separate list in the bfd alive which holds the names of desired sections to be overlapped. In terms of how to specify which sections are possible to be overlapped, I thought of just overloading the use of "OVERLAY". But then I'm aware that OVERLAY is mostly just meant to be syntactic sugar (ftp://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_node/ld_22.html), and the way it is implemented it actually forced the sections to have the LOAD flag set. I think changing that may have too great an implication worldwide and I don't think it's worth it. Considering all of these I think the remaining option is to build the list of accepted NOLOAD sections to be overlapped a list of options in as command line arguments. This patch is suggesting one way of doing that. Additionally, if there is interest in this direction, I guess an additional improvement that could be done would be to add a ld script new option called "OVERLAP" or something similar that would allow the sections to be added to the accepted list in this fashion too.
Since there seems to be enough interest in this bug for people to actually write patches, here is what I think would work and be accepted: In lang_size_sections_1, near the comment "A backwards move of dot", set a static global var whenever seeing "dot < last->vma". This should flag all cases where overlays are used without false triggers. Modify lang_check_section_addresses to check for vma overlap when no overlays are detected. - Use IGNORE_SECTION again to choose interesting sections (the attached patch was wrong in treatment of .tbss) - In the existing lma checking loop, ignore !SEC_LOAD sections. Some rewriting might be necessary to elegantly handle the case of the first section being !SEC_LOAD. - Write another loop to check vmas if overlays not present
Created attachment 9136 [details] Additional possible suggestion with keyword in the linker script Another suggestion with similar structure as the one before, but with accepting an (OVERLAP) keyword to specify a section is acceptable to be overlapped via ld script syntax. Choice of the word or method could be changed if this ever gets adopted of course. Mostly here to suggest an idea.
Created attachment 9137 [details] ADDING OVERLAP keyword with IGNORESECTION (In reply to Alan Modra from comment #7) > Since there seems to be enough interest in this bug for people to actually > write patches, here is what I think would work and be accepted: > > In lang_size_sections_1, near the comment "A backwards move of dot", set a > static global var whenever seeing "dot < last->vma". This should flag all > cases where overlays are used without false triggers. > But this would detect all overlays. It still does not resolve the problem that some overlays are intentional and others are not. This depends on user choice. The fact that one section is or isn't !SEC_LOAD does not necessarily mean that that is a good case or a bad case. We'd be in the same situation as before the 2009 patch, would we not? > Modify lang_check_section_addresses to check for vma overlap when no > overlays are detected. > - Use IGNORE_SECTION again to choose interesting sections (the attached > patch was wrong in treatment of .tbss) Point taken. Reattaching last patch with IGNORE_SECTION back in there. Saw the original 2.24 source was still leaving it out so I left it out too, although I had noticed the difference and was a little bit curious why that define is still left in there for apparently no particular reason. > - In the existing lma checking loop, ignore !SEC_LOAD sections. Some > rewriting might be necessary to elegantly handle the case of the first > section being !SEC_LOAD. Not all !SEC_LOAD sections should be ignored either, or else we'd be in the same situation as we are today (well, almost). The problem is specifically marking which sections are intentional overlay vs which ones are not. The OVERLAY keyword is there, yes, but reading through the world knowledge base it has a different meaning right now so I would not recommend hijacking that keyword for this. > - Write another loop to check vmas if overlays not present This assumes that whoever has overlays intends to have his whole application accept all overlays, and only who does not have overlays at all accepts a clean application. I think these suggestions would be OK if the case would be that it is a general flag that is required to be detected, as per the very first patch submitted, but thinking more about things, I do not think that is true. The same application can have valid overlays and invalid ones, I cannot see any way around this other than allowing the user to specifically mark which ones are valid.
(In reply to Cristian Gavril Olar from comment #9) > I think these suggestions would be OK if the case would be that it is a > general flag that is required to be detected, as per the very first patch > submitted, but thinking more about things, I do not think that is true. The > same application can have valid overlays and invalid ones, I cannot see any > way around this other than allowing the user to specifically mark which ones > are valid. But that being said, if there would be a way to use the existing ld script commands to infer somehow an intentional overlap of noload sections that can clearly show intention and could not happen by mistake in any way, it would also be my preference to reuse existing syntax/options completely it is just that I keep thinking about this and I cannot see any way with just the existing options and syntax. As far as I've noticed: - OVERLAY creates SEC_LOAD sections rather than just allocated (and it has too, in order to perform the function it needs with creation of the extra symbols) - NOLOAD sections will not increment the LMA: and that is just, because otherwise it would start eating up load memory in potentially ROM code (undesirable), so one cannot force increments on NOLOAD sections. I'm mentioning this particular point because I was initially considering that a possibility of handling this would be to use the current syntax and specify something of this sort: /*Make sure on the data side of CMNlibs NOLOAD is used but addresses for data sections are virtual*/ .some.bss 0x1C000000 (NOLOAD) : AT (0xE0000000) { *(S.bss*) *(.__BSS__sect*) } .other.bss 0x1C000000 (NOLOAD) : AT ( LOADADDR (.some.bss) + SIZEOF (.some.bss) ) { *(.other.S.bss*) *(.other.__BSS__sect*) } In my initial opinion I thought this would be a good way to use the current syntax to express intention of overlaps, and then just do LMA checks. But the load address would not increment in this fashion. Initially I did not realize why but reading this ticket and the links in it I realized it makes sense for this to not work because NOLOAD sections cannot increment the LMA address, or else, as the note in https://sourceware.org/ml/binutils/2009-05/msg00330.html says: "Clearly we don't want an image of the bss section in ROM, and the .bss section does not cause the advancement of the allocation point (the LMA)." So it makes sense that LMA does not increase. But I think, at the same time, this shows why with classic commands this problem of intentional vs accidental use cannot be resolved without creating some sort of reinterpretation of the commands which I think is not desirable. But like I said: if it would help anyone, I'd be interested in trying to provide patches/suggestions for a resolution to this problem of: detecting accidental overlaps but allowing intentional ones with classic commands if there is any way of doing that.
(In reply to Alan Modra from comment #7) > Since there seems to be enough interest in this bug for people to actually > write patches, here is what I think would work and be accepted: > > In lang_size_sections_1, near the comment "A backwards move of dot", set a > static global var whenever seeing "dot < last->vma". This should flag all > cases where overlays are used without false triggers. > > Modify lang_check_section_addresses to check for vma overlap when no > overlays are detected. > - Use IGNORE_SECTION again to choose interesting sections (the attached > patch was wrong in treatment of .tbss) > - In the existing lma checking loop, ignore !SEC_LOAD sections. Some > rewriting might be necessary to elegantly handle the case of the first > section being !SEC_LOAD. > - Write another loop to check vmas if overlays not present This would be great (would definitely solve my problem; ld scripts I used didn't use OVERLAY command).
Created attachment 9141 [details] Quick thing to check I'd like to quickly check something with you all. See this attached modified version of the first testcase. I just did a quick check, didn't yet make all of the changes suggested but just plaed some printfs like: if (dot < last->vma && os->bfd_section->size != 0 && dot + os->bfd_section->size <= last->vma) ... printf("Overlays detected!\n"); ... } else { printf("Overlays not detected!\n"); I notice with this, as was my expectation, there are no overlays detected, so then, with no overlays detected, the !SEC_LOAD would pass. But the VMA address check would not. Still this is an intended overlay for my usecase in there. It is just that the intended overlap is of nolaod sections not load sections. Because in either way I do not want nor need memory eaten up by neither of the .bss or .otherbss sections. This usecase is extremely valid and required for me. I'd like to understand the position on this. I would have preferred to stay in tune with vanilla binutils but if it must be, I guess that's the beauty of GPL :) -> we'll just have to fork binutils into something that accepts this use case. If this usecase is accepted as needed (as it definitely is for my usecase) then I would be open to discuss further and help if possible with implementing support for it. Otherwise I wont' bother anymore and go with forking.
Thank you for your patience and contributions to this thread so far. It was in any case very instructive.
The master branch has been updated by Alan Modra <amodra@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a87dd97a2098b7e18ff2574a4e81ae521ef7e6f2 commit a87dd97a2098b7e18ff2574a4e81ae521ef7e6f2 Author: Alan Modra <amodra@gmail.com> Date: Wed Mar 30 17:35:14 2016 +1030 PR18452, ld allows overlapping sections PR 18452 * ldlang.c (maybe_overlays): New static var. (lang_size_sections_1): Set it here. (struct check_sec): New. (sort_sections_by_lma): Adjust for array of structs. (sort_sections_by_vma): New function. (lang_check_section_addresses): Check both LMA and VMA for overlap. * testsuite/ld-scripts/rgn-over7.d: Adjust.
I think the patch I've just committed should catch all the common cases of accidental overlap. Please try it out.
As I was reminded in private email, the patch does not in fact fix the testcase. I'd still like to detect overlays automatically, even ones not using the syntactic sugar of the OVERLAY keyword. So... Perhaps it is reasonably to detect overlays when the current section vma exactly matches the previous section vma. That's what you get when using OVERLAY, and the example of overlays without using OVERLAY in ld.texinfo also starts each overlay section at the same address.
The master branch has been updated by Alan Modra <amodra@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=136a43b762ce7bc692645cc0d9d50c934f9aa392 commit 136a43b762ce7bc692645cc0d9d50c934f9aa392 Author: Alan Modra <amodra@gmail.com> Date: Fri Apr 8 17:52:03 2016 +0930 PR18452, ld allows overlapping sections PR 18452 * ldlang.c (maybe_overlays): Delete. (lang_size_sections_1): Remove code setting maybe_overlays. (lang_check_section_addresses): Instead detect overlays by exact match of section VMAs here. Fix memory leak.
Fixed
Created attachment 9225 [details] Overlay BSS sections working Thank you for this fix. I had a chance to try it out today and yes, this fixes both the original testcase as well as allowing the functionality we require co-exist. Attaching additional testcase here with the additional functionality tested with the last PR which still works. Not sure what's the process of getting from RESOLVED to CLOSED here but as far as I can tell this is a successfully verified fix.