[Bug target/15486] New: -fdata-sections moves COMMON vars to .bss

[Bug target/15486] New: -fdata-sections moves COMMON vars to .bss

[jifl-bugzilla at jifvik dot org]

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).

-- 
           Summary: -fdata-sections moves COMMON vars to .bss
           Product: gcc
           Version: 3.3.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jifl-bugzilla at jifvik dot org
                CC: gcc-bugs at gcc dot gnu dot org
 GCC build triplet: i686-pc-linuxgnulibc2.2
  GCC host triplet: i686-pc-linuxgnulibc2.2
GCC target triplet: arm-elf


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] -fdata-sections moves COMMON vars to .bss

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-05-17 11:27 -------
You can workaround it by -fno-zero-initialized-in-bss.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|target                      |middle-end
           Keywords|                            |wrong-code


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] -fdata-sections moves COMMON vars to .bss

[jifl-bugzilla at jifvik dot org]

------- Additional Comments From jifl-bugzilla at jifvik dot org  2004-05-17 11:32 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] -fdata-sections moves COMMON vars to .bss

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-05-17 11:38 -------
The workaround doesn't work for the testcase since 'x' is not initialized at all.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-05-17 11:40 -------
Regression from GCC 3.2.x.  Not sure if this is "wrong-code" though.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2004-05-17 11:40:41
               date|                            |
            Summary|-fdata-sections moves COMMON|[3.3/3.4/3.5] -fdata-
                   |vars to .bss                |sections moves common vars
                   |                            |to .bss
   Target Milestone|---                         |3.3.4


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[jifl-bugzilla at jifvik dot org]

------- Additional Comments From jifl-bugzilla at jifvik dot org  2004-05-17 21:51 -------
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).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[giovannibajo at libero dot it]

------- Additional Comments From giovannibajo at libero dot it  2004-06-06 03:58 -------
Retargeting to 3.4.1, being a regression on that release branch.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gdr at gcc dot gnu dot org
   Target Milestone|3.3.4                       |3.4.1


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[mmitchel at gcc dot gnu dot org]

------- Additional Comments From mmitchel at gcc dot gnu dot org  2004-06-19 17:46 -------
Postponed until GCC 3.4.2.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|3.4.1                       |3.4.2


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[rearnsha at gcc dot gnu dot org]

------- Additional Comments From rearnsha at gcc dot gnu dot org  2004-06-22 10:26 -------
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.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[jifl-bugzilla at jifvik dot org]

------- Additional Comments From jifl-bugzilla at jifvik dot org  2004-06-22 11:12 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[rearnsha at gcc dot gnu dot org]

------- Additional Comments From rearnsha at gcc dot gnu dot org  2004-06-22 11:33 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[jifl-bugzilla at jifvik dot org]

------- Additional Comments From jifl-bugzilla at jifvik dot org  2004-06-22 19:37 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[jifl-bugzilla at jifvik dot org]

------- Additional Comments From jifl-bugzilla at jifvik dot org  2004-06-22 22:13 -------
That should of course be
 if (DECL_SECTION_NAME (decl) || dont_output_data && DECL_ONE_ONLY(decl))

E&OE :-)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-08-17 23:17 -------
Not really a bug as the C standards say this case is invalid so closing.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[jifl-bugzilla at jifvik dot org]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2935 bytes --]


------- Additional Comments From jifl-bugzilla at jifvik dot org  2004-08-18 01:39 -------
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    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-08-18 02:22 -------
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.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |INVALID


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[jifl-bugzilla at jifvik dot org]

------- Additional Comments From jifl-bugzilla at jifvik dot org  2004-08-18 04:42 -------
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.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[mmitchel at gcc dot gnu dot org]

------- Additional Comments From mmitchel at gcc dot gnu dot org  2004-08-19 21:18 -------
Jason, would you please comment on this discussion, given that one of your
patches seems to be at issue?

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at redhat dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[jason at redhat dot com]

------- Additional Comments From jason at redhat dot com  2004-08-20 21:09 -------
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


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/3.5] -fdata-sections moves common vars to .bss

[mmitchel at gcc dot gnu dot org]

------- Additional Comments From mmitchel at gcc dot gnu dot org  2004-08-29 19:09 -------
Postponed until GCC 3.4.3.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|3.4.2                       |3.4.3


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/4.0 regression] -fdata-sections moves common vars to .bss

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |NEW
      Known to fail|3.3.4 3.4.1 4.0             |3.3.4 3.4.1 4.0.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/4.0 regression] -fdata-sections moves common vars to .bss

[mmitchel at gcc dot gnu dot org]

------- Additional Comments From mmitchel at gcc dot gnu dot org  2004-10-31 02:07 -------
Postponed until GCC 3.4.4.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|3.4.3                       |3.4.4


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/4.0 regression] -fdata-sections moves common vars to .bss

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-12-09 13:15 -------
> 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.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |ebotcazou at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/4.0 regression] -fdata-sections moves common vars to .bss

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-12-14 08:21 -------
Patch at http://gcc.gnu.org/ml/gcc-patches/2004-12/msg00909.html


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/4.0 regression] -fdata-sections moves common vars to .bss

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-12-18 21:50 -------
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



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486

[Bug middle-end/15486] [3.3/3.4/4.0 regression] -fdata-sections moves common vars to .bss

[ebotcazou at gcc dot gnu dot org]

------- Additional Comments From ebotcazou at gcc dot gnu dot org  2004-12-18 21:56 -------
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.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|ebotcazou at gcc dot gnu dot|
                   |org                         |
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|3.4.4                       |4.0.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15486