Fix THUMB mode discovery for LSB-set symbols

[ADodds]

Summary

lldb connecting to lldb-server on ARM android was placing arm-breakpoints in a thumb function. This was fixed by marking a symbol as THUMB if its least significant bit is set while parsing symbols from the elf file.

[clayborg]

So are we missing the CPU map on android ELF files? The $m, $a and $t symbols should take care of this ISA mapping stuff unless they are not present.

[tberghammer]

In D10062#179450, @clayborg wrote:

So are we missing the CPU map on android ELF files? The $m, $a and $t symbols should take care of this ISA mapping stuff unless they are not present.

The ISA mapping symbols are optional and they aren't present in executables where the symtab section is striped (most system library on android).

[tberghammer]

You have to add an entry to the m_address_class_map for arm symbols also because with the current change all address treated as thumb when you don't have mapping symbols.

P.S.: Next time please upload the diff with full context

[ADodds]

In D10062#179593, @tberghammer wrote:

You have to add an entry to the m_address_class_map for arm symbols also because with the current change all address treated as thumb when you don't have mapping symbols.

P.S.: Next time please upload the diff with full context

Perhaps I have misunderstood your concern, but the symbol is only marked as eAddressClassCodeAlternateISA if it is THUMB since that happense inside:

if (symbol_type == eSymbolTypeCode && symbol.st_value & 1)

Are you suggesting that I need to add an 'else' to this check and mark it as eAddressClassCode? I can see that would make sense.

[tberghammer]

In D10062#179982, @ADodds wrote:

In D10062#179593, @tberghammer wrote:

You have to add an entry to the m_address_class_map for arm symbols also because with the current change all address treated as thumb when you don't have mapping symbols.

P.S.: Next time please upload the diff with full context

Perhaps I have misunderstood your concern, but the symbol is only marked as eAddressClassCodeAlternateISA if it is THUMB since that happense inside:

if (symbol_type == eSymbolTypeCode && symbol.st_value & 1)

Are you suggesting that I need to add an 'else' to this check and mark it as eAddressClassCode? I can see that would make sense.

Yes, you have to add an entry for arm symbols also because of the way m_address_class_map is used (see ObjectFileELF::GetAddressClass). Basically an entry means that all address have the specified type until the next entry (ordered by address). The reason behind this layout is that you can query the address class of any address without have to resolve it to a symbol it belongs to.

[ADodds]

In D10062#179997, @tberghammer wrote:

In D10062#179982, @ADodds wrote:

In D10062#179593, @tberghammer wrote:

You have to add an entry to the m_address_class_map for arm symbols also because with the current change all address treated as thumb when you don't have mapping symbols.

P.S.: Next time please upload the diff with full context

Perhaps I have misunderstood your concern, but the symbol is only marked as eAddressClassCodeAlternateISA if it is THUMB since that happense inside:

if (symbol_type == eSymbolTypeCode && symbol.st_value & 1)

Are you suggesting that I need to add an 'else' to this check and mark it as eAddressClassCode? I can see that would make sense.

Yes, you have to add an entry for arm symbols also because of the way m_address_class_map is used (see ObjectFileELF::GetAddressClass). Basically an entry means that all address have the specified type until the next entry (ordered by address). The reason behind this layout is that you can query the address class of any address without have to resolve it to a symbol it belongs to.

Ah right! Yes that makes sense, I will make that change now.

[ADodds]

This patch trys to address the issues raised by Tamas. Arm symbols will now also be added to the m_address_class_map too.

[tberghammer]

LGTM

lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

for (i = 0; i < num_symbols; ++i)
                     }
 
                     continue;
                 }
             }
 
             if (arch.GetMachine() == llvm::Triple::arm)
             {
-                // THUMB functions have the lower bit of their address set. Fixup
-                // the actual address and mark the symbol as THUMB.
-                if (symbol_type == eSymbolTypeCode && symbol.st_value & 1)
+                if (symbol_type == eSymbolTypeCode)
+                {
+                    if (symbol.st_value & 1)
                     {
-                    // Substracting 1 from the address effectively unsets
+                        // Subtracting 1 from the address effectively unsets
                         // the low order bit, which results in the address
                         // actually pointing to the beginning of the symbol.
-                    // This delta will be used below in conjuction with
+                        // This delta will be used below in conjunction with
                         // symbol.st_value to produce the final symbol_value
                         // that we store in the symtab.
                         symbol_value_offset = -1;
                         additional_flags = ARM_ELF_SYM_IS_THUMB;
+                        m_address_class_map[symbol.st_value^1] = eAddressClassCodeAlternateISA;
+                    }
+                    else
+                    {
+                        // This address is ARM
+                        m_address_class_map[symbol.st_value] = eAddressClassCode;
+                    }
                 }
             }
         }
 
         // If the symbol section we've found has no data (SHT_NOBITS), then check the module section
         // list. This can happen if we're parsing the debug file and it has no .text section, for example.
         if (symbol_section_sp && (symbol_section_sp->GetFileSize() == 0))
         {