jasonmolenda created this revision. jasonmolenda added a reviewer: jingham. jasonmolenda added a project: LLDB. Herald added a subscriber: JDevlieghere. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits.
If you add a binary to lldb and do `target modules load -s slide -f name` multiple times, the old entries are not cleared from the SectionLoadList's addr->section map, leading to confusing lldb behavior. SectionLoadList has a section-to-address map (m_sect_to_addr) and an address to section map (m_addr_to_sect) that are registered with the Target. SectionLoadList::SetSectionLoadAddress updates the entry in the m_sect_to_addr map if it exists, or adds it. And it adds a new entry to m_addr_to_sect as long as there isn't a section at that address already. But it never removes the old entry from m_addr_to_sect. This results in each section having multiple entries, with each the load addresses that had been set previously, so load address -> file address translation behaves very oddly. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D130534 Files: lldb/source/Target/SectionLoadList.cpp lldb/test/API/functionalities/multiple-slides/Makefile lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py lldb/test/API/functionalities/multiple-slides/main.c Index: lldb/test/API/functionalities/multiple-slides/main.c =================================================================== --- /dev/null +++ lldb/test/API/functionalities/multiple-slides/main.c @@ -0,0 +1,5 @@ +int first[2048] = { 5 }; +int second[2048] = { 6 }; +int main() { + return first[0] + second[0]; +} Index: lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py =================================================================== --- /dev/null +++ lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py @@ -0,0 +1,44 @@ +""" +Test that a binary can be slid to different load addresses correctly +""" + + + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class MultipleSlidesTestCase(TestBase): + + NO_DEBUG_INFO_TESTCASE = True + def test_mulitple_slides(self): + """Test that a binary can be slid multiple times correctly.""" + self.build() + exe = self.getBuildArtifact("a.out") + err = lldb.SBError() + load_dependent_modules = False + target = self.dbg.CreateTarget(exe, '', '', load_dependent_modules, err) + self.assertTrue(target.IsValid()) + module = target.GetModuleAtIndex(0) + self.assertTrue(module.IsValid()) + + # View the first element of `first` and `second` while + # they have no load address set. + self.expect("p/d ((int*)&first)[0]", substrs=['= 5']) + self.expect("p/d ((int*)&second)[0]", substrs=['= 6']) + + target.SetModuleLoadAddress(module, 1990) + + # View the first element of `first` and `second` with + # a load address . + self.expect("p/d ((int*)&first)[0]", substrs=['= 5']) + self.expect("p/d ((int*)&second)[0]", substrs=['= 6']) + + target.SetModuleLoadAddress(module, 4) + + # View the first element of `first` and `second` with + # a new load address. + self.expect("p/d ((int*)&first)[0]", substrs=['= 5']) + self.expect("p/d ((int*)&second)[0]", substrs=['= 6']) Index: lldb/test/API/functionalities/multiple-slides/Makefile =================================================================== --- /dev/null +++ lldb/test/API/functionalities/multiple-slides/Makefile @@ -0,0 +1,12 @@ +C_SOURCES := main.c +MAKE_DSYM := NO + +include Makefile.rules + +# lldb has a separate bug where this test case +# does not work if we have debug info - after +# sliding the binary, the address of `first` and +# `second` are not slid for some reason on Darwin. +main.o: main.c + $(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@ + Index: lldb/source/Target/SectionLoadList.cpp =================================================================== --- lldb/source/Target/SectionLoadList.cpp +++ lldb/source/Target/SectionLoadList.cpp @@ -116,8 +116,18 @@ } } ats_pos->second = section; - } else + } else { + // Remove the old address->section entry, if + // there is one. + for (const auto &it : m_addr_to_sect) { + if (it.second == section) { + const auto &it_pos = m_addr_to_sect.find(it.first); + m_addr_to_sect.erase(it_pos); + break; + } + } m_addr_to_sect[load_addr] = section; + } return true; // Changed } else {
Index: lldb/test/API/functionalities/multiple-slides/main.c =================================================================== --- /dev/null +++ lldb/test/API/functionalities/multiple-slides/main.c @@ -0,0 +1,5 @@ +int first[2048] = { 5 }; +int second[2048] = { 6 }; +int main() { + return first[0] + second[0]; +} Index: lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py =================================================================== --- /dev/null +++ lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py @@ -0,0 +1,44 @@ +""" +Test that a binary can be slid to different load addresses correctly +""" + + + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class MultipleSlidesTestCase(TestBase): + + NO_DEBUG_INFO_TESTCASE = True + def test_mulitple_slides(self): + """Test that a binary can be slid multiple times correctly.""" + self.build() + exe = self.getBuildArtifact("a.out") + err = lldb.SBError() + load_dependent_modules = False + target = self.dbg.CreateTarget(exe, '', '', load_dependent_modules, err) + self.assertTrue(target.IsValid()) + module = target.GetModuleAtIndex(0) + self.assertTrue(module.IsValid()) + + # View the first element of `first` and `second` while + # they have no load address set. + self.expect("p/d ((int*)&first)[0]", substrs=['= 5']) + self.expect("p/d ((int*)&second)[0]", substrs=['= 6']) + + target.SetModuleLoadAddress(module, 1990) + + # View the first element of `first` and `second` with + # a load address . + self.expect("p/d ((int*)&first)[0]", substrs=['= 5']) + self.expect("p/d ((int*)&second)[0]", substrs=['= 6']) + + target.SetModuleLoadAddress(module, 4) + + # View the first element of `first` and `second` with + # a new load address. + self.expect("p/d ((int*)&first)[0]", substrs=['= 5']) + self.expect("p/d ((int*)&second)[0]", substrs=['= 6']) Index: lldb/test/API/functionalities/multiple-slides/Makefile =================================================================== --- /dev/null +++ lldb/test/API/functionalities/multiple-slides/Makefile @@ -0,0 +1,12 @@ +C_SOURCES := main.c +MAKE_DSYM := NO + +include Makefile.rules + +# lldb has a separate bug where this test case +# does not work if we have debug info - after +# sliding the binary, the address of `first` and +# `second` are not slid for some reason on Darwin. +main.o: main.c + $(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@ + Index: lldb/source/Target/SectionLoadList.cpp =================================================================== --- lldb/source/Target/SectionLoadList.cpp +++ lldb/source/Target/SectionLoadList.cpp @@ -116,8 +116,18 @@ } } ats_pos->second = section; - } else + } else { + // Remove the old address->section entry, if + // there is one. + for (const auto &it : m_addr_to_sect) { + if (it.second == section) { + const auto &it_pos = m_addr_to_sect.find(it.first); + m_addr_to_sect.erase(it_pos); + break; + } + } m_addr_to_sect[load_addr] = section; + } return true; // Changed } else {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits