On Sun, Sep 29, 2024 at 5:13 PM Florian Weimer <fwei...@redhat.com> wrote:
>
> Sometimes this is a user error, sometimes it is more of an ICE.
> In either case, more information about the conflict is helpful.
>
> I used to this to get a better idea about what is going on with
> PR116887. The original diagnostics look like this:
>
> dl-find_object.c: In function ‘_dlfo_process_initial’:
> dl-find_object.c:507:1: error: section type conflict with
> ‘_dlfo_nodelete_mappings’
> 507 | _dlfo_process_initial (void)
> | ^~~~~~~~~~~~~~~~~~~~~
> dl-find_object.c:73:40: note: ‘_dlfo_nodelete_mappings’ was declared here
> 73 | static struct dl_find_object_internal *_dlfo_nodelete_mappings
> | ^~~~~~~~~~~~~~~~~~~~~~~
>
>
> I don't quite understand what is going on (the symbol that's being
> flagged for conflict is somewhat unstable), but at least the new
> diagnostics show that the sectio name, and maybe the flags are useful,
> too:
>
> /tmp/bug.i:6798:1: error: section ‘.data.rel.ro’ type conflict with
> ‘_dlfo_main’
> 6798 | }
> | ^
> /tmp/bug.i:6190:39: note: ‘_dlfo_main’ was declared here
> 6190 | static struct dl_find_object_internal _dlfo_main __attribute__
> ((section (".data.rel.ro")));
> | ^~~~~~~~~~
> /tmp/bug.i:6798:1: error: previous section type: WRITE|NOTYPE|DECLARED|NAMED
> 6798 | }
> | ^
> /tmp/bug.i:6798:1: error: new section type: WRITE|NOTYPE|NAMED|RELRO
>
> I still need help with producing a non-error, non-anchored diagnostic.
> I don't know how to do that.
>
> Thanks,
> Florian
>
> * varasm.cc (section_flags_to_string): New function.
> (get_section): Include name of section in diagnostics.
> Print old and new section flags, as rendered by
> section_flags_to_string.
>
> ---
> gcc/varasm.cc | 80
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 76 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/varasm.cc b/gcc/varasm.cc
> index 4426e7ce6c6..deba15933aa 100644
> --- a/gcc/varasm.cc
> +++ b/gcc/varasm.cc
> @@ -25,6 +25,7 @@ along with GCC; see the file COPYING3. If not see
> We also output the assembler code for constants stored in memory
> and are responsible for combining constants with the same value. */
>
> +#define INCLUDE_STRING
> #include "config.h"
> #include "system.h"
> #include "coretypes.h"
> @@ -276,6 +277,65 @@ get_noswitch_section (unsigned int flags,
> noswitch_section_callback callback)
> return sect;
> }
>
> +/* Return a string describing the section flags. */
> +
> +static std::string
> +section_flags_to_string (unsigned int flags)
> +{
> + if (flags == 0)
> + return "UNNAMED";
> + std::string result;
> + auto append = [&result] (bool f, const char *name)
> + {
> + if (f)
> + {
> + if (!result.empty ())
> + result += '|';
> + result += name;
> + }
> + };
> +
> + append (flags & SECTION_CODE, "CODE");
> + append (flags & SECTION_WRITE, "WRITE");
> + append (flags & SECTION_DEBUG, "DEBUG");
> + append (flags & SECTION_LINKONCE, "LINKONCE");
> + append (flags & SECTION_SMALL, "SMALL");
> + append (flags & SECTION_BSS, "BSS");
> + append (flags & SECTION_MERGE, "MERGE");
> + append (flags & SECTION_STRINGS, "STRINGS");
> + append (flags & SECTION_OVERRIDE, "OVERRIDE");
> + append (flags & SECTION_TLS, "TLS");
> + append (flags & SECTION_NOTYPE, "NOTYPE");
> + append (flags & SECTION_DECLARED, "DECLARED");
> + append (flags & SECTION_NAMED, "NAMED");
> + append (flags & SECTION_NOSWITCH, "NOSWITCH");
> + append (flags & SECTION_COMMON, "COMMON");
> + append (flags & SECTION_RELRO, "RELRO");
> + append (flags & SECTION_EXCLUDE, "EXCLUDE");
> + append (flags & SECTION_RETAIN, "RETAIN");
> + append (flags & SECTION_LINK_ORDER, "LINK_ORDER");
I'm not sure printing these internal flags is of help to the user.
> +
> + unsigned int entsize = flags & SECTION_ENTSIZE;
> + if (entsize != 0)
> + {
> + if (!result.empty ())
> + result += ',';
> + result += "size=";
> + result += std::to_string (entsize);
> + }
> +
> + unsigned int mach_dep = flags / SECTION_MACH_DEP;
> + if (mach_dep != 0)
> + {
> + if (!result.empty ())
> + result += ',';
> + result += "mach=";
> + result += std::to_string (mach_dep);
> + }
> +
> + return result;
> +}
> +
> /* Return the named section structure associated with NAME. Create
> a new section with the given fields if no such structure exists.
> When NOT_EXISTING, then fail if the section already exists. Return
> @@ -312,6 +372,9 @@ get_section (const char *name, unsigned int flags, tree
> decl,
> internal_error ("section already exists: %qs", name);
>
> sect = *slot;
> + unsigned int original_flags = sect->common.flags;
> + unsigned int new_flags = flags;
> +
> /* 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). */
> @@ -355,17 +418,26 @@ get_section (const char *name, unsigned int flags, tree
> decl,
> && decl != sect->named.decl)
> {
> if (decl != NULL && DECL_P (decl))
> - error ("%+qD causes a section type conflict with %qD",
> - decl, sect->named.decl);
> + {
> + error ("%+qD causes a type conflict with %qD"
> + " in section %qs",
> + decl, sect->named.decl);
> + }
> else
> - error ("section type conflict with %qD", sect->named.decl);
> + error ("section %qs type conflict with %qD",
> + name, sect->named.decl);
> inform (DECL_SOURCE_LOCATION (sect->named.decl),
> "%qD was declared here", sect->named.decl);
> }
> else if (decl != NULL && DECL_P (decl))
> - error ("%+qD causes a section type conflict", decl);
> + error ("%+qD causes a section type conflict for section %s",
> + decl, name);
> else
> error ("section type conflict");
> + error ("previous section type: %s",
> + section_flags_to_string (original_flags).c_str ());
> + error ("new section type: %s",
> + section_flags_to_string (new_flags).c_str ());
So these are all cases where neither
/* It is fine if one of the section flags is
SECTION_WRITE | SECTION_RELRO and the other has none of these
flags (i.e. read-only) in named sections and either the
section hasn't been declared yet or has been declared as writable.
In that case just make sure the resulting flags are
SECTION_WRITE | SECTION_RELRO, ie. writable only because of
relocations. */
if (((sect->common.flags ^ flags) & (SECTION_WRITE | SECTION_RELRO))
== (SECTION_WRITE | SECTION_RELRO)
&& (sect->common.flags
& ~(SECTION_DECLARED | SECTION_WRITE | SECTION_RELRO))
== (flags & ~(SECTION_WRITE | SECTION_RELRO))
&& ((sect->common.flags & SECTION_DECLARED) == 0
|| (sect->common.flags & SECTION_WRITE)))
{
sect->common.flags |= (SECTION_WRITE | SECTION_RELRO);
return sect;
}
/* If the SECTION_RETAIN bit doesn't match, return and switch
to a new section later. */
if ((sect->common.flags & SECTION_RETAIN)
!= (flags & SECTION_RETAIN))
return sect;
matched. It should be possible to elaborate on the actual mismatch instead of
dumping all of the random section flags?
> /* Make sure we don't error about one section multiple times. */
> sect->common.flags |= SECTION_OVERRIDE;
> }
>
> base-commit: 786773d4c12fd78e94f58193ff303cba19ca1b19
>
