Fix for Bug 25338

[ravitheja]

Summary

The issue arises because LLDB is not
able to read the vdso library correctly.
The fix adds memory allocation callbacks
to allocate sufficient memory in case the
requested offsets don't fit in the memory
buffer allocated for the ELF.

[labath]

Could you explain what is the nature of the failure in the vdso parsing? Otherwise it's quite hard to see what's the relationship of your patch to it, as it seems that the ObjectFile should be able to handle files which are not fully contained within the initial read (comment on Module::GetMemoryObjectFile says that it's enough if size_to_read contains the file header).

Also, if this fixes a test (I think I remember us disabling a test because of this), can you please enable it again?

include/lldb/Target/Process.h

3145

This definition sounds confusing to me. I would read this as the "start address of the last block ...", which is clearly not what you want. It's probably enough to change it to "*end* address of the last block".

source/Plugins/Process/Linux/NativeProcessLinux.cpp

2973

This will be wrong if the file contains multiple entries for the same file (probably because different segments are mapped with different permissions). Doesn't happen for the vdso, but all other files have this. You should probably look for the *last* segment matching this file and get it's end address.

source/Utility/StringExtractorGDBRemote.cpp

149

Could you also document this packet in docs/lldb-gdb-remote.txt?

[labath]

In D16107#324594, @labath wrote:

Could you explain what is the nature of the failure in the vdso parsing?

Strike that. I see that you have linked the bug # in the title...

[tberghammer]

Can we calculate the size of the object file based on elf headers and the section headers instead of using the /proc file system?

I think a good algorithm would be to take the maximum of the following 2 expression:

Start of section header + Number of section headers x  Size of section headers
Max (for each section: section file address + section size)

[ovyalov]

vdso issue is related to ELF format and I'm wondering whether we can keep the fix contained in ObjectFileELF - for example, if a section that we're trying to read is beyond the boundaries of memory buffer we can read its content from inferior's memory using ObjectFile::m_process_wp.

[ravitheja]

Hi Oleyksiy, actually the problem is not with the Elf Format but its a problem with the ObjectFileElf itself which cannot handle reading from arbitrary offsets in the Elf. Now the ideal solution would be to fix that but as we had discussed last year we agreed to allocate more memory for the vdso. I could try to contain the complete fix in the ObjectFileElf but that may be counterproductive since it already buggy ?

[ravitheja]

Hi Tamas, a modified algorithm may work in this case for now, but this issue may still reoccur for some other Elf Format .

[tberghammer]

Based on your description the vdso file itself looks like a valid elf file to me. If the ObjectFileELF plugin is buggy (it is certainly is as it can't handle the vdso) then I think we should fix that instead of building up a new approach just to work around one of our own bug.

[ravitheja]

Yes the vdso is fine, last year I did discuss this and we here decided that at the moment we can only offer this solution.

[ovyalov]

If vdso bug pertains to ELF format then it looks reasonable to keep the fix within ObjectFileELF.
I experimented a while ago with making ObjectFileELF to read from arbitrary offsets - please see http://reviews.llvm.org/D16151 as reflection of this idea (patch is pretty old and cannot be applied as-is).
Please take a look into it and let us know whether such approach may solve vdso problem.

[ravitheja]

Changes according to D16151

[labath]

I think this looks *much* nicer than the previous version.

I personally would replace the std::function dance with something simpler such as bool use_memory_fallback, but I guess that's a matter of taste.

source/Plugins/ObjectFile/ELF/ObjectFileELF.h

13 ↗

(On Diff #45185)

Please put this right before <vector>

284 ↗

(On Diff #45185)

const non-reference arguments are somewhat pointless. Did you forget a reference here?

307 ↗

(On Diff #45185)

same here

[ravitheja]

correcting previous revision.

[labath]

Looks good as far as I am concerned. @tberghammer, @ovyalov ?

[tberghammer]

Looks good

[ldrumm]

This differential seems to have introduced a regression on x86_64 Android, where std::bad_alloc is thrown during process attach. I've reverted this locally, and filed a bug, but as this is code I'm not familiar with, I'd appreciate it if @ravitheja were able to take a look at this again.

[ravitheja]

Hello, Ok I will take a look.

[ravitheja]

Hello @ldrumm,

I did investigate the issue and my finding is that incorrect values are being passed to the SetDataWithReadMemoryFallback function.
Now I suspect the ElfSectionHeader is wrongly parsed in your case and the size and offsets obtained are wrong. To talk briefly about r258122, it just tries to allocate enough memory for Elf files being loaded by lldb and earlier it would not have read the Elf file so you wouldnt have seen this issue because it would have just returned. Could you maybe check the ElfSectionHeader::Parse function, and see the values it obtains for this elf (mainly the sh_offset and sh_size fields), maybe print them in logs ? I currently don't have an Android x86_64 environment.

include/lldb/Host/common/NativeProcessProtocol.h

public:
         UnregisterNativeDelegate (NativeDelegate &native_delegate);
 
         virtual Error
         GetLoadedModuleFileSpec(const char* module_path, FileSpec& file_spec) = 0;
 
         virtual Error
         GetFileLoadAddress(const llvm::StringRef& file_name, lldb::addr_t& load_addr) = 0;
 
+        virtual Error
+        GetFileEndAddress(const llvm::StringRef& file_name, lldb::addr_t& end_addr) = 0;
+
         //------------------------------------------------------------------
         /// Launch a process for debugging. This method will create an concrete
         /// instance of NativeProcessProtocol, based on the host platform.
         /// (e.g. NativeProcessLinux on linux, etc.)
         ///
         /// @param[in] launch_info
         ///     Information required to launch the process.
         ///

include/lldb/Target/DynamicLoader.h

protected:
     UnloadSectionsCommon(const lldb::ModuleSP module);
 
     /// Locates or creates a module given by @p file and updates/loads the
     /// resulting module at the virtual base address @p base_addr.
     virtual lldb::ModuleSP
     LoadModuleAtAddress(const lldb_private::FileSpec &file,
                         lldb::addr_t link_map_addr,
                         lldb::addr_t base_addr,
-                        bool base_addr_is_offset);
+                        bool base_addr_is_offset,
+                        size_t size_to_read = 512);
 
     const lldb_private::SectionList *
     GetSectionListFromModule(const lldb::ModuleSP module) const;
 
     // Read an unsigned int of the given size from memory at the given addr.
     // Return -1 if the read fails, otherwise return the result as an int64_t.
     int64_t
     ReadUnsignedIntWithSizeInBytes(lldb::addr_t addr, int size_in_bytes);

include/lldb/Target/Process.h

     ///     processes address space, LLDB_INVALID_ADDRESS otherwise.
     //------------------------------------------------------------------
     virtual Error
     GetFileLoadAddress(const FileSpec& file, bool& is_loaded, lldb::addr_t& load_addr)
     {
         return Error("Not supported");
     }
 
+    //------------------------------------------------------------------
+    /// Try to find the end address of a file.
+    /// The end address is defined as the address of the last memory
+    /// region what contains data mapped from the specified file.
+    ///
+    /// @param[in] file
+    ///     The name of the file whose end address we are looking for
+    ///
+    /// @param[out] is_loaded
+    ///     \b True if the file is loaded into the memory and false
+    ///     otherwise.
+    ///
+    /// @param[out] end_addr
+    ///     The end address of the file if it is loaded into the
+    ///     processes address space, LLDB_INVALID_ADDRESS otherwise.
+    //------------------------------------------------------------------
+    virtual Error
+    GetFileEndAddress(const FileSpec& file, bool& is_loaded, lldb::addr_t& end_addr)
+    {
+        return Error("Not supported");
+    }
     size_t
     AddImageToken(lldb::addr_t image_ptr);
 
     lldb::addr_t
     GetImagePtrFromToken(size_t token) const;
 
     void
     ResetImageToken(size_t token);

source/Core/DynamicLoader.cpp

DynamicLoader::GetSectionListFromModule(const ModuleSP module) const
     }
     return sections;
 }
 
 ModuleSP
 DynamicLoader::LoadModuleAtAddress(const FileSpec &file,
                                    addr_t link_map_addr,
                                    addr_t base_addr,
-                                   bool base_addr_is_offset)
+                                   bool base_addr_is_offset,
+                                   size_t size_to_read)
 {
     Target &target = m_process->GetTarget();
     ModuleList &modules = target.GetImages();
     ModuleSP module_sp;
 
     ModuleSpec module_spec (file, target.GetArchitecture());
     if ((module_sp = modules.FindFirstModule (module_spec)))
     {
else
             // address to read the file out of the memory instead of a load bias.
             bool is_loaded;
             lldb::addr_t load_addr;
             Error error = m_process->GetFileLoadAddress(file, is_loaded, load_addr);
             if (error.Success() && is_loaded)
                 base_addr = load_addr;
         }
 
-        if ((module_sp = m_process->ReadModuleFromMemory(file, base_addr)))
+        if ((module_sp = m_process->ReadModuleFromMemory(file, base_addr, size_to_read)))
         {
             UpdateLoadedSections(module_sp, link_map_addr, base_addr, false);
             target.GetImages().AppendIfNeeded(module_sp);
         }
     }
 
     return module_sp;
 }

