[PATCH 1/3] Move gdb/common/diagnostics.h to include/diagnostics.h

[PATCH 1/3] Move gdb/common/diagnostics.h to include/diagnostics.h

[H.J. Lu]

Move gdb/common/diagnostics.h to include/diagnostics.h so that it can
be used in binutils.

gdb/

	* ada-lex.l: Include "diagnostics.h" instead of
	"common/diagnostics.h".
	* unittests/environ-selftests.c: Likewise.
	* common/diagnostics.h: Moved to ../include.

include/

	* diagnostics.h: Moved from ../gdb/common/diagnostics.h.
---
 gdb/ada-lex.l                         |  2 +-
 gdb/unittests/environ-selftests.c     |  2 +-
 {gdb/common => include}/diagnostics.h | 10 +++-------
 3 files changed, 5 insertions(+), 9 deletions(-)
 rename {gdb/common => include}/diagnostics.h (92%)

diff --git a/gdb/ada-lex.l b/gdb/ada-lex.l
index c83a619833..621ebb2a95 100644
--- a/gdb/ada-lex.l
+++ b/gdb/ada-lex.l
@@ -41,7 +41,7 @@ POSEXP  (e"+"?{NUM10})
 
 %{
 
-#include "common/diagnostics.h"
+#include "diagnostics.h"
 
 /* Some old versions of flex generate code that uses the "register" keyword,
    which clang warns about.  This was observed for example with flex 2.5.35,
diff --git a/gdb/unittests/environ-selftests.c b/gdb/unittests/environ-selftests.c
index a66e8c7a48..31b7ebf9c1 100644
--- a/gdb/unittests/environ-selftests.c
+++ b/gdb/unittests/environ-selftests.c
@@ -20,7 +20,7 @@
 #include "defs.h"
 #include "selftest.h"
 #include "common/environ.h"
-#include "common/diagnostics.h"
+#include "diagnostics.h"
 
 static const char gdb_selftest_env_var[] = "GDB_SELFTEST_ENVIRON";
 
diff --git a/gdb/common/diagnostics.h b/include/diagnostics.h
similarity index 92%
rename from gdb/common/diagnostics.h
rename to include/diagnostics.h
index e631f506de..0725664177 100644
--- a/gdb/common/diagnostics.h
+++ b/include/diagnostics.h
@@ -1,7 +1,5 @@
 /* Copyright (C) 2017-2018 Free Software Foundation, Inc.
 
-   This file is part of GDB.
-
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
    the Free Software Foundation; either version 3 of the License, or
@@ -15,10 +13,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifndef COMMON_DIAGNOSTICS_H
-#define COMMON_DIAGNOSTICS_H
-
-#include "common/preprocessor.h"
+#ifndef DIAGNOSTICS_H
+#define DIAGNOSTICS_H
 
 #ifdef __GNUC__
 # define DIAGNOSTIC_PUSH _Pragma ("GCC diagnostic push")
@@ -61,4 +57,4 @@
 
 #endif
 
-#endif /* COMMON_DIAGNOSTICS_H */
+#endif /* DIAGNOSTICS_H */
-- 
2.17.0

[PATCH 2/3] Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION

[H.J. Lu]

Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION for GCC 8 to silence
-Wstringop-truncation warning:

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

	* diagnostics.h (DIAGNOSTIC_STRINGIFY_1): New.
	(DIAGNOSTIC_STRINGIFY): Likewise.
	(DIAGNOSTIC_IGNORE): Replace STRINGIFY with DIAGNOSTIC_STRINGIFY.
	(DIAGNOSTIC_IGNORE_SELF_MOVE): Define empty if not defined.
	(DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER): Likewise.
	(DIAGNOSTIC_IGNORE_UNUSED_FUNCTION): Likewise.
	(DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES): Likewise.
	(DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION): New.
---
 include/diagnostics.h | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/include/diagnostics.h b/include/diagnostics.h
index 0725664177..ec5ac4190e 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -19,8 +19,13 @@
 #ifdef __GNUC__
 # define DIAGNOSTIC_PUSH _Pragma ("GCC diagnostic push")
 # define DIAGNOSTIC_POP _Pragma ("GCC diagnostic pop")
+
+/* Stringification.  */
+# define DIAGNOSTIC_STRINGIFY_1(x) #x
+# define DIAGNOSTIC_STRINGIFY(x) DIAGNOSTIC_STRINGIFY_1 (x)
+
 # define DIAGNOSTIC_IGNORE(option) \
-  _Pragma (STRINGIFY (GCC diagnostic ignored option))
+  _Pragma (DIAGNOSTIC_STRINGIFY (GCC diagnostic ignored option))
 #else
 # define DIAGNOSTIC_PUSH
 # define DIAGNOSTIC_POP
@@ -37,24 +42,36 @@
 # if __has_warning ("-Wenum-compare-switch")
 #  define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES \
    DIAGNOSTIC_IGNORE ("-Wenum-compare-switch")
-# else
-#  define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 # endif
 #elif defined (__GNUC__) /* GCC */
 
-# define DIAGNOSTIC_IGNORE_SELF_MOVE
-# define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER
 # define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
   DIAGNOSTIC_IGNORE ("-Wunused-function")
-# define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 
-#else /* Other compilers */
+# if __GNUC__ >= 8
+#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
+  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
+# endif
+#endif
 
+#ifndef DIAGNOSTIC_IGNORE_SELF_MOVE
 # define DIAGNOSTIC_IGNORE_SELF_MOVE
+#endif
+
+#ifndef DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER
 # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER
+#endif
+
+#ifndef DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
 # define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
+#endif
+
+#ifndef DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 # define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
+#endif
 
+#ifndef DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION
+# define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION
 #endif
 
 #endif /* DIAGNOSTICS_H */
-- 
2.17.0

Re: [PATCH 2/3] Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION

[Nick Clifton]

Hi H.J.

> +# if __GNUC__ >= 8
> +#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
> +  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
> +# endif
> +#endif

Presumably the Wstringop-truncation bug will be fixed in gcc 8.1
so shouldn't the test check the revision number as well ?

Cheers
  Nick

Re: [PATCH 2/3] Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION

[Alan Modra]

On Fri, Jun 01, 2018 at 08:57:20AM +0100, Nick Clifton wrote:
> Hi H.J.
> 
> > +# if __GNUC__ >= 8
> > +#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
> > +  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
> > +# endif
> > +#endif
> 
> Presumably the Wstringop-truncation bug will be fixed in gcc 8.1
> so shouldn't the test check the revision number as well ?

Yes, it has already been fixed.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/3] Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION

