[Lldb-commits] [PATCH] PECOFF

Greg Clayton gclayton at apple.com
Mon Aug 26 10:00:05 PDT 2013

On Aug 23, 2013, at 8:15 PM, Virgile Bello <virgile.bello at gmail.com> 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.


> On Sat, Aug 24, 2013 at 3:33 AM, Greg Clayton <gclayton at apple.com> 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 <virgile.bello at gmail.com> 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
> > lldb-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

More information about the lldb-commits mailing list