Putting an all-zero variable into BSS

Putting an all-zero variable into BSS

[Thomas Koenig]

Hi,

There is a large increase in rodata size for derived types in gfortran
starting with gcc 8 due to the presence of large initializers for
derived types. Among other things, this affects SPEC tonto (PR 84487),
but there are also other bug reports which mention this behavior.

The large default initializers are all filled with zeros which end up in
the rodata section, like this:

         .text
         .globl  __types_module_MOD___def_init_types_module_Archive_type
         .section        .rodata
         .align 32
         .type 
__types_module_MOD___def_init_types_module_Archive_type, @object
         .size 
__types_module_MOD___def_init_types_module_Archive_type, 262144
__types_module_MOD___def_init_types_module_Archive_type:
         .zero   262144

... and so on. To reduce the size of the executable, it would make more
sense to put this into the BSS section.

Question: How?  It would be possible to determine from the Fortran
front end which variables would end up all zeros, and mark them in
some way. Alternatively, the middle or back end could do this,
maybe unter the control of a specal option we can then set from
gfortran (-fzero-initialized-in-bss does not do the trick).

What could be the best path forward?

Regards

	Thomas

Re: Putting an all-zero variable into BSS

[Andreas Schwab]

On Apr 03 2019, Thomas Koenig <tkoenig@netcologne.de> wrote:

> The large default initializers are all filled with zeros which end up in
> the rodata section, like this:
>
>         .text
>         .globl  __types_module_MOD___def_init_types_module_Archive_type
>         .section        .rodata
>         .align 32
>         .type __types_module_MOD___def_init_types_module_Archive_type,
> @object
>         .size __types_module_MOD___def_init_types_module_Archive_type,
> 262144
> __types_module_MOD___def_init_types_module_Archive_type:
>         .zero   262144
>
> ... and so on. To reduce the size of the executable, it would make more
> sense to put this into the BSS section.

Note that .bss is writable.  There is no read-only bss section.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Putting an all-zero variable into BSS

[Thomas Koenig]

Hi Andreas,

> The large default initializers are all filled with zeros which end up in
> the rodata section, like this:
> 
> ... and so on. To reduce the size of the executable, it would make more
> sense to put this into the BSS section.
> 
> Note that .bss is writable.  There is no read-only bss section.

Well, nothing is going to write to it (this is not accessible by
user code), so that should not be a problem.

Regards

	Thomas

Re: Putting an all-zero variable into BSS

[Andreas Schwab]

On Apr 03 2019, Thomas Koenig <tkoenig@netcologne.de> wrote:

> Well, nothing is going to write to it (this is not accessible by
> user code), so that should not be a problem.

Then don't make it read-only.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Putting an all-zero variable into BSS

[Thomas Koenig]

Hi Andreas,

>> Well, nothing is going to write to it (this is not accessible by
>> user code), so that should not be a problem.
> Then don't make it read-only.

