gdb: Fix build error on macOS

Message ID MEAP282MB029370891A815AD567B60CBDDDAE9@MEAP282MB0293.AUSP282.PROD.OUTLOOK.COM
State New
Headers show
Series
  • gdb: Fix build error on macOS
Related show

Commit Message

Philippe Waroquiers via Gdb-patches Oct. 4, 2021, 10:03 a.m.
PR build/28413 notes that the gdb master fails to build on macOS.

Based on De Morgan's law, the expression
!(defined __GNUC__ && !defined __clang__ && __OPTIMIZE__)
is equal to (!defined __GNUC__ || defined __clang__ || !__OPTIMIZE).
The expression above looks weird, maybe there are some problems with
this way of judging. When any one of the conditions is established, the
subsequent judgment will be ignored.

This patch works around the issue by rewriting the expression of
judgment after referring to the nearby notes.

Tested by building on x86_64-Linux and macOS(Mojave & Catalina).

Bug: https://sourceware.org/PR28413
---
 gnulib/import/c++defs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.30.2

Comments

Philippe Waroquiers via Gdb-patches Oct. 4, 2021, 4:14 p.m. | #1
On 2021-10-04 06:03, Enze Li via Gdb-patches wrote:
> PR build/28413 notes that the gdb master fails to build on macOS.

> 

> Based on De Morgan's law, the expression

> !(defined __GNUC__ && !defined __clang__ && __OPTIMIZE__)

> is equal to (!defined __GNUC__ || defined __clang__ || !__OPTIMIZE).

> The expression above looks weird, maybe there are some problems with

> this way of judging. When any one of the conditions is established, the

> subsequent judgment will be ignored.

> 

> This patch works around the issue by rewriting the expression of

> judgment after referring to the nearby notes.

> 

> Tested by building on x86_64-Linux and macOS(Mojave & Catalina).


Thanks for addressing this.  Please send the patch to upstream gnulib,
to the following mailing list:

  https://lists.gnu.org/mailman/listinfo/bug-gnulib

Then we can pull the updated feedback.

Otherwise, we can always have a local patch in
binutils-gdb/gnulib/patches, but if the change is correct, I don't see
why it wouldn't be accepted in upstream gnulib.

Thanks,

Simon
Philippe Waroquiers via Gdb-patches Oct. 5, 2021, 4:51 a.m. | #2
[ add bug-gnulib@gnu.org ]

On 10/4/21 6:03 PM, Enze Li via Gdb-patches wrote:
> PR build/28413 notes that the gdb master fails to build on macOS.

> 

> Based on De Morgan's law, the expression

> !(defined __GNUC__ && !defined __clang__ && __OPTIMIZE__)

> is equal to (!defined __GNUC__ || defined __clang__ || !__OPTIMIZE).

> The expression above looks weird, maybe there are some problems with

> this way of judging. When any one of the conditions is established, the

> subsequent judgment will be ignored.

> 

> This patch works around the issue by rewriting the expression of

> judgment after referring to the nearby notes.

> 

> Tested by building on x86_64-Linux and macOS(Mojave & Catalina).

> 

> Bug: https://sourceware.org/PR28413

> ---

>  gnulib/import/c++defs.h | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/gnulib/import/c++defs.h b/gnulib/import/c++defs.h

> index 39df1bc76bc..dbbae2f1fa2 100644

> --- a/gnulib/import/c++defs.h

> +++ b/gnulib/import/c++defs.h

> @@ -286,7 +286,7 @@

>     _GL_CXXALIASWARN_2 (func, namespace)

>  /* To work around GCC bug <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43881>,

>     we enable the warning only when not optimizing.  */

> -# if !(defined __GNUC__ && !defined __clang__ && __OPTIMIZE__)

> +# if defined __GNUC__ && !defined __clang__ && !__OPTIMIZE__

>  #  define _GL_CXXALIASWARN_2(func,namespace) \

