Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Matthieu Longo <matthieu.longo@arm.com>
To: Andrew Burgess <aburgess@redhat.com>, gdb-patches@sourceware.org
Cc: Luis Machado <luis.machado@amd.com>,
	Luis Machado <luis.machado.foss@gmail.com>,
	Thiago Jung Bauermann <thiago.bauermann@linaro.org>,
	Srinath Parvathaneni <srinath.parvathaneni@arm.com>
Subject: Re: [PATCH v1] gdb/linux-tdep: display ProtectionKey in info proc mappings
Date: Mon, 7 Sep 2026 17:00:52 +0100	[thread overview]
Message-ID: <80ba6000-b240-4a40-8701-082972441108@arm.com> (raw)
In-Reply-To: <87fqzlqe2n.fsf@redhat.com>

On 07/09/2026 16:24, Andrew Burgess wrote:
> 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

You are correct.
Sorry for the noise.

Matthieu

      reply	other threads:[~2026-09-07 16:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28 15:29 Matthieu Longo
2026-07-28 15:30 ` Matthieu Longo
2026-09-07  9:13 ` Matthieu Longo
2026-09-07 15:24   ` Andrew Burgess
2026-09-07 16:00     ` Matthieu Longo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=80ba6000-b240-4a40-8701-082972441108@arm.com \
    --to=matthieu.longo@arm.com \
    --cc=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=luis.machado.foss@gmail.com \
    --cc=luis.machado@amd.com \
    --cc=srinath.parvathaneni@arm.com \
    --cc=thiago.bauermann@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox