On Mon, Feb 26, 2018 at 8:11 PM, Ian Lance Taylor <i...@google.com> 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 <mcgra...@google.com>
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, flagchars);
+ /* default_section_type_flags (above) knows which flags need special
+ handling here, and sets NOTYPE when none of these apply so that the
+ assembler's logic for default types can apply to user-chosen
+ section names. */
if (!(flags & SECTION_NOTYPE))
{
const char *type;
