Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath via gcc-patches]

Committed.

Thanks,
Roland

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Ian Lance Taylor via gcc-patches]

On Wed, Jun 13, 2018 at 5:52 PM, Roland McGrath  wrote:
> A direct cherry-pick of the trunk change
> (db7548a2771bbf34cf7430712af7ac670b429958 / r259969) applies fine to
> today's 8 branch and has no check-gcc regressions on x86_64-linux-gnu.
>
> OK to commit to 8 branch now?

Sure.

Ian

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath via gcc-patches]

A direct cherry-pick of the trunk change
(db7548a2771bbf34cf7430712af7ac670b429958 / r259969) applies fine to
today's 8 branch and has no check-gcc regressions on x86_64-linux-gnu.

OK to commit to 8 branch now?


Thanks,
Roland

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath via gcc-patches]

Committed.

Thanks,
Roland

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Ian Lance Taylor via gcc-patches]

On Sat, Apr 28, 2018 at 2:42 AM, Roland McGrath  wrote:
> I'm back for stage 1!
>
> The same patch from https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01549.html
> rebases cleanly and I didn't change anything but the date on the log entry
> since what I posted there.  The fresh rebase is on the roland/pr77609 git
> branch for your convenience.
>
> It has no check-gcc failures on x86_64-linux-gnu.
>
> OK to commit to trunk now?
>
> When will be the right time to raise the question of backporting it,
> perhaps shortly after the 8 release?

This is OK for trunk.  Thanks.

I  think a backport would be OK after a month.

Ian

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath via gcc-patches]

ping

On Sat, Apr 28, 2018 at 2:42 AM Roland McGrath  wrote:

> I'm back for stage 1!

> The same patch from
https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01549.html
> rebases cleanly and I didn't change anything but the date on the log entry
> since what I posted there.  The fresh rebase is on the roland/pr77609 git
> branch for your convenience.

> It has no check-gcc failures on x86_64-linux-gnu.

> OK to commit to trunk now?

> When will be the right time to raise the question of backporting it,
> perhaps shortly after the 8 release?


> Thanks,
> Roland

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath via gcc-patches]

I'm back for stage 1!

The same patch from https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01549.html
rebases cleanly and I didn't change anything but the date on the log entry
since what I posted there.  The fresh rebase is on the roland/pr77609 git
branch for your convenience.

It has no check-gcc failures on x86_64-linux-gnu.

OK to commit to trunk now?

When will be the right time to raise the question of backporting it,
perhaps shortly after the 8 release?


Thanks,
Roland

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath via gcc-patches]

OK.  I'll come back in stage 1.

Thanks,
Roland

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Ian Lance Taylor via gcc-patches]

On Wed, Feb 28, 2018 at 3:56 PM, Roland McGrath  wrote:
> On Tue, Feb 27, 2018 at 8:14 PM, Ian Lance Taylor  wrote:
>> Still OK, but it should wait until after the tree is back in stage 1.
>
> I've been hoping to get this fixed on stable branches (6, 7).
> Are you saying that this bug can only be fixed in 9?
> Or will it be OK to backport to 6, 7, and 8 once stage 1 opens for 9?

Since you already ran into trouble, I'm worried that will be other
trouble.  At least, it's hard for me to convince myself that there
won't be any other trouble.  And I don't think we should introduce
trouble at this point in the release cycle.  So, yes, I'd prefer to
see this fixed in GCC 9, and, once people have had a chance to test it
on various systems, backport to earlier versions if appropriate.

Ian

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath via gcc-patches]

On Tue, Feb 27, 2018 at 8:14 PM, Ian Lance Taylor  wrote:
> Still OK, but it should wait until after the tree is back in stage 1.

I've been hoping to get this fixed on stable branches (6, 7).
Are you saying that this bug can only be fixed in 9?
Or will it be OK to backport to 6, 7, and 8 once stage 1 opens for 9?

Thanks,
Roland

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Ian Lance Taylor via gcc-patches]

