tberghammer added a comment.

In http://reviews.llvm.org/D12556#238457, @clayborg wrote:

> Changing all $d symbols to always say they are 
> eAddressClassDataIntermixedCode is wrong because the symbols in the .data 
> section now would be marked as eAddressClassDataIntermixedCode.


We only use the m_address_class_map for the code section (when the address 
class of the section is eAddressClassCode) so we won't mark the symbols in the 
.data section as eAddressClassDataIntermixedCode.

> To clarify a few things, lets say we have the following code:

> 

> 0x1000: bx <addr> Non-tail call in a no return function

>  0x1004: [data-pool] Marked with $d mapping symbol

> 

> Should just claim that 0x1000 is eAddressClassCode and that 0x1004 is 
> eAddressClassData. We don't need a new eAddressClassDataIntermixedCode for 
> this, it should just say eAddressClassData for 0x1004.


This is the current implementation (before this change).

> For return addresses we that are on the stack on in the LR, we should 
> actually be sanitizing them before we start doing lookups. So the return 
> address would be 0x1005 in this case you are talking about right? Maybe we 
> always get the address class of the return address and check if it is 
> eAddressClassData. If it is, we know something is wrong since the return 
> address can't go to data and we work around this by recognizing that fact.


It is incorrect in some sense if we see that the return address points to a 
data section but in the described case this is the best what LLDB can say (and 
it is true that if the function return, then it will try to execute some code 
from the data segment), and from this address the unwinding code can continue 
without any issue (assuming lr is saved somewhere).

> I would rather not just say that all data is eAddressClassDataIntermixedCode 
> for ARM and Thumb, that seems like the wrong fix.


What is your opinion about changing GetOpcodeLoadAddress and 
GetCallableLoadAddress to always mask out the LSB on arm/thumb and never return 
LLDB_INVALID_ADDRESS? It would fix the issue and will be (mostly) consistent 
with the other architectures where we don't do any checking, but it will drop a 
slight safety net for the case when something went seriously wrong and we 
really ended up in a data section (we don't have this check for other 
architectures).


http://reviews.llvm.org/D12556



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

Reply via email to