Use DIAG_IGNORE_NEEDS_COMMENT to silence -Wstringop-truncation

Message ID 20180507134315.GA27623@gmail.com
State New
Headers show
Series
  • Use DIAG_IGNORE_NEEDS_COMMENT to silence -Wstringop-truncation
Related show

Commit Message

H.J. Lu May 7, 2018, 1:43 p.m.
On Wed, Apr 25, 2018 at 10:58:59PM -0400, Carlos O'Donell wrote:
> On 04/25/2018 10:43 PM, Alan Modra wrote:

> > On Wed, Apr 25, 2018 at 07:57:22AM -0500, Carlos O'Donell wrote:

> >> Two comments here:

> >>

> >> * The FSF version of gcc should be identified here in an easy to understand way.

> >> * Consider abstracting away some of the verbose GCC_VERSION checking behind

> >>   some helper macros.

> > 

> > I'll support these ideas if/when binutils has more than a few instances

> > where we need to modify gcc diagnostics.  We have just two at the

> > moment (and two more in files generated by bison so those don't

> > count).

>  

> Mentioning the gcc version, like you did in the second version of

> the patch is very useful for future developers wanting to cleanup

> and remove old pragmas in the future when gcc 8 is no longer supported.

> It's a good habit regardless of the project or number of warnings.

> 

> But overall you're right, you don't have enough of these that it's that

> much of a problem.

> 

> Again, more of a comment, than any direct review :-)

> 


Here is the patch.  Tested with cross binutils to arm, aarch64, ppc
and s390.  OK for master?


H.J.
---
GCC 8 warns about destination size with -Wstringop-truncation:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643

Copy DIAG_PUSH_NEEDS_COMMENT, DIAG_POP_NEEDS_COMMENT and
DIAG_IGNORE_NEEDS_COMMENT from glibc 2.27 to silence it.

	* bfd-in.h (DIAG_PUSH_NEEDS_COMMENT): New.
	(DIAG_POP_NEEDS_COMMENT): Likewse.
	(_DIAG_STR1): Likewse.
	(_DIAG_STR): Likewse.
	(DIAG_IGNORE_NEEDS_COMMENT): Likewse.
	* bfd-in2.h: Regenerated.
	* elf32-arm.c (elf32_arm_nabi_write_core_note): Use
	DIAG_PUSH_NEEDS_COMMENT, DIAG_IGNORE_NEEDS_COMMENT and
	DIAG_POP_NEEDS_COMMENT to silence GCC 8 warnings with
	-Wstringop-truncation.
	* elf32-ppc.c (ppc_elf_write_core_note): Likewse.
	* elf32-s390.c (elf_s390_write_core_note): Likewse.
	* elf64-ppc.c (ppc64_elf_write_core_note): Likewse.
	* elf64-s390.c (elf_s390_write_core_note): Likewse.
	* elfxx-aarch64.c (_bfd_aarch64_elf_write_core_note): Likewse.
---
 bfd/bfd-in.h        | 38 ++++++++++++++++++++++++++++++++++++++
 bfd/bfd-in2.h       | 38 ++++++++++++++++++++++++++++++++++++++
 bfd/elf32-arm.c     |  9 +++++++++
 bfd/elf32-ppc.c     |  9 +++++++++
 bfd/elf32-s390.c    |  9 +++++++++
 bfd/elf64-ppc.c     |  9 +++++++++
 bfd/elf64-s390.c    |  9 +++++++++
 bfd/elfxx-aarch64.c |  9 +++++++++
 8 files changed, 130 insertions(+)

-- 
2.17.0

Comments

Alan Modra May 8, 2018, 3:32 a.m. | #1
On Mon, May 07, 2018 at 06:43:15AM -0700, H.J. Lu wrote:
> GCC 8 warns about destination size with -Wstringop-truncation:

> 

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643

> 

> Copy DIAG_PUSH_NEEDS_COMMENT, DIAG_POP_NEEDS_COMMENT and

> DIAG_IGNORE_NEEDS_COMMENT from glibc 2.27 to silence it.


I'm inclined to think we don't need this patch.  It certainly isn't
needed for binutils releases, as we don't enable -Werror by default for
releases.  The problem mostly affect developers using bleeding edge
toolchains, and I have every confidence that the gcc bug will be fixed
quickly.

