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;