clayborg added a comment. 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55356/new/ https://reviews.llvm.org/D55356 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits