Re: [PATCH v1] gdb/linux-tdep: display ProtectionKey in info proc mappings

[Andrew Burgess <aburgess@redhat.com>]

Matthieu Longo <matthieu.longo@arm.com> writes:

> On 28/07/2026 16:29, Matthieu Longo wrote:
>> Memory Protection Keys provide a mechanism for enforcing page-based
>> protections without requiring modification of the page tables
>> when an application changes protection domains. [1]
>> 
>> The "ProtectionKey" field may be present in /proc/PID/smaps on x86_64,
>> PowerPC, and AArch64 systems since Linux 4.9, when the kernel is built
>> with Memory Protection Keys support.
>> 
>> Store the parsed PKey in smaps_data as an optional value on systems
>> where the kernel reports memory protection keys.
>> 
>> Parse /proc/PID/smaps alongside /proc/PID/maps when printing 'info
>> proc mappings', and add a PKey column when at least one smaps entry
>> contains a "ProtectionKey" field.
>> This lets 'info proc mappings' show the protection key associated with
>> each mapped address range.
>> 
>> [1]: https://docs.kernel.org/core-api/protection-keys.html,
>>      https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/
>>      linux.git/commit/?id=c1192f842841
>> ---
>>  gdb/linux-tdep.c | 93 ++++++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 78 insertions(+), 15 deletions(-)
>> 
>> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
>> index 2e9cb14d5bf..2cdf93f3193 100644
>> --- a/gdb/linux-tdep.c
>> +++ b/gdb/linux-tdep.c
>> @@ -124,6 +124,7 @@ struct smaps_data
>>  
>>    ULONGEST rss;
>>    ULONGEST swap;
>> +  std::optional<int> pkey;
>>  };
>>  
>>  /* Whether to take the /proc/PID/coredump_filter into account when
>> @@ -907,6 +908,11 @@ extract_string_view_from_buffer (gdb::array_view<char> &buffer,
>>    return {gdb::array_view<char> (start, it), next_start};
>>  }
>>  
>> +/* Forward declaration of parse_smaps_data used in linux_info_proc below.  */
>> +
>> +static std::vector<struct smaps_data>
>> +parse_smaps_data (const file_reader_t<char> &freader);
>> +
>>  /* Implement the "info proc" command.  */
>>  
>>  static void
>> @@ -1010,33 +1016,69 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
>>      }
>>    if (mappings_f)
>>      {
>> +      /* Parse the content of /proc/PID/smaps.  */
>> +      bool pkey_column = false;
>> +      std::vector<struct smaps_data> smaps_data;
>> +      file_reader_t<char> smaps_freader
>> +	(string_printf ("/proc/%ld/smaps", ptid.lwp ()));
>> +      if (smaps_freader.error ())
>> +	warning (_("unable to open /proc file '%s'"),
>> +		 smaps_freader.c_filepath ());
>> +      else if (smaps_freader)
>> +	{
>> +	  smaps_data = parse_smaps_data (smaps_freader);
>> +
>> +	  /* Check whether "ProtectionKey" field is present.  */
>> +	  pkey_column
>> +	    = std::any_of (smaps_data.cbegin (), smaps_data.cend (),
>> +			   [] (const struct smaps_data &d) -> bool
>> +			   { return d.pkey.has_value (); });
>> +	}
>> +
>> +      /* Parse the content of /proc/PID/maps.  */
>> +      std::vector<struct mapping> maps_data;
>>        file_reader_t<char> map_freader
>>  	(string_printf ("/proc/%ld/maps", ptid.lwp ()));
>> -      if (map_freader)
>> +      if (map_freader.error ())
>> +	warning (_("unable to open /proc file '%s'"),
>> +		 map_freader.c_filepath ());
>> +      else if (map_freader)
>> +	{
>> +	  auto content = map_freader.view ();
>> +	  for (auto it = content.begin (); it != content.end ();)
>> +	    {
>> +	      auto [line, next_line_begin]
>> +		= extract_string_view_from_buffer (content, it, '\n');
>> +	      it = next_line_begin;
>> +
>> +	      /* read_mapping() expects a null-terminated string.  */
>> +	      *std::prev (it) = '\0';
>> +	      maps_data.emplace_back (read_mapping (line.data ()));
>> +	   }
>> +	}
>> +
>> +      gdb_assert (maps_data.size () == smaps_data.size ());
>> +
>> +      if (maps_data.size () > 0)
>>  	{
>>  	  gdb_printf (_("Mapped address spaces:\n\n"));
>> -	  ui_out_emit_table emitter (current_uiout, 6, -1, "ProcMappings");
>> +	  int n_cols = (pkey_column ? 7 : 6);
>> +	  ui_out_emit_table emitter (current_uiout, n_cols, -1, "ProcMappings");
>>  
>>  	  int width = gdbarch_addr_bit (gdbarch) == 32 ? 10 : 18;
>>  	  current_uiout->table_header (width, ui_left, "start", "Start Addr");
>>  	  current_uiout->table_header (width, ui_left, "end", "End Addr");
>>  	  current_uiout->table_header (width, ui_left, "size", "Size");
>>  	  current_uiout->table_header (width, ui_left, "offset", "Offset");
>> +	  if (pkey_column)
>> +	    current_uiout->table_header (4, ui_left, "pkey", "PKey");
>>  	  current_uiout->table_header (5, ui_left, "perms", "Perms");
>>  	  current_uiout->table_header (0, ui_left, "objfile", "File");
>>  	  current_uiout->table_body ();
>>  
>> -	  auto content = map_freader.view ();
>> -	  for (auto it = content.begin (); it != content.end ();)
>> +	  auto it_smaps = smaps_data.cbegin ();
>> +	  for (const auto &m: maps_data)
>>  	    {
>> -	      auto [line, next_line_begin]
>> -		= extract_string_view_from_buffer (content, it, '\n');
>> -	      it = next_line_begin;
>> -
>> -	      /* read_mapping() expects a null-terminated string.  */
>> -	      *std::prev (it) = '\0';
>> -	      struct mapping m = read_mapping (line.data ());
>> -
>>  	      ui_out_emit_tuple tuple_emitter (current_uiout, nullptr);
>>  	      current_uiout->field_core_addr ("start", gdbarch, m.addr);
>>  	      current_uiout->field_core_addr ("end", gdbarch, m.endaddr);
>> @@ -1047,15 +1089,21 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
>>  						     m.endaddr - m.addr));
>>  	      current_uiout->field_string ("offset",
>>  					   paddress (gdbarch, m.offset));
>> +	      if (pkey_column)
>> +		{
>> +		  if (it_smaps->pkey.has_value ())
>> +		    current_uiout->field_signed ("pkey",
>> +						 it_smaps->pkey.value ());
>> +		  else
>> +		    current_uiout->field_skip ("pkey");
>> +		}
>>  	      current_uiout->field_string ("perms", m.permissions);
>>  	      current_uiout->field_string ("objfile", m.filename,
>>  					   file_name_style.style ());
>>  	      current_uiout->text ("\n");
>> +	      ++it_smaps;
>>  	    }
>>  	}
>> -      else
>> -	warning (_("unable to open /proc file '%s'"),
>> -		 map_freader.c_filepath ());
>>      }
>>    if (status_f)
>>      {
>> @@ -1605,6 +1653,7 @@ parse_smaps_data (const file_reader_t<char> &freader)
>>        int mapping_file_p;
>>        ULONGEST rss = -1;
>>        ULONGEST swap = -1;
>> +      int pkey = -1;
>>  
>>        memset (&v, 0, sizeof (v));
>>        struct mapping m = read_mapping (line);
>> @@ -1705,6 +1754,18 @@ parse_smaps_data (const file_reader_t<char> &freader)
>>  		  mapping_anon_p = 1;
>>  		}
>>  	    }
>> +
>> +	  if (streq (keyword, "ProtectionKey:"))
>> +	    {
>> +	      if (sscanf (line, "%*s%d", &pkey) != 1)
>> +		{
>> +		  /* Remove the trailing column when printing the warning.  */
>> +		  keyword[sizeof ("ProtectionKey:") - 2] = '\0';
>> +		  warning (_("Error parsing %s's value in {s,}maps file '%s'"),
>> +			   keyword, freader.c_filepath ());
>> +		  break;
>> +		}
>> +	    }
>>  	}
>>        /* Save the smaps entry to the vector.  */
>>  	struct smaps_data map;
>> @@ -1724,6 +1785,8 @@ parse_smaps_data (const file_reader_t<char> &freader)
>>  	map.inode = m.inode;
>>  	map.rss = rss;
>>  	map.swap = swap;
>> +	if (pkey != -1)
>> +	  map.pkey.emplace (pkey);
>>  
>>  	smaps.emplace_back (map);
>>      }
>
> Ping.

Isn't this patch blocked on some other patches being merged?  I'm not
sure it needs to be pinged if it doesn't apply yet.  Apologies if I'm
wrong here.

Thanks,
Andrew