[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.


================
Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:651-659
+          ArchSpec header_arch;
+          GetArchitecture(header_arch);
+          if (header_arch.GetMachine() == llvm::Triple::arm) {
+            if (function_rva & 1) {
+              // For Thumb we need the last bit to be 0 so that the address
+              // points to the right beginning of the symbol.
+              function_rva ^= 1;
----------------
Shouldn't this happen for all thumb symbols? Other object file parsers define something like:

```
#define THUMB_ADDRESS_BIT_MASK 0xfffffffffffffffeull
```
And do things like:

```
function_rva &=THUMB_ADDRESS_BIT_MASK;
```

Makes it a bit cleaner to read.

It would be better to create a  "bool is_arm = ...;" similar to other symbol file plug-ins. Determine that once before all symbols are being parsed. Here it is being done for each symbol in the loop. 

Be aware that you must answer the address class correctly later with:

```
AddressClass ObjectFilePECOFF::GetAddressClass(lldb::addr_t file_addr);
```

It must return "eAddressClassCode" for ARM address ranges, and "eAddressClassCodeAlternateISA" for Thumb address ranges. Check how "ObjectFileMachO::GetAddressClass()" or "ObjectFileELF::GetAddressClass()" does this. For mach-o we set a bit in each symbol's flags... I will comment below where you could do that. The address class map is used to set breakpoints correctly in Thumb or ARM code and must be done accurately.


https://reviews.llvm.org/D39315