[binutils-gdb] gdbsupport: change path_join parameter to array_view<const char *>

[Simon Marchi <simark@sourceware.org>]

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

commit 6f1d2f789bfaf457565787ff8a59108039bc637e
Author: Simon Marchi <simon.marchi@efficios.com>
Date:   Tue Jul 19 10:27:41 2022 -0400

    gdbsupport: change path_join parameter to array_view<const char *>
    
    When a GDB built with -D_GLIBCXX_DEBUG=1 reads a binary with a single
    character name, we hit this assertion failure:
    
        $ ./gdb -q --data-directory=data-directory -nx ./x
        /usr/include/c++/12.1.0/string_view:239: constexpr const std::basic_string_view<_CharT, _Traits>::value_type& std::basic_string_view<_CharT, _Traits>::operator[](size_type) const [with _CharT = char; _Traits = std::char_traits<char>; const_reference = const char&; size_type = long unsigned int]: Assertion '__pos < this->_M_len' failed.
    
    The backtrace:
    
        #3  0x00007ffff6c0f002 in std::__glibcxx_assert_fail (file=<optimized out>, line=<optimized out>, function=<optimized out>, condition=<optimized out>) at /usr/src/debug/gcc/libstdc++-v3/src/c++11/debug.cc:60
        #4  0x000055555da8a864 in std::basic_string_view<char, std::char_traits<char> >::operator[] (this=0x7fffffffcc30, __pos=1) at /usr/include/c++/12.1.0/string_view:239
        #5  0x00005555609dcb88 in path_join[abi:cxx11](gdb::array_view<std::basic_string_view<char, std::char_traits<char> > const>) (paths=...) at /home/simark/src/binutils-gdb/gdbsupport/pathstuff.cc:203
        #6  0x000055555e0443f4 in path_join<char const*, char const*> () at /home/simark/src/binutils-gdb/gdb/../gdbsupport/pathstuff.h:84
        #7  0x00005555609dc336 in gdb_realpath_keepfile[abi:cxx11](char const*) (filename=0x6060000a8d40 "/home/simark/build/binutils-gdb-one-target/gdb/./x") at /home/simark/src/binutils-gdb/gdbsupport/pathstuff.cc:122
        #8  0x000055555ebd2794 in exec_file_attach (filename=0x7fffffffe0f9 "./x", from_tty=1) at /home/simark/src/binutils-gdb/gdb/exec.c:471
        #9  0x000055555f2b3fb0 in catch_command_errors (command=0x55555ebd1ab6 <exec_file_attach(char const*, int)>, arg=0x7fffffffe0f9 "./x", from_tty=1, do_bp_actions=false) at /home/simark/src/binutils-gdb/gdb/main.c:513
        #10 0x000055555f2b7e11 in captured_main_1 (context=0x7fffffffdb60) at /home/simark/src/binutils-gdb/gdb/main.c:1209
        #11 0x000055555f2b9144 in captured_main (data=0x7fffffffdb60) at /home/simark/src/binutils-gdb/gdb/main.c:1319
        #12 0x000055555f2b9226 in gdb_main (args=0x7fffffffdb60) at /home/simark/src/binutils-gdb/gdb/main.c:1344
        #13 0x000055555d938c5e in main (argc=5, argv=0x7fffffffdcf8) at /home/simark/src/binutils-gdb/gdb/gdb.c:32
    
    The problem is this line in path_join:
    
        gdb_assert (strlen (path) == 0 || !IS_ABSOLUTE_PATH (path));
    
    ... where `path` is "x".  IS_ABSOLUTE_PATH eventually calls
    HAS_DRIVE_SPEC_1:
    
        #define HAS_DRIVE_SPEC_1(dos_based, f)                  \
          ((f)[0] && ((f)[1] == ':') && (dos_based))
    
    This macro accesses indices 0 and 1 of the input string.  However, `f`
    is a string_view of length 1, so it's incorrect to try to access index
    1.  We know that the string_view's underlying object is a null-terminated
    string, so in practice there's no harm.  But as far as the string_view
    is concerned, index 1 is considered out of bounds.
    
    This patch makes the easy fix, that is to change the path_join parameter
    from a vector of to a vector of `const char *`.  Another solution would
    be to introduce a non-standard gdb::cstring_view class, which would be a
    view over a null-terminated string.  With that class, it would be
    correct to access index 1, it would yield the NUL character.  If there
    is interest in having this class (it has been mentioned a few times in
    the past) I can do it and use it here.
    
    This was found by running tests such as gdb.ada/arrayidx.exp, which
    produce 1-char long filenames, so adding a new test is not necessary.
    
    Change-Id: Ia41a16c7243614636b18754fd98a41860756f7af