I tried this, and while it solves the executable size problem, it
causes an OpenMP regression (see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84487#c22 ),
so now I am out of ideas.

Oh well, I would have liked fixing this before 9.0, but it
seems that this may not be possible.

Re: Putting an all-zero variable into BSS

[Richard Biener]

On Thu, Apr 4, 2019 at 9:53 PM Thomas Koenig <tkoenig@netcologne.de> wrote:
>
> Hi Andreas,
>
> >> Well, nothing is going to write to it (this is not accessible by
> >> user code), so that should not be a problem.
> > Then don't make it read-only.
>
> I tried this, and while it solves the executable size problem, it
> causes an OpenMP regression (see
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84487#c22 ),
> so now I am out of ideas.
>
> Oh well, I would have liked fixing this before 9.0, but it
> seems that this may not be possible.

Putting readonly data into .rodata isn't required by the C standard I think
so we could freely choose .bss for data exceeding a reasonable
size limit.  IIRC GCC behaved one or another way in the past already
and the last change might be due to security concerns.  Btw, large
all-zeros constant objects don't make very much sense...

Richard.

Re: Putting an all-zero variable into BSS

[Thomas Koenig]

Am 05.04.19 um 12:15 schrieb Richard Biener:

> Putting readonly data into .rodata isn't required by the C standard I think
> so we could freely choose .bss for data exceeding a reasonable
> size limit.

That would be the best solution, I think.

> IIRC GCC behaved one or another way in the past already
> and the last change might be due to security concerns.

I cannot speak to that. If there is concern for C, we could also
limit this to Fortran.

> Btw, large
> all-zeros constant objects don't make very much sense...

I am well aware of this, but we're not going to change this
before the GCC 9 release :-)

So, would it be possible for you to make the change wrt .bss?
I would not have the first idea where to start looking.

Regards

	Thomas

Re: Putting an all-zero variable into BSS

[Richard Biener]

On April 6, 2019 3:59:41 PM GMT+02:00, Thomas Koenig <tkoenig@netcologne.de> wrote:
>Am 05.04.19 um 12:15 schrieb Richard Biener:
>
>> Putting readonly data into .rodata isn't required by the C standard I
>think
>> so we could freely choose .bss for data exceeding a reasonable
>> size limit.
>
>That would be the best solution, I think.
>
>> IIRC GCC behaved one or another way in the past already
>> and the last change might be due to security concerns.
>
>I cannot speak to that. If there is concern for C, we could also
>limit this to Fortran.
>
>> Btw, large
>> all-zeros constant objects don't make very much sense...
>
>I am well aware of this, but we're not going to change this
>before the GCC 9 release :-)
>
>So, would it be possible for you to make the change wrt .bss?
>I would not have the first idea where to start looking.

I don't know without looking, but I'd start at assemble_variable in varasm.c.

Richard. 

>Regards
>
>	Thomas

Re: Putting an all-zero variable into BSS

[Thomas Koenig]

[-- Attachment #1: Type: text/plain, Size: 303 bytes --]

Hi Richard,

> I don't know without looking, but I'd start at assemble_variable in varasm.c.

Thanks.  I've done that, and this is what a patch could look like.
However, I will not have time to formally submit this until next
weekend.

In the meantime, comments are still welcome :-)

Regards

	Thomas


[-- Attachment #2: p4.diff --]
[-- Type: text/x-patch, Size: 1462 bytes --]

Index: fortran/trans-decl.c
===================================================================
--- fortran/trans-decl.c	(Revision 270182)
+++ fortran/trans-decl.c	(Arbeitskopie)
@@ -1865,7 +1865,10 @@ gfc_get_symbol_decl (gfc_symbol * sym)
 
   if (sym->attr.vtab
       || (sym->name[0] == '_' && gfc_str_startswith (sym->name, "__def_init")))
-    TREE_READONLY (decl) = 1;
+    {
+      TREE_READONLY (decl) = 1;
+      DECL_ARTIFICIAL (decl) = 1;
+    }
 
   return decl;
 }
