Re: [Lldb-commits] [PATCH] D42443: [SymbolFilePDB] Add support for function symbols
Well... it would be *very*-nice-to-have thing. I would very much love to be able to run at least one pdb test from linux. (So, I'm not opposing this in any way, just pointing out what needs to happen). On 1 February 2018 at 18:35, Zachary Turner wrote: > Yea good point. We can probably punt on this for now as it’s just a > nice-to-have > > On Thu, Feb 1, 2018 at 1:38 AM Pavel Labath via Phabricator > wrote: >> >> labath added inline comments. >> >> >> >> Comment at: lit/SymbolFile/PDB/func-symbols.test:4 >> +RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/FuncSymbols.cpp /o >> %T/FuncSymbols.cpp.obj >> +RUN: link %T/FuncSymbolsTestMain.cpp.obj %T/FuncSymbols.cpp.obj /DEBUG >> /nodefaultlib /Entry:main /OUT:%T/FuncSymbolsTest.exe >> +RUN: lldb-test symbols %T/FuncSymbolsTest.exe | FileCheck %s >> >> zturner wrote: >> > I bet we could get rid of `REQUIRES: windows` if we changed this to >> > `lld-link`. You're already specifying `/nodefaultlib /entry:main`, and no >> > windows header files are included, so maybe it's worth a try? >> That would be great, but you'd probably still need something like >> REQUIRES:lld, as lld is not currently not a mandatory requirement for >> building/testing. >> >> >> Repository: >> rL LLVM >> >> https://reviews.llvm.org/D42443 >> >> >> > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions
labath added a comment. Yes, it's mostly stylistic differences, but there is one more important thing that we need to figure out, and that's making sure that the test only runs if we actually have XML support compiled-in. If there's isn't anything in the SB API that would tell us that (I cursory look doesn't reveal anything), then we may need to get some help from the build system for that. OTOH, maybe an new SB API for determining the lldb feature set would actually be useful... Comment at: include/lldb/Target/Process.h:1958 + virtual bool WriteObjectFile(llvm::ArrayRef entries, + Status &error); owenpshaw wrote: > labath wrote: > > This (return bool + by-ref Status) is a bit weird of an api. Could you just > > return Status here (but I would not be opposed to going llvm all the way > > and returning `llvm::Error`). > Open to whatever. I preferred how it made calling code a little simpler. > > ``` > if (!WriteObjectFile(entries, error)) > return error; > ``` > > rather than > > > ``` > error = WriteObjectFile(entries); > if (!error.Sucess()) > return error > ``` That extra line is a bit of a nuissance. We could fix that, but as the long term solution is to use llvm::Error everywhere, i'm not motivated to do that. So if the extra line bothers you, just use llvm::Error. Then the call-size becomes: ``` if (llvm::Error E = WriteObjectFile(...)) return Status(std::move(E)); ``` The constant conversion between Status and Error is a bit annoying, but as more places start using this, it should get better. (My main issue with your syntax is that it's not DRY) Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:2807 + // memory, must happen in order of increasing address. + std::vector sortedEntries(entries); + std::stable_sort(std::begin(sortedEntries), std::end(sortedEntries), owenpshaw wrote: > labath wrote: > > Let's avoid copying the entries here. I can see two options: > > - Require that the entries are already sorted on input > > - pass them to this function as `std::vector` (and then have > > the caller call with `std::move(entries)`). > I didn't love the copy either, but figured 1) it was pretty cheap given the > expected values 2) it eliminates an unexpected modification of the passed > array. I prefer having the sort in the process file, since it's really the > only scope that knows about the sorting requirement. I agree it likely to be cheap, but the thing you fear (2) can be easily avoided. Note that I deliberately did not add any reference qualifications to the type in the comment above. If you take the argument as a value type, then you leave it up to the caller to decide whether a copy takes place. If he calls it as `WriteMemoryObject(my_vector)`, then it's copied (and the original is left unmodified). If he calls it as `WriteMemoryObject(std::move(my_vector))`, then it's moved (but that's very obvious from looking at the call-site). So in the worst case, you get one copy, just like you would here, and in the best case, you get no copies. In your case, the caller doesn't need the vector, so we save a copy. To me, that makes this option a clear win. Isn't C++11 great? :D Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:2812-2821 + m_allow_flash_writes = true; + if (Process::WriteObjectFile(sortedEntries, error)) +error = FlashDone(); + else +// Even though some of the writing failed, try to send a flash done if +// some of the writing succeeded so the flash state is reset to normal, +// but don't stomp on the error status that was set in the write failure owenpshaw wrote: > labath wrote: > > This makes the control flow quite messy. The base function is not so > > complex that you have to reuse it at all costs. I think we should just > > implement the loop ourselves (and call some write function while passing > > the "allow_flash_writes" as an argument). > Guess we have different definitions of messy :) > > Wouldn't any other approach pretty much require duplicating WriteMemory, > WriteMemoryPrivate, and DoWriteMemory? > > - It seemed like the breakpoint logic in WriteMemory could be important when > writing an object to ram, so I wanted to preserve that > - The loop in WriteMemoryPrivate is fairly trivial, but why add a second one > if not needed? > - DoWriteMemory is mostly code that is common to both ram and flash writes, > especially if a ram-only version would still need to check if address is > flash so it could report an error. > Yeah, I think I'll have to agree with you here, although this makes the whole separate-function approach look far less attractive to me. If we are going to reuse all of that logic that then we might make it obvious that we are doing that by having a bool flag on those functions like Greg suggested a couple of comments back.
[Lldb-commits] [PATCH] D42828: Fix for read-past-end-of-array buglet in ProcessElfCore.cpp while reading linux notes
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good, just make sure to not include extra \0 bytes. Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:668 return status.ToError(); - thread_data.name = prpsinfo.pr_fname; + thread_data.name.assign (prpsinfo.pr_fname, sizeof (prpsinfo.pr_fname)); SetID(prpsinfo.pr_pid); In case the name *is* null-terminated, this will forcibly include the \0 bytes into the string, which is not good. I think this should be something like `assign(pr_fname, strnlen(pr_fname, sizeof(pf_fname))` (maybe there is a more c++-y way of writing that, but I couldn't think of one). I think adding a check for the thread name in the test still has value (asan will just check we don't do anything stupid, but it won't verify we actually produce the right name in the end), but I can do that as a follow-up. Repository: rL LLVM https://reviews.llvm.org/D42828 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42836: [dotest] make debug info variant accessible in setUp()
labath created this revision. labath added reviewers: aprantl, jingham. Herald added subscribers: JDevlieghere, eraman. This changes the way we store the debug info variant to make it available earlier in the test bringup: instead of it being set by the test wrapper method, it is set as a *property* of the wrapper method. This way, we can inspect it as soon as self.testMethodName is initialized. The retrieval is implemented by a new function TestBase.getDebugInfo(), and all that's necessary to make it work is to change self.debug_info into self.getDebugInfo(). While searching for debug_info occurences i noticed that TestLogging is being replicated for no good reason, so I removed the replication there. https://reviews.llvm.org/D42836 Files: packages/Python/lldbsuite/test/decorators.py packages/Python/lldbsuite/test/functionalities/exec/TestExec.py packages/Python/lldbsuite/test/lldbinline.py packages/Python/lldbsuite/test/lldbtest.py packages/Python/lldbsuite/test/logging/TestLogging.py Index: packages/Python/lldbsuite/test/logging/TestLogging.py === --- packages/Python/lldbsuite/test/logging/TestLogging.py +++ packages/Python/lldbsuite/test/logging/TestLogging.py @@ -19,6 +19,7 @@ mydir = TestBase.compute_mydir(__file__) append_log_file = "lldb-commands-log-append.txt" truncate_log_file = "lldb-commands-log-truncate.txt" +NO_DEBUG_INFO_TESTCASE = True @classmethod def classCleanup(cls): @@ -28,23 +29,11 @@ def test(self): self.build() -if self.debug_info == "dsym": -self.command_log_tests("dsym") -else: -self.command_log_tests("dwarf") - -def command_log_tests(self, type): exe = self.getBuildArtifact("a.out") self.expect("file " + exe, patterns=["Current executable set to .*a.out"]) -log_file = os.path.join( -self.getBuildDir(), -"lldb-commands-log-%s-%s-%s.txt" % -(type, - os.path.basename( - self.getCompiler()), -self.getArchitecture())) +log_file = os.path.join(self.getBuildDir(), "lldb-commands-log.txt") if (os.path.exists(log_file)): os.remove(log_file) @@ -75,7 +64,6 @@ "Something was written to the log file.") # Check that lldb truncates its log files -@no_debug_info_test def test_log_truncate(self): if (os.path.exists(self.truncate_log_file)): os.remove(self.truncate_log_file) @@ -99,7 +87,6 @@ self.assertEquals(contents.find("bacon"), -1) # Check that lldb can append to a log file -@no_debug_info_test def test_log_append(self): if (os.path.exists(self.append_log_file)): os.remove(self.append_log_file) Index: packages/Python/lldbsuite/test/lldbtest.py === --- packages/Python/lldbsuite/test/lldbtest.py +++ packages/Python/lldbsuite/test/lldbtest.py @@ -850,9 +850,6 @@ self.setPlatformWorkingDir() self.enableLogChannelsForCurrentTest() -# Initialize debug_info -self.debug_info = None - lib_dir = os.environ["LLDB_LIB_DIR"] self.dsym = None self.framework_dir = None @@ -1520,7 +1517,7 @@ """Platform specific way to build the default binaries.""" if not testdir: testdir = self.mydir -if self.debug_info: +if self.getDebugInfo(): raise Exception("buildDefault tests must set NO_DEBUG_INFO_TESTCASE") module = builder_module() self.makeBuildDir() @@ -1539,7 +1536,7 @@ """Platform specific way to build binaries with dsym info.""" if not testdir: testdir = self.mydir -if self.debug_info <> "dsym": +if self.getDebugInfo() != "dsym": raise Exception("NO_DEBUG_INFO_TESTCASE must build with buildDefault") module = builder_module() @@ -1558,7 +1555,7 @@ """Platform specific way to build binaries with dwarf maps.""" if not testdir: testdir = self.mydir -if self.debug_info <> "dwarf": +if self.getDebugInfo() != "dwarf": raise Exception("NO_DEBUG_INFO_TESTCASE must build with buildDefault") module = builder_module() @@ -1577,7 +1574,7 @@ """Platform specific way to build binaries with dwarf maps.""" if not testdir: testdir = self.mydir -if self.debug_info <> "dwo": +if self.getDebugInfo() != "dwo": raise Exception("NO_DEBUG_INFO_TESTCASE must build with buildDefault") module = builder_module() @@ -1596,7 +1593,7 @@ """Platform specific way to build binaries with gmodules info.""" if not testdir: testdir = self.mydir -if self.debug_info <> "gmodul
[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
JDevlieghere added inline comments. Comment at: packages/Python/lldbsuite/test/lldbinline.py:96-98 +return "-N dwarf %s" % (testdir) else: +return "-N dsym %s" % (testdir) labath wrote: > aprantl wrote: > > labath wrote: > > > xiaobai wrote: > > > > Good opportunity to move away from the old style of formatting strings > > > > to a newer style? It doesn't make a huge difference, I just think it'd > > > > be nice to do. :) > > > > `return "-N dwarf {}".format(testdir)` > > > > `return "-N dwarf {}".format(testdir)` > > > > > > > > This is supported in Python 2.7, but I'm not sure if we can assume that > > > > version or greater to be present. > > > 2.7 is definitely fine, and we use `{}` format in plently of places > > > already. (But I don't have any beef with the %s format either). > > In this case I think this should just be `return "-N dsym " + testdir` > :D +1. I agree with Pavel that we should adopt the new formatters. Let's be idiomatic :-) Comment at: packages/Python/lldbsuite/test/lldbtest.py:728 +if not os.path.isdir(path): +raise OSError(errno.ENOTDIR, "%s is not a directory"%path) `"{} is not a directory".format(path)` https://reviews.llvm.org/D42763 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42836: [dotest] make debug info variant accessible in setUp()
labath updated this revision to Diff 132557. labath added a comment. Move getDebugInfo definition to "Base" class (fixes lldb-mi tests). https://reviews.llvm.org/D42836 Files: packages/Python/lldbsuite/test/decorators.py packages/Python/lldbsuite/test/functionalities/exec/TestExec.py packages/Python/lldbsuite/test/lldbinline.py packages/Python/lldbsuite/test/lldbtest.py packages/Python/lldbsuite/test/logging/TestLogging.py Index: packages/Python/lldbsuite/test/logging/TestLogging.py === --- packages/Python/lldbsuite/test/logging/TestLogging.py +++ packages/Python/lldbsuite/test/logging/TestLogging.py @@ -19,6 +19,7 @@ mydir = TestBase.compute_mydir(__file__) append_log_file = "lldb-commands-log-append.txt" truncate_log_file = "lldb-commands-log-truncate.txt" +NO_DEBUG_INFO_TESTCASE = True @classmethod def classCleanup(cls): @@ -28,23 +29,11 @@ def test(self): self.build() -if self.debug_info == "dsym": -self.command_log_tests("dsym") -else: -self.command_log_tests("dwarf") - -def command_log_tests(self, type): exe = self.getBuildArtifact("a.out") self.expect("file " + exe, patterns=["Current executable set to .*a.out"]) -log_file = os.path.join( -self.getBuildDir(), -"lldb-commands-log-%s-%s-%s.txt" % -(type, - os.path.basename( - self.getCompiler()), -self.getArchitecture())) +log_file = os.path.join(self.getBuildDir(), "lldb-commands-log.txt") if (os.path.exists(log_file)): os.remove(log_file) @@ -75,7 +64,6 @@ "Something was written to the log file.") # Check that lldb truncates its log files -@no_debug_info_test def test_log_truncate(self): if (os.path.exists(self.truncate_log_file)): os.remove(self.truncate_log_file) @@ -99,7 +87,6 @@ self.assertEquals(contents.find("bacon"), -1) # Check that lldb can append to a log file -@no_debug_info_test def test_log_append(self): if (os.path.exists(self.append_log_file)): os.remove(self.append_log_file) Index: packages/Python/lldbsuite/test/lldbtest.py === --- packages/Python/lldbsuite/test/lldbtest.py +++ packages/Python/lldbsuite/test/lldbtest.py @@ -850,9 +850,6 @@ self.setPlatformWorkingDir() self.enableLogChannelsForCurrentTest() -# Initialize debug_info -self.debug_info = None - lib_dir = os.environ["LLDB_LIB_DIR"] self.dsym = None self.framework_dir = None @@ -1404,6 +1401,10 @@ option_str += " -C " + comp return option_str +def getDebugInfo(self): +method = getattr(self, self.testMethodName) +return getattr(method, "debug_info", None) + # == # Build methods supported through a plugin interface # == @@ -1520,7 +1521,7 @@ """Platform specific way to build the default binaries.""" if not testdir: testdir = self.mydir -if self.debug_info: +if self.getDebugInfo(): raise Exception("buildDefault tests must set NO_DEBUG_INFO_TESTCASE") module = builder_module() self.makeBuildDir() @@ -1539,7 +1540,7 @@ """Platform specific way to build binaries with dsym info.""" if not testdir: testdir = self.mydir -if self.debug_info <> "dsym": +if self.getDebugInfo() != "dsym": raise Exception("NO_DEBUG_INFO_TESTCASE must build with buildDefault") module = builder_module() @@ -1558,7 +1559,7 @@ """Platform specific way to build binaries with dwarf maps.""" if not testdir: testdir = self.mydir -if self.debug_info <> "dwarf": +if self.getDebugInfo() != "dwarf": raise Exception("NO_DEBUG_INFO_TESTCASE must build with buildDefault") module = builder_module() @@ -1577,7 +1578,7 @@ """Platform specific way to build binaries with dwarf maps.""" if not testdir: testdir = self.mydir -if self.debug_info <> "dwo": +if self.getDebugInfo() != "dwo": raise Exception("NO_DEBUG_INFO_TESTCASE must build with buildDefault") module = builder_module() @@ -1596,7 +1597,7 @@ """Platform specific way to build binaries with gmodules info.""" if not testdir: testdir = self.mydir -if self.debug_info <> "gmodules": +if self.getDebugInfo() != "gmodules": raise Exception("NO_DEBUG_INFO_TESTCASE must build with buildDefault") module = builder_module() @
[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
labath added inline comments. Comment at: packages/Python/lldbsuite/test/api/listeners/TestListener.py:26 TestBase.setUp(self) -self.build() aprantl wrote: > labath wrote: > > I'm confused by these changes. I was under the impression that setUp() runs > > before each test method (and hence this should be NFC). Can you explain the > > reasoning behind this? > (see also my previous comment) > `setUp` runs before everything else, but it runs once per testcase and > `self.testMethodName` is not initialized yet. Therefore we can only run > `self.build()` in `self.setUp()` in NO_DEBUG_INFO_TESTCASEs. I'm not sure where you got the idea that testMethodName is not initialized. It's initialized as one of the first things in Base.setUp (lldbtest.py:776). This also demonstrates that the setUp method is run once *per test function* (as otherwise testMethodName would make no sense; also check out third_party/Python/module/unittest2/unittest2/case.py:379 to see how tests are run). The problem I think you are having is that self.**debug_info** is not initialized. I was curious at hard it would be to make this work, so I played around with it a bit and the result was D42836. It's not that I think this change is that bad (although I would prefer if we could keep doing these things in setUp), but I think this could also help make happen the ideas about building binaries once per test case (class) -- the way I'd do that is that I would enumerate all variants that are needed to build in setUpClass (and build them); then the actual test could fetch the corresponding binary according to the variant. Comment at: packages/Python/lldbsuite/test/lldbtest.py:721-728 +try: +os.makedirs(path) +except OSError as e: +import errno +if e.errno != errno.EEXIST: +raise +if not os.path.isdir(path): Can we use the (new) lldbutil.mkdir_p here? If there's some circular dependency, maybe we could move the function somewhere else? Comment at: packages/Python/lldbsuite/test/lldbtest.py:1517-1518 testdir = self.mydir +if not testname: +testname = self.testMethodName if self.debug_info: Why was this necessary? Could we either always pass the name by argument, or always fetch it from `self`? Maybe it will be possible after `D42836`? https://reviews.llvm.org/D42763 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42828: Fix for read-past-end-of-array buglet in ProcessElfCore.cpp while reading linux notes
clayborg added inline comments. Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:668 return status.ToError(); - thread_data.name = prpsinfo.pr_fname; + thread_data.name.assign (prpsinfo.pr_fname, sizeof (prpsinfo.pr_fname)); SetID(prpsinfo.pr_pid); labath wrote: > In case the name *is* null-terminated, this will forcibly include the \0 > bytes into the string, which is not good. > I think this should be something like `assign(pr_fname, strnlen(pr_fname, > sizeof(pf_fname))` (maybe there is a more c++-y way of writing that, but I > couldn't think of one). > > I think adding a check for the thread name in the test still has value (asan > will just check we don't do anything stupid, but it won't verify we actually > produce the right name in the end), but I can do that as a follow-up. This is how we cap segment lengths on ObjectFileMachO: ``` ConstString const_segname(load_cmd.segname, std::min(strlen(load_cmd.segname), sizeof(load_cmd.segname))); ``` This won't keep ASAN happy though as the strlen might go off the end. You might be able to check the last byte since I am guessing they will zero fill by default: ``` if (prpsinfo.pr_fname[sizeof (prpsinfo.pr_fname)-1] != '\0') thread_data.name.assign (prpsinfo.pr_fname, sizeof (prpsinfo.pr_fname)); else thread_data.name = prpsinfo.pr_fname; ``` Repository: rL LLVM https://reviews.llvm.org/D42828 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42836: [dotest] make debug info variant accessible in setUp()
aprantl added a comment. That looks great, to make sure I understand this correctly, is the following accurate? If we apply this on top of https://reviews.llvm.org/D42763, and we have tests/my/testA.py: class TestA(Base): def setUp(self): ... def testAone(self): ... def testAtwo(self): ... and we are testing two variants, "dwo", and "dwarf". dotest.py will instantiate 4 objects: Aone_dwo = new TestA { debug_info="dwo" } Aone_dwarf = new TestA { debug_info="dwo" } Atwo_dwo = new TestA { debug_info="dwo" } Atwo_dwarf = new TestA { debug_info="dwarf" } and schedule Aone_dwo.setUp() Aone_dwarf.setUp() Atwo_dwo.setUp() Atwo_dwarf.setUp() and then Aone_dwo.testAone() Aone_dwarf.testAone() Atwo_dwo.testAtwo() Atwo_dwarf.testAtwo() Is this accurate, or is there something missing to make it behave this way? https://reviews.llvm.org/D42836 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42852: Fix upper->lower case for /usr/lib/debug/.build-id/**.debug
jankratochvil created this revision. jankratochvil added reviewers: labath, clayborg. I have found the lookup by build-id (when lookup by /usr/lib/debug/path/name/exec.debug failed) does not work as LLDB tries the build-id hex string in uppercase but Fedora uses lowercase. xubuntu-16.10 also uses lowercase during my test: /usr/lib/debug/.build-id/6c/61f3566329f43d03f812ae7057e9e7391b5ff6.debug So I do not see when it could work, I haven't found any such regression-looking change in GIT history. https://reviews.llvm.org/D42852 Files: packages/Python/lldbsuite/test/linux/buildidcase/Makefile packages/Python/lldbsuite/test/linux/buildidcase/TargetSymbolsBuildidCase.py packages/Python/lldbsuite/test/linux/buildidcase/main.c source/Host/common/Symbols.cpp Index: source/Host/common/Symbols.cpp === --- source/Host/common/Symbols.cpp +++ source/Host/common/Symbols.cpp @@ -245,6 +245,8 @@ // Some debug files are stored in the .build-id directory like this: // /usr/lib/debug/.build-id/ff/e7fe727889ad82bb153de2ad065b2189693315.debug uuid_str = module_uuid.GetAsString(""); + std::transform(uuid_str.begin(), uuid_str.end(), uuid_str.begin(), + ::tolower); uuid_str.insert(2, 1, '/'); uuid_str = uuid_str + ".debug"; } Index: packages/Python/lldbsuite/test/linux/buildidcase/main.c === --- /dev/null +++ packages/Python/lldbsuite/test/linux/buildidcase/main.c @@ -0,0 +1,3 @@ +int main() { + return 0; +} Index: packages/Python/lldbsuite/test/linux/buildidcase/TargetSymbolsBuildidCase.py === --- /dev/null +++ packages/Python/lldbsuite/test/linux/buildidcase/TargetSymbolsBuildidCase.py @@ -0,0 +1,42 @@ +""" Testing separate debug info loading by its .build-id. """ +import os +import time +import lldb +import sys +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TargetSymbolsBuildidCase(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +def setUp(self): +TestBase.setUp(self) +self.source = 'main.c' + +@no_debug_info_test # Prevent the genaration of the dwarf version of this test +@skipUnlessPlatform(['linux']) +def test_target_buildid_case(self): +self.build(clean=True) +exe = self.getBuildArtifact("stripped.out") + +self.target = self.dbg.CreateTarget(exe) +self.assertTrue(self.target, VALID_TARGET) + +main_bp = self.target.BreakpointCreateByName("main", "stripped.out") +self.assertTrue(main_bp, VALID_BREAKPOINT) + +self.process = self.target.LaunchSimple( +None, None, self.get_process_working_directory()) +self.assertTrue(self.process, PROCESS_IS_VALID) + +# The stop reason of the thread should be breakpoint. +self.assertTrue(self.process.GetState() == lldb.eStateStopped, +STOPPED_DUE_TO_BREAKPOINT) + +exe_module = self.target.GetModuleAtIndex(0) + +# Check that symbols are now loaded and main.c is in the output. +self.expect("frame select", substrs=['main.c']) Index: packages/Python/lldbsuite/test/linux/buildidcase/Makefile === --- /dev/null +++ packages/Python/lldbsuite/test/linux/buildidcase/Makefile @@ -0,0 +1,20 @@ +LEVEL = ../../make +CXX_SOURCES := main.cpp +LD_EXTRAS += -Wl,--build-id=sha1 + +localall: stripped.out .build-id + +.PHONY: .build-id +stripped.out .build-id: a.out + $(OBJCOPY) -j .note.gnu.build-id -O binary $< tmp + rm -rf .build-id + fn=`od -An -tx1 Index: source/Host/common/Symbols.cpp === --- source/Host/common/Symbols.cpp +++ source/Host/common/Symbols.cpp @@ -245,6 +245,8 @@ // Some debug files are stored in the .build-id directory like this: // /usr/lib/debug/.build-id/ff/e7fe727889ad82bb153de2ad065b2189693315.debug uuid_str = module_uuid.GetAsString(""); + std::transform(uuid_str.begin(), uuid_str.end(), uuid_str.begin(), + ::tolower); uuid_str.insert(2, 1, '/'); uuid_str = uuid_str + ".debug"; } Index: packages/Python/lldbsuite/test/linux/buildidcase/main.c === --- /dev/null +++ packages/Python/lldbsuite/test/linux/buildidcase/main.c @@ -0,0 +1,3 @@ +int main() { + return 0; +} Index: packages/Python/lldbsuite/test/linux/buildidcase/TargetSymbolsBuildidCase.py === --- /dev/null +++ packages/Python/lldbsuite/test/linux/buildidcase/TargetSymbolsBuildidCase.py @@ -0,0 +1,42 @@ +""" Testing separate debug info loading by its .build-id. """ +import os +
[Lldb-commits] [PATCH] D42836: [dotest] make debug info variant accessible in setUp()
labath added a comment. In https://reviews.llvm.org/D42836#996288, @aprantl wrote: > That looks great, to make sure I understand this correctly, is the following > accurate? > > If we apply this on top of https://reviews.llvm.org/D42763, and we have > > tests/my/testA.py: > class TestA(Base): > def setUp(self): > ... > def testAone(self): > ... > def testAtwo(self): > ... > > > and we are testing two variants, "dwo", and "dwarf". > > dotest.py will instantiate 4 objects: > > Aone_dwo = new TestA { debug_info="dwo" } > Aone_dwarf = new TestA { debug_info="dwo" } > Atwo_dwo = new TestA { debug_info="dwo" } > Atwo_dwarf = new TestA { debug_info="dwarf" } > > > and schedule > > Aone_dwo.setUp() > Aone_dwarf.setUp() > Atwo_dwo.setUp() > Atwo_dwarf.setUp() > > > and then > > Aone_dwo.testAone() > Aone_dwarf.testAone() > Atwo_dwo.testAtwo() > Atwo_dwarf.testAtwo() > > > Is this accurate, or is there something missing to make it behave this way? Not completely accurate. The steps are the correct, but the order is different. It will be something like for test in ["testAdwarf", "testAdwo", "testBdwarf", "testBdwo"]: A = new TestA(test) # initializes A._testMethodName A.setUp() # can access debug info variant through A._testMethodName (a.k.a A.testMethodName) getattr(A, test)() A.tearDown() I don't know whether this distinction matters for you right now (?) https://reviews.llvm.org/D42836 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42853: Resolve binary symlinks before finding its separate .debug file
jankratochvil created this revision. jankratochvil added reviewers: labath, clayborg. Herald added a subscriber: aprantl. I have found LLDB cannot find separate debug info of Fedora /usr/bin/gdb. It is because: lrwxrwxrwx 1 root root 14 Jan 25 20:41 /usr/bin/gdb -> ../libexec/gdb* -rwxr-xr-x 1 root root 10180296 Jan 25 20:41 /usr/libexec/gdb* ls: cannot access '/usr/lib/debug/usr/bin/gdb-8.0.1-35.fc27.x86_64.debug': No such file or directory -r--r--r-- 1 root root 29200464 Jan 25 20:41 /usr/lib/debug/usr/libexec/gdb-8.0.1-35.fc27.x86_64.debug FYI that `-8.0.1-35.fc27.x86_64.debug` may look confusing, it was always just `.debug` before. Why is `/usr/bin/gdb` a symlink is offtopic for this bugreport, Fedora has it so for some reasons. It is always safest to look at the .debug file only after resolving all symlinks on the binary file. I hope it should not cause any regressions. https://reviews.llvm.org/D42853 Files: packages/Python/lldbsuite/test/linux/sepdebugsymlink/Makefile packages/Python/lldbsuite/test/linux/sepdebugsymlink/TestSymbolsSepDebugSymlink.py packages/Python/lldbsuite/test/linux/sepdebugsymlink/main.c source/Host/common/Symbols.cpp Index: source/Host/common/Symbols.cpp === --- source/Host/common/Symbols.cpp +++ source/Host/common/Symbols.cpp @@ -9,6 +9,7 @@ #include "lldb/Host/Symbols.h" #include "lldb/Core/ModuleSpec.h" +#include "lldb/Host/FileSystem.h" #include "lldb/Symbol/ObjectFile.h" #include "lldb/Target/Target.h" #include "lldb/Utility/ArchSpec.h" @@ -221,7 +222,11 @@ Target::GetDefaultDebugFileSearchPaths()); // Add module directory. -const ConstString &file_dir = module_spec.GetFileSpec().GetDirectory(); +FileSpec module_file_spec = module_spec.GetFileSpec(); +// We keep the unresolved pathname if it fails. +FileSystem::ResolveSymbolicLink(module_file_spec, module_file_spec); + +const ConstString &file_dir = module_file_spec.GetDirectory(); debug_file_search_paths.AppendIfUnique( FileSpec(file_dir.AsCString("."), true)); @@ -274,7 +279,7 @@ FileSpec file_spec(filename, true); if (llvm::sys::fs::equivalent(file_spec.GetPath(), - module_spec.GetFileSpec().GetPath())) + module_file_spec.GetPath())) continue; if (file_spec.Exists()) { Index: packages/Python/lldbsuite/test/linux/sepdebugsymlink/main.c === --- /dev/null +++ packages/Python/lldbsuite/test/linux/sepdebugsymlink/main.c @@ -0,0 +1,3 @@ +int main() { + return 0; +} Index: packages/Python/lldbsuite/test/linux/sepdebugsymlink/TestSymbolsSepDebugSymlink.py === --- /dev/null +++ packages/Python/lldbsuite/test/linux/sepdebugsymlink/TestSymbolsSepDebugSymlink.py @@ -0,0 +1,42 @@ +""" Testing separate debug info loading for base binary with a symlink. """ +import os +import time +import lldb +import sys +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestSymbolsSepDebugSymlink(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +def setUp(self): +TestBase.setUp(self) +self.source = 'main.c' + +@no_debug_info_test # Prevent the genaration of the dwarf version of this test +@skipUnlessPlatform(['linux']) +def test_sepdebug_symlink_case(self): +self.build(clean=True) +exe = self.getBuildArtifact("dirsymlink/stripped.symlink") + +self.target = self.dbg.CreateTarget(exe) +self.assertTrue(self.target, VALID_TARGET) + +main_bp = self.target.BreakpointCreateByName("main", "stripped.symlink") +self.assertTrue(main_bp, VALID_BREAKPOINT) + +self.process = self.target.LaunchSimple( +None, None, self.get_process_working_directory()) +self.assertTrue(self.process, PROCESS_IS_VALID) + +# The stop reason of the thread should be breakpoint. +self.assertTrue(self.process.GetState() == lldb.eStateStopped, +STOPPED_DUE_TO_BREAKPOINT) + +exe_module = self.target.GetModuleAtIndex(0) + +# Check that symbols are now loaded and main.c is in the output. +self.expect("frame select", substrs=['main.c']) Index: packages/Python/lldbsuite/test/linux/sepdebugsymlink/Makefile === --- /dev/null +++ packages/Python/lldbsuite/test/linux/sepdebugsymlink/Makefile @@ -0,0 +1,20 @@ +LEVEL = ../../make +CXX_SOURCES := main.cpp + +localall: dirsymlink + +dirreal: a.out + $(RM) -r $@ + mkdir $@ + $(OBJCOPY) --only-keep-debug $< $@/stripped.debug + $(OBJCOPY) --strip-all --add-gnu-debuglink=$@/stripped.debug $< $@/stripped.out + +dirsymlink: dirreal + $(RM)
[Lldb-commits] [PATCH] D42852: Fix upper->lower case for /usr/lib/debug/.build-id/**.debug
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Many file systems are case insensitive so that is probably why it works on some computers. https://reviews.llvm.org/D42852 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42853: Resolve binary symlinks before finding its separate .debug file
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Makes sense for me, but make sure Pavel is ok with this as well before committing. https://reviews.llvm.org/D42853 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r324115 - Use an alternative approach to prevent Spotlight from indexing the build directory.
Author: adrian Date: Fri Feb 2 10:32:29 2018 New Revision: 324115 URL: http://llvm.org/viewvc/llvm-project?rev=324115&view=rev Log: Use an alternative approach to prevent Spotlight from indexing the build directory. Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest.py lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py lldb/trunk/test/CMakeLists.txt Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest.py?rev=324115&r1=324114&r2=324115&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Fri Feb 2 10:32:29 2018 @@ -1200,14 +1200,6 @@ def run_suite(): build_dir = configuration.test_build_dir lldbutil.mkdir_p(build_dir) -# Create a marker for Spotlight to never index $BUILD_DIR. LLDB -# queries Spotlight to locate .dSYM bundles based on the UUID -# embedded in a binary, and because the UUID is a hash of filename -# and .text section, there *will* be conflicts inside $BUILD_DIR. -if platform.system() == "Darwin": -with open(os.path.join(build_dir, '.metadata_never_index'), 'w+'): -pass - target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2] checkLibcxxSupport() Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py?rev=324115&r1=324114&r2=324115&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py Fri Feb 2 10:32:29 2018 @@ -163,7 +163,7 @@ def create_parser(): '--build-dir', dest='test_build_dir', metavar='Test build directory', -default='lldb-test-build', +default='lldb-test-build.noindex', help='The root build directory for the tests. It will be removed before running.') # Configuration options Modified: lldb/trunk/test/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/CMakeLists.txt?rev=324115&r1=324114&r2=324115&view=diff == --- lldb/trunk/test/CMakeLists.txt (original) +++ lldb/trunk/test/CMakeLists.txt Fri Feb 2 10:32:29 2018 @@ -57,13 +57,18 @@ set(LLDB_TEST_USER_ARGS "" CACHE STRING "Specify additional arguments to pass to test runner. For example: '-C gcc -C clang -A i386 -A x86_64'") +# The .nodindex suffix is a marker for Spotlight to never index the +# build directory. LLDB queries Spotlight to locate .dSYM bundles +# based on the UUID embedded in a binary, and because the UUID is a +# hash of filename and .text section, there *will* be conflicts inside +# the build directory. set(LLDB_TEST_COMMON_ARGS --arch=${LLDB_TEST_ARCH} --executable $ -s ${CMAKE_BINARY_DIR}/lldb-test-traces --build-dir - ${CMAKE_BINARY_DIR}/lldb-test-build + ${CMAKE_BINARY_DIR}/lldb-test-build.noindex -S nm -u CXXFLAGS -u CFLAGS ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r324119 - Add the ability to restrict the breakpoint to a module
Author: jingham Date: Fri Feb 2 10:39:25 2018 New Revision: 324119 URL: http://llvm.org/viewvc/llvm-project?rev=324119&view=rev Log: Add the ability to restrict the breakpoint to a module for run_to_{source,name}_breakpoint. Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py?rev=324119&r1=324118&r2=324119&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py Fri Feb 2 10:39:25 2018 @@ -769,9 +769,10 @@ def run_to_breakpoint_do_run(test, targe def run_to_name_breakpoint (test, bkpt_name, launch_info = None, exe_name = "a.out", +bkpt_module = None, in_cwd = True): """Start up a target, using exe_name as the executable, and run it to - a breakpoint set by name on bkpt_name. + a breakpoint set by name on bkpt_name restricted to bkpt_module. If you want to pass in launch arguments or environment variables, you can optionally pass in an SBLaunchInfo. If you @@ -781,22 +782,30 @@ def run_to_name_breakpoint (test, bkpt_n And if your executable isn't in the CWD, pass in the absolute path to the executable in exe_name, and set in_cwd to False. + If you need to restrict the breakpoint to a particular module, + pass the module name (a string not a FileSpec) in bkpt_module. If + nothing is passed in setting will be unrestricted. + If the target isn't valid, the breakpoint isn't found, or hit, the function will cause a testsuite failure. If successful it returns a tuple with the target process and - thread that hit the breakpoint. + thread that hit the breakpoint, and the breakpoint that we set + for you. """ target = run_to_breakpoint_make_target(test, exe_name, in_cwd) -breakpoint = target.BreakpointCreateByName(bkpt_name) +breakpoint = target.BreakpointCreateByName(bkpt_name, bkpt_module) + + test.assertTrue(breakpoint.GetNumLocations() > 0, "No locations found for name breakpoint: '%s'."%(bkpt_name)) return run_to_breakpoint_do_run(test, target, breakpoint, launch_info) def run_to_source_breakpoint(test, bkpt_pattern, source_spec, launch_info = None, exe_name = "a.out", + bkpt_module = None, in_cwd = True): """Start up a target, using exe_name as the executable, and run it to a breakpoint set by source regex bkpt_pattern. @@ -807,7 +816,7 @@ def run_to_source_breakpoint(test, bkpt_ target = run_to_breakpoint_make_target(test, exe_name, in_cwd) # Set the breakpoints breakpoint = target.BreakpointCreateBySourceRegex( -bkpt_pattern, source_spec) +bkpt_pattern, source_spec, bkpt_module) test.assertTrue(breakpoint.GetNumLocations() > 0, 'No locations found for source breakpoint: "%s", file: "%s", dir: "%s"'%(bkpt_pattern, source_spec.GetFilename(), source_spec.GetDirectory())) return run_to_breakpoint_do_run(test, target, breakpoint, launch_info) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42853: Resolve binary symlinks before finding its separate .debug file
jingham added a comment. Can you fix the test case as mentioned inline. If something doesn't work please just ping me and I'll fix it. I'd like to get off propagating this boiler-plate... Comment at: packages/Python/lldbsuite/test/linux/sepdebugsymlink/TestSymbolsSepDebugSymlink.py:23-37 +exe = self.getBuildArtifact("dirsymlink/stripped.symlink") + +self.target = self.dbg.CreateTarget(exe) +self.assertTrue(self.target, VALID_TARGET) + +main_bp = self.target.BreakpointCreateByName("main", "stripped.symlink") +self.assertTrue(main_bp, VALID_BREAKPOINT) You should be able to do all this with the run_to_name_breakpoint. Pass "dirsymlink/stripped.symlink" as exe_name. I just added the ability to limit the breakpoint to a particular module, so you can do that as well. Having all this duplicated boiler-plate made Adrian's laudable job of building out of tree way more tedious than it should have been. We should centralize this whenever possible. https://reviews.llvm.org/D42853 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42836: [dotest] make debug info variant accessible in setUp()
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Great. The important thing for me was that a new object is constructed for each permutation of test function / variant. https://reviews.llvm.org/D42836 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions
owenpshaw updated this revision to Diff 132641. owenpshaw added a comment. - adjust WriteObjectFile signature to return Status and take an std::vector - match project formatting style I toyed with adding allow_flash to as an argument, but didn't really like it because it seemed to bring things back to basically allowing arbitrary flash writes via WriteMemory, and would affect all the process subclasses when only one (gdb) actually needs it. I still like the member flag because it keeps the flash writing very private to ProcessGDB. What can I do to get the xml dependency check in the test? Not clear on who I should talk to or where to start. https://reviews.llvm.org/D42145 Files: include/lldb/Host/XML.h include/lldb/Target/MemoryRegionInfo.h include/lldb/Target/Process.h packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteLoad.py source/Host/common/XML.cpp source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp source/Plugins/ObjectFile/ELF/ObjectFileELF.h source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp source/Plugins/Process/gdb-remote/ProcessGDBRemote.h source/Symbol/ObjectFile.cpp source/Target/Process.cpp Index: source/Target/Process.cpp === --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -2533,6 +2533,17 @@ return 0; } +Status Process::WriteObjectFile(std::vector entries) { + Status error; + for (const auto &Entry : entries) { +WriteMemory(Entry.Dest, Entry.Contents.data(), Entry.Contents.size(), +error); +if (!error.Success()) + break; + } + return error; +} + #define USE_ALLOCATE_MEMORY_CACHE 1 addr_t Process::AllocateMemory(size_t size, uint32_t permissions, Status &error) { Index: source/Symbol/ObjectFile.cpp === --- source/Symbol/ObjectFile.cpp +++ source/Symbol/ObjectFile.cpp @@ -659,22 +659,31 @@ SectionList *section_list = GetSectionList(); if (!section_list) return Status("No section in object file"); + + std::vector writeEntries; + + // Create a list of write entries from loadable sections size_t section_count = section_list->GetNumSections(0); for (size_t i = 0; i < section_count; ++i) { +Process::WriteEntry entry; SectionSP section_sp = section_list->GetSectionAtIndex(i); -addr_t addr = target.GetSectionLoadList().GetSectionLoadAddress(section_sp); -if (addr != LLDB_INVALID_ADDRESS) { - DataExtractor section_data; - // We can skip sections like bss - if (section_sp->GetFileSize() == 0) -continue; - section_sp->GetSectionData(section_data); - lldb::offset_t written = process->WriteMemory( - addr, section_data.GetDataStart(), section_data.GetByteSize(), error); - if (written != section_data.GetByteSize()) -return error; -} +entry.Dest = target.GetSectionLoadList().GetSectionLoadAddress(section_sp); +if (entry.Dest == LLDB_INVALID_ADDRESS) + continue; +// We can skip sections like bss +if (section_sp->GetFileSize() == 0) + continue; +DataExtractor section_data; +section_sp->GetSectionData(section_data); +entry.Contents = llvm::ArrayRef(section_data.GetDataStart(), + section_data.GetByteSize()); +writeEntries.push_back(entry); } + + error = process->WriteObjectFile(std::move(writeEntries)); + if (!error.Success()) +return error; + if (set_pc) { ThreadList &thread_list = process->GetThreadList(); ThreadSP curr_thread(thread_list.GetSelectedThread()); Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h === --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -144,6 +144,8 @@ size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size, Status &error) override; + Status WriteObjectFile(std::vector entries) override; + size_t DoWriteMemory(lldb::addr_t addr, const void *buf, size_t size, Status &error) override; @@ -302,6 +304,11 @@ int64_t m_breakpoint_pc_offset; lldb::tid_t m_initial_tid; // The initial thread ID, given by stub on attach + bool m_allow_flash_writes; + using FlashRangeVector = lldb_private::RangeVector; + using FlashRange = FlashRangeVector::Entry; + FlashRangeVector m_erased_flash_ranges; + //-- // Accessors //-- @@ -408,6 +415,12 @@ Status UpdateAutomaticSignalFiltering() override; + Status FlashErase(lldb::addr_t
[Lldb-commits] [PATCH] D42852: Fix upper->lower case for /usr/lib/debug/.build-id/**.debug
jankratochvil updated this revision to Diff 132668. jankratochvil added a comment. Simplify the testcase using: lldbutil.run_to_name_breakpoint https://reviews.llvm.org/D42852 Files: packages/Python/lldbsuite/test/linux/buildidcase/Makefile packages/Python/lldbsuite/test/linux/buildidcase/TestTargetSymbolsBuildidCase.py packages/Python/lldbsuite/test/linux/buildidcase/main.c source/Host/common/Symbols.cpp Index: source/Host/common/Symbols.cpp === --- source/Host/common/Symbols.cpp +++ source/Host/common/Symbols.cpp @@ -245,6 +245,8 @@ // Some debug files are stored in the .build-id directory like this: // /usr/lib/debug/.build-id/ff/e7fe727889ad82bb153de2ad065b2189693315.debug uuid_str = module_uuid.GetAsString(""); + std::transform(uuid_str.begin(), uuid_str.end(), uuid_str.begin(), + ::tolower); uuid_str.insert(2, 1, '/'); uuid_str = uuid_str + ".debug"; } Index: packages/Python/lldbsuite/test/linux/buildidcase/main.c === --- /dev/null +++ packages/Python/lldbsuite/test/linux/buildidcase/main.c @@ -0,0 +1,3 @@ +int main() { + return 0; +} Index: packages/Python/lldbsuite/test/linux/buildidcase/TestTargetSymbolsBuildidCase.py === --- /dev/null +++ packages/Python/lldbsuite/test/linux/buildidcase/TestTargetSymbolsBuildidCase.py @@ -0,0 +1,25 @@ +""" Testing separate debug info loading by its .build-id. """ +import os +import time +import lldb +import sys +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestTargetSymbolsBuildidCase(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +def setUp(self): +TestBase.setUp(self) +self.source = 'main.c' + +@no_debug_info_test # Prevent the genaration of the dwarf version of this test +@skipUnlessPlatform(['linux']) +def test_target_symbols_buildid_case(self): +self.build(clean=True) +exe = self.getBuildArtifact("stripped.out") + +lldbutil.run_to_name_breakpoint(self, "main", exe_name = exe) Index: packages/Python/lldbsuite/test/linux/buildidcase/Makefile === --- /dev/null +++ packages/Python/lldbsuite/test/linux/buildidcase/Makefile @@ -0,0 +1,20 @@ +LEVEL = ../../make +C_SOURCES := main.c +LD_EXTRAS += -Wl,--build-id=sha1 + +all: stripped.out + +.PHONY: .build-id +stripped.out .build-id: a.out + $(OBJCOPY) -j .note.gnu.build-id -O binary $< tmp + rm -rf .build-id + fn=`od -An -tx1 Index: source/Host/common/Symbols.cpp === --- source/Host/common/Symbols.cpp +++ source/Host/common/Symbols.cpp @@ -245,6 +245,8 @@ // Some debug files are stored in the .build-id directory like this: // /usr/lib/debug/.build-id/ff/e7fe727889ad82bb153de2ad065b2189693315.debug uuid_str = module_uuid.GetAsString(""); + std::transform(uuid_str.begin(), uuid_str.end(), uuid_str.begin(), + ::tolower); uuid_str.insert(2, 1, '/'); uuid_str = uuid_str + ".debug"; } Index: packages/Python/lldbsuite/test/linux/buildidcase/main.c === --- /dev/null +++ packages/Python/lldbsuite/test/linux/buildidcase/main.c @@ -0,0 +1,3 @@ +int main() { + return 0; +} Index: packages/Python/lldbsuite/test/linux/buildidcase/TestTargetSymbolsBuildidCase.py === --- /dev/null +++ packages/Python/lldbsuite/test/linux/buildidcase/TestTargetSymbolsBuildidCase.py @@ -0,0 +1,25 @@ +""" Testing separate debug info loading by its .build-id. """ +import os +import time +import lldb +import sys +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestTargetSymbolsBuildidCase(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +def setUp(self): +TestBase.setUp(self) +self.source = 'main.c' + +@no_debug_info_test # Prevent the genaration of the dwarf version of this test +@skipUnlessPlatform(['linux']) +def test_target_symbols_buildid_case(self): +self.build(clean=True) +exe = self.getBuildArtifact("stripped.out") + +lldbutil.run_to_name_breakpoint(self, "main", exe_name = exe) Index: packages/Python/lldbsuite/test/linux/buildidcase/Makefile === --- /dev/null +++ packages/Python/lldbsuite/test/linux/buildidcase/Makefile @@ -0,0 +1,20 @@ +LEVEL = ../../make +C_SOURCES := main.c +LD_EXTRAS += -Wl,--build-id=sha1 + +all: stripped.out + +.PHONY: .build-id +stripped.out .build-id: a.out + $(OBJCOPY) -j .n
[Lldb-commits] [PATCH] D42853: Resolve binary symlinks before finding its separate .debug file
jankratochvil updated this revision to Diff 132670. jankratochvil marked an inline comment as done. jankratochvil added a comment. Simplify the testcase by: lldbutil.run_to_name_breakpoint https://reviews.llvm.org/D42853 Files: packages/Python/lldbsuite/test/linux/sepdebugsymlink/Makefile packages/Python/lldbsuite/test/linux/sepdebugsymlink/TestTargetSymbolsSepDebugSymlink.py packages/Python/lldbsuite/test/linux/sepdebugsymlink/main.c source/Host/common/Symbols.cpp Index: source/Host/common/Symbols.cpp === --- source/Host/common/Symbols.cpp +++ source/Host/common/Symbols.cpp @@ -9,6 +9,7 @@ #include "lldb/Host/Symbols.h" #include "lldb/Core/ModuleSpec.h" +#include "lldb/Host/FileSystem.h" #include "lldb/Symbol/ObjectFile.h" #include "lldb/Target/Target.h" #include "lldb/Utility/ArchSpec.h" @@ -221,7 +222,11 @@ Target::GetDefaultDebugFileSearchPaths()); // Add module directory. -const ConstString &file_dir = module_spec.GetFileSpec().GetDirectory(); +FileSpec module_file_spec = module_spec.GetFileSpec(); +// We keep the unresolved pathname if it fails. +FileSystem::ResolveSymbolicLink(module_file_spec, module_file_spec); + +const ConstString &file_dir = module_file_spec.GetDirectory(); debug_file_search_paths.AppendIfUnique( FileSpec(file_dir.AsCString("."), true)); @@ -276,7 +281,7 @@ FileSpec file_spec(filename, true); if (llvm::sys::fs::equivalent(file_spec.GetPath(), - module_spec.GetFileSpec().GetPath())) + module_file_spec.GetPath())) continue; if (file_spec.Exists()) { Index: packages/Python/lldbsuite/test/linux/sepdebugsymlink/main.c === --- /dev/null +++ packages/Python/lldbsuite/test/linux/sepdebugsymlink/main.c @@ -0,0 +1,3 @@ +int main() { + return 0; +} Index: packages/Python/lldbsuite/test/linux/sepdebugsymlink/TestTargetSymbolsSepDebugSymlink.py === --- /dev/null +++ packages/Python/lldbsuite/test/linux/sepdebugsymlink/TestTargetSymbolsSepDebugSymlink.py @@ -0,0 +1,25 @@ +""" Testing separate debug info loading for base binary with a symlink. """ +import os +import time +import lldb +import sys +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestTargetSymbolsSepDebugSymlink(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +def setUp(self): +TestBase.setUp(self) +self.source = 'main.c' + +@no_debug_info_test # Prevent the genaration of the dwarf version of this test +@skipUnlessPlatform(['linux']) +def test_target_symbols_sepdebug_symlink_case(self): +self.build(clean=True) +exe = self.getBuildArtifact("dirsymlink/stripped.symlink") + +lldbutil.run_to_name_breakpoint(self, "main", exe_name = exe) Index: packages/Python/lldbsuite/test/linux/sepdebugsymlink/Makefile === --- /dev/null +++ packages/Python/lldbsuite/test/linux/sepdebugsymlink/Makefile @@ -0,0 +1,20 @@ +LEVEL = ../../make +C_SOURCES := main.c + +all: dirsymlink + +dirreal: a.out + $(RM) -r $@ + mkdir $@ + $(OBJCOPY) --only-keep-debug $< $@/stripped.debug + $(OBJCOPY) --strip-all --add-gnu-debuglink=$@/stripped.debug $< $@/stripped.out + +dirsymlink: dirreal + $(RM) -r $@ + mkdir $@ + ln -s ../$Index: source/Host/common/Symbols.cpp === --- source/Host/common/Symbols.cpp +++ source/Host/common/Symbols.cpp @@ -9,6 +9,7 @@ #include "lldb/Host/Symbols.h" #include "lldb/Core/ModuleSpec.h" +#include "lldb/Host/FileSystem.h" #include "lldb/Symbol/ObjectFile.h" #include "lldb/Target/Target.h" #include "lldb/Utility/ArchSpec.h" @@ -221,7 +222,11 @@ Target::GetDefaultDebugFileSearchPaths()); // Add module directory. -const ConstString &file_dir = module_spec.GetFileSpec().GetDirectory(); +FileSpec module_file_spec = module_spec.GetFileSpec(); +// We keep the unresolved pathname if it fails. +FileSystem::ResolveSymbolicLink(module_file_spec, module_file_spec); + +const ConstString &file_dir = module_file_spec.GetDirectory(); debug_file_search_paths.AppendIfUnique( FileSpec(file_dir.AsCString("."), true)); @@ -276,7 +281,7 @@ FileSpec file_spec(filename, true); if (llvm::sys::fs::equivalent(file_spec.GetPath(), - module_spec.GetFileSpec().GetPath())) + module_file_spec.GetPath())) continue; if (file_spec.Exists()) { Index: packages/Python/lldbsuite/test/linux/sepdebugsymlink/main.c
[Lldb-commits] [PATCH] D42868: Fix a crash in *NetBSD::Factory::Launch
krytarowski created this revision. krytarowski added reviewers: labath, joerg. Herald added a subscriber: llvm-commits. krytarowski edited the summary of this revision. We cannot call process_up->SetState() inside the NativeProcessNetBSD::Factory::Launch function because it triggers a NULL pointer deference. The generic code for launching a process in: GDBRemoteCommunicationServerLLGS::LaunchProcess sets the m_debugged_process_up pointer after a successful call to m_process_factory.Launch(). If we attempt to call process_up->SetState() inside a platform specific Launch function we end up dereferencing a NULL pointer in NativeProcessProtocol::GetCurrentThreadID(). Follow the Linux case and move SetState() to: NativeProcessNetBSD::NativeProcessNetBSD. Sponsored by Repository: rL LLVM https://reviews.llvm.org/D42868 Files: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp Index: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp === --- source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp +++ source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp @@ -113,7 +113,6 @@ for (const auto &thread : process_up->m_threads) static_cast(*thread).SetStoppedBySignal(SIGSTOP); - process_up->SetState(StateType::eStateStopped); return std::move(process_up); } @@ -160,6 +159,8 @@ m_sigchld_handle = mainloop.RegisterSignal( SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, status); assert(m_sigchld_handle && status.Success()); + + SetState(StateType::eStateStopped, false); } // Handles all waitpid events from the inferior process. Index: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp === --- source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp +++ source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp @@ -113,7 +113,6 @@ for (const auto &thread : process_up->m_threads) static_cast(*thread).SetStoppedBySignal(SIGSTOP); - process_up->SetState(StateType::eStateStopped); return std::move(process_up); } @@ -160,6 +159,8 @@ m_sigchld_handle = mainloop.RegisterSignal( SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, status); assert(m_sigchld_handle && status.Success()); + + SetState(StateType::eStateStopped, false); } // Handles all waitpid events from the inferior process. ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42868: Fix a crash in *NetBSD::Factory::Launch
krytarowski added a comment. (gdb) r Starting program: /public/llvm-build/bin/lldb-server g \*:1234 -- /usr/bin/look [New process 15150] Thread 1 received signal SIGSEGV, Segmentation fault. lldb_private::NativeProcessProtocol::GetCurrentThreadID (this=0x0) at /public/llvm/tools/lldb/include/lldb/Host/common/NativeProcessProtocol.h:178 178 lldb::tid_t GetCurrentThreadID() { return m_current_thread_id; } (gdb) bt #0 lldb_private::NativeProcessProtocol::GetCurrentThreadID (this=0x0) at /public/llvm/tools/lldb/include/lldb/Host/common/NativeProcessProtocol.h:178 #1 0x00600fab in lldb_private::process_gdb_remote::GDBRemoteCommunicationServerLLGS::SendStopReasonForState (this=0x7f7fddc0, process_state=lldb::eStateStopped) at /public/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1641 #2 0x0060135a in lldb_private::process_gdb_remote::GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped (this=0x7f7fddc0, process=0x7f7ff7b7c000) at /public/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:835 #3 0x006014d1 in lldb_private::process_gdb_remote::GDBRemoteCommunicationServerLLGS::ProcessStateChanged (this=0x7f7fddc0, process=0x7f7ff7b7c000, state=lldb::eStateStopped) at /public/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:870 #4 0x005412e1 in lldb_private::NativeProcessProtocol::SynchronouslyNotifyProcessStateChanged (this=0x7f7ff7b7c000, state=lldb::eStateStopped) at /public/llvm/tools/lldb/source/Host/common/NativeProcessProtocol.cpp:336 #5 0x00542e5f in lldb_private::NativeProcessProtocol::SetState (this=0x7f7ff7b7c000, state=lldb::eStateStopped, notify_delegates=true) at /public/llvm/tools/lldb/source/Host/common/NativeProcessProtocol.cpp:422 #6 0x0059fc77 in lldb_private::process_netbsd::NativeProcessNetBSD::Factory::Launch (this=0x7f7fe3a8, launch_info=..., native_delegate=..., mainloop=...) at /public/llvm/tools/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:114 #7 0x005fd82a in lldb_private::process_gdb_remote::GDBRemoteCommunicationServerLLGS::LaunchProcess (this=0x7f7fddc0) at /public/llvm/tools/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:233 #8 0x00418e16 in handle_launch (gdb_server=..., argc=1, argv=0x7f7fe6f8) at /public/llvm/tools/lldb/tools/lldb-server/lldb-gdbserver.cpp:195 #9 0x0041a228 in main_gdbserver (argc=1, argv=0x7f7fe6f8) at /public/llvm/tools/lldb/tools/lldb-server/lldb-gdbserver.cpp:525 #10 0x0041befd in main (argc=5, argv=0x7f7fe6d8) at /public/llvm/tools/lldb/tools/lldb-server/lldb-server.cpp:58 (gdb) Repository: rL LLVM https://reviews.llvm.org/D42868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images
krytarowski added a comment. Extracted from: https://reviews.llvm.org/D32149. Repository: rL LLVM https://reviews.llvm.org/D42870 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images
krytarowski created this revision. krytarowski added reviewers: joerg, labath. Herald added subscribers: llvm-commits, emaste. Split the recognition into NetBSD executables & shared libraries and core(5) files. Introduce new owner type: "NetBSD-CORE", as core(5) files are not tagged in the same way as regular NetBSD executables. Stop using incorrectly ABI_TAG and ABI_SIZE. Introduce IDENT_TAG, IDENT_DECSZ, IDENT_NAMESZ and PROCINFO. The new values detects correctly the NetBSD images. Sponsored by Repository: rL LLVM https://reviews.llvm.org/D42870 Files: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp === --- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -53,6 +53,7 @@ const char *const LLDB_NT_OWNER_FREEBSD = "FreeBSD"; const char *const LLDB_NT_OWNER_GNU = "GNU"; const char *const LLDB_NT_OWNER_NETBSD = "NetBSD"; +const char *const LLDB_NT_OWNER_NETBSDCORE = "NetBSD-CORE"; const char *const LLDB_NT_OWNER_OPENBSD = "OpenBSD"; const char *const LLDB_NT_OWNER_CSR = "csr"; const char *const LLDB_NT_OWNER_ANDROID = "Android"; @@ -68,8 +69,10 @@ const elf_word LLDB_NT_GNU_BUILD_ID_TAG = 0x03; -const elf_word LLDB_NT_NETBSD_ABI_TAG = 0x01; -const elf_word LLDB_NT_NETBSD_ABI_SIZE = 4; +const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_TAG = 1; +const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_DESCSZ = 4; +const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_NAMESZ = 7; +const elf_word LLDB_NT_NETBSD_NT_PROCINFO = 1; // GNU ABI note OS constants const elf_word LLDB_NT_GNU_ABI_OS_LINUX = 0x00; @@ -1335,25 +1338,41 @@ // The note.n_name == LLDB_NT_OWNER_GNU is valid for Linux platform arch_spec.GetTriple().setOS(llvm::Triple::OSType::Linux); } -// Process NetBSD ELF notes. +// Process NetBSD ELF executables and shared libraries else if ((note.n_name == LLDB_NT_OWNER_NETBSD) && - (note.n_type == LLDB_NT_NETBSD_ABI_TAG) && - (note.n_descsz == LLDB_NT_NETBSD_ABI_SIZE)) { - // Pull out the min version info. + (note.n_type == LLDB_NT_NETBSD_NT_NETBSD_IDENT_TAG) && + (note.n_descsz == LLDB_NT_NETBSD_NT_NETBSD_IDENT_DESCSZ) && + (note.n_namesz == LLDB_NT_NETBSD_NT_NETBSD_IDENT_NAMESZ)) { + // Pull out the version info. uint32_t version_info; if (data.GetU32(&offset, &version_info, 1) == nullptr) { error.SetErrorString("failed to read NetBSD ABI note payload"); return error; } - + // Convert the version info into a major/minor/patch number. + // #define __NetBSD_Version__ MMmmrrpp00 + // + // M = major version + // m = minor version; a minor number of 99 indicates current. + // r = 0 (since NetBSD 3.0 not used) + // p = patchlevel + const uint32_t version_major = version_info / 1; + const uint32_t version_minor = (version_info % 1) / 100; + const uint32_t version_patch = (version_info % 1) / 100; + char os_name[32]; + snprintf(os_name, sizeof(os_name), + "netbsd%" PRIu32 ".%" PRIu32 ".%" PRIu32, version_major, + version_minor, version_patch); + // Set the elf OS version to NetBSD. Also clear the vendor. + arch_spec.GetTriple().setOSName(os_name); + arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor); +} +// Process NetBSD ELF core(5) notes +else if ((note.n_name == LLDB_NT_OWNER_NETBSDCORE) && + (note.n_type == LLDB_NT_NETBSD_NT_PROCINFO)) { // Set the elf OS version to NetBSD. Also clear the vendor. arch_spec.GetTriple().setOS(llvm::Triple::OSType::NetBSD); arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor); - - if (log) -log->Printf( -"ObjectFileELF::%s detected NetBSD, min version constant %" PRIu32, -__FUNCTION__, version_info); } // Process OpenBSD ELF notes. else if (note.n_name == LLDB_NT_OWNER_OPENBSD) { Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp === --- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -53,6 +53,7 @@ const char *const LLDB_NT_OWNER_FREEBSD = "FreeBSD"; const char *const LLDB_NT_OWNER_GNU = "GNU"; const char *const LLDB_NT_OWNER_NETBSD = "NetBSD"; +const char *const LLDB_NT_OWNER_NETBSDCORE = "NetBSD-CORE"; const char *const LLDB_NT_OWNER_OPENBSD = "OpenBSD"; const char *const LLDB_NT_OWNER_CSR = "csr"; const char *const LLDB_NT_OWNER_ANDROID = "Android"; @@ -68,8 +69,10 @@ const elf_word LLDB_NT_GNU_BUILD_ID_TAG = 0x03; -const elf_word LLDB_NT_NETBSD_ABI_TAG = 0x01; -const elf_word LLDB_NT_NETBSD_ABI_SIZE = 4; +const elf_word LLDB_NT_NETBSD_NT
[Lldb-commits] [PATCH] D42868: Fix a crash in *NetBSD::Factory::Launch
krytarowski added inline comments. Comment at: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:116 static_cast(*thread).SetStoppedBySignal(SIGSTOP); - process_up->SetState(StateType::eStateStopped); Another option is to call: ``` process_up->SetState(StateType::eStateStopped, false); ``` Both work. Repository: rL LLVM https://reviews.llvm.org/D42868 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r324156 - Fix a copy of a fixed length, possibly non-nul terminated, string
Author: jmolenda Date: Fri Feb 2 14:48:45 2018 New Revision: 324156 URL: http://llvm.org/viewvc/llvm-project?rev=324156&view=rev Log: Fix a copy of a fixed length, possibly non-nul terminated, string into a std::string so we don't run off the end of the array when there is no nul byte in ProcessElfCore::parseLinuxNotes. Found with ASAN testing. Differential revision: https://reviews.llvm.org/D42828 Modified: lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp Modified: lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp?rev=324156&r1=324155&r2=324156&view=diff == --- lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp (original) +++ lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp Fri Feb 2 14:48:45 2018 @@ -665,7 +665,7 @@ llvm::Error ProcessElfCore::parseLinuxNo Status status = prpsinfo.Parse(note.data, arch); if (status.Fail()) return status.ToError(); - thread_data.name = prpsinfo.pr_fname; + thread_data.name.assign (prpsinfo.pr_fname, strnlen (prpsinfo.pr_fname, sizeof (prpsinfo.pr_fname))); SetID(prpsinfo.pr_pid); break; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42828: Fix for read-past-end-of-array buglet in ProcessElfCore.cpp while reading linux notes
This revision was automatically updated to reflect the committed changes. Closed by commit rL324156: Fix a copy of a fixed length, possibly non-nul terminated, string (authored by jmolenda, committed by ). Changed prior to commit: https://reviews.llvm.org/D42828?vs=132500&id=132688#toc Repository: rL LLVM https://reviews.llvm.org/D42828 Files: lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp Index: lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp === --- lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -665,7 +665,7 @@ Status status = prpsinfo.Parse(note.data, arch); if (status.Fail()) return status.ToError(); - thread_data.name = prpsinfo.pr_fname; + thread_data.name.assign (prpsinfo.pr_fname, strnlen (prpsinfo.pr_fname, sizeof (prpsinfo.pr_fname))); SetID(prpsinfo.pr_pid); break; } Index: lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp === --- lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -665,7 +665,7 @@ Status status = prpsinfo.Parse(note.data, arch); if (status.Fail()) return status.ToError(); - thread_data.name = prpsinfo.pr_fname; + thread_data.name.assign (prpsinfo.pr_fname, strnlen (prpsinfo.pr_fname, sizeof (prpsinfo.pr_fname))); SetID(prpsinfo.pr_pid); break; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42868: Fix a crash in *NetBSD::Factory::Launch
krytarowski updated this revision to Diff 132689. Repository: rL LLVM https://reviews.llvm.org/D42868 Files: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp Index: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp === --- source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp +++ source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp @@ -113,7 +113,7 @@ for (const auto &thread : process_up->m_threads) static_cast(*thread).SetStoppedBySignal(SIGSTOP); - process_up->SetState(StateType::eStateStopped); + process_up->SetState(StateType::eStateStopped, false); return std::move(process_up); } Index: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp === --- source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp +++ source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp @@ -113,7 +113,7 @@ for (const auto &thread : process_up->m_threads) static_cast(*thread).SetStoppedBySignal(SIGSTOP); - process_up->SetState(StateType::eStateStopped); + process_up->SetState(StateType::eStateStopped, false); return std::move(process_up); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images
davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. Please add a test case. Repository: rL LLVM https://reviews.llvm.org/D42870 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images
krytarowski added a comment. What would the test do? Repository: rL LLVM https://reviews.llvm.org/D42870 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images
clayborg added a comment. Probably take a ELF file that is NetBSD and obj2yaml it. The test would run yaml2obj on it and then test that things are recognized correctly via the SB interfaces (check triple is correct)? Repository: rL LLVM https://reviews.llvm.org/D42870 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images
clayborg added a comment. You would check in the YAML code for the NetBSD ELF file so that the test case can make it into an ELF file, run the test, verify things, and then cleanup the created ELF file. Repository: rL LLVM https://reviews.llvm.org/D42870 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images
davide added a comment. In https://reviews.llvm.org/D42870#996899, @clayborg wrote: > Probably take a ELF file that is NetBSD and obj2yaml it. The test would run > yaml2obj on it and then test that things are recognized correctly via the SB > interfaces (check triple is correct)? The SBApi in this case is a sledgehammer. I think `lldb-test` or an unitest would be a better/lightweight alternative. Repository: rL LLVM https://reviews.llvm.org/D42870 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images
krytarowski added a comment. Is there a working example of this? I would clone an existing code for Linux or other supported OS and adapt it for NetBSD. Please note that I'm in the process of restoration LLDB (lldb-server) so I cannot execute regular tests, at least in close time. Repository: rL LLVM https://reviews.llvm.org/D42870 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images
davide added a subscriber: zturner. davide added a comment. In https://reviews.llvm.org/D42870#996913, @krytarowski wrote: > Is there a working example of this? I would clone an existing code for Linux > or other supported OS and adapt it for NetBSD. > > Please note that I'm in the process of restoration LLDB (lldb-server) so I > cannot execute regular tests, at least in close time. cc: @zturner / @labath Repository: rL LLVM https://reviews.llvm.org/D42870 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images
jingham added a comment. There is code in the GDBRemoteTestBase that makes targets from yaml2obj. See the createTarget method in packages/Python/lldbsuite/functionalities/gdb_remote_client/gdbclientutils.py. Actually, that's method should be pulled out of the gdb_remote_client and moved to TestBase in lldbtest.py, it doesn't appear to have any dependencies on being a gdb_remote_client test, and it seems like a generally useful feature. Note that gdb_remote_client directory also has an example of making a little ELF file (a.yaml). It looks like the GDBRemoteTestBase class also has some mechanism for cleaning up temporary files, but I'm not sure that's necessary, since the generated files should all go into the build directory now Adrian is done with that work. I don't see why the test cases should have to do this by hand. Anyway, if that were in lldbtest (maybe createTargetFromYaml), then you could make a dotest test by copying the packages/Python/lldbsuite/test/sample_test/TestSampleTest.py somewhere, remove the build stage, and in the test body just do something like: target = self.createTargetFromYaml(path_to_yaml) self.assertTrue(target.GetTriple() =="whateverItsSupposedToBe", "Got the right target") You only need to use lldb-server to run the dotest.py based tests if you need to run the process, which you don't here. You should be able to run all the tests that don't run processes, though of course if you don't have an lldb-server you can't actually launch anything regardless of what test harness is doing the launching. Repository: rL LLVM https://reviews.llvm.org/D42870 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r324158 - Turn off the deprecated ALWAYS_SEARCH_USER_PATHS feature
Author: jmolenda Date: Fri Feb 2 16:37:46 2018 New Revision: 324158 URL: http://llvm.org/viewvc/llvm-project?rev=324158&view=rev Log: Turn off the deprecated ALWAYS_SEARCH_USER_PATHS feature in debugserver. This is already set this way in the lldb project files but not in debugserver. Updating for consistency. Modified: lldb/trunk/tools/debugserver/debugserver.xcodeproj/project.pbxproj Modified: lldb/trunk/tools/debugserver/debugserver.xcodeproj/project.pbxproj URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/debugserver.xcodeproj/project.pbxproj?rev=324158&r1=324157&r2=324158&view=diff == --- lldb/trunk/tools/debugserver/debugserver.xcodeproj/project.pbxproj (original) +++ lldb/trunk/tools/debugserver/debugserver.xcodeproj/project.pbxproj Fri Feb 2 16:37:46 2018 @@ -687,6 +687,7 @@ 1DEB914F08733D8E0010E9CD /* Debug */ = { isa = XCBuildConfiguration; buildSettings = { + ALWAYS_SEARCH_USER_PATHS = NO; "ARCHS[sdk=iphoneos*]" = ( arm64, armv7, @@ -731,6 +732,7 @@ 1DEB915008733D8E0010E9CD /* Release */ = { isa = XCBuildConfiguration; buildSettings = { + ALWAYS_SEARCH_USER_PATHS = NO; "ARCHS[sdk=iphoneos*]" = ( armv7, armv7s, @@ -775,6 +777,7 @@ 262419A11198A93E00067686 /* BuildAndIntegration */ = { isa = XCBuildConfiguration; buildSettings = { + ALWAYS_SEARCH_USER_PATHS = NO; "ARCHS[sdk=iphoneos*]" = arm64; "ARCHS[sdk=macosx*]" = "$(ARCHS_STANDARD_64_BIT)"; CLANG_WARN_BOOL_CONVERSION = YES; @@ -1436,6 +1439,7 @@ 4968B7A916657FAE00741ABB /* DebugClang */ = { isa = XCBuildConfiguration; buildSettings = { + ALWAYS_SEARCH_USER_PATHS = NO; "ARCHS[sdk=iphoneos*]" = ( arm64, armv7, @@ -1581,6 +1585,7 @@ 940AD5251B1FE3B10051E88F /* DebugPresubmission */ = { isa = XCBuildConfiguration; buildSettings = { + ALWAYS_SEARCH_USER_PATHS = NO; "ARCHS[sdk=iphoneos*]" = ( arm64, armv7, @@ -1810,6 +1815,7 @@ 94D72C871ADF10AA00A3F718 /* CustomSwift-Debug */ = { isa = XCBuildConfiguration; buildSettings = { + ALWAYS_SEARCH_USER_PATHS = NO; "ARCHS[sdk=iphoneos*]" = ( arm64, armv7, @@ -1946,6 +1952,7 @@ 94D72C891ADF10B000A3F718 /* CustomSwift-Release */ = { isa = XCBuildConfiguration; buildSettings = { + ALWAYS_SEARCH_USER_PATHS = NO; "ARCHS[sdk=iphoneos*]" = ( armv7, armv7s, ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D42210: Re-enable logging on the child side of a fork() in lldb-server platform mode
Hi Pavel, I closed this phabracator, I've been working on this on-and-off and I think a different approach would be better. I am working up a patch where I change tools/lldb-server/lldb-platform.cpp from using a fork() model to using std::thread's. The main thread listens for incoming connections, and when one is received, it starts up a new std::thread to handle that connection. I have a PortCoordinator singleton object that manages the ports we can use for communications. Newly created threads request ports from (& free them when the thread is finished) so that it would be possible to run multiple tests at the same time. The current code fork()'s when it receives a connection, and gives the child process a copy of all the ports that are available. Because it's in a separate process, if we were to receive a second connection and fork off a new process, it would have the same list of ports listed as available. When debugserver is being used, this is a problem - when the lldb-server platform thread/process gets a qLaunchGDBServer packet, we run debugserver saying "hey debugserver listen for a connection on port N" and then tell the remote lldb process "there's a debugserver waiting to hear from you on port N" - so lldb-server can't test whether port N is in use or not. (there was also a problem in GDBRemoteCommunication::StartDebugserverProcess where it has a url like localhost: and then it tries to run debugserver in --named-pipe mode even though we already have a port # to use in the url.) The final problem is that TCPSocket::Accept waits for an incoming connection on the assigned port #, and when it comes in it gets a new file descriptor for that connection. It should resume listening to that assigned port for the next connection that comes in ... but today TCPSocket::Accept calls CloseListenSockets() so after opening the first lldb-server platform connection that comes in, when we go to accept() the second, the socket has been closed and we exit immediately. To quickly recap the POSIX API (I never do network programming so I have to remind myself of this every time), the workflow for a server like this usually looks like parentfd = socket(AF_INET, SOCK_STREAM, 0); optval = 1; setsockopt(parentfd, SOL_SOCKET, SO_REUSEADDR, (const void *)&optval , sizeof(int)); serveraddr.sin_family = AF_INET; serveraddr.sin_addr.s_addr = htonl(INADDR_ANY); serveraddr.sin_port = htons((unsigned short)portno); bind(parentfd, (struct sockaddr *) &serveraddr, sizeof(serveraddr); listen(parentfd, 100); // allow 100 connections to queue up -- whatever childfd = accept(parentfd, (struct sockaddr *) &clientaddr, &clientlen); and then we go back to accept()'ing on parentfd after we've spun off something to talk to childfd. I've been doing all of my work on an old branch for reasons that are boring, so some/all of this may be fixed on top of tree already! But this is what I had to do to get my branch to work correctly on a remote system. I also noticed that dotest.py won't run multiple debug sessions with a remote target. That was part of my goal, to speed these up, but it seems to run in --no-multiprocess mode currently. I'll be picking this up next week - my changes right now are a mess, and they're against this old branch that I have to work on, but I'll get them cleaned up & updated to top of tree and make up a patchset. But I wanted to give you a heads up on where I'm headed as this touches a lot of your code. Thanks! J > On Jan 18, 2018, at 3:44 AM, Pavel Labath via Phabricator > wrote: > > labath added a comment. > > In https://reviews.llvm.org/D42210#979608, @jasonmolenda wrote: > >> Jim remembers some problem with logging across a fork()'ed process, Pavel >> does this ring a bell? This change might be completely bogus -- but it's >> very difficult to debug the child side of an lldb-server platform today. > > > I believe Jim is thinking of https://reviews.llvm.org/D38938. The issue is > that if another thread holds the logging mutex while we fork(), the mutex > will remain in a bogus state in the child process. This would mean that any > operation on the Log subsystem (such as enabling logging) could hang. We hold > the mutex for just a couple of instructions (just enough to copy a > shared_ptr), but after some code reshuffles, we were hitting this case very > often in liblldb. > > Now, I don't think this can happen here as at the point where we are forking, > the platform process is single-threaded. So, enabling logging here should be > safe, but only accidentally. It should be possible to make this always safe, > but that will need to be done with a very steady hand. My opinion is we > should not bother with that (i.e., just check this in as-is) until we > actually need more threads in the platform, particularly as I think the > long-term direction here should be to replace the fork with a new thread for > handling the