clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Two issues we need to resolve:
- I don't think AddressClass should have Absolute as a value. Absolute values
in symbol tables are just numbers, not necessarily addresses.
- This change won't work for ARM
================
Comment at: include/lldb/lldb-enumerations.h:829
+ eAddressClassRuntime,
+ eAddressClassTypeAbsoluteAddress
};
----------------
I would suggest removing "Address" from the end of the enum name. It is already
in an enum that starts with "eAddressClass". I also question why any address
class should be absolute. Absolute symbols are usually not addresses, but just
values.
================
Comment at: source/Core/Address.cpp:993-1003
+ // Get address class based on loaded Section type
+ SectionSP section_sp(GetSection());
+ if (section_sp) {
+ const SectionType section_type = section_sp->GetType();
+ AddressClass addr_class;
+ addr_class = ObjectFile::SectionTypeToAddressClass(section_type);
+ if (addr_class != eAddressClassTypeAbsoluteAddress)
----------------
This won't work correctly for ARM binaries. ".text" can be filled with ARM,
Thumb and Data and there is a CPU map that can help unwind this. The above code
will just ay "eAddressClassCode" for all ".text". So this won't work.
My guess is the right fix here is to check if the address has a valid section
before calling the code below and removing all code above.
```
if (!GetSection())
return eAddressClassInvalid;
```
GetFileAddress will just return the m_offset if the section isn't valid. One
could argue that Address::GetFileAddress() should only return the file address
if the section is valid though, perhaps that should be the change we make here.
https://reviews.llvm.org/D35784
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