On Tue, Feb 27, 2018 at 6:01 PM, Roland McGrath  wrote:
> On Mon, Feb 26, 2018 at 8:11 PM, Ian Lance Taylor  wrote:
>> You are recreating the conditions used in
>> default_elf_asm_named_section, so I think you ought to have comments
>> referring back and forth between them.
>>
>> This is OK with the two additional comments.
>
> Thanks for the review.  I've added those comments.
>
> However, in testing on x86_64-linux-gnu it caused a regression in:
> gcc/testsuite/gcc.target/i386/pr25254.c
> which got the "section type conflict" error.
>
> This is because x86_64_elf_select_section for that case calls:
> get_section (".lrodata", SECTION_LARGE, NULL)
> but something else had previously instantiated the section via
> the section_type_flags logic that now adds in SECTION_NOTYPE.
>
> I addressed this by making get_section accept having SECTION_NOTYPE and not
> as a non-conflict if none of SECTION_BSS et al is present.  That seemed
> like a better bet than finding every get_section caller and making sure
> they use SECTION_NOTYPE when appropriate.  But I'm not sure if there might
> be some downside to that logic or if there is a third way to resolve this
> that's better than either of those two.
>
> Here's the new patch I'd like to commit.  It has no regressions on
> x86_64-linux-gnu, but I'm not set up to test other configurations.
>
>
> gcc/
> 2018-02-27  Roland McGrath  
>
> PR other/77609
> * varasm.c (default_section_type_flags): Set SECTION_NOTYPE for
> any section for which we don't know a specific type it should have,
> regardless of name.  Previously this was done only for the exact
> names ".init_array", ".fini_array", and ".preinit_array".
> (default_elf_asm_named_section): Add comment about
> relationship with default_section_type_flags and SECTION_NOTYPE.
> (get_section): Don't consider it a type conflict if one side has
> SECTION_NOTYPE and the other doesn't, as long as neither has the
> SECTION_BSS et al used in the default_section_type_flags logic.

Still OK, but it should wait until after the tree is back in stage 1.

Ian

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath via gcc-patches]

On Mon, Feb 26, 2018 at 8:11 PM, Ian Lance Taylor  wrote:
> You are recreating the conditions used in
> default_elf_asm_named_section, so I think you ought to have comments
> referring back and forth between them.
>
> This is OK with the two additional comments.

Thanks for the review.  I've added those comments.

However, in testing on x86_64-linux-gnu it caused a regression in:
gcc/testsuite/gcc.target/i386/pr25254.c
which got the "section type conflict" error.

This is because x86_64_elf_select_section for that case calls:
get_section (".lrodata", SECTION_LARGE, NULL)
but something else had previously instantiated the section via
the section_type_flags logic that now adds in SECTION_NOTYPE.

I addressed this by making get_section accept having SECTION_NOTYPE and not
as a non-conflict if none of SECTION_BSS et al is present.  That seemed
like a better bet than finding every get_section caller and making sure
they use SECTION_NOTYPE when appropriate.  But I'm not sure if there might
be some downside to that logic or if there is a third way to resolve this
that's better than either of those two.

Here's the new patch I'd like to commit.  It has no regressions on
x86_64-linux-gnu, but I'm not set up to test other configurations.


gcc/
2018-02-27  Roland McGrath  

PR other/77609
* varasm.c (default_section_type_flags): Set SECTION_NOTYPE for
any section for which we don't know a specific type it should have,
regardless of name.  Previously this was done only for the exact
names ".init_array", ".fini_array", and ".preinit_array".
(default_elf_asm_named_section): Add comment about
relationship with default_section_type_flags and SECTION_NOTYPE.
(get_section): Don't consider it a type conflict if one side has
SECTION_NOTYPE and the other doesn't, as long as neither has the
SECTION_BSS et al used in the default_section_type_flags logic.

