Re: [PATCH 2/2] Improve reporting of section conflict errors

["H.J. Lu" <hjl.tools@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1600 bytes --]

On Thu, Dec 16, 2010 at 7:46 AM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Thu, 16 Dec 2010, Andi Kleen wrote:
>
>> +/* Print section flags F into BUF */
>
> End comment with ".  */".
>
>> +     { "code", SECTION_CODE, 0 },
> [...]
>
>> +  if (f)
>> +    sprintf (buf, "rest %x", f);
>
> Are these strings meant to be English language or literal programming
> language text?  If English, there should be appropriate arrangements for
> translation.
>
>> +       char buf[1024];
>> +
>
> I agree a variable size buffer would be better - but the minimum is
> runtime checks to avoid overflow in the case of long translations etc. (so
> don't use sprintf etc. without knowing there is enough space left at
> runtime).
>
> It would be nice to eliminate direct use of sprintf from GCC - even if you
> do know there is enough space, using snprintf is better discipline.  (And
> asprintf/vasprintf - in favour of xasprintf/xvasprintf - if someone could
> just approve <http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01448.html and
> <http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01449.html>.)
>
>> +       inform (DECL_SOURCE_LOCATION (decl),
>> +               "New section declaration differs in: %s",
>> +               section_flags_string (buf, flags));
>
> Diagnostics start with lowercase letters.
>

Is this patch OK for trunk?

Thanks.

-- 
H.J.
---
2010-12-16  Andi Kleen	<ak@linux.intel.com>
	    H.J. Lu  <hongjiu.lu@intel.com>

	* varasm.c (section_flags_string): New.
	(get_section): Print more details on conflicting section flags.

[-- Attachment #2: gcc-section-flag-1.patch --]
[-- Type: text/plain, Size: 2371 bytes --]

2010-12-16  Andi Kleen	<ak@linux.intel.com>
	    H.J. Lu  <hongjiu.lu@intel.com>

	* varasm.c (section_flags_string): New.
	(get_section): Print more details on conflicting section flags.

diff --git a/gcc/varasm.c b/gcc/varasm.c
index ed44610..d91497c 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -268,6 +268,47 @@ get_noswitch_section (unsigned int flags, noswitch_section_callback callback)
   return sect;
 }
 
+/* Return a string for section flags F.  */
+
+static char *
+section_flags_string (unsigned int f)
+{
+  static const struct section_flag
+    {
+      const char *name;
+      unsigned int flag;
+    }
+  section_flags[] =
+    {
+	{ "code", SECTION_CODE },
+	{ "write", SECTION_WRITE },
+	{ "debug", SECTION_DEBUG },
+	{ "linkonce", SECTION_LINKONCE },
+	{ "small", SECTION_SMALL },
+	{ "bss", SECTION_BSS },
+	{ "forget", SECTION_FORGET },
+	{ "merge", SECTION_MERGE },
+	{ "strings", SECTION_STRINGS },
+	{ "override", SECTION_OVERRIDE },
+	{ "tls", SECTION_TLS },
+	{ "common", SECTION_COMMON },
+	{ "unnamed", SECTION_UNNAMED},
+	{ "named", SECTION_NAMED },
+	{ "noswitch", SECTION_NOSWITCH }
+    };
+  unsigned int i;
+  char *flag = NULL;
+  for (i = 0; i < ARRAY_SIZE (section_flags); i++)
+    if ((f & section_flags[i].flag) != 0)
+      {
+	if (flag)
+	  flag = concat (flag, " ", section_flags[i].name, NULL);
+	else
+	  flag = concat (section_flags[i].name, NULL);
+      }
+  return flag;
+}
+
 /* Return the named section structure associated with NAME.  Create
    a new section with the given fields if no such structure exists.  */
 
@@ -290,15 +331,24 @@ get_section (const char *name, unsigned int flags, tree decl)
     }
   else
     {
+      unsigned int oflags;
+
       sect = *slot;
-      if ((sect->common.flags & ~SECTION_DECLARED) != flags
+      oflags = sect->common.flags & ~SECTION_DECLARED;
+      if (oflags != flags
 	  && ((sect->common.flags | flags) & SECTION_OVERRIDE) == 0)
 	{
+	  char *str;
 	  /* Sanity check user variables for flag changes.  */
 	  if (decl == 0)
 	    decl = sect->named.decl;
 	  gcc_assert (decl);
 	  error ("%+D causes a section type conflict", decl);
+	  flags = (oflags ^ flags) & ~SECTION_OVERRIDE;
+	  str = section_flags_string (flags);
+	  inform (DECL_SOURCE_LOCATION (decl), 
+		  "new section declaration differs in: %s", str);
+	  free (str);
 	}
     }
   return sect;