Also, not all binutils users have glibc installed.  We can't depend on
a macro defined in glibc's /usr/include/features.h, __GNUC_PREREQ.

-- 
Alan Modra
Australia Development Lab, IBM
Carlos O'Donell May 15, 2018, 2:09 a.m. | #2
On 05/07/2018 11:32 PM, Alan Modra wrote:
> On Mon, May 07, 2018 at 06:43:15AM -0700, H.J. Lu wrote:

>> GCC 8 warns about destination size with -Wstringop-truncation:

>>

>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643

>>

>> Copy DIAG_PUSH_NEEDS_COMMENT, DIAG_POP_NEEDS_COMMENT and

>> DIAG_IGNORE_NEEDS_COMMENT from glibc 2.27 to silence it.

> 

> I'm inclined to think we don't need this patch.  It certainly isn't

> needed for binutils releases, as we don't enable -Werror by default for

> releases.  The problem mostly affect developers using bleeding edge

> toolchains, and I have every confidence that the gcc bug will be fixed

> quickly.


New Year's Resolution! Enable -Werror ;-)

> Also, not all binutils users have glibc installed.  We can't depend on

> a macro defined in glibc's /usr/include/features.h, __GNUC_PREREQ.

 
Just to be clear the patch doesn't depend on glibc being installed, it
just copies the presently used macros from glibc for use in binutils.

... and then uses them to fix the gcc 8 issues that crop up with -Werror.

-- 
Cheers,
Carlos.
Alan Modra May 17, 2018, 1:54 a.m. | #3
On Mon, May 14, 2018 at 10:09:53PM -0400, Carlos O'Donell wrote:
> On 05/07/2018 11:32 PM, Alan Modra wrote:

> > Also, not all binutils users have glibc installed.  We can't depend on

> > a macro defined in glibc's /usr/include/features.h, __GNUC_PREREQ.

>  

> Just to be clear the patch doesn't depend on glibc being installed, it

> just copies the presently used macros from glibc for use in binutils.

> 

> ... and then uses them to fix the gcc 8 issues that crop up with -Werror.


I checked HJ's patch again, and stand by my comment about
__GNUC_PREREQ.  This macro is not defined anywhere in the binutils
sources, nor is it defined by HJ's patch.

-- 
Alan Modra
Australia Development Lab, IBM
Carlos O'Donell May 17, 2018, 2:33 a.m. | #4
On 05/16/2018 09:54 PM, Alan Modra wrote:
> On Mon, May 14, 2018 at 10:09:53PM -0400, Carlos O'Donell wrote:

>> On 05/07/2018 11:32 PM, Alan Modra wrote:

>>> Also, not all binutils users have glibc installed.  We can't depend on

>>> a macro defined in glibc's /usr/include/features.h, __GNUC_PREREQ.

>>  

>> Just to be clear the patch doesn't depend on glibc being installed, it

>> just copies the presently used macros from glibc for use in binutils.

>>

>> ... and then uses them to fix the gcc 8 issues that crop up with -Werror.

> 

> I checked HJ's patch again, and stand by my comment about

> __GNUC_PREREQ.  This macro is not defined anywhere in the binutils

> sources, nor is it defined by HJ's patch.


My apologies, I misread your original statement. I completely agree.

Why do we need __GNUC_PREREQ macro?

You can unconditionally use the DIAG_* macros, and if you're
__GNUC__ >= 8 then they evaluate to meaningful things, otherwise
nothing.

-- 
Cheers,
Carlos.

Patch