Index: varasm.c
===================================================================
--- varasm.c	(Revision 270182)
+++ varasm.c	(Arbeitskopie)
@@ -1007,9 +1007,13 @@ decode_reg_name (const char *name)
 bool
 bss_initializer_p (const_tree decl, bool named)
 {
-  /* Do not put non-common constants into the .bss section, they belong in
-     a readonly section, except when NAMED is true.  */
-  return ((!TREE_READONLY (decl) || DECL_COMMON (decl) || named)
+  /* Do not put non-common constants into the .bss section, they
+     belong in a readonly section, except when NAMED is true or when
+     we are dealing with an artificial declaration above a certain
+     size.  */
+  return ((!TREE_READONLY (decl) || DECL_COMMON (decl) || named
+	   || ((tree_to_uhwi (DECL_SIZE_UNIT (decl)) > 256
+		&& DECL_ARTIFICIAL (decl))))
 	  && (DECL_INITIAL (decl) == NULL
 	      /* In LTO we have no errors in program; error_mark_node is used
 	         to mark offlined constructors.  */

Re: Putting an all-zero variable into BSS

[Andrew Haley]

On 4/7/19 5:03 PM, Thomas Koenig wrote:
> Hi Richard,
> 
>> I don't know without looking, but I'd start at assemble_variable in varasm.c.
> 
> Thanks.  I've done that, and this is what a patch could look like.
> However, I will not have time to formally submit this until next
> weekend.
> 
> In the meantime, comments are still welcome :-)

Did you look at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83100

This was the change that caused this behaviour.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: Putting an all-zero variable into BSS

[Richard Biener]

On Mon, Apr 8, 2019 at 10:38 AM Andrew Haley <aph@redhat.com> wrote:
>
> On 4/7/19 5:03 PM, Thomas Koenig wrote:
> > Hi Richard,
> >
> >> I don't know without looking, but I'd start at assemble_variable in varasm.c.
> >
> > Thanks.  I've done that, and this is what a patch could look like.
> > However, I will not have time to formally submit this until next
> > weekend.
> >
> > In the meantime, comments are still welcome :-)
>
> Did you look at
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83100
>
> This was the change that caused this behaviour.

Actually that just changed the behavior for DECL_COMMONs
which may of course match the fortran case here in case you
bisected this.  OTOH DECL_COMMONs are tentative and
do not have an initializer (and not go to .rodata either).

Richard.

>
> --
> Andrew Haley
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: Putting an all-zero variable into BSS

[Richard Biener]

On Mon, Apr 8, 2019 at 11:33 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 10:38 AM Andrew Haley <aph@redhat.com> wrote:
> >
> > On 4/7/19 5:03 PM, Thomas Koenig wrote:
> > > Hi Richard,
> > >
> > >> I don't know without looking, but I'd start at assemble_variable in varasm.c.
> > >
> > > Thanks.  I've done that, and this is what a patch could look like.
> > > However, I will not have time to formally submit this until next
> > > weekend.
> > >
> > > In the meantime, comments are still welcome :-)
> >
> > Did you look at
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83100
> >
> > This was the change that caused this behaviour.
>
> Actually that just changed the behavior for DECL_COMMONs
> which may of course match the fortran case here in case you
> bisected this.  OTOH DECL_COMMONs are tentative and
> do not have an initializer (and not go to .rodata either).

That is, the C testcase

const char x[1024*1024] = {};

reproduces the "issue".  The comment in bss_initializer_p though
explicitely says

  /* Do not put non-common constants into the .bss section, they belong in
     a readonly section, except when NAMED is true.  */
  return ((!TREE_READONLY (decl) || DECL_COMMON (decl) || named)

(where named refers to explicit .bss section marked decls).  Note
the docs for -fzero-initialized-in-bss doesn't mention that this doesn't
apply for readonly variables.

Richard.

> Richard.
>
> >
> > --
> > Andrew Haley
> > Java Platform Lead Engineer
> > Red Hat UK Ltd. <https://www.redhat.com>
> > EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: Putting an all-zero variable into BSS

[Joseph Myers]

On Mon, 8 Apr 2019, Richard Biener wrote:

> That is, the C testcase
> 
> const char x[1024*1024] = {};
> 
> reproduces the "issue".  The comment in bss_initializer_p though
> explicitely says
> 
>   /* Do not put non-common constants into the .bss section, they belong in
>      a readonly section, except when NAMED is true.  */
>   return ((!TREE_READONLY (decl) || DECL_COMMON (decl) || named)
> 
> (where named refers to explicit .bss section marked decls).  Note
> the docs for -fzero-initialized-in-bss doesn't mention that this doesn't
> apply for readonly variables.

I'd say it's a security risk (breaks expected hardening properties) for a 
const variable with static storage duration not to end up in read-only 
memory, regardless of its size or contents, so such variables should not 
be put in BSS by default.

-- 
Joseph S. Myers
joseph@codesourcery.com