[H.J. Lu]

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

On Fri, Jun 1, 2018 at 3:19 AM, Alan Modra <amodra@gmail.com> wrote:
> On Fri, Jun 01, 2018 at 08:57:20AM +0100, Nick Clifton wrote:
>> Hi H.J.
>>
>> > +# if __GNUC__ >= 8
>> > +#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
>> > +  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
>> > +# endif
>> > +#endif
>>
>> Presumably the Wstringop-truncation bug will be fixed in gcc 8.1
>> so shouldn't the test check the revision number as well ?
>
> Yes, it has already been fixed.
>

The bug is in GCC 8.1 and will be fixed in GCC 8.2.  Here is the
updated patch with

+# if __GNUC__ == 8 && __GNUC_MINOR__ < 2
+#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
+  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
+# endif

OK for master?

-- 
H.J.

[-- Attachment #2: 0001-Add-DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION.patch --]
[-- Type: text/x-patch, Size: 2799 bytes --]

From bb1d3ddd78abf25b30e11cf25e4b2ec7b6ef0688 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 21 May 2018 05:06:39 -0700
Subject: [PATCH 1/2] Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION

Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION for GCC 8.1 to silence
-Wstringop-truncation warning:

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

	* diagnostics.h (DIAGNOSTIC_STRINGIFY_1): New.
	(DIAGNOSTIC_STRINGIFY): Likewise.
	(DIAGNOSTIC_IGNORE): Replace STRINGIFY with DIAGNOSTIC_STRINGIFY.
	(DIAGNOSTIC_IGNORE_SELF_MOVE): Define empty if not defined.
	(DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER): Likewise.
	(DIAGNOSTIC_IGNORE_UNUSED_FUNCTION): Likewise.
	(DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES): Likewise.
	(DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION): New.
---
 include/diagnostics.h | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/include/diagnostics.h b/include/diagnostics.h
index 0725664177..f7412d4a38 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -19,8 +19,13 @@
 #ifdef __GNUC__
 # define DIAGNOSTIC_PUSH _Pragma ("GCC diagnostic push")
 # define DIAGNOSTIC_POP _Pragma ("GCC diagnostic pop")
+
+/* Stringification.  */
+# define DIAGNOSTIC_STRINGIFY_1(x) #x
+# define DIAGNOSTIC_STRINGIFY(x) DIAGNOSTIC_STRINGIFY_1 (x)
+
 # define DIAGNOSTIC_IGNORE(option) \
-  _Pragma (STRINGIFY (GCC diagnostic ignored option))
+  _Pragma (DIAGNOSTIC_STRINGIFY (GCC diagnostic ignored option))
 #else
 # define DIAGNOSTIC_PUSH
 # define DIAGNOSTIC_POP
@@ -37,24 +42,36 @@
 # if __has_warning ("-Wenum-compare-switch")
 #  define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES \
    DIAGNOSTIC_IGNORE ("-Wenum-compare-switch")
-# else
-#  define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 # endif
 #elif defined (__GNUC__) /* GCC */
 
-# define DIAGNOSTIC_IGNORE_SELF_MOVE
-# define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER
 # define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
   DIAGNOSTIC_IGNORE ("-Wunused-function")
-# define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 
-#else /* Other compilers */
+# if __GNUC__ == 8 && __GNUC_MINOR__ < 2
+#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
+  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
+# endif
+#endif
 
+#ifndef DIAGNOSTIC_IGNORE_SELF_MOVE
 # define DIAGNOSTIC_IGNORE_SELF_MOVE
+#endif
+
+#ifndef DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER
 # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER
+#endif
+
+#ifndef DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
 # define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
+#endif
+
+#ifndef DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
 # define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
+#endif
 
+#ifndef DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION
+# define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION
 #endif
 
 #endif /* DIAGNOSTICS_H */
-- 
2.17.0

Re: [PATCH 2/3] Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION

[Nick Clifton]

Hi H.J.

> +# if __GNUC__ == 8 && __GNUC_MINOR__ < 2
> +#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
> +  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
> +# endif
> 
> OK for master?

Approved - please apply.

Cheers
  Nick

Re: [PATCH 2/3] Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION

[Pedro Alves]

On 06/04/2018 01:13 PM, Nick Clifton wrote:
> Hi H.J.
> 
>> +# if __GNUC__ == 8 && __GNUC_MINOR__ < 2
>> +#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
>> +  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
>> +# endif
>>
>> OK for master?
> 
> Approved - please apply.

Please don't.  This is again going against the intention of
the header.  The GCC version checks should be put in the
places where the warning needs to be suppressed.
The current patch makes all current and future uses of
DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION only apply to
GCC 8.1.  That is incorrect.  Consider what you will
have to do to suppress some -Wstrinop-truncation warning
with DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION on GCC 8.2 or
GCC 9 or whatever.

Also, you can use GCC_VERSION to make the version check
a little simpler.

Thanks,
Pedro Alves

Re: [PATCH 2/3] Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION

[H.J. Lu]

On Mon, Jun 4, 2018 at 5:19 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/04/2018 01:13 PM, Nick Clifton wrote:
>> Hi H.J.
>>
>>> +# if __GNUC__ == 8 && __GNUC_MINOR__ < 2
>>> +#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
>>> +  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
>>> +# endif
>>>
>>> OK for master?
>>
>> Approved - please apply.
>
> Please don't.  This is again going against the intention of
> the header.  The GCC version checks should be put in the
> places where the warning needs to be suppressed.
> The current patch makes all current and future uses of
> DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION only apply to
> GCC 8.1.  That is incorrect.  Consider what you will
> have to do to suppress some -Wstrinop-truncation warning
> with DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION on GCC 8.2 or
> GCC 9 or whatever.

This is what my original patch intended to do.  But diagnostics.h from
GDB doesn't support GCC version.   Should I extend it to match glibc?

> Also, you can use GCC_VERSION to make the version check
> a little simpler.
>
> Thanks,
> Pedro Alves



-- 
H.J.

Re: [PATCH 2/3] Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION

[H.J. Lu]

On Mon, Jun 4, 2018 at 5:46 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Jun 4, 2018 at 5:19 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 06/04/2018 01:13 PM, Nick Clifton wrote:
>>> Hi H.J.
>>>
>>>> +# if __GNUC__ == 8 && __GNUC_MINOR__ < 2
>>>> +#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
>>>> +  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
>>>> +# endif
>>>>
>>>> OK for master?
>>>
>>> Approved - please apply.
>>
>> Please don't.  This is again going against the intention of
>> the header.  The GCC version checks should be put in the
>> places where the warning needs to be suppressed.
>> The current patch makes all current and future uses of
>> DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION only apply to
>> GCC 8.1.  That is incorrect.  Consider what you will
>> have to do to suppress some -Wstrinop-truncation warning
>> with DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION on GCC 8.2 or
>> GCC 9 or whatever.
>
> This is what my original patch intended to do.  But diagnostics.h from
> GDB doesn't support GCC version.   Should I extend it to match glibc?
>
>> Also, you can use GCC_VERSION to make the version check
>> a little simpler.
>>
>> Thanks,
>> Pedro Alves

My second patch is needed for Darwin.   I will check it in as is
and update it with a follow up patch to support GCC version.


-- 
H.J.

Re: [PATCH 2/3] Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION

[Pedro Alves]

On 06/04/2018 02:30 PM, H.J. Lu wrote:
> On Mon, Jun 4, 2018 at 5:46 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Jun 4, 2018 at 5:19 AM, Pedro Alves <palves@redhat.com> wrote:
>>> On 06/04/2018 01:13 PM, Nick Clifton wrote:
>>>> Hi H.J.
>>>>
>>>>> +# if __GNUC__ == 8 && __GNUC_MINOR__ < 2
>>>>> +#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
>>>>> +  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
>>>>> +# endif
>>>>>
>>>>> OK for master?
>>>>
>>>> Approved - please apply.
>>>
>>> Please don't.  This is again going against the intention of
>>> the header.  The GCC version checks should be put in the
>>> places where the warning needs to be suppressed.
>>> The current patch makes all current and future uses of
>>> DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION only apply to
>>> GCC 8.1.  That is incorrect.  Consider what you will
>>> have to do to suppress some -Wstrinop-truncation warning
>>> with DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION on GCC 8.2 or
>>> GCC 9 or whatever.
>>
>> This is what my original patch intended to do.  But diagnostics.h from
>> GDB doesn't support GCC version.   Should I extend it to match glibc?
>>
>>> Also, you can use GCC_VERSION to make the version check
>>> a little simpler.
> 
> My second patch is needed for Darwin.   I will check it in as is
> and update it with a follow up patch to support GCC version.

For the record, I think it would have been better procedure
to split and push the DIAGNOSTIC_STRINGIFY bit only [1], while
giving people reasonable time to respond to the rest.

https://sourceware.org/ml/gdb-patches/2018-06/msg00062.html

Thanks,
Pedro Alves

Re: [PATCH 2/3] Add DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION

[Pedro Alves]

On 06/04/2018 01:46 PM, H.J. Lu wrote:
> On Mon, Jun 4, 2018 at 5:19 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 06/04/2018 01:13 PM, Nick Clifton wrote:
>>> Hi H.J.
>>>
>>>> +# if __GNUC__ == 8 && __GNUC_MINOR__ < 2
>>>> +#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
>>>> +  DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
>>>> +# endif
>>>>
>>>> OK for master?
>>>
>>> Approved - please apply.
>>
>> Please don't.  This is again going against the intention of
>> the header.  The GCC version checks should be put in the
>> places where the warning needs to be suppressed.
>> The current patch makes all current and future uses of
>> DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION only apply to
>> GCC 8.1.  That is incorrect.  Consider what you will
>> have to do to suppress some -Wstrinop-truncation warning
>> with DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION on GCC 8.2 or
>> GCC 9 or whatever.
> 

> This is what my original patch intended to do.  But diagnostics.h from
> GDB doesn't support GCC version. > Should I extend it to match glibc?

diagnostics.h doesn't support a GCC version parameter on purpose, as I
explained before.  A single GCC version argument like glibc does
only works for GCC, and is in fact not used by the macro at all,
it is only there for conveniently grepping.  It's a nop argument:

#define DIAG_IGNORE_NEEDS_COMMENT(version, option)     \
  _Pragma (_DIAG_STR (GCC diagnostic ignored option))

I've detailed more here on why I don't think that's right:

 https://sourceware.org/ml/binutils/2018-05/msg00191.html



We mainly want to use DIAGNOSTIC_IGNORE_XXX to disable warnings 
around code that even though is seemingly smelly, is actually
what we want to write.  See these examples:

 https://sourceware.org/ml/gdb-patches/2017-06/msg00622.html
 https://sourceware.org/ml/gdb-patches/2017-12/msg00529.html

The case in question is a little different, as the warning is
a false positive in these particular spots.  So I think we should
disable it in these particular spots, only.  I.e., I think a version
check in the client side (like your original patch, IIRC, though
implemented differently) is appropriate, like:

+	DIAGNOSTIC_PUSH;
+	/* GCC 8.1 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+#if GCC_VERSION == 8001
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
+#endif
 	strncpy (data + 44, va_arg (ap, const char *), 80);
+	DIAGNOSTIC_POP;

It would be fine with me to add the GCC version check to diagnostics.h,
if, when you consider what to do when you need to disable 
-Wstringop-truncation in some future GCC version, in some
other spot in the code, you decide that if that happens,
it's OK to disable -Wstringop-truncation everywhere 
DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION is used at that point
in time, even if the current spots that use
DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION won't trigger false positive
with the newer GCC version that is forcing you to change the
version check.  That seems to go counter to the expressed desire
to only disable -Wstringop-truncation in these spots with 8.1
but not 8.2, though.  That's a design conflict.

>> Also, you can use GCC_VERSION to make the version check
>> a little simpler.

Thanks,
Pedro Alves

[PATCH 3/3] Use DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION to silence GCC 8

[H.J. Lu]

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

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

Use DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION to silence it.

	PR binutils/23146
	* bfd-in.h: Include "diagnostics.h".
	* bfd-in2.h: Regenerated.
	* elf32-arm.c (elf32_arm_nabi_write_core_note): Use
	DIAGNOSTIC_PUSH, DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION and
	DIAGNOSTIC_POP 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        | 1 +
 bfd/bfd-in2.h       | 1 +
 bfd/elf32-arm.c     | 7 +++++++
 bfd/elf32-ppc.c     | 7 +++++++
 bfd/elf32-s390.c    | 7 +++++++
 bfd/elf64-ppc.c     | 7 +++++++
 bfd/elf64-s390.c    | 7 +++++++
 bfd/elfxx-aarch64.c | 7 +++++++
 8 files changed, 44 insertions(+)

diff --git a/bfd/bfd-in.h b/bfd/bfd-in.h
index 481587e458..1d477c3f5f 100644
--- a/bfd/bfd-in.h
+++ b/bfd/bfd-in.h
@@ -34,6 +34,7 @@ extern "C" {
 
 #include "ansidecl.h"
 #include "symcat.h"
+#include "diagnostics.h"
 #include <stdarg.h>
 #include <sys/stat.h>
 
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index c64eee1fe1..4a5c51fdf8 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -41,6 +41,7 @@ extern "C" {
 
 #include "ansidecl.h"
 #include "symcat.h"
+#include "diagnostics.h"
 #include <stdarg.h>
 #include <sys/stat.h>
 
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index 5a3b58ff0d..4e04a2f699 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -2174,7 +2174,14 @@ 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);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
 	strncpy (data + 44, va_arg (ap, const char *), 80);
+	DIAGNOSTIC_POP;
 	va_end (ap);
 
 	return elfcore_write_note (abfd, buf, bufsiz,
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index 462c8af311..b5cb14ecbf 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -2411,7 +2411,14 @@ 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);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
 	strncpy (data + 48, va_arg (ap, const char *), 80);
+	DIAGNOSTIC_POP;
 	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..b9e1602aa2 100644
--- a/bfd/elf32-s390.c
+++ b/bfd/elf32-s390.c
@@ -3951,7 +3951,14 @@ elf_s390_write_core_note (bfd *abfd, char *buf, int *bufsiz,
 	va_end (ap);
 
 	strncpy (data + 28, fname, 16);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
 	strncpy (data + 44, psargs, 80);
+	DIAGNOSTIC_POP;
 	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 b166558945..bf314b3571 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -3041,7 +3041,14 @@ 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);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
 	strncpy (data + 56, va_arg (ap, const char *), 80);
+	DIAGNOSTIC_POP;
 	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..4552bd6ee8 100644
--- a/bfd/elf64-s390.c
+++ b/bfd/elf64-s390.c
@@ -3760,7 +3760,14 @@ elf_s390_write_core_note (bfd *abfd, char *buf, int *bufsiz,
 	va_end (ap);
 
 	strncpy (data + 40, fname, 16);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
 	strncpy (data + 56, psargs, 80);
+	DIAGNOSTIC_POP;
 	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..f418581048 100644
--- a/bfd/elfxx-aarch64.c
+++ b/bfd/elfxx-aarch64.c
@@ -659,7 +659,14 @@ _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);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
 	strncpy (data + 56, va_arg (ap, const char *), 80);
+	DIAGNOSTIC_POP;
 	va_end (ap);
 
 	return elfcore_write_note (abfd, buf, bufsiz, "CORE",
-- 
2.17.0

Re: [PATCH 3/3] Use DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION to silence GCC 8

[Nick Clifton]

Hi H.J. 

> 	PR binutils/23146
> 	* bfd-in.h: Include "diagnostics.h".
> 	* bfd-in2.h: Regenerated.
> 	* elf32-arm.c (elf32_arm_nabi_write_core_note): Use
> 	DIAGNOSTIC_PUSH, DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION and
> 	DIAGNOSTIC_POP 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.

Approved - please apply.

Cheers
  Nick

Re: [PATCH 3/3] Use DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION to silence GCC 8

[H.J. Lu]

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

On Fri, Jun 1, 2018 at 12:59 AM, Nick Clifton <nickc@redhat.com> wrote:
> Hi H.J.
>
>>       PR binutils/23146
>>       * bfd-in.h: Include "diagnostics.h".
>>       * bfd-in2.h: Regenerated.
>>       * elf32-arm.c (elf32_arm_nabi_write_core_note): Use
>>       DIAGNOSTIC_PUSH, DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION and
>>       DIAGNOSTIC_POP 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.
>
> Approved - please apply.
>

This is the patch I am checking in.

-- 
H.J.

[-- Attachment #2: 0001-Use-DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION-to-silence.patch --]
[-- Type: text/x-patch, Size: 6792 bytes --]

From 30a2bf66aa5b1a22ea14cbc5eb10d12cae5657c1 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 4 Jun 2018 09:48:28 -0700
Subject: [PATCH] Use DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION to silence GCC 8.1

GCC 8.1 warns about destination size with -Wstringop-truncation:

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

Use DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION to silence it.

bfd/

	PR binutils/23146
	* bfd-in.h: Include "diagnostics.h".
	* bfd-in2.h: Regenerated.
	* elf32-arm.c (elf32_arm_nabi_write_core_note): Use
	DIAGNOSTIC_PUSH, DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION and
	DIAGNOSTIC_POP to silence GCC 8.1 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.

include/

	* diagnostics.h (DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION): Always
	define for GCC.
---
 bfd/bfd-in.h          | 1 +
 bfd/bfd-in2.h         | 1 +
 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 +++++++++
 include/diagnostics.h | 4 +---
 9 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/bfd/bfd-in.h b/bfd/bfd-in.h
index 481587e458..1d477c3f5f 100644
--- a/bfd/bfd-in.h
+++ b/bfd/bfd-in.h
@@ -34,6 +34,7 @@ extern "C" {
 
 #include "ansidecl.h"
 #include "symcat.h"
+#include "diagnostics.h"
 #include <stdarg.h>
 #include <sys/stat.h>
 
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index ef62f31953..65735f026e 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -41,6 +41,7 @@ extern "C" {
 
 #include "ansidecl.h"
 #include "symcat.h"
+#include "diagnostics.h"
 #include <stdarg.h>
 #include <sys/stat.h>
 
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index dbfd838fbe..870111b5ed 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);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8.1 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+#if GCC_VERSION == 8001
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
+#endif
 	strncpy (data + 44, va_arg (ap, const char *), 80);
+	DIAGNOSTIC_POP;
 	va_end (ap);
 
 	return elfcore_write_note (abfd, buf, bufsiz,
diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index 3482baca20..ea8dbed981 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);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8.1 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+#if GCC_VERSION == 8001
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
+#endif
 	strncpy (data + 48, va_arg (ap, const char *), 80);
+	DIAGNOSTIC_POP;
 	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 99ceb76d3d..ebda1dacdc 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);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8.1 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+#if GCC_VERSION == 8001
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
+#endif
 	strncpy (data + 44, psargs, 80);
+	DIAGNOSTIC_POP;
 	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 f543cb0288..16199fbe3e 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);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8.1 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+#if GCC_VERSION == 8001
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
+#endif
 	strncpy (data + 56, va_arg (ap, const char *), 80);
+	DIAGNOSTIC_POP;
 	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 612557fa74..93a3c7c22a 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);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8.1 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+#if GCC_VERSION == 8001
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
+#endif
 	strncpy (data + 56, psargs, 80);
+	DIAGNOSTIC_POP;
 	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 af37f828d5..3ea8dadf6d 100644
--- a/bfd/elfxx-aarch64.c
+++ b/bfd/elfxx-aarch64.c
@@ -640,7 +640,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);
+	DIAGNOSTIC_PUSH;
+	/* GCC 8.1 warns about 80 equals destination size with
+	   -Wstringop-truncation:
+	   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
+	 */
+#if GCC_VERSION == 8001
+	DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION;
+#endif
 	strncpy (data + 56, va_arg (ap, const char *), 80);
+	DIAGNOSTIC_POP;
 	va_end (ap);
 
 	return elfcore_write_note (abfd, buf, bufsiz, "CORE",
diff --git a/include/diagnostics.h b/include/diagnostics.h
index f7412d4a38..4a674106dc 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -48,10 +48,8 @@
 # define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
   DIAGNOSTIC_IGNORE ("-Wunused-function")
 
-# if __GNUC__ == 8 && __GNUC_MINOR__ < 2
-#  define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
+# define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
   DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
-# endif
 #endif
 
 #ifndef DIAGNOSTIC_IGNORE_SELF_MOVE
-- 
2.17.1

Re: [PATCH 3/3] Use DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION to silence GCC 8

[Pedro Alves]

On 06/04/2018 05:58 PM, H.J. Lu wrote:

> This is the patch I am checking in.

Great, thank you!

Pedro Alves

Re: [PATCH 3/3] Use DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION to silence GCC 8

[Tulio Magno Quites Machado Filho]

"H.J. Lu" <hjl.tools@gmail.com> writes:

> GCC 8.1 warns about destination size with -Wstringop-truncation:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85643
>
> Use DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION to silence it.
>
> bfd/
>
> 	PR binutils/23146
> 	* bfd-in.h: Include "diagnostics.h".
> 	* bfd-in2.h: Regenerated.

bfd.h is an installed header, while diagnostics.h is not, causing build
failures on software that include bfd.h, e.g.:

In file included from opagent.c:65:
bfd.h:44:10: fatal error: diagnostics.h: No such file or directory
 #include "diagnostics.h"
          ^~~~~~~~~~~~~~~

-- 
Tulio Magno

Re: [PATCH 1/3] Move gdb/common/diagnostics.h to include/diagnostics.h

[Simon Marchi]

On 2018-05-21 08:15, H.J. Lu wrote:
> Move gdb/common/diagnostics.h to include/diagnostics.h so that it can
> be used in binutils.
> 
> gdb/
> 
> 	* ada-lex.l: Include "diagnostics.h" instead of
> 	"common/diagnostics.h".
> 	* unittests/environ-selftests.c: Likewise.
> 	* common/diagnostics.h: Moved to ../include.
> 
> include/
> 
> 	* diagnostics.h: Moved from ../gdb/common/diagnostics.h.

This series looks OK from the GDB side.

Thanks,

Simon

Re: [PATCH 1/3] Move gdb/common/diagnostics.h to include/diagnostics.h

[Nick Clifton]

Hi H.J.

> gdb/
> 
> 	* ada-lex.l: Include "diagnostics.h" instead of
> 	"common/diagnostics.h".
> 	* unittests/environ-selftests.c: Likewise.
> 	* common/diagnostics.h: Moved to ../include.
> 
> include/
> 
> 	* diagnostics.h: Moved from ../gdb/common/diagnostics.h.

Approved from the binutils side as well.  Please commit.

Cheers
  Nick

Re: [PATCH 1/3] Move gdb/common/diagnostics.h to include/diagnostics.h

[John Marshall]

On 21 May 2018, at 13:15, H dot J dot Lu <hjl dot tools at gmail dot com> wrote:
> Move gdb/common/diagnostics.h to include/diagnostics.h so that it can
> be used in binutils.

This patch broke building gdb on MacOS with clang (i.e., after ./configure with no options):

  CXX    gdb.o
In file included from ../../../binutils-gdb/gdb/gdb.c:19:
In file included from ../../../binutils-gdb/gdb/defs.h:531:
In file included from ../../../binutils-gdb/gdb/gdbarch.h:39:
In file included from ../../../binutils-gdb/gdb/frame.h:72:
In file included from ../../../binutils-gdb/gdb/language.h:26:
../../../binutils-gdb/gdb/symtab.h:1361:1: error: _Pragma takes a parenthesized string literal
DEF_VEC_P (symtab_ptr);

> --- a/gdb/common/diagnostics.h
> +++ b/include/diagnostics.h
> [snip]
> @@ -15,10 +13,8 @@
>    You should have received a copy of the GNU General Public License
>    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> 
> -#ifndef COMMON_DIAGNOSTICS_H
> -#define COMMON_DIAGNOSTICS_H
> -
> -#include "common/preprocessor.h"

Putting this #include back fixes the build. Apparently in this configuration, include/diagnostics.h doesn't otherwise have a definition of STRINGIFY whereas on Linux or other platforms it does, via some coincidence of different host-related includes or something.

Cheers,

    John

Re: [PATCH 1/3] Move gdb/common/diagnostics.h to include/diagnostics.h

[H.J. Lu]

On Mon, Jun 4, 2018 at 3:51 AM, John Marshall
<John.W.Marshall@glasgow.ac.uk> wrote:
> On 21 May 2018, at 13:15, H dot J dot Lu <hjl dot tools at gmail dot com> wrote:
>> Move gdb/common/diagnostics.h to include/diagnostics.h so that it can
>> be used in binutils.
>
> This patch broke building gdb on MacOS with clang (i.e., after ./configure with no options):
>
>   CXX    gdb.o
> In file included from ../../../binutils-gdb/gdb/gdb.c:19:
> In file included from ../../../binutils-gdb/gdb/defs.h:531:
> In file included from ../../../binutils-gdb/gdb/gdbarch.h:39:
> In file included from ../../../binutils-gdb/gdb/frame.h:72:
> In file included from ../../../binutils-gdb/gdb/language.h:26:
> ../../../binutils-gdb/gdb/symtab.h:1361:1: error: _Pragma takes a parenthesized string literal
> DEF_VEC_P (symtab_ptr);
>
>> --- a/gdb/common/diagnostics.h
>> +++ b/include/diagnostics.h
>> [snip]
>> @@ -15,10 +13,8 @@
>>    You should have received a copy of the GNU General Public License
>>    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>
>> -#ifndef COMMON_DIAGNOSTICS_H
>> -#define COMMON_DIAGNOSTICS_H
>> -
>> -#include "common/preprocessor.h"
>
> Putting this #include back fixes the build. Apparently in this configuration, include/diagnostics.h doesn't otherwise have a definition of STRINGIFY whereas on Linux or other platforms it does, via some coincidence of different host-related includes or something.

Please add

#include "common/preprocessor.h"

to gdb.c.


-- 
H.J.

Re: [PATCH 1/3] Move gdb/common/diagnostics.h to include/diagnostics.h

[John Marshall]

On 4 Jun 2018, at 14:12, H.J. Lu <hjl.tools@gmail.com> wrote:
> Please add
> 
> #include "common/preprocessor.h"
> 
> to gdb.c.

The problem occurs in other source files too. (And I'm not a GDB maintainer, so I won't be adding anything to anything :-))

>    Move gdb/common/diagnostics.h to include/diagnostics.h so that it can
>    be used in binutils.

If the intention is to use include/diagnostics.h in binutils outwith GDB, then gdb/common/preprocessor.h will need to follow diagnostics.h to include/. (Or at least a definition of STRINGIFY will need to be in include/ -- but preprocessor.h seems as generic as diagnostics.h in any case, so it seems appropriate to just move it all.)

    John

Re: [PATCH 1/3] Move gdb/common/diagnostics.h to include/diagnostics.h

[Pedro Alves]

On 06/04/2018 02:12 PM, H.J. Lu wrote:
> On Mon, Jun 4, 2018 at 3:51 AM, John Marshall
> <John.W.Marshall@glasgow.ac.uk> wrote:
>> On 21 May 2018, at 13:15, H dot J dot Lu <hjl dot tools at gmail dot com> wrote:
>>> Move gdb/common/diagnostics.h to include/diagnostics.h so that it can
>>> be used in binutils.
>>
>> This patch broke building gdb on MacOS with clang (i.e., after ./configure with no options):
>>
>>   CXX    gdb.o
>> In file included from ../../../binutils-gdb/gdb/gdb.c:19:
>> In file included from ../../../binutils-gdb/gdb/defs.h:531:
>> In file included from ../../../binutils-gdb/gdb/gdbarch.h:39:
>> In file included from ../../../binutils-gdb/gdb/frame.h:72:
>> In file included from ../../../binutils-gdb/gdb/language.h:26:
>> ../../../binutils-gdb/gdb/symtab.h:1361:1: error: _Pragma takes a parenthesized string literal
>> DEF_VEC_P (symtab_ptr);

I think clang is printing a bogus location here.  symtab.h includes
vec.h, which includes diagnostics.h and does:

/* clang has a bug that makes it warn (-Wunused-function) about unused functions
   that are the result of the DEF_VEC_* macro expansion.  See:

     https://bugs.llvm.org/show_bug.cgi?id=22712

   We specifically ignore this warning for the vec functions when the compiler
   is clang.  */
#ifdef __clang__
# define DIAGNOSTIC_IGNORE_UNUSED_VEC_FUNCTION \
    DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
#else
# define DIAGNOSTIC_IGNORE_UNUSED_VEC_FUNCTION
#endif

and that's most certainly what is tripping on the _Pragma+STRINGIFY.

>>
>>> --- a/gdb/common/diagnostics.h
>>> +++ b/include/diagnostics.h
>>> [snip]
>>> @@ -15,10 +13,8 @@
>>>    You should have received a copy of the GNU General Public License
>>>    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>
>>> -#ifndef COMMON_DIAGNOSTICS_H
>>> -#define COMMON_DIAGNOSTICS_H
>>> -
>>> -#include "common/preprocessor.h"
>>
>> Putting this #include back fixes the build. Apparently in this configuration, include/diagnostics.h doesn't otherwise have a definition of STRINGIFY whereas on Linux or other platforms it does, via some coincidence of different host-related includes or something.
> 
> Please add
> 
> #include "common/preprocessor.h"
> 
> to gdb.c.
> 

I don't think so, see above.

Where does binutils get the definition of STRINGIFY from?

Your other patch does:

> --- a/include/diagnostics.h
> +++ b/include/diagnostics.h
> @@ -19,8 +19,13 @@
>  #ifdef __GNUC__
>  # define DIAGNOSTIC_PUSH _Pragma ("GCC diagnostic push")
>  # define DIAGNOSTIC_POP _Pragma ("GCC diagnostic pop")
> +
> +/* Stringification.  */
> +# define DIAGNOSTIC_STRINGIFY_1(x) #x
> +# define DIAGNOSTIC_STRINGIFY(x) DIAGNOSTIC_STRINGIFY_1 (x)
> +
>  # define DIAGNOSTIC_IGNORE(option) \
> -  _Pragma (STRINGIFY (GCC diagnostic ignored option))
> +  _Pragma (DIAGNOSTIC_STRINGIFY (GCC diagnostic ignored option))
>  #else

So I'm surprised by your suggestion.   That DIAGNOSTIC_STRINGIFY
bit should be split out of that other patch and pushed in
separately, IMO.  Alternatively, preprocessor.h should be shared too.

Thanks,
Pedro Alves

Re: [PATCH 1/3] Move gdb/common/diagnostics.h to include/diagnostics.h

[H.J. Lu]

On Mon, Jun 4, 2018 at 6:35 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/04/2018 02:12 PM, H.J. Lu wrote:
>> On Mon, Jun 4, 2018 at 3:51 AM, John Marshall
>> <John.W.Marshall@glasgow.ac.uk> wrote:
>>> On 21 May 2018, at 13:15, H dot J dot Lu <hjl dot tools at gmail dot com> wrote:
>>>> Move gdb/common/diagnostics.h to include/diagnostics.h so that it can
>>>> be used in binutils.
>>>
>>> This patch broke building gdb on MacOS with clang (i.e., after ./configure with no options):
>>>
>>>   CXX    gdb.o
>>> In file included from ../../../binutils-gdb/gdb/gdb.c:19:
>>> In file included from ../../../binutils-gdb/gdb/defs.h:531:
>>> In file included from ../../../binutils-gdb/gdb/gdbarch.h:39:
>>> In file included from ../../../binutils-gdb/gdb/frame.h:72:
>>> In file included from ../../../binutils-gdb/gdb/language.h:26:
>>> ../../../binutils-gdb/gdb/symtab.h:1361:1: error: _Pragma takes a parenthesized string literal
>>> DEF_VEC_P (symtab_ptr);
>
> I think clang is printing a bogus location here.  symtab.h includes
> vec.h, which includes diagnostics.h and does:
>
> /* clang has a bug that makes it warn (-Wunused-function) about unused functions
>    that are the result of the DEF_VEC_* macro expansion.  See:
>
>      https://bugs.llvm.org/show_bug.cgi?id=22712
>
>    We specifically ignore this warning for the vec functions when the compiler
>    is clang.  */
> #ifdef __clang__
> # define DIAGNOSTIC_IGNORE_UNUSED_VEC_FUNCTION \
>     DIAGNOSTIC_IGNORE_UNUSED_FUNCTION
> #else
> # define DIAGNOSTIC_IGNORE_UNUSED_VEC_FUNCTION
> #endif
>
> and that's most certainly what is tripping on the _Pragma+STRINGIFY.
>
>>>
>>>> --- a/gdb/common/diagnostics.h
>>>> +++ b/include/diagnostics.h
>>>> [snip]
>>>> @@ -15,10 +13,8 @@
>>>>    You should have received a copy of the GNU General Public License
>>>>    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>>
>>>> -#ifndef COMMON_DIAGNOSTICS_H
>>>> -#define COMMON_DIAGNOSTICS_H
>>>> -
>>>> -#include "common/preprocessor.h"
>>>
>>> Putting this #include back fixes the build. Apparently in this configuration, include/diagnostics.h doesn't otherwise have a definition of STRINGIFY whereas on Linux or other platforms it does, via some coincidence of different host-related includes or something.
>>
>> Please add
>>
>> #include "common/preprocessor.h"
>>
>> to gdb.c.
>>
>
> I don't think so, see above.
>
> Where does binutils get the definition of STRINGIFY from?
>
> Your other patch does:
>
>> --- a/include/diagnostics.h
>> +++ b/include/diagnostics.h
>> @@ -19,8 +19,13 @@
>>  #ifdef __GNUC__
>>  # define DIAGNOSTIC_PUSH _Pragma ("GCC diagnostic push")
>>  # define DIAGNOSTIC_POP _Pragma ("GCC diagnostic pop")
>> +
>> +/* Stringification.  */
>> +# define DIAGNOSTIC_STRINGIFY_1(x) #x
>> +# define DIAGNOSTIC_STRINGIFY(x) DIAGNOSTIC_STRINGIFY_1 (x)
>> +
>>  # define DIAGNOSTIC_IGNORE(option) \
>> -  _Pragma (STRINGIFY (GCC diagnostic ignored option))
>> +  _Pragma (DIAGNOSTIC_STRINGIFY (GCC diagnostic ignored option))
>>  #else
>
> So I'm surprised by your suggestion.   That DIAGNOSTIC_STRINGIFY
> bit should be split out of that other patch and pushed in
> separately, IMO.  Alternatively, preprocessor.h should be shared too.
>

I pushed my second patch and will submit a follow up patch to address
your concern on GCC version.


-- 
H.J.