[Lldb-commits] [PATCH] D56537: ObjectFilePECOFF: Create a "container" section spanning the entire module image

[Pavel Labath via Phabricator via lldb-commits]

labath marked an inline comment as done.
labath added a comment.

In D56537#1393202 <https://reviews.llvm.org/D56537#1393202>, @amccarth wrote:

> My knowledge of PECOFF is even more limited, but it's my understanding that the ImageBase is the _preferred_ address for the module.  That doesn't guarantee that's the actual address it would be loaded at.  Not just because of ASLR but also because there may be conflicts with modules that have already been loaded.  Is GetBaseAddress supposed to return the actual base address or the preferred one?


Well, it kind of returns both. The returned Address object stores the address in a section-relative form. So, if you ask it for the "file address", it will return the "address, as known to the object file", or the "preferred load address", or however you like to call it. OTOH, you can also ask it for the "load address" for a specific target, which will consult the target for the load address of the section, and return the actual load address in a specific target. This is the main reason why I needed to create the extra container section sitting on top of everything (though that has other benefits too).



================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1370
 
-    auto section = section_list->GetSectionAtIndex(section_idx);
+    auto section = section_list->FindSectionByID(section_id);
     if (!section)
----------------
clayborg wrote:
> How fast is this? Do we need a local cache so we aren't looking up a section for each symbol? Maybe a locally cached vector since sections are represented by indexes? 
It's not particularly fast (linear search), but the number of sections is generally small. FindSectionByID is also used in other places for Symtab construction (e.g. ObjectFileELF; ObjectFileMachO does some complicated thing, which I believe involves caching), so it doesn't seem to be too bad. If it turns out we need to speed up the lookup here, then I think it should be done a bit more generically, so that all users can benefit from this.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56537/new/

https://reviews.llvm.org/D56537