On 11/12/2018 20:10, Jim Ingham via Phabricator wrote:
jingham added a comment.
In D55356#1327280 <https://reviews.llvm.org/D55356#1327280>, @clayborg wrote:
In D55356#1327242 <https://reviews.llvm.org/D55356#1327242>, @labath wrote:
In D55356#1327224 <https://reviews.llvm.org/D55356#1327224>, @clayborg wrote:
In D55356#1327099 <https://reviews.llvm.org/D55356#1327099>, @labath wrote:
Actually, this now causes an lldb-mi test to fail, but it's not clear to me if the
problem is in the test, or this patch. This issue happens when lldb-mi is printing the
"library loaded" message after a module gets added to a not-yet-running target.
It tries to print the load address by first getting the base address and then converting
that to a load address.
Before this patch, that would always fail, because well.. ELF and PECOFF had
this function unimplemented, and for MachO the base address was
section-relative, and so it wasn't resolved to a load address without the
section being loaded. However, with this patch, in the ELF (and presumably
PECOFF) case, the load address is not section-relative and so the
`GetLoadAddress` function happily returns the address.
Is this the expected behavior here? (i.e.,
object_file->GetLoadAddress().GetLoadAddress(target) returning a valid value
even though the target is not running)
Not unless someone has manually set the section load address in the test?
The test is not setting the load address manually. This simply falls out of how
`Address::GetLoadAddress` is implemented:
addr_t Address::GetLoadAddress(Target *target) const {
SectionSP section_sp(GetSection());
if (section_sp) {
...
} else {
// We don't have a section so the offset is the load address
return m_offset;
}
}
So, where's the bug here? It's not clear to me how to change
`Address::GetLoadAddress` to do something different than what it is doing now.
So this is a bug really. When there is no section, we should specify what the m_offset is
using lldb_private::AddressType in maybe a new ivar name "m_offset_type". Then
GetBaseAddress() would specify eAddressTypeFile. And the above code would become:
addr_t Address::GetLoadAddress(Target *target) const {
SectionSP section_sp(GetSection());
if (section_sp) {
...
} else if (m_offset_type == eAddressTypeLoad) {
// We don't have a section so the offset is the load address
return m_offset;
}
}
We just need to be careful and see if we can not make lldb_private::Address
get any bigger byte size wise if we can at all avoid it.
I must be missing something. If there's a file around so that we can express
this address relative to the file, why would it ever not be expressed as a
section + offset? If there's not a file around, then what does it mean to say
this address ie eAddressTypeFile but we don't know the file?
I think the issue here is the difference in how elf and MachO files are
loaded. Elf has this strange dual view of the file, where the same data
is represented both as "sections", and "segments". Sections are the
thing we all know (.text, .data, .debug_info, etc.), and are used by
most tools (lldb, linker, ...). However, this is *not* what the kernel
uses when it loads a binary into memory. The loader uses "segments" instead.
It is segments who describe where will a piece of file be loaded into
memory. Normally, segments span one or more sections, but this is not a
requirement. And almost always segments will contain some additional
data besides section content. This data is generally "junk" but it is
there because it enables the linker to "pack" the elf file more efficiently.
A typical elf file might look something like
-------------------
| elf header |
-------------------
| segment headers |
-------------------
| .text |
-------------------
| other sections |
-------------------
| section headers |
-------------------
The segment headers might say "load everything from the start of file to
the end of .text section at address 0x40000". So the load address of
this file (and the base for all kinds of offsets) would be "0x40000",
but that does not correspond to any section (the file address of the
.text section would probably be something like 0x40123).
So, it almost sounds to me like we would need some kind of a
module-relative (in addition to section-relative) address. Then, this
address would be represented as "module+0", and GetLoadAddress(target)
could translate that the same way as it does section-relative addresses.
Though that may be overkill here. For my purposes it would be sufficient
to just have a function which always returns an file address (regardless
of whether it points to a section or not), and I can use it to compute
offsets (kind of like my original patch did). We could keep the
Module.GetBaseAddress returning LLDB_INVALID_ADDRESS for cases when the
base address cannot be represented as a section offset.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