Diff:
---
 gdb/dwarf2/line-header.c | 4 ++--
 gdbsupport/pathstuff.cc  | 8 ++++----
 gdbsupport/pathstuff.h   | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/gdb/dwarf2/line-header.c b/gdb/dwarf2/line-header.c
index 73db3765c09..3a47557806e 100644
--- a/gdb/dwarf2/line-header.c
+++ b/gdb/dwarf2/line-header.c
@@ -69,13 +69,13 @@ line_header::file_file_name (const file_entry &fe) const
 
   const char *dir = fe.include_dir (this);
   if (dir != nullptr)
-    ret = path_join (dir, ret);
+    ret = path_join (dir, ret.c_str ());
 
   if (IS_ABSOLUTE_PATH (ret))
     return ret;
 
   if (m_comp_dir != nullptr)
-    ret = path_join (m_comp_dir, ret);
+    ret = path_join (m_comp_dir, ret.c_str ());
 
   return ret;
 }
diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc
index af10c6ebd2e..390f10b1b5e 100644
--- a/gdbsupport/pathstuff.cc
+++ b/gdbsupport/pathstuff.cc
@@ -191,21 +191,21 @@ child_path (const char *parent, const char *child)
 /* See gdbsupport/pathstuff.h.  */
 
 std::string
-path_join (gdb::array_view<const gdb::string_view> paths)
+path_join (gdb::array_view<const char *> paths)
 {
   std::string ret;
 
   for (int i = 0; i < paths.size (); ++i)
     {
-      const gdb::string_view path = paths[i];
+      const char *path = paths[i];
 
       if (i > 0)
-	gdb_assert (path.empty () || !IS_ABSOLUTE_PATH (path));
+	gdb_assert (strlen (path) == 0 || !IS_ABSOLUTE_PATH (path));
 
       if (!ret.empty () && !IS_DIR_SEPARATOR (ret.back ()))
 	  ret += '/';
 
-      ret.append (path.begin (), path.end ());
+      ret.append (path);
     }
 
   return ret;
diff --git a/gdbsupport/pathstuff.h b/gdbsupport/pathstuff.h
index d01db89e085..e0b5bc4f355 100644
--- a/gdbsupport/pathstuff.h
+++ b/gdbsupport/pathstuff.h
@@ -67,7 +67,7 @@ extern const char *child_path (const char *parent, const char *child);
    The first element can be absolute or relative.  All the others must be
    relative.  */
 
-extern std::string path_join (gdb::array_view<const gdb::string_view> paths);
+extern std::string path_join (gdb::array_view<const char *> paths);
 
 /* Same as the above, but accept paths as distinct parameters.  */
 
@@ -78,10 +78,10 @@ path_join (Args... paths)
   /* It doesn't make sense to join less than two paths.  */
   gdb_static_assert (sizeof... (Args) >= 2);
 
-  std::array<gdb::string_view, sizeof... (Args)> views
-    { gdb::string_view (paths)... };
+  std::array<const char *, sizeof... (Args)> path_array
+    { paths... };
 
-  return path_join (gdb::array_view<const gdb::string_view> (views));
+  return path_join (gdb::array_view<const char *> (path_array));
 }
 
 /* Return whether PATH contains a directory separator character.  */