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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
