This is a regression from GCC 3.2.1 (at least). If I create a file foo.c containing simply: int x; and compile it with: arm-elf-gcc -c foo.c then I can get the following with 'arm-elf-nm foo.o' 00000004 C x However if I compile with arm-elf-gcc -c -fdata-sections foo.c then with arm-elf-nm I get: 00000000 B x i.e. x is in the BSS. This results in breaking the long-standing behaviour of common variables being merged. If you typed the following: cat >foo2.c int x=0; int main(){ return x; } arm-elf-gcc -c -fdata-sections foo.c arm-elf-gcc -c -fdata-sections foo2.c arm-elf-gcc -o foo foo.o foo2.o then you get: foo2.o(.data.x+0x0): multiple definition of `x' foo.o(.bss.x+0x0): first defined here I can see how this behaviour would be beneficial if "-fno-common" was passed to gcc and if not using "legacy" code (actually the real code I'm having trouble with is derived from a BSD kernel of about 4 years ago, so not that old), but in the absence of -fno-common, -fdata-sections should not operate on uninitialised data. Perhaps the default should be -fno-common, and I should be required to pass -fcommon to get the old behaviour, I'm not bothered. Or alternatively perhaps this treatment of uninitialised data should only happen with a separate -fbss-sections option (for symmetry with -ffunction-sections and -fdata-sections).
You can workaround it by -fno-zero-initialized-in-bss.
I think in my case not using -fdata-sections would be a better (or less bad :-)) workaround than increasing the initialized data section that much. But might be useful for others to know if they search for this bug in the archives.
The workaround doesn't work for the testcase since 'x' is not initialized at all.
Regression from GCC 3.2.x. Not sure if this is "wrong-code" though.
No I wouldn't consider it wrong code. I think it's sensible to allow nice modern programs to benefit from garbage collection of BSS sections. But since it breaks older code, only as an option (whether it be default on or default off I'm not bothered).
Retargeting to 3.4.1, being a regression on that release branch.
Postponed until GCC 3.4.2.
I'm not convinced this is even a bug. Looking at the assembly code generated for the test case int x; we see: .file "foo.c" .global x .section .bss.x,"aw",%nobits .align 2 .type x, %object .size x, 4 x: .space 4 So indeed the compiler has done as asked and given X a section of its own (.bss.x). I'm not aware that it is possible to do this using the common model - at least, not in ELF. Section 6.2.2 of the C99 rationale describes 4 linkage models: Common, Relaxed Ref/Def, Strict Ref/Def and Initialization. GCC normally implements the Relaxed Ref/Def model, since this is tradionally what Unix uses; but I don't think it would be unreasonable to assert that -fdata-sections requires the Strict Ref/Def model. My inclination would be to document that -fdata-sections implies -fno-common.
Firstly the compiler has arguably not done as asked, because -fdata-sections did not used to affect the bss section at all. That makes this a regression. As I suggested, perhaps this should be a separate -fbss-sections option. Secondly, what you are suggesting is deliberately breaking traditional unix code, including moderately recent BSD kernels, as that's where the eCos code came from (specifically TCP/IP). Sometimes such a thing could be justified if an optimisation cannot be achieved any other way, but that is not true here: a separate -fbss-sections option or similar (-fdata-sections -fno-bss-sections if you want to retain this behaviour for -fdata-sections) could do this. Finally, this is not quite the same as saying -fdata-sections implies -fno-common. -fno-common is used to, essentially, put BSS data in the .data section. That is not what -fdata-sections is now doing, although it is what one would expect it to do if you did pass -fno-common (if you see the distinction I mean). So it would be incorrect to document that -fdata-sections implies -fno-common.
Just because the compiler changed behaviour doesn't necessarily make it a regression. Perhaps the change was intentional. It could be equally argued that failing to put uninitialized data in separate sections was the bug that was being fixed when the change was introduced. It would be necessary to do some archeology to find out which patch caused the change, and why it was proposed in the first place.
I'm not arguing against the current behaviour as such. I think the current behaviour would be a good idea as it allows GC of BSS sections; but it should not be rolled into an existing option with an existing known action, breaking long-standing behaviour used in lots of code. Or if it is, there should be a way to stop it in this case. Anyway, after some archaeology (:-)), I have found the patch that did it: http://gcc.gnu.org/ml/gcc-patches/2002-03/msg00921.html Specifically moving resolve_unique_section() before the check of DECL_SECTION_NAME which is: /* If the decl has been given an explicit section name, then it isn't common, and shouldn't be handled as such. */ if (DECL_SECTION_NAME (decl) || dont_output_data) ; /* We don't implement common thread-local data at present. */ else [etc.] It isn't clear what the rationale for this change was: the "more fallout" referred to here is I believe subsequent to http://gcc.gnu.org/ml/gcc-patches/2002-03/msg00768.html perhaps which is in turn after Kaveh's changes to add -fno-zero-initialized-in-bss in http://gcc.gnu.org/ml/gcc-patches/2002-03/msg00062.html Jason presumably had come across some reason why Kaveh's change broke something. But I don't think it was deliberately intended to remove common variable support when using -fdata-sections. Given the other purpose of resolve_unique_section() I would guess it's to do with the correct implementation of transparent unions, which are used in several high-profile places in glibc, including most socket functions and an argument to the wait() family of functions. Are there other ways DECL_SECTION_NAME could be set before this point? As to how to address this then, I'm not too hot on GCC internals, but the old behaviour could be returned simply by making the above cited if actually: if (DECL_ONE_ONLY(decl) && DECL_SECTION_NAME (decl) || dont_output_data) This would then allow the else clauses to be followed as expected. This is on the assumption that the reasoning behind Jason's change is as per above. If it's something else, hopefully the above info helps someone more knowledgable than I work out what the rationale is. Although I would say that someone should consider allowing -fdata-sections (or a new -fbss-sections option) to operate on the BSS as well in future, due to the space savings that could give, as long as there's a way to get back the current behaviour.
That should of course be if (DECL_SECTION_NAME (decl) || dont_output_data && DECL_ONE_ONLY(decl)) E&OE :-)
Not really a bug as the C standards say this case is invalid so closing.
Richard Earnshaw gave a most helpful analysis describing the various linkage models mentioned in 6.22 of the C99 standard rationale. I quote from there about the relaxed ref/def model that GCC previously implemented: "The UNIX operating system C compiler and linker implement this model, which is recognized as a common extension to the C language (see §K.5.11). UNIX C programs which take advantage of this model are standard conforming in their environment, but are not maximally portable (not strictly conforming)." In other words, the programs are indeed standard conforming, not "invalid". Strict conformance is akin to -pedantic - preventing an extension. Are you perhaps saying that 6.9 para 5 of C99 mandates only one definition? Actually that does not follow, as that only applies if the object is declared with an external definition. It does not apply, as in the "legacy" code in question, if the object is always defined (therefore becomes common), and never declared with extern. The standard does not mandate breaking such code. The code I have isn't really legacy code - it's only a few years old and was taken from BSD post-1999 even. At the very least to be completely pedantic about standards conformance, and break compilation of such code which has been deliberately been maintained to work to date goes against the GNU spirit of making things work. Especially when the change is clearly unintentional. I've even suggested what I believe to be a simple fix. Breaking a large body of past code should not be something done lightly, certainly not in a patch that doesn't mention it and was intended to fix other issues; and it isn't a decision that should be taken by one person just in this bug. At the very least there are other ramifications: I suggest if Andrew wants this change to be permanent, he needs to submit a change to the -fno-common, and __attribute__((nocommon)), __attribute__((section(...))) documentation which are all clearly wrong if common data is eliminated. There are also quite a few comments in GCC that need to be fixed to reflect this proposed new reality; and some code that could probably now be removed. I've reopened this bug to indicate that either this issue is still live as gcc should not be suddenly (and without warning or prior deprecation) breaking common Unix code; or that someone needs to fix the documentation and other bits as above. At least that way with a proposed doc patch, the removal of common variable support can be brought to a wider audience than this bug report.
What is invalid about this problem is that x is initialized in one TU but not another, I will note note that on darwin this will fail no matter what.
Then remove the initialiser. The problem is just the same. I didn't realise it would make a difference to anyone's opinion. In my previous reply, I did lose a little perspective rather mistakenly (the passage of time since this bug was submitted I guess) - this misbehaviour is specific to -fdata-sections. What this means is that gcc behaves differently with -fdata-sections and without, and breaks code with -fdata-sections, and not without, which is not meant to happen. Again: the change was not intentional, and is almost certainly easily restored to the previous behaviour.
Jason, would you please comment on this discussion, given that one of your patches seems to be at issue?
Subject: Re: [3.3/3.4/3.5] -fdata-sections moves common vars to .bss I believe that the problem which my patch was addressing was that Kaveh's patch was causing variables to end up in .bss which needed to be in a special section. I'm not sure why that would be, since asm_emit_uninitialized does call resolve_unique_section. If you want to try reverting my patch and seeing if anything breaks, I'm open to that. Jason
Postponed until GCC 3.4.3.
Postponed until GCC 3.4.4.
> I believe that the problem which my patch was addressing was that Kaveh's > patch was causing variables to end up in .bss which needed to be in a > special section. I'm not sure why that would be, since > asm_emit_uninitialized does call resolve_unique_section. If you want to > try reverting my patch and seeing if anything breaks, I'm open to that. I don't think your patch can be reverted: the point is that resolve_unique_section must be called before DECL_SECTION_NAME is tested. I'm going to try to restore the original behaviour with an ad-hoc approach.
Patch at http://gcc.gnu.org/ml/gcc-patches/2004-12/msg00909.html
Subject: Bug 15486 CVSROOT: /cvs/gcc Module name: gcc Changes by: ebotcazou@gcc.gnu.org 2004-12-18 21:49:56 Modified files: gcc : ChangeLog varasm.c gcc/testsuite : ChangeLog Added files: gcc/testsuite/gcc.dg: fdata-sections-1.c Log message: PR middle-end/15486 * varasm.c (asm_emit_uninitialised): Return early if a custom section is requested. (assemble_variable): Revert 2002-03-15 patch. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.6883&r2=2.6884 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/varasm.c.diff?cvsroot=gcc&r1=1.468&r2=1.469 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.4781&r2=1.4782 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/fdata-sections-1.c.diff?cvsroot=gcc&r1=NONE&r2=1.1
See http://gcc.gnu.org/ml/gcc-patches/2004-12/msg01295.html Fixed in the upcoming 4.0.0 only, as per the RM's ruling.