This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by nitesh.jain on Feb 28 2017, 4:22 AM.

Details

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.

Diff Detail

Event Timeline

nitesh.jain created this revision.Feb 28 2017, 4:22 AM
labath edited edge metadata.Feb 28 2017, 8:04 AM

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 requested changes to this revision.Feb 28 2017, 8:45 AM

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

This revision now requires changes to proceed.Feb 28 2017, 8:45 AM

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.

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;
    }

...
...
...
}

Attach log file

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.

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 added a comment.Mar 7 2017, 5:36 AM

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...

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 edited edge metadata.
nitesh.jain retitled this revision from [LLDB][MIPS] Fix typo in MatchesModuleSpec() to [LLDB][MIPS] Regain Module Name if memory_info.GetName() is empty.

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 added inline comments.
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 ?

nitesh.jain retitled this revision from [LLDB][MIPS] Regain Module Name if memory_info.GetName() is empty to [LLDB][MIPS] Check if memory_info.GetName() is empty before finding corresponding module..
nitesh.jain edited the summary of this revision. (Show Details)
nitesh.jain marked an inline comment as done.
labath accepted this revision.Mar 15 2017, 7:59 AM

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

clayborg accepted this revision.Mar 16 2017, 7:45 PM

Looks good, sorry for the delay.

This revision is now accepted and ready to land.Mar 16 2017, 7:45 PM
This revision was automatically updated to reflect the committed changes.