On Sun, 2024-09-29 at 17:12 +0200, Florian Weimer 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.

I'm not quite sure what you mean by "non-error" and "non-anchored". 

By "non-error", do you mean that this should this be a warning?  If so,
use warning_at.  You can use 0 for the option_id whilst prototyping. 
Or use "inform" to get a note.

By "non-anchored", do you mean "not associated with any particular
source location"?  If so, use error_at/warning_at and use
UNKNOWN_LOCATION for the location_t ("error" and "warning" implicitly
use the "input_location" global variable; it's usually best to instead
specify a location, or use UNKNOWN_LOCATION for "global" problems).

Or am I misunderstanding?

Hope this is helpful
Dave




> 
> 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");
> +
> +  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 ());
>         /* Make sure we don't error about one section multiple
> times.  */
>         sect->common.flags |= SECTION_OVERRIDE;
>       }
> 
> base-commit: 786773d4c12fd78e94f58193ff303cba19ca1b19
> 

Reply via email to