diff --git a/gcc/varasm.c b/gcc/varasm.c
index 6e345d39d31..e488f866011 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -296,6 +296,17 @@ get_section (const char *name, unsigned int
flags, tree decl)
   else
 {
   sect = *slot;
+  /* It is fine if one of the sections has SECTION_NOTYPE as long as
+ the other has none of the contrary flags (see the logic at the end
+ of default_section_type_flags, below).  */
+  if (((sect->common.flags ^ flags) & SECTION_NOTYPE)
+  && !((sect->common.flags | flags)
+   & (SECTION_CODE | SECTION_BSS | SECTION_TLS | SECTION_ENTSIZE
+  | (HAVE_COMDAT_GROUP ? SECTION_LINKONCE : 0
+{
+  sect->common.flags |= SECTION_NOTYPE;
+  flags |= SECTION_NOTYPE;
+}
   if ((sect->common.flags & ~SECTION_DECLARED) != flags
  && ((sect->common.flags | flags) & SECTION_OVERRIDE) == 0)
{
@@ -6361,15 +6372,23 @@ default_section_type_flags (tree decl, const
char *name, int reloc)
   || strncmp (name, ".gnu.linkonce.tb.", 17) == 0)
 flags |= SECTION_TLS | SECTION_BSS;

-  /* These three sections have special ELF types.  They are neither
- SHT_PROGBITS nor SHT_NOBITS, so when changing sections we don't
- want to print a section type (@progbits or @nobits).  If someone
- is silly enough to emit code or TLS variables to one of these
- sections, then don't handle them specially.  */
-  if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS))
-  && (strcmp (name, ".init_array") == 0
- || strcmp (name, ".fini_array") == 0
- || strcmp (name, ".preinit_array") == 0))
+  /* Various sections have special ELF types that the assembler will
+ assign by default based on the name.  They are neither SHT_PROGBITS
+ nor SHT_NOBITS, so when changing sections we don't want to print a
+ section type (@progbits or @nobits).  Rather than duplicating the
+ assembler's knowledge of what those special name patterns are, just
+ let the assembler choose the type if we don't know a specific
+ reason to set it to something other than the default.  SHT_PROGBITS
+ is the default for sections whose name is not specially known to
+ the assembler, so it does no harm to leave the choice to the
+ assembler when @progbits is the best thing we know to use.  If
+ someone is silly enough to emit code or TLS variables to one of
+ these sections, then don't handle them specially.
+
+ default_elf_asm_named_section (below) handles the BSS, TLS, ENTSIZE, and
+ LINKONCE cases when NOTYPE is not set, so leave those to its logic.  */
+  if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS | SECTION_ENTSIZE))
+  && !(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE)))
 flags |= SECTION_NOTYPE;

   return flags;
@@ -6455,6 +6474,10 @@ default_elf_asm_named_section (const char
*name, unsigned int flags,

   fprintf (asm_out_file, "\t.section\t%s,\"%s\"", name, 

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Ian Lance Taylor via gcc-patches]

On Sat, Feb 24, 2018 at 1:54 AM, Roland McGrath via gcc-patches
 wrote:
> Anybody want to look at this?
>
> It rebases identically on today's trunk.  I'd like to commit it to
> trunk and gcc-7-branch and gcc-6-branch ideally.

You are recreating the conditions used in
default_elf_asm_named_section, so I think you ought to have comments
referring back and forth between them.

This is OK with the two additional comments.

Ian



> On Thu, Sep 22, 2016 at 1:15 PM, Roland McGrath  wrote:
>> ping?
>>
>> On Thu, Sep 15, 2016 at 4:09 PM, Roland McGrath  wrote:
>>> This fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77609 (which I've
>>> just filed).
>>>
>>> OK for trunk?
>>>
>>> I'm not sure if this kind of fix is appropriate for gcc-6-branch or not,
>>> but I'd like to backport it there too if it is acceptable.
>>>
>>>
>>> Thanks,
>>> Roland
>>>
>>>
>>> gcc/
>>> 2016-09-15  Roland McGrath  <
>>>
>>> PR other/77609
>>> * varasm.c (default_section_type_flags): Set SECTION_NOTYPE for
>>> any section for which we don't know a specific type it should have,
>>> regardless of name.  Previously this was done only for the exact
>>> names ".init_array", ".fini_array", and ".preinit_array".
>>>
>>> diff --git a/gcc/varasm.c b/gcc/varasm.c
>>> index 00a9b30..0d7ea38 100644
>>> --- a/gcc/varasm.c
>>> +++ b/gcc/varasm.c
>>> @@ -6210,15 +6210,20 @@ default_section_type_flags (tree decl, const
>>> char *name, int reloc)
>>>|| strncmp (name, ".gnu.linkonce.tb.", 17) == 0)
>>>  flags |= SECTION_TLS | SECTION_BSS;
>>>
>>> -  /* These three sections have special ELF types.  They are neither
>>> - SHT_PROGBITS nor SHT_NOBITS, so when changing sections we don't
>>> - want to print a section type (@progbits or @nobits).  If someone
>>> - is silly enough to emit code or TLS variables to one of these
>>> - sections, then don't handle them specially.  */
>>> -  if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS))
>>> -  && (strcmp (name, ".init_array") == 0
>>> - || strcmp (name, ".fini_array") == 0
>>> - || strcmp (name, ".preinit_array") == 0))
>>> +  /* Various sections have special ELF types that the assembler will
>>> + assign by default based on the name.  They are neither SHT_PROGBITS
>>> + nor SHT_NOBITS, so when changing sections we don't want to print a
>>> + section type (@progbits or @nobits).  Rather than duplicating the
>>> + assembler's knowledge of what those special name patterns are, just
>>> + let the assembler choose the type if we don't know a specific
>>> + reason to set it to something other than the default.  SHT_PROGBITS
>>> + is the default for sections whose name is not specially known to
>>> + the assembler, so it does no harm to leave the choice to the
>>> + assembler when @progbits is the best thing we know to use.  If
>>> + someone is silly enough to emit code or TLS variables to one of
>>> + these sections, then don't handle them specially.  */
>>> +  if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS | 
>>> SECTION_ENTSIZE))
>>> +  && !(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE)))
>>>  flags |= SECTION_NOTYPE;
>>>
>>>return flags;

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath via gcc-patches]

