Bug 28417 - std::string no longer allows accepting nullptr_t since it is undefined behavior after yesterday's change on libstdc++.
Summary: std::string no longer allows accepting nullptr_t since it is undefined behavi...
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: gold (show other bugs)
Version: 2.38
: P2 normal
Target Milestone: 2.38
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-05 03:50 UTC by cqwrteur
Modified: 2021-11-05 08:10 UTC (History)
4 users (show)

See Also:
Host: x86_64-w64-mingw32
Target: x86_64-ubuntu-linux-gnu
Build: x86_64-linux-gnu
Last reconfirmed:


Attachments
just remove name_(NULL), (244 bytes, patch)
2021-10-05 03:57 UTC, cqwrteur
Details | Diff
another NULL undefined behavior bug fixed. (438 bytes, patch)
2021-10-05 04:01 UTC, cqwrteur
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description cqwrteur 2021-10-05 03:50:53 UTC
../../../../binutils-gdb/gold/options.h: In constructor 'gold::Search_directory::Search_directory()':
../../../../binutils-gdb/gold/options.h:614:7: error: use of deleted function 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(std::nullptr_t) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; std::nullptr_t = std::nullptr_t]'
  614 |     : name_(NULL), put_in_sysroot_(false), is_in_sysroot_(false)
      |       ^~~~~~~~~~~
libtool: link: x86_64-w64-mingw32-gcc -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -Wstack-usage=262144 -I../../../../binutils-gdb/binutils/../zlib -g -O2 -Wl,--stack -Wl,12582912 -o .libs/bfdtest2.exe bfdtest2.o  ../bfd/.libs/libbfd.a -L/home/cqwrteur/myhome/gcc_build/glibc231_win_canadian/binutils-gdb/zlib -lz ../libiberty/libiberty.a
In file included from /home/cqwrteur/cross/x86_64-w64-mingw32/x86_64-w64-mingw32/include/c++/12.0.0/string:53,
                 from ../../../../binutils-gdb/gold/expression.cc:25:

std::string str(nullptr); was considered undefined behavior, Now it is forbidden by ISO C++ standard since C++23. libstdc++ has updated this behavior.


https://github.com/gcc-mirror/gcc/commit/cf876562c592193732f869e9f96034a42d0fad89#diff-ba6ba5a2bddd4d1cf66ef3699e6111d9c6aa1cf0192c97e6d59a048b4a851b91


See Jonathan Wakely's words
Comment 1 cqwrteur 2021-10-05 03:57:22 UTC
Created attachment 13700 [details]
just remove name_(NULL),

Simple but correct patch.
Comment 2 cqwrteur 2021-10-05 03:58:41 UTC
Here is another problem.
../../../../binutils-gdb/gold/incremental.cc:2289:12: error: use of deleted function 'std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(std::nullptr_t) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>; std::nullptr_t = std::nullptr_t]'
 2289 |     return NULL;
      |            ^~~~
In file included from /home/cqwrteur/cross/x86_64-w64-mingw32/x86_64-w64-mingw32/include/c++/12.0.0/string:53,
                 from ../../../../binutils-gdb/gold/options.h:43,
                 from ../../../../binutils-gdb/gold/incremental.cc:30:
/home/cqwrteur/cross/x86_64-w64-mingw32/x86_64-w64-mingw32/include/c++/12.0.0/bits/basic_string.h:626:7: note: declared here
  626 |       basic_string(nullptr_t) = delete;
      |       ^~~~~~~~~~~~
Comment 3 cqwrteur 2021-10-05 04:01:01 UTC
Created attachment 13701 [details]
another NULL undefined behavior bug fixed.
Comment 4 Jonathan Wakely 2021-10-05 08:06:19 UTC
Yes, the patch is correct.

To construct an empty string either use "" as the initializer, or just use the default constructor. Creating a string from NULL or nullptr is completely bogus and has been undefined behaviour since C++98:

  basic_string(const charT* s, const Allocator& a = Allocator());

  Requires: s shall not be a null pointer.

And GCC's std::string has turned that into an exception since forever:

	// NB: Not required, but considered best practice.
	if (__gnu_cxx::__is_null_pointer(__beg) && __beg != __end)
	  std::__throw_logic_error(__N("basic_string::"
				       "_M_construct null not valid"));


Which suggests neither of these pieces of code ever executes, so could be removed.
Comment 5 Martin Liska 2021-10-14 11:33:58 UTC
@Cary: Can you please send the patch to the mailing list?
Comment 6 Jonathan Wakely 2021-10-14 11:42:41 UTC
Search_directory() says:

// We need a default constructor because we put this in a
// std::vector

That is not true since C++11. Since the constructor is broken (has undefined behaviour, which libstdc++ turns into an exception) and isn't needed for the stated purpose, maybe it should just be removed.
Comment 7 Alan Modra 2021-10-14 13:09:48 UTC
gold continues to be built with older compilers such as gcc-4.8 which default to c++98.  So beware relying on c++11.  (I've been tripped up using c++11 features in gold patches.)
Comment 8 Jonathan Wakely 2021-10-14 14:15:39 UTC
Then default constructing the string member is the right thing to do.
Comment 9 cqwrteur 2021-10-15 11:56:10 UTC
(In reply to Alan Modra from comment #7)
> gold continues to be built with older compilers such as gcc-4.8 which
> default to c++98.  So beware relying on c++11.  (I've been tripped up using
> c++11 features in gold patches.)

I am sure my patch works under C++98. It is not C++11 feature.
Comment 10 Jonathan Wakely 2021-10-15 12:17:00 UTC
Alan was replying to comment 6 where I said the default constructor could just be removed.
Comment 11 cvs-commit@gcc.gnu.org 2021-10-20 21:30:35 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 068a039b8bd7c7386bb0d88f0ae245b0fe4132e9
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Oct 20 10:09:57 2021 +1030

    PR28417, std::string no longer allows accepting nullptr_t
    
            PR 28417
            * incremental.cc (Sized_relobj_incr::do_section_name): Avoid
            std:string undefined behaviour.
            * options.h (Search_directory::Search_directory): Likewise.
Comment 12 cvs-commit@gcc.gnu.org 2021-10-21 11:36:39 UTC
The binutils-2_37-branch branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 0effb90eb7c9a50408d98ce174f1b4bc5562f017
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Oct 20 10:09:57 2021 +1030

    PR28417, std::string no longer allows accepting nullptr_t
    
            PR 28417
            * incremental.cc (Sized_relobj_incr::do_section_name): Avoid
            std:string undefined behaviour.
            * options.h (Search_directory::Search_directory): Likewise.
    
    (cherry picked from commit 068a039b8bd7c7386bb0d88f0ae245b0fe4132e9)
Comment 13 Alan Modra 2021-11-05 08:10:46 UTC
Fixed