[PATCH v3 5/8] gdb/linux: refactor /proc mapping parsing to use array_view

[Matthieu Longo <matthieu.longo@arm.com>]

Refactor read_mapping and smaps parsing to operate on gdb::array_view<char>
instead of relying on NUL-terminated strings and strtok_r to split mapping
headers.

Track the amount of data returned by target_fileio_read_stralloc and use
extract_next_field to iterate over mapping lines.

Update read_mapping to parse fields using bounded iterators and diagnose
malformed addresses, separators, offsets, and inode values.

Adjust the callers of parse_smaps_data accordingly.
---
 gdb/linux-tdep.c | 167 +++++++++++++++++++++++++++++------------------
 1 file changed, 103 insertions(+), 64 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index bd4c1d5540f..772b561eb96 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -510,35 +510,60 @@ struct mapping
 /* Service function for corefiles and info proc.  */
 
 static mapping
-read_mapping (const char *line)
+read_mapping (gdb::array_view<char> line)
 {
   struct mapping mapping;
-  const char *p = line;
 
-  mapping.addr = strtoulst (p, &p, 16);
-  if (*p == '-')
-    p++;
-  mapping.endaddr = strtoulst (p, &p, 16);
+  decltype(line)::const_iterator it = line.begin ();
+  decltype(line)::const_iterator it_next;
 
-  p = skip_spaces (p);
-  const char *permissions_start = p;
-  while (*p && !c_isspace (*p))
-    p++;
-  mapping.permissions = std::string (permissions_start,
-				     (size_t) (p - permissions_start));
+  mapping.addr = strtoulst (it, &it_next, 16);
+  if (it == it_next)
+    error (_("failed to parse start address"));
+  else
+    it = it_next;
 
-  mapping.offset = strtoulst (p, &p, 16);
+  if (*it++ != '-')
+    error (_("expected separator '-'"));
 
-  p = skip_spaces (p);
-  const char *device_start = p;
-  while (*p && !c_isspace (*p))
-    p++;
-  mapping.device = {device_start, (size_t) (p - device_start)};
+  mapping.endaddr = strtoulst (it, &it_next, 16);
+  if (it == it_next)
+    error (_("failed to parse end address"));
+  else
+    it = it_next;
 
-  mapping.inode = strtoulst (p, &p, 10);
+  /* Skip spaces.  */
+  it = std::find_if_not (it, line.cend (), c_isspace);
 
-  p = skip_spaces (p);
-  mapping.filename = p;
+  it_next = std::find_if (it, line.cend (), c_isspace);
+  mapping.permissions = std::string (it, it_next);
+  it = it_next;
+
+  mapping.offset = strtoulst (it, &it_next, 16);
+  if (it == it_next)
+    error (_("failed to parse offset"));
+  else
+    it = it_next;
+
+  /* Skip spaces.  */
+  it = std::find_if_not (it, line.cend (), c_isspace);
+
+  it_next = std::find_if (it, line.cend (), c_isspace);
+  mapping.device = std::string (it, it_next);
+  it = it_next;
+
+  mapping.inode = strtoulst (it, &it_next, 10);
+  if (it == it_next)
+    error (_("failed to parse inode"));
+  else
+    it = it_next;
+
+  /* Skip spaces.  */
+  it = std::find_if_not (it, line.cend (), c_isspace);
+
+  mapping.filename = it;
+  /* Ensure that the line ends with '0'.  */
+  *line.end () = '\0';
 
   return mapping;
 }
