Re: [PATCH v4 07/11] gdb: Handle shadow stack pointer register unwinding for amd64 linux.

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Hello Christina,

I have one late, small suggestion on this patch. I wouldn't have
mentioned it but if there's going to be a v5 for some other reason then
it might be worth making the change, if you agree with it:

Christina Schimpe <christina.schimpe@intel.com> writes:

> @@ -3036,6 +3043,46 @@ show_dump_excluded_mappings (struct ui_file *file, int from_tty,
>  		      " flag is %s.\n"), value);
>  }
>  
> +/* See linux-tdep.h.  */
> +
> +bool
> +linux_address_in_shadow_stack_mem_range
> +  (CORE_ADDR addr, std::pair<CORE_ADDR, CORE_ADDR> *range)

All callers pass a range argument, and this function doesn't even check
if the pointer is nullptr before dereferencing it. For such cases, using
a reference rather than a pointer is better, since the compiler will
make sure that the argument is never nullptr.

> +{
> +  if (!target_has_execution () || current_inferior ()->fake_pid_p)
> +    return false;
> +
> +  const int pid = current_inferior ()->pid;
> +
> +  std::string smaps_file = string_printf ("/proc/%d/smaps", pid);
> +
> +  gdb::unique_xmalloc_ptr<char> data
> +    = target_fileio_read_stralloc (nullptr, smaps_file.c_str ());
> +
> +  if (data == nullptr)
> +    return false;
> +
> +  const std::vector<smaps_data> smaps
> +    = parse_smaps_data (data.get (), std::move (smaps_file));
> +
> +  auto find_addr_mem_range = [&addr] (const smaps_data &map)
> +    {
> +      bool addr_in_mem_range
> +	= (addr >= map.start_address && addr < map.end_address);
> +      return (addr_in_mem_range && map.vmflags.shadow_stack_memory);
> +    };
> +  auto it = std::find_if (smaps.begin (), smaps.end (), find_addr_mem_range);
> +
> +  if (it != smaps.end ())
> +    {
> +      range->first = it->start_address;
> +      range->second = it->end_address;
> +      return true;
> +    }
> +
> +  return false;
> +}

-- 
Thiago