Anybody want to look at this?

It rebases identically on today's trunk.  I'd like to commit it to
trunk and gcc-7-branch and gcc-6-branch ideally.


Thanks,
Roland

On Thu, Sep 22, 2016 at 1:15 PM, Roland McGrath  wrote:
> ping?
>
> On Thu, Sep 15, 2016 at 4:09 PM, Roland McGrath  wrote:
>> This fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77609 (which I've
>> just filed).
>>
>> OK for trunk?
>>
>> I'm not sure if this kind of fix is appropriate for gcc-6-branch or not,
>> but I'd like to backport it there too if it is acceptable.
>>
>>
>> Thanks,
>> Roland
>>
>>
>> gcc/
>> 2016-09-15  Roland McGrath  <
>>
>> PR other/77609
>> * varasm.c (default_section_type_flags): Set SECTION_NOTYPE for
>> any section for which we don't know a specific type it should have,
>> regardless of name.  Previously this was done only for the exact
>> names ".init_array", ".fini_array", and ".preinit_array".
>>
>> diff --git a/gcc/varasm.c b/gcc/varasm.c
>> index 00a9b30..0d7ea38 100644
>> --- a/gcc/varasm.c
>> +++ b/gcc/varasm.c
>> @@ -6210,15 +6210,20 @@ default_section_type_flags (tree decl, const
>> char *name, int reloc)
>>|| strncmp (name, ".gnu.linkonce.tb.", 17) == 0)
>>  flags |= SECTION_TLS | SECTION_BSS;
>>
>> -  /* These three sections have special ELF types.  They are neither
>> - SHT_PROGBITS nor SHT_NOBITS, so when changing sections we don't
>> - want to print a section type (@progbits or @nobits).  If someone
>> - is silly enough to emit code or TLS variables to one of these
>> - sections, then don't handle them specially.  */
>> -  if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS))
>> -  && (strcmp (name, ".init_array") == 0
>> - || strcmp (name, ".fini_array") == 0
>> - || strcmp (name, ".preinit_array") == 0))
>> +  /* Various sections have special ELF types that the assembler will
>> + assign by default based on the name.  They are neither SHT_PROGBITS
>> + nor SHT_NOBITS, so when changing sections we don't want to print a
>> + section type (@progbits or @nobits).  Rather than duplicating the
>> + assembler's knowledge of what those special name patterns are, just
>> + let the assembler choose the type if we don't know a specific
>> + reason to set it to something other than the default.  SHT_PROGBITS
>> + is the default for sections whose name is not specially known to
>> + the assembler, so it does no harm to leave the choice to the
>> + assembler when @progbits is the best thing we know to use.  If
>> + someone is silly enough to emit code or TLS variables to one of
>> + these sections, then don't handle them specially.  */
>> +  if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS | 
>> SECTION_ENTSIZE))
>> +  && !(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE)))
>>  flags |= SECTION_NOTYPE;
>>
>>return flags;

Re: [PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath]

ping?