source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.h

protected:
     UnloadSections(const lldb::ModuleSP module) override;
 
     /// Locates or creates a module given by @p file and updates/loads the
     /// resulting module at the virtual base address @p base_addr.
     lldb::ModuleSP
     LoadModuleAtAddress(const lldb_private::FileSpec &file,
                         lldb::addr_t link_map_addr,
                         lldb::addr_t base_addr,
-                        bool base_addr_is_offset) override;
+                        bool base_addr_is_offset,
+                        size_t size_to_read = 512) override;
 
     /// Callback routine invoked when we hit the breakpoint on process entry.
     ///
     /// This routine is responsible for resolving the load addresses of all
     /// dependent modules required by the inferior and setting up the rendezvous
     /// breakpoint.
     static bool
     EntryBreakpointHit(void *baton, 

source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp

 }
 
 /// Helper for the entry breakpoint callback.  Resolves the load addresses
 /// of all dependent modules.
 ModuleSP
 DynamicLoaderHexagonDYLD::LoadModuleAtAddress(const FileSpec &file,
                                               addr_t link_map_addr,
                                               addr_t base_addr,
-                                              bool base_addr_is_offset)
+                                              bool base_addr_is_offset,
+                                              size_t size_to_read)
 {
     Target &target = m_process->GetTarget();
     ModuleList &modules = target.GetImages();
     ModuleSP module_sp;
 
     ModuleSpec module_spec (file, target.GetArchitecture());
 
     // check if module is currently loaded

source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

DynamicLoaderPOSIXDYLD::LoadAllCurrentModules()
 
     // The rendezvous class doesn't enumerate the main module, so track
     // that ourselves here.
     ModuleSP executable = GetTargetExecutable();
     m_loaded_modules[executable] = m_rendezvous.GetLinkMapAddress();
     if (m_vdso_base != LLDB_INVALID_ADDRESS)
     {
         FileSpec file_spec("[vdso]", false);
-        ModuleSP module_sp = LoadModuleAtAddress(file_spec, LLDB_INVALID_ADDRESS, m_vdso_base, false);
+        bool is_loaded;
+        lldb::addr_t end_addr;
+        size_t size_to_read = 512;
+        Error error = m_process->GetFileEndAddress(file_spec, is_loaded, end_addr);
+        if (error.Success() && is_loaded)
+            size_to_read = end_addr - m_vdso_base;
+        ModuleSP module_sp = LoadModuleAtAddress(file_spec, LLDB_INVALID_ADDRESS, m_vdso_base, false, size_to_read);
         if (module_sp.get())
         {
             module_list.Append(module_sp);
         }
     }
     for (I = m_rendezvous.begin(), E = m_rendezvous.end(); I != E; ++I)
     {
         ModuleSP module_sp = LoadModuleAtAddress(I->file_spec, I->link_addr, I->base_addr, true);

source/Plugins/Process/Linux/NativeProcessLinux.h

public:
         DoStopIDBumped (uint32_t newBumpId) override;
 
         Error
         GetLoadedModuleFileSpec(const char* module_path, FileSpec& file_spec) override;
 
         Error
         GetFileLoadAddress(const llvm::StringRef& file_name, lldb::addr_t& load_addr) override;
 
+        Error
+        GetFileEndAddress(const llvm::StringRef& file_name, lldb::addr_t& end_addr) override;
+
         NativeThreadLinuxSP
         GetThreadByID(lldb::tid_t id);
 
         // ---------------------------------------------------------------------
         // Interface used by NativeRegisterContext-derived classes.
         // ---------------------------------------------------------------------
         static Error
         PtraceWrapper(int req,

source/Plugins/Process/Linux/NativeProcessLinux.cpp

Error error = ProcFileReader::ProcessLineByLine (GetID (), "maps",
             }
  
             // Return true to continue reading the proc file
             return true;
         });
     return error; 
 }
 
