jasonmolenda wrote:

>The main reason i added the change in `GDBRemoteCommunicationClient.cpp` is 
>that it looks like that the `Symbol` class is meant to work with addresses, 
>but i guess we can still move part of the logic in the symbol class, i just 
>wanted to avoid touching it in case other places were always expecting valid 
>addresses.

My naive reaction after reading this code over very briefly is that 
`Symbol::GetLoadAddress` should return a virtual address that is valid in this 
Process.  It seems like Greg's change in 
f0697d7c3fb5296cfec1718206aceb77b7ca9ab8 to `Symbol::ValueIsAddress` broke this 
for Symbols that have a base address that is an absolute address (outside of 
any Section), but I haven't read his patch closely enough to be confident in 
why he made that change.  I _suspect_ he was making a change that didn't take 
symbols with an absolute address into account -- something that only comes up 
with firmware style debugging, normally, so he may have simply made a mistake 
here.

The patch, currently is basically "If Symbol::GetLoadAddress failed, get the 
_file address_ and use it as-is, we'll just assume this is an absolute VA".  I 
think this should be handled correctly in Symbol::GetLoadAddress, or else we'll 
have other callers that will misbehave with an absolute VA in a Symbol, like 
this.

https://github.com/llvm/llvm-project/pull/200134
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to