DavidSpickett added a comment.

> What do you think about locating this change in ToAddress instead of 
> Target::GetBreakableLoadAddress? It looks like the one caller to 
> GetBreakableLoadAddress is Target::CreateBreakpoint(addr_t addr) - which is 
> probably called by an SBTarget method if we want to think of the most general 
> purpose use case. I think stripping non-addressable bits in 
> OptionArgParser::ToAddress is the right thing to do - but I don't have an 
> opinion about whether it should be done in Target::GetBreakableLoadAddress or 
> not.

I had not checked the other lookup commands, so yeah this sounds good to me. 
Doing this fixing in fewer places is always better. Put up your change and I'll 
check the test from this patch against it.

> One thing to note is that I also have an SBValue method that needs to be 
> upstreamed, SBValue::GetValueAsAddress(). So if someone has an SBValue of a 
> function pointer and they want to take the value of that func ptr and call 
> SBTarget::CreateBreakpoint on it, I would say "well yes, you need to use 
> GetValueAsAddress before you pass that to CreateBreakpoint".

If I understand correctly, this would effectively run FixCodeAddress on the 
value? One thing that I found doing the SB API for MTE (which was shelved, but 
besides the point) is that there was no way to fix addresses via the API (or 
get access to the ABI plugin in general). I ended up just adding FixDataAddress 
to SBProcess as a temporary thing. So something like ValueAsAddress could be 
very useful.

We have this distinction (that doesn't mean much) between code and data fixes. 
For now a single method will work fine, I'm hoping the code/data difference 
never gets to a point where we have to be really careful about it.
(as you say "This process isn't using TBI, but the high bits end up being used 
for signing so it's the same effect.", so FixCode and FixData are the same 
thing in that case)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136938

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

Reply via email to