+Error
+NativeProcessLinux::GetFileEndAddress(const llvm::StringRef& file_name, lldb::addr_t& end_addr)
+{
+    end_addr = LLDB_INVALID_ADDRESS;
+    Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_BREAKPOINTS));
+    Error error = ProcFileReader::ProcessLineByLine (GetID (), "maps",
+        [&] (const std::string &line) -> bool
+        {
+            StringRef maps_row(line);
+
+            SmallVector<StringRef, 16> maps_columns;
+            maps_row.split(maps_columns, StringRef(" "), -1, false);
+
+            if (maps_columns.size() < 6)
+            {
+                // Return true to continue reading the proc file
+                return true;
+            }
+
+            if (maps_columns[5] == file_name)
+            {
+                StringExtractor addr_extractor(maps_columns[0].str().c_str());
+                addr_extractor.GetHexMaxU64(false, LLDB_INVALID_ADDRESS);
+                char ch = addr_extractor.GetChar('\0');
+                if (ch != '\0' && ch != '-')
+                    return true;
+
+                end_addr = addr_extractor.GetHexMaxU64(false, LLDB_INVALID_ADDRESS);
+                // Return false to stop reading the proc file further
+                return false;
+            }
+
+            return true;
+        });
+    return error;
+}
+
 NativeThreadLinuxSP
 NativeProcessLinux::GetThreadByID(lldb::tid_t tid)
 {
     return std::static_pointer_cast<NativeThreadLinux>(NativeProcessProtocol::GetThreadByID(tid));
 }
 
 Error
 NativeProcessLinux::ResumeThread(NativeThreadLinux &thread, lldb::StateType state, int signo)

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

