[LLDB][MIPS] Check if memory_info.GetName() is empty before finding corresponding module.

[nitesh.jain]

Summary

In case of core file , the memory_info.GetName() is empty for "vdso" module. This causes application elf sections to be load at incorrect addressses since "vdso" module get match with "elf application module". This patch add check "memory_info.GetName() " is empty before matching modules.

[labath]

I am not sure this is correct... looking at the other checks in this function, they seem to be following the same pattern... could you explain where you ran into this?

[clayborg]

This logic is wrong. If there is no object name, true should be returned. The old logic is correct.

[clayborg]

Object name is used for a module spec where the file is "/tmp/foo.a" and the object name is "bar.o". So this is for named objects within a container file like a BSD archive. If there is no object name, it doesn't need to be matched. If there is an object name, then the object name must match. All of the other tests are returning false when a there is no match. The original code is correct.

[nitesh.jain]

The issue was observed while parsing "vdso module" for the core file. When the DynamicLoader::LoadModuleAtAddress is called to load "vdso" module. Since "vdso" doesn't match with any of the module in the target.GetImages(), the check_alternative_file_name becomes true. This cause MatchesModuleSpec to return true and application binary to reload again at vdso base address(AT_SYSINFO_EHDR) thus all load addresses of application elf's sections becomes incorrect. Please find attach log file.
DynamicLoader::LoadModuleAtAddress(...)
{

...
...
...
if (check_alternative_file_name) {
        ...
        ...
        ...
        **FileSpec(memory_info.GetName().AsCString(), false),**  ** // memory_info.GetName().AsCString() is empty here**
        target.GetArchitecture());

    if ((module_sp = modules.FindFirstModule(new_module_spec))) {  // FindFirstModule returns application elf's module
      UpdateLoadedSections(module_sp, link_map_addr, base_addr, false); // load addresses of application elf's sections gets updated to vdso's address (which is incorrect)
      return module_sp;
    }

...
...
...
}

[nitesh.jain]

Attach log file

module.log19 KBDownload

[clayborg]

So a ModuleSpec allows you to specify a module by path, UUID and many other things. This is falling down for a magic file that doesn't actually exist right? "vsdo" is just a made up name for the table of loaded shared libraries? Is that correct? I need to understand what is going on before I can offer correct guidance.

[nitesh.jain]

In D30454#693614, @clayborg wrote:

So a ModuleSpec allows you to specify a module by path, UUID and many other things. This is falling down for a magic file that doesn't actually exist right?

Yes.

"vsdo" is just a made up name for the table of loaded shared libraries? Is that correct? I need to understand what is going on before I can offer correct guidance.

I am not sure about that. From Man page, the "vDSO" (virtual dynamic shared object) is a small shared library that the kernel automatically maps into the address space of all user-space applications.

Thanks

[labath]

The VDSO is a pretty standard shared library in most aspects -- it contains the implementations of a couple of functions whose implementation can depend on the specific hardware revision (the fastest way to make a syscall, the fastest way to obtain system time, etc.). The main special thing about it is that it does not exist as a file on the disk. In case of a live process, we load it by directly reading the contents out of inferior memory. I am not sure what we should do in the core file case...

[clayborg]

How does VSDO get loaded when debugging a normal process? In the core file case you probably need to create the VSDO module from memory by locating it somehow and add it to the target before the dynamic loader goes looking for it. Wouldn't that fix the issue?

[nitesh.jain]

In case of debugging normal process, the memory_info.GetName() is not empty and hence module is read from memory (m_process->ReadModuleFromMemory) and the corresponding sections are updated. In case of core file , the memory_info.GetName() is empty which causes FindFirstModule to matches with the first module in ModuleSpecList and result in loading wrong module at incorrect sections addresses. This patch regain "vdso" module name if memory_info.GetName() is empty.

[labath]

source/Core/DynamicLoader.cpp

193 ↗

(On Diff #91833)

This is wrong. The only reason we are querying the GetMemoryRegionInfo is to retrieve the file name, so falling back to the original makes whole effort pointless.

Would it work for you if you modified the if above to only retry these checks only if GetMemoryRegionInfo actually returned a valid (non-empty) filename ?

[labath]

looks good to me, but please give Greg a chance to respond first.

[clayborg]

Looks good, sorry for the delay.

source/Core/Module.cpp

bool Module::MatchesModuleSpec(const ModuleSpec &module_ref) {
   const ArchSpec &arch = module_ref.GetArchitecture();
   if (arch.IsValid()) {
     if (!m_arch.IsCompatibleMatch(arch))
       return false;
   }
 
   const ConstString &object_name = module_ref.GetObjectName();
   if (object_name) {
-    if (object_name != GetObjectName())
-      return false;
-  }
+    if (object_name == GetObjectName())
       return true;
   }
+  return false;
+}
 
 bool Module::FindSourceFile(const FileSpec &orig_spec,
                             FileSpec &new_spec) const {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   return m_source_mappings.FindFile(orig_spec, new_spec);
 }
 
 bool Module::RemapSourceFile(llvm::StringRef path,