clayborg added a comment.

In D71372#1782567 <https://reviews.llvm.org/D71372#1782567>, @jingham wrote:

> In D71372#1782549 <https://reviews.llvm.org/D71372#1782549>, @clayborg wrote:
>
> > In D71372#1782538 <https://reviews.llvm.org/D71372#1782538>, @jingham wrote:
> >
> > > In D71372#1782536 <https://reviews.llvm.org/D71372#1782536>, @clayborg 
> > > wrote:
> > >
> > > > I believe it is ok for permissions to succeed as long as they don't 
> > > > return invalid permissions. For any address outside any mapped ranges, 
> > > > we should have zero as the permissions. Looking up address mappings for 
> > > > zero will return
> > > >
> > > > [0x00000000 - 0x100000000) ---
> > > >
> > > > no permisssions are represented as "---". Then you can take the end 
> > > > address and look it up, and continue. So as long as we aren't getting 
> > > > bogus values back, we are good.
> > >
> > >
> > > Then what does the bool return mean?
> >
> >
> > If there is no memory map info in the process plug-in at all, then false 
> > will be returned.
>
>
> That means to answer the question "did GetLoadAddressPermissions return valid 
> permissions for load address 0xABCD" I have to check both the return value 
> and if any of the permission values are unknown?  That seems like an awkward 
> API.


That it is. Not sure if I added it, but blame shows Kate Stone from the great 
code convention checkin. Would be fine to fix it and update any call sites. 
Looks like it could be turned into:

  bool Process::LoadAddressHasAllPermissions(lldb::addr_t load_addr, uint32_t 
permissions);

The permissions is now an input parameter. The only other client is 
RegisterContextLLDB and all call sites do things like:

  process->GetLoadAddressPermissions(current_pc_addr, permissions) && 
(permissions & ePermissionsExecutable) == 0)

or

  if (process.GetLoadAddressPermissions(candidate, permissions) &&permissions & 
lldb::ePermissionsExecutable)

So it would be easy to fix. Please do!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71372/new/

https://reviews.llvm.org/D71372



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to