protected:
     Handle_jThreadsInfo (StringExtractorGDBRemote &packet);
 
     PacketResult
     Handle_qWatchpointSupportInfo (StringExtractorGDBRemote &packet);
 
     PacketResult
     Handle_qFileLoadAddress (StringExtractorGDBRemote &packet);
 
+    PacketResult
+    Handle_qFileEndAddress (StringExtractorGDBRemote &packet);
+
     void
     SetCurrentThreadID (lldb::tid_t tid);
 
     lldb::tid_t
     GetCurrentThreadID () const;
 
     void
     SetContinueThreadID (lldb::tid_t tid);

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

GDBRemoteCommunicationServerLLGS::RegisterPacketHandlers()
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_P,
                                   &GDBRemoteCommunicationServerLLGS::Handle_P);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qC,
                                   &GDBRemoteCommunicationServerLLGS::Handle_qC);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qfThreadInfo,
                                   &GDBRemoteCommunicationServerLLGS::Handle_qfThreadInfo);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qFileLoadAddress,
                                   &GDBRemoteCommunicationServerLLGS::Handle_qFileLoadAddress);
+    RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qFileEndAddress,
+                                  &GDBRemoteCommunicationServerLLGS::Handle_qFileEndAddress);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qGetWorkingDir,
                                   &GDBRemoteCommunicationServerLLGS::Handle_qGetWorkingDir);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qMemoryRegionInfo,
                                   &GDBRemoteCommunicationServerLLGS::Handle_qMemoryRegionInfo);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qMemoryRegionInfoSupported,
                                   &GDBRemoteCommunicationServerLLGS::Handle_qMemoryRegionInfoSupported);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qProcessInfo,
                                   &GDBRemoteCommunicationServerLLGS::Handle_qProcessInfo);
