[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add memory tag reading to lldb-server

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked 2 inline comments as done.
DavidSpickett added inline comments.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1357
+
+  if (error.Fail())
+    return error;
----------------
DavidSpickett wrote:
> DavidSpickett wrote:
> > omjavaid wrote:
> > > ptrace request is a success if number of tags requested is not equal to no of tags read? If not then this and following condition may be redundant.
> > Well ptracewrapper doesn't check the iovec, but I'll check the kernel source to see if it's actually possible for it to fail that way.
> In `linux/arch/arm64/kernel/mte.c` `__access_remote_tags` there is a comment:
> ```
> +/*
> + * Access MTE tags in another process' address space as given in mm. Update
> + * the number of tags copied. Return 0 if any tags copied, error otherwise.
> + * Inspired by __access_remote_vm().
> + */
> ```
> 
> *any tags* being the key words.
> 
> So the scenario is:
> * ask to read from addr X in page 0, with length of pagesize+some so the range spills into page 1
> * kernel can access page 0, reads tags until the end of the page
> * tries to access page 1 to read the rest, fails, returns 0 (success) since *some* tags were read
> * we see the ptrace call succeeded but with less tags than we expected
> 
> I don't see it's worth dealing with this corner case here since lldb will look before it leaps. It would have errored much earlier here because either page 1 isn't in the tracee's memory regions or it wasn't MTE enabled.
> 
> 
Added a comment in the code too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95601