[Lldb-commits] [PATCH] D17450: Add support for handling absolute symbols in ELF

[Tamas Berghammer via lldb-commits]

tberghammer added a comment.

In http://reviews.llvm.org/D17450#357477, @clayborg wrote:

> We should probably make it such that if a section has the type eSectionTypeAbsoluteAddress that we do the right thing inside the following functions:
>  Section::GetLoadBaseAddress(...);
>  Section::Slide(...);
>  SectionLoadList::SetSectionLoadAddress(...);
>  SectionLoadList::GetSectionLoadAddress(...);
>  SectionLoadList::SetSectionUnloaded(...);
>  SectionLoadList::ResolveLoadAddress(...);


I don't think any of these function should behave differently for absolute sections. For example calling SetSectionLoadAddress on an absolute section wouldn't make too much sense but I think they should still behave the same way and the caller of them have to make sure they are not called with incorrect values (see ObjectFileELF.cpp:931). For any other function you mentioned the type of the section shouldn't matter at all. We can possibly add some assert to some of these functions to verify they are not called in incorrect scenarios but I am not sure it is a good idea (changing a soft failure to a crash)


================
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2092
@@ -2085,3 +2091,3 @@
         SymbolType symbol_type = eSymbolTypeInvalid;
         Elf64_Half symbol_idx = symbol.st_shndx;
 
----------------
clayborg wrote:
> This variable has a really bad name. We should change "symbol_idx" to "section_idx" and make this const:
> ```
> const Elf64_Half section_idx = symbol.st_shndx;
> ```
Done

================
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2291
@@ +2290,3 @@
+
+        if (symbol_section_sp == nullptr && symbol.st_size != 0)
+        {
----------------
clayborg wrote:
> Shouldn't this just be:
> 
> ```
> if (!symbol_section_sp && symbol.st_shndx != SHN_UNDEF  && symbol.st_size != 0)
> ```
> 
> We need to know if a symbol has an address and it only has an address if symbol.st_shndx != SHN_UNDEF. All other section types mean that the symbol actually has an address
I agree that my original condition is wrong but I think it should be the following because we want to create a virtual section for absolute symbols only:
```
if (symbol_section_sp == nullptr && symbol.st_shndx == SHN_ABS && symbol.st_size != 0)
```


http://reviews.llvm.org/D17450