GDBRemoteCommunicationServerLLGS::Handle_qFileLoadAddress (StringExtractorGDBRemote &packet)
     if (file_load_address == LLDB_INVALID_ADDRESS)
         return SendErrorResponse(1); // File not loaded
 
     StreamGDBRemote response;
     response.PutHex64(file_load_address);
     return SendPacketNoLock(response.GetData(), response.GetSize());
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_qFileEndAddress (StringExtractorGDBRemote &packet)
+{
+    Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
+    if (log)
+        log->Printf ("GDBRemoteCommunicationServerLLGS::%s qFileEndAddress Start", __FUNCTION__);
+    // Fail if we don't have a current process.
+    if (!m_debugged_process_sp ||
+            m_debugged_process_sp->GetID () == LLDB_INVALID_PROCESS_ID)
+        return SendErrorResponse(67);
+
+    packet.SetFilePos(strlen("qFileEndAddress:"));
+    if (packet.GetBytesLeft() == 0)
+        return SendErrorResponse(68);
+    if (log)
+        log->Printf ("GDBRemoteCommunicationServerLLGS::%s qFileEndAddress", __FUNCTION__);
+    std::string file_name;
+    packet.GetHexByteString(file_name);
+
+    if (log)
+        log->Printf ("GDBRemoteCommunicationServerLLGS::%s %s", __FUNCTION__, file_name.c_str());
+    lldb::addr_t file_end_address = LLDB_INVALID_ADDRESS;
+    Error error = m_debugged_process_sp->GetFileEndAddress(file_name, file_end_address);
+    if (log)
+        log->Printf ("GDBRemoteCommunicationServerLLGS::%s %" PRIu64, __FUNCTION__, file_end_address);
+    if (error.Fail())
+        return SendErrorResponse(69);
+
+    if (file_end_address == LLDB_INVALID_ADDRESS)
+        return SendErrorResponse(1); // File not loaded
+
+    StreamGDBRemote response;
+    response.PutHex64(file_end_address);
+    return SendPacketNoLock(response.GetData(), response.GetSize());
+}
+
 void
 GDBRemoteCommunicationServerLLGS::MaybeCloseInferiorTerminalConnection ()
 {
     Log *log (GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
 
     // Tell the stdio connection to shut down.
     if (m_stdio_communication.IsConnected())
     {

source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

GetHostOSVersion(uint32_t &major,
                      uint32_t &update) override;
 
     size_t
     LoadModules() override;
 
     Error
     GetFileLoadAddress(const FileSpec& file, bool& is_loaded, lldb::addr_t& load_addr) override;
 
+    Error
+    GetFileEndAddress(const FileSpec& file, bool& is_loaded, lldb::addr_t& end_addr) override;
+
     void
     ModulesDidLoad (ModuleList &module_list) override;
 
     StructuredData::ObjectSP
     GetLoadedDynamicLibrariesInfos (lldb::addr_t image_list_address, lldb::addr_t image_count) override;
 
 protected:
     friend class ThreadGDBRemote;

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

if (response.IsNormalResponse())
         is_loaded = true;
         load_addr = response.GetHexMaxU64(false, LLDB_INVALID_ADDRESS);
         return Error();
     }
 
     return Error("Unknown error happened during sending the load address packet");
 }
 
+Error
+ProcessGDBRemote::GetFileEndAddress(const FileSpec& file, bool& is_loaded, lldb::addr_t& end_addr)
+{
+    is_loaded = false;
+    end_addr = LLDB_INVALID_ADDRESS;
+
+    std::string file_path = file.GetPath(false);
+    if (file_path.empty ())
+        return Error("Empty file name specified");
+
+    StreamString packet;
+    packet.PutCString("qFileEndAddress:");
+    packet.PutCStringAsRawHex8(file_path.c_str());
+
+    StringExtractorGDBRemote response;
+    if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString().c_str(), response, false) != GDBRemoteCommunication::PacketResult::Success)
+        return Error("Sending qFileEndAddress packet failed");
+
+    if (response.IsErrorResponse())
+    {
+        if (response.GetError() == 1)
+        {
+            // The file is not loaded into the inferior
+            is_loaded = false;
+            end_addr = LLDB_INVALID_ADDRESS;
+            return Error();
+        }
+
+        return Error("Fetching file end address from remote server returned an error");
+    }
+
+    if (response.IsNormalResponse())
+    {
+        is_loaded = true;
+        end_addr = response.GetHexMaxU64(false, LLDB_INVALID_ADDRESS);
+        return Error();
+    }
+
+    return Error("Unknown error happened during sending the end address packet");
+}
 
 void
 ProcessGDBRemote::ModulesDidLoad (ModuleList &module_list)
 {
     // We must call the lldb_private::Process::ModulesDidLoad () first before we do anything
     Process::ModulesDidLoad (module_list);
 
     // After loading shared libraries, we can ask our remote GDB server if

source/Utility/StringExtractorGDBRemote.h

enum ServerPacketType
         eServerPacketType_qKillSpawnedProcess,
         eServerPacketType_qLaunchSuccess,
         eServerPacketType_qModuleInfo,
         eServerPacketType_qProcessInfoPID,
         eServerPacketType_qSpeedTest,
         eServerPacketType_qUserName,
         eServerPacketType_qGetWorkingDir,
         eServerPacketType_qFileLoadAddress,
+        eServerPacketType_qFileEndAddress,
         eServerPacketType_QEnvironment,
         eServerPacketType_QLaunchArch,
         eServerPacketType_QSetDisableASLR,
         eServerPacketType_QSetDetachOnError,
         eServerPacketType_QSetSTDIN,
         eServerPacketType_QSetSTDOUT,
         eServerPacketType_QSetSTDERR,
         eServerPacketType_QSetWorkingDir,

source/Utility/StringExtractorGDBRemote.cpp

case 'q':
             break;
 
         case 'E':
             if (PACKET_STARTS_WITH ("qEcho:"))                  return eServerPacketType_qEcho;
             break;
 
         case 'F':
             if (PACKET_STARTS_WITH ("qFileLoadAddress:"))       return eServerPacketType_qFileLoadAddress;
+            if (PACKET_STARTS_WITH ("qFileEndAddress:"))       return eServerPacketType_qFileEndAddress;
             break;
 
         case 'G':
             if (PACKET_STARTS_WITH ("qGroupName:"))             return eServerPacketType_qGroupName;
             if (PACKET_MATCHES ("qGetWorkingDir"))              return eServerPacketType_qGetWorkingDir;
             if (PACKET_MATCHES ("qGetPid"))                     return eServerPacketType_qGetPid;
             if (PACKET_STARTS_WITH ("qGetProfileData;"))        return eServerPacketType_qGetProfileData;
             if (PACKET_MATCHES ("qGDBServerVersion"))           return eServerPacketType_qGDBServerVersion;