diff --git a/bfd/bfd-in.h b/bfd/bfd-in.h
index 50a8b52e96..5d9e311081 100644
--- a/bfd/bfd-in.h
+++ b/bfd/bfd-in.h
@@ -71,6 +71,44 @@  extern "C" {
 #define LITMEMCPY(DEST,STR2) memcpy ((DEST), (STR2), sizeof (STR2) - 1)
 #define LITSTRCPY(DEST,STR2) memcpy ((DEST), (STR2), sizeof (STR2))
 
+#if __GNUC__ >= 8
+/* These are copied from glibc 2.27.  */
+
+/* The macros to control diagnostics are structured like this, rather
+   than a single macro that both pushes and pops diagnostic state and
+   takes the affected code as an argument, because the GCC pragmas
+   work by disabling the diagnostic for a range of source locations
+   and do not work when all the pragmas and the affected code are in a
+   single macro expansion.  */
+
+/* Push diagnostic state.  */
+#define DIAG_PUSH_NEEDS_COMMENT _Pragma ("GCC diagnostic push")
+
+/* Pop diagnostic state.  */
+#define DIAG_POP_NEEDS_COMMENT _Pragma ("GCC diagnostic pop")
+
+#define _DIAG_STR1(s) #s
+#define _DIAG_STR(s) _DIAG_STR1(s)
+
+/* Ignore the diagnostic OPTION.  VERSION is the most recent GCC
+   version for which the diagnostic has been confirmed to appear in
+   the absence of the pragma (in the form MAJOR.MINOR for GCC 4.x,
+   just MAJOR for GCC 5 and later).  Uses of this pragma should be
+   reviewed when the GCC version given is no longer supported for
+   building glibc; the version number should always be on the same
+   source line as the macro name, so such uses can be found with grep.
+   Uses should come with a comment giving more details of the
+   diagnostic, and an architecture on which it is seen if possibly
+   optimization-related and not in architecture-specific code.  This
+   macro should only be used if the diagnostic seems hard to fix (for
+   example, optimization-related false positives).  */
+#define DIAG_IGNORE_NEEDS_COMMENT(version, option)     \
+  _Pragma (_DIAG_STR (GCC diagnostic ignored option))
+#else
+#define DIAG_PUSH_NEEDS_COMMENT
+#define DIAG_POP_NEEDS_COMMENT
+#define DIAG_IGNORE_NEEDS_COMMENT(version, option)
+#endif
 
 #define BFD_SUPPORTS_PLUGINS @supports_plugins@
 
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 48226860c7..9bee17bad5 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -78,6 +78,44 @@  extern "C" {
 #define LITMEMCPY(DEST,STR2) memcpy ((DEST), (STR2), sizeof (STR2) - 1)
 #define LITSTRCPY(DEST,STR2) memcpy ((DEST), (STR2), sizeof (STR2))
 
+#if __GNUC__ >= 8
+/* These are copied from glibc 2.27.  */
+
+/* The macros to control diagnostics are structured like this, rather
+   than a single macro that both pushes and pops diagnostic state and
+   takes the affected code as an argument, because the GCC pragmas
+   work by disabling the diagnostic for a range of source locations
+   and do not work when all the pragmas and the affected code are in a
+   single macro expansion.  */
+
+/* Push diagnostic state.  */
+#define DIAG_PUSH_NEEDS_COMMENT _Pragma ("GCC diagnostic push")
+
+/* Pop diagnostic state.  */
+#define DIAG_POP_NEEDS_COMMENT _Pragma ("GCC diagnostic pop")
+
+#define _DIAG_STR1(s) #s
+#define _DIAG_STR(s) _DIAG_STR1(s)
+
+/* Ignore the diagnostic OPTION.  VERSION is the most recent GCC
+   version for which the diagnostic has been confirmed to appear in
+   the absence of the pragma (in the form MAJOR.MINOR for GCC 4.x,
+   just MAJOR for GCC 5 and later).  Uses of this pragma should be
+   reviewed when the GCC version given is no longer supported for
+   building glibc; the version number should always be on the same
+   source line as the macro name, so such uses can be found with grep.
+   Uses should come with a comment giving more details of the
+   diagnostic, and an architecture on which it is seen if possibly
+   optimization-related and not in architecture-specific code.  This
+   macro should only be used if the diagnostic seems hard to fix (for
+   example, optimization-related false positives).  */
+#define DIAG_IGNORE_NEEDS_COMMENT(version, option)     \
+  _Pragma (_DIAG_STR (GCC diagnostic ignored option))
+#else
+#define DIAG_PUSH_NEEDS_COMMENT
+#define DIAG_POP_NEEDS_COMMENT
+#define DIAG_IGNORE_NEEDS_COMMENT(version, option)
+#endif
 
 #define BFD_SUPPORTS_PLUGINS @supports_plugins@
 
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index 5a3b58ff0d..da8a528e36 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -2174,7 +2174,16 @@  elf32_arm_nabi_write_core_note (bfd *abfd, char *buf, int *bufsiz,
 	va_start (ap, note_type);
 	memset (data, 0, sizeof (data));
 	strncpy (data + 28, va_arg (ap, const char *), 16);
+	DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (8, 0)
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAG_IGNORE_NEEDS_COMMENT (8, "-Wstringop-truncation");
+#endif
 	strncpy (data + 44, va_arg (ap, const char *), 80);
+	DIAG_POP_NEEDS_COMMENT;
 	va_end (ap);
 
 	return elfcore_write_note (abfd, buf, bufsiz,
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index 462c8af311..c8ddcffdac 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -2411,7 +2411,16 @@  ppc_elf_write_core_note (bfd *abfd, char *buf, int *bufsiz, int note_type, ...)
 	va_start (ap, note_type);
 	memset (data, 0, sizeof (data));
 	strncpy (data + 32, va_arg (ap, const char *), 16);
+	DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (8, 0)
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAG_IGNORE_NEEDS_COMMENT (8, "-Wstringop-truncation");
+#endif
 	strncpy (data + 48, va_arg (ap, const char *), 80);
+	DIAG_POP_NEEDS_COMMENT;
 	va_end (ap);
 	return elfcore_write_note (abfd, buf, bufsiz,
 				   "CORE", note_type, data, sizeof (data));
diff --git a/bfd/elf32-s390.c b/bfd/elf32-s390.c
index b3603bd865..60433a38e5 100644
--- a/bfd/elf32-s390.c
+++ b/bfd/elf32-s390.c
@@ -3951,7 +3951,16 @@  elf_s390_write_core_note (bfd *abfd, char *buf, int *bufsiz,
 	va_end (ap);
 
 	strncpy (data + 28, fname, 16);
+	DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (8, 0)
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAG_IGNORE_NEEDS_COMMENT (8, "-Wstringop-truncation");
+#endif
 	strncpy (data + 44, psargs, 80);
+	DIAG_POP_NEEDS_COMMENT;
 	return elfcore_write_note (abfd, buf, bufsiz, "CORE", note_type,
 				   &data, sizeof (data));
       }
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 09377d102c..91c5edae20 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -3041,7 +3041,16 @@  ppc64_elf_write_core_note (bfd *abfd, char *buf, int *bufsiz, int note_type,
 	va_start (ap, note_type);
 	memset (data, 0, sizeof (data));
 	strncpy (data + 40, va_arg (ap, const char *), 16);
+	DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (8, 0)
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAG_IGNORE_NEEDS_COMMENT (8, "-Wstringop-truncation");
+#endif
 	strncpy (data + 56, va_arg (ap, const char *), 80);
+	DIAG_POP_NEEDS_COMMENT;
 	va_end (ap);
 	return elfcore_write_note (abfd, buf, bufsiz,
 				   "CORE", note_type, data, sizeof (data));
diff --git a/bfd/elf64-s390.c b/bfd/elf64-s390.c
index bfa02340ca..1773c208b4 100644
--- a/bfd/elf64-s390.c
+++ b/bfd/elf64-s390.c
@@ -3760,7 +3760,16 @@  elf_s390_write_core_note (bfd *abfd, char *buf, int *bufsiz,
 	va_end (ap);
 
 	strncpy (data + 40, fname, 16);
+	DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (8, 0)
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAG_IGNORE_NEEDS_COMMENT (8, "-Wstringop-truncation");
+#endif
 	strncpy (data + 56, psargs, 80);
+	DIAG_POP_NEEDS_COMMENT;
 	return elfcore_write_note (abfd, buf, bufsiz, "CORE", note_type,
 				   &data, sizeof (data));
       }
diff --git a/bfd/elfxx-aarch64.c b/bfd/elfxx-aarch64.c
index 45a732db2b..6450c5f1c6 100644
--- a/bfd/elfxx-aarch64.c
+++ b/bfd/elfxx-aarch64.c
@@ -659,7 +659,16 @@  _bfd_aarch64_elf_write_core_note (bfd *abfd, char *buf, int *bufsiz, int note_ty
 	va_start (ap, note_type);
 	memset (data, 0, sizeof (data));
 	strncpy (data + 40, va_arg (ap, const char *), 16);
+	DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (8, 0)
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAG_IGNORE_NEEDS_COMMENT (8, "-Wstringop-truncation");
+#endif
 	strncpy (data + 56, va_arg (ap, const char *), 80);
+	DIAG_POP_NEEDS_COMMENT;
 	va_end (ap);
 
 	return elfcore_write_note (abfd, buf, bufsiz, "CORE",