>      _GL_WARN_ON_USE (func, \

>                       "The symbol ::" #func " refers to the system function. " \

> @@ -314,7 +314,7 @@

>     _GL_CXXALIASWARN1_2 (func, rettype, parameters_and_attributes, namespace)

>  /* To work around GCC bug <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43881>,

>     we enable the warning only when not optimizing.  */

> -# if !(defined __GNUC__ && !defined __clang__ && __OPTIMIZE__)

> +# if defined __GNUC__ && !defined __clang__ && !__OPTIMIZE__

>  #  define _GL_CXXALIASWARN1_2(func,rettype,parameters_and_attributes,namespace) \

>      _GL_WARN_ON_USE_CXX (func, rettype, rettype, parameters_and_attributes, \

>                           "The symbol ::" #func " refers to the system function. " \

>
Philippe Waroquiers via Gdb-patches Oct. 5, 2021, 8:23 a.m. | #3
Thanks for the review.

I noticed that the upstream gnulib and the gnulib used in gdb have different directory structures, and it would be confused to forward this patch to them directly. I will soon remake a patch based on the upstream gnulib and send it to the bug-gnulib mailing list.

Thanks,
Enze

On 10/5/21 12:14 AM, Simon Marchi wrote:
> 

> 

> On 2021-10-04 06:03, Enze Li via Gdb-patches wrote:

>> PR build/28413 notes that the gdb master fails to build on macOS.

>>

>> Based on De Morgan's law, the expression

>> !(defined __GNUC__ && !defined __clang__ && __OPTIMIZE__)

>> is equal to (!defined __GNUC__ || defined __clang__ || !__OPTIMIZE).

>> The expression above looks weird, maybe there are some problems with

>> this way of judging. When any one of the conditions is established, the

>> subsequent judgment will be ignored.

>>

>> This patch works around the issue by rewriting the expression of

>> judgment after referring to the nearby notes.

>>

>> Tested by building on x86_64-Linux and macOS(Mojave & Catalina).

> 

> Thanks for addressing this.  Please send the patch to upstream gnulib,

> to the following mailing list:

> 

>   https://lists.gnu.org/mailman/listinfo/bug-gnulib

> 

> Then we can pull the updated feedback.

> 

> Otherwise, we can always have a local patch in

> binutils-gdb/gnulib/patches, but if the change is correct, I don't see

> why it wouldn't be accepted in upstream gnulib.

> 

> Thanks,

> 

> Simon

>
Bruno Haible Oct. 6, 2021, 12:05 a.m. | #4
Enze Li wrote:
> > PR build/28413 notes that the gdb master fails to build on macOS.


The build fails due to the combination of '-Werror' and a useful
warning emitted by Gnulib.

But Gnulib does not support '-Werror'. To fix the issue, you need to
filter out the '-Werror' option. Or not add it in the first place.

> > Based on De Morgan's law, the expression

> > !(defined __GNUC__ && !defined __clang__ && __OPTIMIZE__)

> > is equal to (!defined __GNUC__ || defined __clang__ || !__OPTIMIZE).

> > The expression above looks weird, maybe there are some problems with

> > this way of judging. When any one of the conditions is established, the

> > subsequent judgment will be ignored.

> > 

> > This patch works around the issue by rewriting the expression of

> > judgment after referring to the nearby notes.


This text does not explain what the patch does.

> > --- a/gnulib/import/c++defs.h

> > +++ b/gnulib/import/c++defs.h

> > @@ -286,7 +286,7 @@

> >     _GL_CXXALIASWARN_2 (func, namespace)

> >  /* To work around GCC bug <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43881>,

> >     we enable the warning only when not optimizing.  */

> > -# if !(defined __GNUC__ && !defined __clang__ && __OPTIMIZE__)

> > +# if defined __GNUC__ && !defined __clang__ && !__OPTIMIZE__

> >  #  define _GL_CXXALIASWARN_2(func,namespace) \

> >      _GL_WARN_ON_USE (func, \

> >                       "The symbol ::" #func " refers to the system function. " \


The previous code enables a useful warning for clang, and does not enable it
for GCC because that would trigger a GCC bug.

Your patch swaps the cases of clang and GCC. Thus it removes a useful warning
for clang builds, and triggers a known GCC bug in the GCC builds.

> > @@ -314,7 +314,7 @@

> >     _GL_CXXALIASWARN1_2 (func, rettype, parameters_and_attributes, namespace)

> >  /* To work around GCC bug <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43881>,

> >     we enable the warning only when not optimizing.  */

> > -# if !(defined __GNUC__ && !defined __clang__ && __OPTIMIZE__)

> > +# if defined __GNUC__ && !defined __clang__ && !__OPTIMIZE__

> >  #  define _GL_CXXALIASWARN1_2(func,rettype,parameters_and_attributes,namespace) \

> >      _GL_WARN_ON_USE_CXX (func, rettype, rettype, parameters_and_attributes, \

> >                           "The symbol ::" #func " refers to the system function. " \


Likewise.

So, the patch is bad on all accounts.

Bruno

Patch

diff --git a/gnulib/import/c++defs.h b/gnulib/import/c++defs.h
index 39df1bc76bc..dbbae2f1fa2 100644
--- a/gnulib/import/c++defs.h
+++ b/gnulib/import/c++defs.h
@@ -286,7 +286,7 @@ 
    _GL_CXXALIASWARN_2 (func, namespace)
 /* To work around GCC bug <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43881>,
    we enable the warning only when not optimizing.  */
-# if !(defined __GNUC__ && !defined __clang__ && __OPTIMIZE__)
+# if defined __GNUC__ && !defined __clang__ && !__OPTIMIZE__
 #  define _GL_CXXALIASWARN_2(func,namespace) \
     _GL_WARN_ON_USE (func, \
                      "The symbol ::" #func " refers to the system function. " \
@@ -314,7 +314,7 @@ 
    _GL_CXXALIASWARN1_2 (func, rettype, parameters_and_attributes, namespace)
 /* To work around GCC bug <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43881>,
    we enable the warning only when not optimizing.  */
-# if !(defined __GNUC__ && !defined __clang__ && __OPTIMIZE__)
+# if defined __GNUC__ && !defined __clang__ && !__OPTIMIZE__
 #  define _GL_CXXALIASWARN1_2(func,rettype,parameters_and_attributes,namespace) \
     _GL_WARN_ON_USE_CXX (func, rettype, rettype, parameters_and_attributes, \
                          "The symbol ::" #func " refers to the system function. " \