Bug 25731

Thanks for reporting.  I'll push a patch directly, since this is obvious, but next time don't hesitate to send your patch to the gdb-patches mailing list, as described here:

https://sourceware.org/gdb/wiki/ContributionChecklist

Hmm, in fact I can't reproduce locally.  Here's how this file is built for me:

$ gdb gnumake gdbtypes.o V=1
g++ -x c++ -std=gnu++11 -g3 -O0   -I. -I../../binutils-gdb/gdb -I../../binutils-gdb/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../binutils-gdb/gdb/../include/opcode -I../../binutils-gdb/gdb/../readline/readline/.. -I../../binutils-gdb/gdb/../zlib -I../bfd -I../../binutils-gdb/gdb/../bfd -I../../binutils-gdb/gdb/../include -I../libdecnumber -I../../binutils-gdb/gdb/../libdecnumber -I./../intl -I../../binutils-gdb/gdb/../gnulib/import -I../gnulib/import -I../../binutils-gdb/gdb/.. -I..  -DTUI=1    -I/System/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7 -I/System/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7  -I../../binutils-gdb/gdb/.. -pthread -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wno-sign-compare -Wno-mismatched-tags -Wno-error=deprecated-register -Wredundant-move -Wmissing-declarations -Wmissing-prototypes -Wformat -Wformat-nonliteral -Werror -c -o gdbtypes.o -MT gdbtypes.o -MMD -MP -MF ./.deps/gdbtypes.Tpo ../../binutils-gdb/gdb/gdbtypes.c
$ gdb which g++
/usr/bin/g++
$ gdb g++ --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/4.2.1
Apple clang version 11.0.0 (clang-1100.0.33.17)
Target: x86_64-apple-darwin19.3.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

Do you have any idea why I don't see the same failure as you?

(In reply to Simon Marchi from comment #2)
> Hmm, in fact I can't reproduce locally.  Here's how this file is built for
> me:
> 
> $ gdb gnumake gdbtypes.o V=1
> ...
> $ gdb which g++
> ...
> $ gdb g++ --version

Err, ignore the leading "gdb" in those commands, that was part of my prompt.

I noticed that we already include stdlib.h in common-defs.h:

https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdbsupport/common-defs.h;h=e42d2b80c045361cfa19f52cb146ae32c58f61b3;hb=HEAD#l87

However, according to [1], C++ standard library implementations are not mandated to make stdlib.h populate the std namespace.  I guess your C++ library doesn't.

[1] https://en.cppreference.com/w/cpp/header

So since we use std::abs, the correct thing to do would indeed be to include cstdlib.  I tried to swap stdlib.h for cstdlib in common-defs.h, I just stumbled on some old hack in ada-exp.y that can probably be removed.  But otherwise it builds for me.

Can you see if the patch below builds fine for you on macOS?


From 8ca602f1a6b05cb431b663c8ed0c09537fe31960 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Thu, 26 Mar 2020 13:27:07 -0400
Subject: [PATCH] allo

---
 gdb/ada-exp.y            | 3 ---
 gdbsupport/common-defs.h | 2 +-
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/gdb/ada-exp.y b/gdb/ada-exp.y
index c5e1e14299e..6e65b212e24 100644
--- a/gdb/ada-exp.y
+++ b/gdb/ada-exp.y
@@ -720,9 +720,6 @@ primary     :       '*' primary             %prec '.'
 
 static struct obstack temp_parse_space;
 
-/* The following kludge was found necessary to prevent conflicts between */
-/* defs.h and non-standard stdlib.h files.  */
-#define qsort __qsort__dummy
 #include "ada-lex.c"
 
 int
diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h
index e42d2b80c04..e1a431c6939 100644
--- a/gdbsupport/common-defs.h
+++ b/gdbsupport/common-defs.h
@@ -84,7 +84,7 @@
 
 #include <stdarg.h>
 #include <stdio.h>
-#include <stdlib.h>
+#include <cstdlib>
 #include <stddef.h>
 #include <stdint.h>
 #include <string.h>
-- 
2.25.1

Indeed with proposed patch it builds perfectly fine.
Though I don't have an answer to your question why the same problem is not reproduced on your side, sorry.

And thanks a lot for the prompt fix!

Hi Simon,

Do you still have any doubts about your fix from comment #4?
Otherwise it would be good to get it merged so it won't slip through the cracks.

FWIW it looks fine to me.

Sorry for bugging you but is there a chance to get proposed fix committed sometime soon?

I'll send something on the list.

Patch posted here: https://sourceware.org/pipermail/gdb-patches/2020-April/167501.html

Yet another gentle reminder :)
There seems to be a discussion on the mailing list which stops here: https://sourceware.org/pipermail/gdb-patches/2020-April/167520.html

Do we need a re-spin or it might be fixed-up by the committer?

Thanks for the reminder, I'm not very good at tracking my own patches... I'll push it, as nobody opposed, we'll see if it breaks something somewhere.

The master branch has been updated by Simon Marchi <simark@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ff8885c3be6f42ed90a7b0ec0028fad26861cd94

commit ff8885c3be6f42ed90a7b0ec0028fad26861cd94
Author: Simon Marchi <simon.marchi@polymtl.ca>
Date:   Mon Apr 27 09:19:48 2020 -0400

    gdbsupport: include cstdlib in common-defs.h
    
    In PR 25731 [1], the following build failure was reported:
    
        ../../binutils-gdb/gdb/gdbtypes.c:1254:10: error: no member named 'abs' in namespace 'std'; did you mean simply 'abs'?
                    = ((std::abs (stride) * element_count) + 7) / 8;
                        ^~~~~~~~
                        abs
        /usr/include/stdlib.h:129:6: note: 'abs' declared here
        int      abs(int) __pure2;
                 ^
    The original report was using:
    
        $ gcc -v
        Apple LLVM version 8.0.0 (clang-800.0.42.1)
        Target: x86_64-apple-darwin15.6.0
    
    Note that I was _not_ able to reproduce using:
    
        $ g++ --version
        Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/4.2.1
        Apple clang version 11.0.0 (clang-1100.0.33.17)
        Target: x86_64-apple-darwin19.3.0
    
    The proposed fix is to include <cstdlib> in addition to <stdlib.h>.
    
    Here's an excerpt of [2] relevant to this problem:
    
        These headers [speaking of the .h form] are allowed to also declare
        the same names in the std namespace, and the corresponding cxxx
        headers are allowed to also declare the same names in the global
        namespace: including <cstdlib> definitely provides std::malloc and
        may also provide ::malloc.  Including <stdlib.h> definitely provides
        ::malloc and may also provide std::malloc
    
    Since we use std::abs, we should not assume that our include of stdlib.h
    declares an `abs` function in the `std` namespace.
    
    If we replace the include of stdlib.h with cstdlib, then we fall in the
    opposite situation.  A standard C++ library may decide to only put the
    declarations in the std namespace, requiring us to prefix all standard
    functions with `std::`.  I'm not against that, but for the moment I think the
    safest way forward is to just include both.
    
    Note that I don't know what effect this patch can have on any stdlib.h fix
    provided by gnulib.
    
    [1] https://sourceware.org/bugzilla/show_bug.cgi?id=25731
    [2] https://en.cppreference.com/w/cpp/header#C_compatibility_headers
    
    gdbsupport/ChangeLog:
    
            * common-defs.h: Include cstdlib.h.

Fixed.