On Thu, Sep 15, 2016 at 4:09 PM, Roland McGrath  wrote:
> This fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77609 (which I've
> just filed).
>
> OK for trunk?
>
> I'm not sure if this kind of fix is appropriate for gcc-6-branch or not,
> but I'd like to backport it there too if it is acceptable.
>
>
> Thanks,
> Roland
>
>
> gcc/
> 2016-09-15  Roland McGrath  <
>
> PR other/77609
> * varasm.c (default_section_type_flags): Set SECTION_NOTYPE for
> any section for which we don't know a specific type it should have,
> regardless of name.  Previously this was done only for the exact
> names ".init_array", ".fini_array", and ".preinit_array".
>
> diff --git a/gcc/varasm.c b/gcc/varasm.c
> index 00a9b30..0d7ea38 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -6210,15 +6210,20 @@ default_section_type_flags (tree decl, const
> char *name, int reloc)
>|| strncmp (name, ".gnu.linkonce.tb.", 17) == 0)
>  flags |= SECTION_TLS | SECTION_BSS;
>
> -  /* These three sections have special ELF types.  They are neither
> - SHT_PROGBITS nor SHT_NOBITS, so when changing sections we don't
> - want to print a section type (@progbits or @nobits).  If someone
> - is silly enough to emit code or TLS variables to one of these
> - sections, then don't handle them specially.  */
> -  if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS))
> -  && (strcmp (name, ".init_array") == 0
> - || strcmp (name, ".fini_array") == 0
> - || strcmp (name, ".preinit_array") == 0))
> +  /* Various sections have special ELF types that the assembler will
> + assign by default based on the name.  They are neither SHT_PROGBITS
> + nor SHT_NOBITS, so when changing sections we don't want to print a
> + section type (@progbits or @nobits).  Rather than duplicating the
> + assembler's knowledge of what those special name patterns are, just
> + let the assembler choose the type if we don't know a specific
> + reason to set it to something other than the default.  SHT_PROGBITS
> + is the default for sections whose name is not specially known to
> + the assembler, so it does no harm to leave the choice to the
> + assembler when @progbits is the best thing we know to use.  If
> + someone is silly enough to emit code or TLS variables to one of
> + these sections, then don't handle them specially.  */
> +  if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS | SECTION_ENTSIZE))
> +  && !(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE)))
>  flags |= SECTION_NOTYPE;
>
>return flags;

[PATCH PR other/77609] Let the assembler choose ELF section types for miscellaneous named sections

[Roland McGrath]

This fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77609 (which I've
just filed).

OK for trunk?

I'm not sure if this kind of fix is appropriate for gcc-6-branch or not,
but I'd like to backport it there too if it is acceptable.


Thanks,
Roland


gcc/
2016-09-15  Roland McGrath  <

PR other/77609
* varasm.c (default_section_type_flags): Set SECTION_NOTYPE for
any section for which we don't know a specific type it should have,
regardless of name.  Previously this was done only for the exact
names ".init_array", ".fini_array", and ".preinit_array".

diff --git a/gcc/varasm.c b/gcc/varasm.c
index 00a9b30..0d7ea38 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -6210,15 +6210,20 @@ default_section_type_flags (tree decl, const
char *name, int reloc)
   || strncmp (name, ".gnu.linkonce.tb.", 17) == 0)
 flags |= SECTION_TLS | SECTION_BSS;

-  /* These three sections have special ELF types.  They are neither
- SHT_PROGBITS nor SHT_NOBITS, so when changing sections we don't
- want to print a section type (@progbits or @nobits).  If someone
- is silly enough to emit code or TLS variables to one of these
- sections, then don't handle them specially.  */
-  if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS))
-  && (strcmp (name, ".init_array") == 0
- || strcmp (name, ".fini_array") == 0
- || strcmp (name, ".preinit_array") == 0))
+  /* Various sections have special ELF types that the assembler will
+ assign by default based on the name.  They are neither SHT_PROGBITS
+ nor SHT_NOBITS, so when changing sections we don't want to print a
+ section type (@progbits or @nobits).  Rather than duplicating the
+ assembler's knowledge of what those special name patterns are, just
+ let the assembler choose the type if we don't know a specific
+ reason to set it to something other than the default.  SHT_PROGBITS
+ is the default for sections whose name is not specially known to
+ the assembler, so it does no harm to leave the choice to the
+ assembler when @progbits is the best thing we know to use.  If
+ someone is silly enough to emit code or TLS variables to one of
+ these sections, then don't handle them specially.  */
+  if (!(flags & (SECTION_CODE | SECTION_BSS | SECTION_TLS | SECTION_ENTSIZE))
+  && !(HAVE_COMDAT_GROUP && (flags & SECTION_LINKONCE)))
 flags |= SECTION_NOTYPE;

   return flags;