On Aug 23, 2013, at 8:15 PM, Virgile Bello <[email protected]> wrote:
> I see, sorry about that, I realized I got confused because Section also have > a "file_vm_addr" in its constructor (in .h, in .cpp it becomes "file_addr"), > accessible through GetFileAddress(). But this one seems to actually be a VM > space address, not a real file offset. > Some examples: Yes, "file_addr" actually means a VM address in the file itself. "file_offset" would mean the offset inside the file, one is an address, the other is an offset. > - In ObjectFile, comments for parameter file_vm_addr/GetFileAddress() when > creating Section: // File VM address == addresses as they are found in the > object file > - SetSectionLoadAddress (section_sp, section_sp->GetFileAddress() + offset) < > Using GetFileAddress to compute Section load address > > Since Section used GetFileAddress() for VM addresses as written in assembly, > I thought GetFileOffset() would also be in VM space as well (I didn't notice > GetFileAddress vs GetFileOffset, thinking all GetFile would work in the same > address space). > > Anyway, thanks for the review, now I understand what's wrong and I will > change it. However I still need this VM Base Address to be available for my > ObjectFile to be available externally, so that lldbProcessWindows can apply > module offset. As an example, if PECOFF module is supposed to be loaded at VM > Base Address = 0x400000 but actually loaded at 0x500000, debugger needs to > call module->SetLoadAddress(offset = 0x500000 - 0x400000), so that all > Sections are shifted properly. Is this a rigid slide that is applied to all sections? Or just the text section? If this is a section specific thing, you will want to consider adding code the lldb_private::Section. If this is a rigid slide of all sections all the time when the sections are loaded, then you can add a new call to ObjectFile and return 0 by default, and then override this for PECOFF. > > Note that all addresses within PECOFF will be written in VM addr space as if > module is loaded at 0x400000, which is why it should be considered the Module > VM Address, same as Section's file_vm_addr/GetFileAddress() is currently > being used for Section VM address offseting. > > So, if I understood right: > - Is it OK to add a function such as ObjectFile::GetVirtualFileAddress() or > something like that to represent this concept? If yes, what name? See above, but if this is a rigid slide, then yes add something to ObjectFile, else add to Section. It might be easier to add this to lldb_private::Section and then when you are telling a section to load itself with SetSectionLoadAddress, the adjustment can happen automatically. This way the dynamic loader doesn't need to worry about any adjustment. Maybe something like: lldb::addr_t Section::GetLoadAddressOffset(); The default would return 0, and the PECOFF sections could set this. > - In that case, shouldn't Section::GetFileOffset be renamed to > GetVirtualFileAddress() as well, to avoid future confusion between VM address > and real file container address if a function name starts with GetFile? No. GetFileAddress() is the virtual address. GetFileOffset() is the offset in bytes for the section data that is in the object file on disk. > - Otherwise, any other idea on how to proceed? I would try to add extra data to the Sections with: lldb::addr_t Section::GetLoadAddressOffset(); void Section::SetLoadAddressOffset(lldb::addr_t load_addr_offset); You would then need to apply this to each section as you create the sections in the ObjectFilePECOFF. This is the most flexible because it doesn't assume a rigid slide for all sections. > I might have misunderstood so don't hesitate to tell me if I'm totally wrong! Let me know if you need clarification on anything above. Greg > > > > On Sat, Aug 24, 2013 at 3:33 AM, Greg Clayton <[email protected]> wrote: > Actually this one is not correct. The m_file_offset is the offset of the > PECOFF file within the file itself. So m_file_offset should be zero for all > files that aren't in containers (like universal files on Darwin that contain > multiple architecture slices, or .o files within a BSD archive (.a file)). > > > On Aug 23, 2013, at 10:10 AM, Virgile Bello <[email protected]> wrote: > > > File offset is not set properly in PECOFF. > > > > http://llvm-reviews.chandlerc.com/D1488 > > > > Files: > > source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp > > > > Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp > > =================================================================== > > --- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp > > +++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp > > @@ -434,6 +434,8 @@ > > m_coff_header_opt.data_dirs[i].vmaddr = > > m_data.GetU32(offset_ptr); > > m_coff_header_opt.data_dirs[i].vmsize = > > m_data.GetU32(offset_ptr); > > } > > + > > + m_file_offset = m_coff_header_opt.image_base; > > } > > } > > } > > <D1488.1.patch>_______________________________________________ > > lldb-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