@@ -985,8 +1010,9 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
   if (mappings_f)
     {
       xsnprintf (filename, sizeof filename, "/proc/%ld/maps", ptid.lwp ());
+      LONGEST len = 0;
       gdb::unique_xmalloc_ptr<char> map
-	= target_fileio_read_stralloc (NULL, filename);
+	= target_fileio_read_stralloc (NULL, filename, &len);
       if (map != NULL)
 	{
 	  gdb_printf (_("Mapped address spaces:\n\n"));
@@ -1001,12 +1027,13 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
 	  current_uiout->table_header (0, ui_left, "objfile", "File");
 	  current_uiout->table_body ();
 
-	  char *saveptr;
-	  for (const char *line = strtok_r (map.get (), "\n", &saveptr);
-	       line != nullptr;
-	       line = strtok_r (nullptr, "\n", &saveptr))
+	  gdb::array_view<char> content (map.get (), len);
+	  for (auto it = content.begin (); it != content.end ();)
 	    {
-	      struct mapping m = read_mapping (line);
+	      auto [line, next_line_begin]
+		= extract_next_field (content, it, '\n');
+	      it = next_line_begin;
+	      mapping m = read_mapping (line);
 
 	      ui_out_emit_tuple tuple_emitter (current_uiout);
 	      current_uiout->field_core_addr ("start", gdbarch, m.addr);
@@ -1559,20 +1586,22 @@ parse_smaps_key_value (const char *keyword, const char *line,
    into the SMAPS vector.  */
 
 static std::vector<smaps_data>
-parse_smaps_data (const char *data,
+parse_smaps_data (gdb::array_view<char> data,
 		  const std::string &maps_filename)
 {
-  char *line, *t;
-
-  gdb_assert (data != nullptr);
-
-  line = strtok_r ((char *) data, "\n", &t);
-
   std::vector<smaps_data> smaps;
 
-  while (line != NULL)
+  for (auto it = data.begin (); it != data.end ();)
     {
+      auto [region_header_line, next_line_begin]
+	= extract_next_field (data, it, '\n');
+      it = next_line_begin;
+
+      /* Parse a region's header.  */
+      mapping m = read_mapping (region_header_line);
+
       struct smaps_vmflags v;
+      memset (&v, 0, sizeof (v));
       int read, write, exec, priv;
       int has_anonymous = 0;
       int mapping_anon_p;
@@ -1580,8 +1609,6 @@ parse_smaps_data (const char *data,
       ULONGEST rss = -1;
       ULONGEST swap = -1;
 
-      memset (&v, 0, sizeof (v));
-      struct mapping m = read_mapping (line);
       mapping_anon_p = mapping_is_anonymous_p (m.filename);
       /* If the mapping is not anonymous, then we can consider it
 	 to be file-backed.  These two states (anonymous or
@@ -1614,7 +1641,8 @@ parse_smaps_data (const char *data,
 
       /* Try to detect if region should be dumped by parsing smaps
 	 counters.  */
-      for (line = strtok_r (NULL, "\n", &t);
+      char *line, *t;
+      for (line = strtok_r (it, "\n", &t);
 	   line != NULL && line[0] >= 'A' && line[0] <= 'Z';
 	   line = strtok_r (NULL, "\n", &t))
 	{
@@ -1679,26 +1707,31 @@ parse_smaps_data (const char *data,
 		}
 	    }
 	}
+
       /* Save the smaps entry to the vector.  */
-	smaps_data map;
-
-	map.start_address = m.addr;
-	map.end_address = m.endaddr;
-	map.filename = m.filename;
-	map.vmflags = v;
-	map.read = read? true : false;
-	map.write = write? true : false;
-	map.exec = exec? true : false;
-	map.priv = priv? true : false;
-	map.has_anonymous = has_anonymous;
-	map.mapping_anon_p = mapping_anon_p? true : false;
-	map.mapping_file_p = mapping_file_p? true : false;
-	map.offset = m.offset;
-	map.inode = m.inode;
-	map.rss = rss;
-	map.swap = swap;
-
-	smaps.emplace_back (map);
+      smaps_data map;
+
+      map.start_address = m.addr;
+      map.end_address = m.endaddr;
+      map.filename = m.filename;
+      map.vmflags = v;
+      map.read = read? true : false;
+      map.write = write? true : false;
+      map.exec = exec? true : false;
+      map.priv = priv? true : false;
+      map.has_anonymous = has_anonymous;
+      map.mapping_anon_p = mapping_anon_p? true : false;
+      map.mapping_file_p = mapping_file_p? true : false;
+      map.offset = m.offset;
+      map.inode = m.inode;
+      map.rss = rss;
+      map.swap = swap;
+
+      smaps.emplace_back (map);
+
+      if (line == nullptr)
+	break;
+      it = line;
     }
 
   return smaps;
@@ -1716,15 +1749,17 @@ linux_process_address_in_memtag_page (CORE_ADDR address)
   ptid_t ptid = get_ptid_for_slash_proc ();
   std::string smaps_file = string_printf ("/proc/%ld/smaps", ptid.lwp ());
 
+  LONGEST len = 0;
   gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (NULL, smaps_file.c_str ());
+    = target_fileio_read_stralloc (NULL, smaps_file.c_str (), &len);
 
   if (data == nullptr)
     return false;
 
   /* Parse the contents of smaps into a vector.  */
+  gdb::array_view<char> content (data.get (), len);
   std::vector<smaps_data> smaps
-    = parse_smaps_data (data.get (), smaps_file);
+    = parse_smaps_data (content, smaps_file);
 
   for (const smaps_data &map : smaps)
     {
@@ -1805,22 +1840,24 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
 
   std::string maps_filename = string_printf ("/proc/%ld/smaps", ptid.lwp ());
 
+  LONGEST len = 0;
   gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (NULL, maps_filename.c_str ());
+    = target_fileio_read_stralloc (NULL, maps_filename.c_str (), &len);
 
   if (data == NULL)
     {
       /* Older Linux kernels did not support /proc/PID/smaps.  */
       maps_filename = string_printf ("/proc/%ld/maps", ptid.lwp ());
-      data = target_fileio_read_stralloc (NULL, maps_filename.c_str ());
+      data = target_fileio_read_stralloc (NULL, maps_filename.c_str (), &len);
 
       if (data == nullptr)
 	return false;
     }
 
   /* Parse the contents of smaps into a vector.  */
+  gdb::array_view<char> content (data.get (), len);
   std::vector<smaps_data> smaps
-    = parse_smaps_data (data.get (), maps_filename);
+    = parse_smaps_data (content, maps_filename);
 
   for (const smaps_data &map : smaps)
     {
@@ -3263,14 +3300,16 @@ linux_address_in_shadow_stack_mem_range
   ptid_t ptid = get_ptid_for_slash_proc ();
   std::string smaps_file = string_printf ("/proc/%ld/smaps", ptid.lwp ());
 
+  LONGEST len = 0;
   gdb::unique_xmalloc_ptr<char> data
-    = target_fileio_read_stralloc (nullptr, smaps_file.c_str ());
+    = target_fileio_read_stralloc (nullptr, smaps_file.c_str (), &len);
 
   if (data == nullptr)
     return false;
 
+  gdb::array_view<char> content (data.get (), len);
   const std::vector<smaps_data> smaps
-    = parse_smaps_data (data.get (), smaps_file);
+    = parse_smaps_data (content, smaps_file);
 
   auto find_addr_mem_range = [&addr] (const smaps_data &map)
     {
-- 
2.55.0