[Lldb-commits] [PATCH] D42443: [SymbolFilePDB] Add support for function symbols
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] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
labath added a comment.
I am not sure this actually creates enough separation. Plenty of tests have
more then one test method per file:
TestFoo.py:
class FooTestCase(TestBase):
NO_DEBUG_INFO_TESTCASE = True
def test_bar(self):
...
def test_baz(self):
...
If I understand correctly, you patch will run both `test_bar` and `test_baz` in
the same build folder (TestFoo.default). Shouldn't we separate those as well ?
(e.g. `TestFoo.test_bar`, `TestFoo.test_baz`) ?
The good thing about this is that you don't need to worry about debug info
variants in this code at all. You just take the test name, and the debug info
will be encoded in there (if I drop the NO_DEBUG_INFO_TESTCASE, the metaclass
will "magically" create test methods "test_bar_dwarf", "test_bar_dwo", ...)
(Technically that still leaves the possibility that a single file will have two
classes, and each of them will have the same method name, but no test currently
does that, so we can just disallow that).
Comment at: packages/Python/lldbsuite/test/lldbinline.py:96-98
+return "-N dwarf %s" % (testdir)
else:
+return "-N dsym %s" % (testdir)
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).
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] D42145: [lldb] Use vFlash commands when writing to target's flash memory regions
labath added a comment.
I think we're slowly getting there, but we could cleanup the implementation a
bit.
I am also not sure that the `WriteObjectFile` name really captures what this
function does, but I don't have a suggestion for a better name either
Comment at: include/lldb/Target/Process.h:559
+ struct WriteEntry{
+lldb::addr_t Dest;
Please run clang format over your diff.
Comment at: include/lldb/Target/Process.h:1958
+ virtual bool WriteObjectFile(llvm::ArrayRef entries,
+ Status &error);
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`).
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),
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)`).
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
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).
https://reviews.llvm.org/D42145
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r323953 - mock_gdb_server: rectify ack handling code
Author: labath
Date: Thu Feb 1 03:29:06 2018
New Revision: 323953
URL: http://llvm.org/viewvc/llvm-project?rev=323953&view=rev
Log:
mock_gdb_server: rectify ack handling code
The mock server was sending acks back in response to spurious acks from
the client, but the client was not prepared to handle these. Most of the
time this would work because the only time the client was sending
unsolicited acks is after the initial connection, and there reply-ack
would get ignored in the "flush all packets from the server" loop which
came after the ack. However, this loop had only a 10ms delay, and
sometimes this was not enough to catch the reply (which meant the
connection got out of sync, and test failed).
Since this behavior not consistent with how lldb-server handles this
situation (it just ignores the ack), I fix the mock server to do the
same.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py?rev=323953&r1=323952&r2=323953&view=diff
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
(original)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
Thu Feb 1 03:29:06 2018
@@ -200,7 +200,6 @@ class MockGDBServer:
_receivedData = None
_receivedDataOffset = None
_shouldSendAck = True
-_isExpectingAck = False
def __init__(self, port = 0):
self.responder = MockGDBServerResponder()
@@ -237,7 +236,6 @@ class MockGDBServer:
except:
return
self._shouldSendAck = True
-self._isExpectingAck = False
self._receivedData = ""
self._receivedDataOffset = 0
data = None
@@ -329,21 +327,15 @@ class MockGDBServer:
def _handlePacket(self, packet):
if packet is self.PACKET_ACK:
-# If we are expecting an ack, we'll just ignore it because there's
-# nothing else we're supposed to do.
-#
-# However, if we aren't expecting an ack, it's likely the initial
-# ack that lldb client sends, and observations of real servers
-# suggest we're supposed to ack back.
-if not self._isExpectingAck:
-self._client.sendall('+')
+# Ignore ACKs from the client. For the future, we can consider
+# adding validation code to make sure the client only sends ACKs
+# when it's supposed to.
return
response = ""
# We'll handle the ack stuff here since it's not something any of the
# tests will be concerned about, and it'll get turned off quicly
anyway.
if self._shouldSendAck:
self._client.sendall('+')
-self._isExpectingAck = True
if packet == "QStartNoAckMode":
self._shouldSendAck = False
response = "OK"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior
labath added inline comments. Comment at: lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py:335-337 +# However, if we aren't expecting an ack, it's likely the initial +# ack that lldb client sends, and observations of real servers +# suggest we're supposed to ack back. Which servers were you observing here? I tried lldb-server and gdbserver and they don't send the ack back (It seems like a bad idea anyway, as it could lead to an infinite ack ping-pong). I have removed this code as it was causing flakyness on the bots. If there is a server that does this, and we care about supporting it, then we can add a new targeted test which emulates this behavior, but then we need to also fix the client to handle this situation better. Repository: rL LLVM https://reviews.llvm.org/D42195 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r323974 - Extend windows->android XFAIL on TestLoadUnload
Author: labath Date: Thu Feb 1 07:35:55 2018 New Revision: 323974 URL: http://llvm.org/viewvc/llvm-project?rev=323974&view=rev Log: Extend windows->android XFAIL on TestLoadUnload This fails regardless of the android architecture or compiler used. The important bit is the mismatch in path separators. Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py?rev=323974&r1=323973&r2=323974&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py Thu Feb 1 07:35:55 2018 @@ -217,8 +217,6 @@ class LoadUnloadTestCase(TestBase): @expectedFailureAll( bugnumber="llvm.org/pr25805", hostoslist=["windows"], -compiler="gcc", -archs=["i386"], triple='.*-android') @skipIfFreeBSD # llvm.org/pr14424 - missing FreeBSD Makefiles/testcase support @skipIfWindows # Windows doesn't have dlopen and friends, dynamic libraries work differently ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D39307: Fix global data symbol resolution
clayborg added a comment. It would be nice if we can try and disable the redeclaration lookups from clang when in debugger mode. I can't remember if we already have such a flag in the compiler options. If we do, we should try to just disable this when in debug expression mode and see how the test suite fares. Unless we are able to lookup the name within the AST that is currently being built so we can re-find the exact same variable declaration and hand it back, disabling this seems to be the correct thing to do for debug expression. Comments? https://reviews.llvm.org/D39307 ___ 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
clayborg added a comment. In https://reviews.llvm.org/D42145#994556, @labath wrote: > I think we're slowly getting there, but we could cleanup the implementation a > bit. > > I am also not sure that the `WriteObjectFile` name really captures what this > function does, but I don't have a suggestion for a better name either Looking at the API I wouldn't immediately assume that WriteObjectFile would be flash aware and WriteMemory wouldn't... How about we add a new parameter to Process::WriteMemory that like "bool write_flash". It can default to false. Then if this is true, then we do the extra legwork to try and figure out the memory map. If false, just write the memory. https://reviews.llvm.org/D42145 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
aprantl added a comment.
> I am not sure this actually creates enough separation.
That's a good point. If I manage to extract the testname somehow via Python
reflection magic, I could also get rid of the unintuitive self.mydir tuple.
I'll give that a try.
Comment at:
packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py:39-40
(_COMP_DIR_SYM_LINK_PROP, pwd_symlink))
-lldbutil.run_break_set_by_file_and_line(self, self.src_path, self.line)
+src_path = self.getBuildArtifact(_SRC_FILE)
+lldbutil.run_break_set_by_file_and_line(self, src_path, self.line)
xiaobai wrote:
> Instead of making a temporary variable `src_path` that gets used once in each
> instance, why not just insert `self.getBuildArtifact(_SRC_FILE)` directly
> into `lldbutil.run_break_set_by_file_and_line()`?
While most tests have their sources in the source directory, some use generated
source file, which are located in the build directory, so that is unfortunately
not an option.
Comment at: packages/Python/lldbsuite/test/lldbinline.py:96-98
+return "-N dwarf %s" % (testdir)
else:
+return "-N dsym %s" % (testdir)
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`
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] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
aprantl updated this revision to Diff 132417.
aprantl added a comment.
Address review feedback from Alex.
https://reviews.llvm.org/D42763
Files:
packages/Python/lldbsuite/test/api/listeners/TestListener.py
packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py
packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_on_vectors/TestValueOfVectorVariable.py
packages/Python/lldbsuite/test/lldbinline.py
packages/Python/lldbsuite/test/lldbtest.py
packages/Python/lldbsuite/test/plugins/builder_base.py
packages/Python/lldbsuite/test/plugins/builder_darwin.py
packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py
packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
Index: packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
===
--- packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
+++ packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
@@ -20,16 +20,16 @@
def setUp(self):
# Call super's setUp().
TestBase.setUp(self)
-# Get the full path to our executable to be attached/debugged.
-self.exe = self.getBuildArtifact(self.testMethodName)
-self.d = {'EXE': self.testMethodName}
@add_test_categories(['pyapi'])
def test_with_process_launch_api(self):
"""Test SBValue::GetValueDidChange"""
-self.build(dictionary=self.d)
-self.setTearDownCleanup(dictionary=self.d)
-target = self.dbg.CreateTarget(self.exe)
+# Get the full path to our executable to be attached/debugged.
+exe = self.getBuildArtifact(self.testMethodName)
+d = {'EXE': exe}
+self.build(dictionary=d)
+self.setTearDownCleanup(dictionary=d)
+target = self.dbg.CreateTarget(exe)
breakpoint = target.BreakpointCreateBySourceRegex(
"break here", lldb.SBFileSpec("main.c"))
Index: packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
===
--- packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
+++ packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
@@ -73,10 +73,7 @@
str(compileUnit),
"The compile unit should match",
exe=False,
-substrs=[
-os.path.join(
-self.mydir,
-'main.c')])
+substrs=[self.getSourcePath('main.c')])
function = context.GetFunction()
self.assertTrue(function)
@@ -88,12 +85,12 @@
lineEntry = context.GetLineEntry()
#print("line entry:", lineEntry)
+reldir, _ = self.mydir
self.expect(
lineEntry.GetFileSpec().GetDirectory(),
"The line entry should have the correct directory",
exe=False,
-substrs=[
-self.mydir])
+substrs=[reldir])
self.expect(
lineEntry.GetFileSpec().GetFilename(),
"The line entry should have the correct filename",
Index: packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
===
--- packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
+++ packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
@@ -19,6 +19,8 @@
def setUp(self):
# Call super's setUp().
TestBase.setUp(self)
+
+def setup_test(self):
# Get the full path to our executable to be debugged.
self.exe = self.getBuildArtifact("process_io")
self.local_input_file = self.getBuildArtifact("input.txt")
@@ -38,6 +40,7 @@
@expectedFlakeyLinux(bugnumber="llvm.org/pr26437")
def test_stdin_by_api(self):
"""Exercise SBProcess.PutSTDIN()."""
+self.setup_test()
self.build()
self.create_target()
self.run_process(True)
@@ -49,6 +52,7 @@
@expectedFlakeyLinux(bugnumber="llvm.org/pr26437")
def test_stdin_redirection(self):
"""Exercise SBLaunchInfo::AddOpenFileAction() for STDIN without specifying STDOUT or STDERR."""
+self.setup_test()
self.build()
self.create_target()
self.redirect_stdin()
@@ -62,6 +66,7 @@
@skipIfDarwinEmbedded # debugserver can't create/write files on the device
def test_stdout_redirection(self):
"""Exercise SBLaunchInfo::AddOpenFileAction() for STDOUT without specifying STDIN or STDERR."""
+self.setup_test()
[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/lldbinline.py:96-98
+return "-N dwarf %s" % (testdir)
else:
+return "-N dsym %s" % (testdir)
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
Comment at:
packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py:85
"""Create target, spawn a process, and attach to it with process id."""
-self.build(dictionary=self.d)
-self.setTearDownCleanup(dictionary=self.d)
-target = self.dbg.CreateTarget(self.exe)
+exe = self.getBuildArtifact(self.testMethodName)
+d = {'EXE': exe}
I think `self.testMethodName` is what you're looking for. (There also seems to
be a `self._testMethodName` but I'm not sure what's the difference between the
two.)
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] D42195: [lldb] Generic base for testing gdb-remote behavior
owenpshaw added inline comments. Comment at: lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py:335-337 +# However, if we aren't expecting an ack, it's likely the initial +# ack that lldb client sends, and observations of real servers +# suggest we're supposed to ack back. labath wrote: > Which servers were you observing here? I tried lldb-server and gdbserver and > they don't send the ack back (It seems like a bad idea anyway, as it could > lead to an infinite ack ping-pong). > > I have removed this code as it was causing flakyness on the bots. If there is > a server that does this, and we care about supporting it, then we can add a > new targeted test which emulates this behavior, but then we need to also fix > the client to handle this situation better. Just took another look and I misunderstood what was going on. The server does send an opening +, but not in response to the client. I was observing the packets from openocd, where lldb sends a + and then receives a + from openocd, and assumed one was a response to the other. ``` (lldb) log enable gdb-remote packets (lldb) gdb-remote < 1> send packet: + history[1] tid=0x0307 < 1> send packet: + < 1> read packet: + < 19> send packet: $QStartNoAckMode#b0 < 1> read packet: + < 6> read packet: $OK#9a < 1> send packet: + ``` Can't find any docs about these opening acks, but looking at openocd code, it just always sends a + at the start of a connection. Should the test server do the same? Repository: rL LLVM https://reviews.llvm.org/D42195 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42195: [lldb] Generic base for testing gdb-remote behavior
clayborg added a comment. I am pretty sure that the ack that gets sent should happen in response to getting an ACK. Note that LLDB send an ACK and then waits for an ACK. This helps us see if anything is there. So yes, this code can always just respond to an unsolicited ACK with an ACK. Repository: rL LLVM https://reviews.llvm.org/D42195 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
aprantl added a comment. lib/IR -> test/IR 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] D42195: [lldb] Generic base for testing gdb-remote behavior
labath added a comment. In https://reviews.llvm.org/D42195#995087, @clayborg wrote: > I am pretty sure that the ack that gets sent should happen in response to > getting an ACK. Note that LLDB send an ACK and then waits for an ACK. This > helps us see if anything is there. So yes, this code can always just respond > to an unsolicited ACK with an ACK. LLDB sends an ACK, and then waits for anything. That "anything" can also be "nothing" (see `GDBRemoteCommunicationClient::HandshakeWithServer`). lldb-server currently does not respond to the initial ACK, which is good, because the length of the initial client wait (10ms) would mean this reply-ack would often get missed, and the connection would get out of sync (as this code demonstrated). If we really want to have our servers send an initial ack, then we need to fix the client. Repository: rL LLVM https://reviews.llvm.org/D42195 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D42443: [SymbolFilePDB] Add support for function symbols
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 < revi...@reviews.llvm.org> 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
owenpshaw added a comment. Thanks! Overall it's feeling like we're getting down to mainly differences in personal preferences and judgements. Not sure if you're looking for a discussion, which I'm happy to have, if you're just looking for changes to definitely be made. If it's the latter, I think it'd be more efficient to just hand this off so the changes can be made without a lot of back and forth. Comment at: include/lldb/Target/Process.h:1958 + virtual bool WriteObjectFile(llvm::ArrayRef entries, + Status &error); 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 ``` 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), 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. 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 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. https://reviews.llvm.org/D42145 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
aprantl updated this revision to Diff 132433.
aprantl added a comment.
Update the diff with Pavel's awesome suggestion of using self.testMethodName.
Note that that I haven't fixed all the resulting test error yet, I'm working
through those now.
https://reviews.llvm.org/D42763
Files:
packages/Python/lldbsuite/test/api/listeners/TestListener.py
packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py
packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_on_vectors/TestValueOfVectorVariable.py
packages/Python/lldbsuite/test/lldbinline.py
packages/Python/lldbsuite/test/lldbtest.py
packages/Python/lldbsuite/test/plugins/builder_base.py
packages/Python/lldbsuite/test/plugins/builder_darwin.py
packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py
packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
Index: packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
===
--- packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
+++ packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
@@ -20,16 +20,16 @@
def setUp(self):
# Call super's setUp().
TestBase.setUp(self)
-# Get the full path to our executable to be attached/debugged.
-self.exe = self.getBuildArtifact(self.testMethodName)
-self.d = {'EXE': self.testMethodName}
@add_test_categories(['pyapi'])
def test_with_process_launch_api(self):
"""Test SBValue::GetValueDidChange"""
-self.build(dictionary=self.d)
-self.setTearDownCleanup(dictionary=self.d)
-target = self.dbg.CreateTarget(self.exe)
+# Get the full path to our executable to be attached/debugged.
+exe = self.getBuildArtifact(self.testMethodName)
+d = {'EXE': exe}
+self.build(dictionary=d)
+self.setTearDownCleanup(dictionary=d)
+target = self.dbg.CreateTarget(exe)
breakpoint = target.BreakpointCreateBySourceRegex(
"break here", lldb.SBFileSpec("main.c"))
Index: packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
===
--- packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
+++ packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
@@ -73,10 +73,7 @@
str(compileUnit),
"The compile unit should match",
exe=False,
-substrs=[
-os.path.join(
-self.mydir,
-'main.c')])
+substrs=[self.getSourcePath('main.c')])
function = context.GetFunction()
self.assertTrue(function)
@@ -88,12 +85,12 @@
lineEntry = context.GetLineEntry()
#print("line entry:", lineEntry)
+reldir, _ = self.mydir
self.expect(
lineEntry.GetFileSpec().GetDirectory(),
"The line entry should have the correct directory",
exe=False,
-substrs=[
-self.mydir])
+substrs=[reldir])
self.expect(
lineEntry.GetFileSpec().GetFilename(),
"The line entry should have the correct filename",
Index: packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
===
--- packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
+++ packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
@@ -19,6 +19,8 @@
def setUp(self):
# Call super's setUp().
TestBase.setUp(self)
+
+def setup_test(self):
# Get the full path to our executable to be debugged.
self.exe = self.getBuildArtifact("process_io")
self.local_input_file = self.getBuildArtifact("input.txt")
@@ -38,6 +40,7 @@
@expectedFlakeyLinux(bugnumber="llvm.org/pr26437")
def test_stdin_by_api(self):
"""Exercise SBProcess.PutSTDIN()."""
+self.setup_test()
self.build()
self.create_target()
self.run_process(True)
@@ -49,6 +52,7 @@
@expectedFlakeyLinux(bugnumber="llvm.org/pr26437")
def test_stdin_redirection(self):
"""Exercise SBLaunchInfo::AddOpenFileAction() for STDIN without specifying STDOUT or STDERR."""
+self.setup_test()
self.build()
self.create_target()
self.redirect_stdin()
@@ -62,6 +66,7 @@
@skipIfDarwinEmbedded # debugserver can't create/write files on the device
def test_stdout_redirection(se
Re: [Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
Now that we aren't mixing variants, would it be possible to have a test class
claim that all the tests use the same binary file? At present to get
self-contained tests you often need to do roughly the same thing many times, on
the same binary. You only need to build it once per variant in that case. Of
course, not all tests would behave this way, some play tricks with the binary
products mid-test. But I bet the majority of test classes with more than one
test method just build exactly the same thing as all the other tests, and don't
modify it.
Jim
> On Feb 1, 2018, at 9:55 AM, Pavel Labath via Phabricator
> wrote:
>
> labath added inline comments.
>
>
>
> Comment at: packages/Python/lldbsuite/test/lldbinline.py:96-98
> +return "-N dwarf %s" % (testdir)
> else:
> +return "-N dsym %s" % (testdir)
>
> 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
>
>
>
> Comment at:
> packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py:85
> """Create target, spawn a process, and attach to it with process
> id."""
> -self.build(dictionary=self.d)
> -self.setTearDownCleanup(dictionary=self.d)
> -target = self.dbg.CreateTarget(self.exe)
> +exe = self.getBuildArtifact(self.testMethodName)
> +d = {'EXE': exe}
>
> I think `self.testMethodName` is what you're looking for. (There also seems
> to be a `self._testMethodName` but I'm not sure what's the difference between
> the two.)
>
>
> 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] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
aprantl added a comment. @jingham wrote: > Now that we aren't mixing variants, would it be possible to have a test class > claim that all the tests use the same binary file? At present to get > self-contained tests you often need to do roughly the same thing many times, > on the same binary. You only need to build it once per variant in that case. > Of course, not all tests would behave this way, some play tricks with the > binary products mid-test. But I bet the majority of test classes with more > than one test method just build exactly the same thing as all the other > tests, and don't modify it. Yes, but I would like to keep this for a separate patch, since it is nontrivial: - For NO_DEBUG_INFO_TESTCASEs this is as easy as invoking buildDefault() in self.setUp(). - For tests with variants, we first would need to define a method setUpVariant() that behaves like setUp() in that is invoked for each variant but before any tests are executed. https://reviews.llvm.org/D42763 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
Yeah, no reason to pile on this patch, mostly I was making sure I understood what we could do... Jim > On Feb 1, 2018, at 11:24 AM, Adrian Prantl via Phabricator > wrote: > > aprantl added a comment. > > @jingham wrote: > >> Now that we aren't mixing variants, would it be possible to have a test >> class claim that all the tests use the same binary file? At present to get >> self-contained tests you often need to do roughly the same thing many times, >> on the same binary. You only need to build it once per variant in that >> case. Of course, not all tests would behave this way, some play tricks with >> the binary products mid-test. But I bet the majority of test classes with >> more than one test method just build exactly the same thing as all the other >> tests, and don't modify it. > > Yes, but I would like to keep this for a separate patch, since it is > nontrivial: > > - For NO_DEBUG_INFO_TESTCASEs this is as easy as invoking buildDefault() in > self.setUp(). > - For tests with variants, we first would need to define a method > setUpVariant() that behaves like setUp() in that is invoked for each variant > but before any tests are executed. > > > 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] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
aprantl updated this revision to Diff 132456.
aprantl added a comment.
Fix broken testcases.
https://reviews.llvm.org/D42763
Files:
packages/Python/lldbsuite/test/api/listeners/TestListener.py
packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py
packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_on_vectors/TestValueOfVectorVariable.py
packages/Python/lldbsuite/test/lldbinline.py
packages/Python/lldbsuite/test/lldbtest.py
packages/Python/lldbsuite/test/plugins/builder_base.py
packages/Python/lldbsuite/test/plugins/builder_darwin.py
packages/Python/lldbsuite/test/python_api/hello_world/TestHelloWorld.py
packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
packages/Python/lldbsuite/test/python_api/symbol-context/two-files/TestSymbolContextTwoFiles.py
packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
Index: packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
===
--- packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
+++ packages/Python/lldbsuite/test/python_api/value_var_update/TestValueVarUpdate.py
@@ -20,16 +20,16 @@
def setUp(self):
# Call super's setUp().
TestBase.setUp(self)
-# Get the full path to our executable to be attached/debugged.
-self.exe = self.getBuildArtifact(self.testMethodName)
-self.d = {'EXE': self.testMethodName}
@add_test_categories(['pyapi'])
def test_with_process_launch_api(self):
"""Test SBValue::GetValueDidChange"""
-self.build(dictionary=self.d)
-self.setTearDownCleanup(dictionary=self.d)
-target = self.dbg.CreateTarget(self.exe)
+# Get the full path to our executable to be attached/debugged.
+exe = self.getBuildArtifact(self.testMethodName)
+d = {'EXE': exe}
+self.build(dictionary=d)
+self.setTearDownCleanup(dictionary=d)
+target = self.dbg.CreateTarget(exe)
breakpoint = target.BreakpointCreateBySourceRegex(
"break here", lldb.SBFileSpec("main.c"))
Index: packages/Python/lldbsuite/test/python_api/symbol-context/two-files/TestSymbolContextTwoFiles.py
===
--- packages/Python/lldbsuite/test/python_api/symbol-context/two-files/TestSymbolContextTwoFiles.py
+++ packages/Python/lldbsuite/test/python_api/symbol-context/two-files/TestSymbolContextTwoFiles.py
@@ -22,7 +22,6 @@
"""Test lookup by address in a module with multiple compilation units"""
self.build()
exe = self.getBuildArtifact("a.out")
-
target = self.dbg.CreateTarget(exe)
self.assertTrue(target, VALID_TARGET)
@@ -45,7 +44,6 @@
compile unit contains DW_AT_ranges and DW_AT_ranges_base attributes."""
self.build()
exe = self.getBuildArtifact("a.out")
-
target = self.dbg.CreateTarget(exe)
self.assertTrue(target, VALID_TARGET)
Index: packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
===
--- packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
+++ packages/Python/lldbsuite/test/python_api/symbol-context/TestSymbolContext.py
@@ -73,10 +73,7 @@
str(compileUnit),
"The compile unit should match",
exe=False,
-substrs=[
-os.path.join(
-self.mydir,
-'main.c')])
+substrs=[self.getSourcePath('main.c')])
function = context.GetFunction()
self.assertTrue(function)
@@ -92,8 +89,7 @@
lineEntry.GetFileSpec().GetDirectory(),
"The line entry should have the correct directory",
exe=False,
-substrs=[
-self.mydir])
+substrs=[self.mydir])
self.expect(
lineEntry.GetFileSpec().GetFilename(),
"The line entry should have the correct filename",
Index: packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
===
--- packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
+++ packages/Python/lldbsuite/test/python_api/process/io/TestProcessIO.py
@@ -19,6 +19,8 @@
def setUp(self):
# Call super's setUp().
TestBase.setUp(self)
+
+def setup_test(self):
# Get the full path to our executable to be debugged.
self.exe = self.getBuildArtifact("process_io")
self.local_input_file = self.getBuildArtifact("input.txt")
@@ -38,6 +40,7 @@
[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() 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? 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] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file
aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/api/listeners/TestListener.py:26 TestBase.setUp(self) -self.build() 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. 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] [lldb] r324008 - Remove unused Args variable, and #include of Args.h. NFC.
Author: jingham
Date: Thu Feb 1 13:31:14 2018
New Revision: 324008
URL: http://llvm.org/viewvc/llvm-project?rev=324008&view=rev
Log:
Remove unused Args variable, and #include of Args.h. NFC.
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=324008&r1=324007&r2=324008&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Thu Feb
1 13:31:14 2018
@@ -25,7 +25,6 @@
#include "lldb/Core/Module.h"
#include "lldb/Core/Value.h"
#include "lldb/Host/Host.h"
-#include "lldb/Interpreter/Args.h"
#include "lldb/Symbol/ClangASTImporter.h"
#include "lldb/Symbol/ClangExternalASTSourceCommon.h"
#include "lldb/Symbol/ClangUtil.h"
@@ -2083,7 +2082,6 @@ bool DWARFASTParserClang::ParseTemplateP
if (!parent_die)
return false;
- Args template_parameter_names;
for (DWARFDIE die = parent_die.GetFirstChild(); die.IsValid();
die = die.GetSibling()) {
const dw_tag_t tag = die.Tag();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r324010 - Added lldbutil.run_to_name_breakpoint and use it in one test.
Author: jingham
Date: Thu Feb 1 13:35:50 2018
New Revision: 324010
URL: http://llvm.org/viewvc/llvm-project?rev=324010&view=rev
Log:
Added lldbutil.run_to_name_breakpoint and use it in one test.
Using the "run_to_{source,name}_breakpoint will allow us to remove
a lot of boiler-plate from the testsuite. We mostly use source
breakpoints, but some tests use by name ones so this was needed.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py?rev=324010&r1=324009&r2=324010&view=diff
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
(original)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
Thu Feb 1 13:35:50 2018
@@ -41,27 +41,9 @@ class ReturnValueTestCase(TestBase):
"""Test getting return values from stepping out."""
self.build()
exe = self.getBuildArtifact("a.out")
-error = lldb.SBError()
-
-self.target = self.dbg.CreateTarget(exe)
-self.assertTrue(self.target, VALID_TARGET)
-
-inner_sint_bkpt = self.target.BreakpointCreateByName("inner_sint", exe)
-self.assertTrue(inner_sint_bkpt, VALID_BREAKPOINT)
-
-# Now launch the process, and do not stop at entry point.
-self.process = self.target.LaunchSimple(
-None, None, self.get_process_working_directory())
+(self.target, self.process, thread, inner_sint_bkpt) =
lldbutil.run_to_name_breakpoint(self, "inner_sint", exe_name = exe)
-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)
-
-# Now finish, and make sure the return value is correct.
-thread = lldbutil.get_stopped_thread(
-self.process, lldb.eStopReasonBreakpoint)
+error = lldb.SBError()
# inner_sint returns the variable value, so capture that here:
in_int = thread.GetFrameAtIndex(0).FindVariable(
@@ -86,10 +68,8 @@ class ReturnValueTestCase(TestBase):
# Run again and we will stop in inner_sint the second time outer_sint
is called.
# Then test stepping out two frames at once:
-
-self.process.Continue()
-thread_list = lldbutil.get_threads_stopped_at_breakpoint(
-self.process, inner_sint_bkpt)
+
+thread_list = lldbutil.continue_to_breakpoint(self.process,
inner_sint_bkpt)
self.assertTrue(len(thread_list) == 1)
thread = thread_list[0]
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=324010&r1=324009&r2=324010&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py Thu Feb 1 13:35:50
2018
@@ -734,39 +734,18 @@ def get_crashed_threads(test, process):
threads.append(thread)
return threads
-def run_to_source_breakpoint(test, bkpt_pattern, source_spec,
- launch_info = None, exe_name = "a.out",
- 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.
-
- If you want to pass in launch arguments or environment
- variables, you can optionally pass in an SBLaunchInfo. If you
- do that, remember to set the working directory as well.
-
- If your executable isn't called a.out, you can pass that in.
- 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 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.
-"""
+# Helper functions for run_to_{source,name}_breakpoint:
+def run_to_breakpoint_make_target(test, exe_name, in_cwd):
if in_cwd:
exe = test.getBuildArtifact(exe_name)
# Create the target
target = test.dbg.CreateTarget(exe)
test.assertTrue(target, "Target: %s is not valid."%(exe_name))
+return target
-# Set the breakpoints
-breakpoint = target.BreakpointCreateBySourceRegex(
-bkpt_patter
[Lldb-commits] [lldb] r324013 - Make use of physical footprint as memory measurement.
Author: hanming
Date: Thu Feb 1 13:46:40 2018
New Revision: 324013
URL: http://llvm.org/viewvc/llvm-project?rev=324013&view=rev
Log:
Make use of physical footprint as memory measurement.
Remove obsolete measurements.
This check in requires at least 10.11
Reviewed: Jason Molenda, Jim Ingham
Xcode Memory gauge should show the jetsam ledger
footprint rather than anonymous
Modified:
lldb/trunk/tools/debugserver/source/DNBDefs.h
lldb/trunk/tools/debugserver/source/MacOSX/MachTask.mm
lldb/trunk/tools/debugserver/source/MacOSX/MachVMMemory.cpp
lldb/trunk/tools/debugserver/source/MacOSX/MachVMMemory.h
Modified: lldb/trunk/tools/debugserver/source/DNBDefs.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/DNBDefs.h?rev=324013&r1=324012&r2=324013&view=diff
==
--- lldb/trunk/tools/debugserver/source/DNBDefs.h (original)
+++ lldb/trunk/tools/debugserver/source/DNBDefs.h Thu Feb 1 13:46:40 2018
@@ -347,9 +347,7 @@ enum DNBProfileDataScanType {
eProfileHostMemory = (1 << 5),
- eProfileMemory = (1 << 6), // By default, excludes eProfileMemoryDirtyPage.
- eProfileMemoryDirtyPage =
- (1 << 7), // Assume eProfileMemory, get Dirty Page size as well.
+ eProfileMemory = (1 << 6),
eProfileMemoryAnonymous =
(1 << 8), // Assume eProfileMemory, get Anonymous memory as well.
Modified: lldb/trunk/tools/debugserver/source/MacOSX/MachTask.mm
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/MacOSX/MachTask.mm?rev=324013&r1=324012&r2=324013&view=diff
==
--- lldb/trunk/tools/debugserver/source/MacOSX/MachTask.mm (original)
+++ lldb/trunk/tools/debugserver/source/MacOSX/MachTask.mm Thu Feb 1 13:46:40
2018
@@ -348,23 +348,13 @@ std::string MachTask::GetProfileData(DNB
threads_used_usec);
}
-#if defined(HOST_VM_INFO64_COUNT)
vm_statistics64_data_t vminfo;
-#else
- struct vm_statistics vminfo;
-#endif
uint64_t physical_memory;
- mach_vm_size_t rprvt = 0;
- mach_vm_size_t rsize = 0;
- mach_vm_size_t vprvt = 0;
- mach_vm_size_t vsize = 0;
- mach_vm_size_t dirty_size = 0;
- mach_vm_size_t purgeable = 0;
mach_vm_size_t anonymous = 0;
+ mach_vm_size_t phys_footprint = 0;
if (m_vm_memory.GetMemoryProfile(scanType, task, task_info,
m_process->GetCPUType(), pid, vminfo,
- physical_memory, rprvt, rsize, vprvt, vsize,
- dirty_size, purgeable, anonymous)) {
+ physical_memory, anonymous,
phys_footprint)) {
std::ostringstream profile_data_stream;
if (scanType & eProfileHostCPU) {
@@ -413,51 +403,20 @@ std::string MachTask::GetProfileData(DNB
profile_data_stream << "total:" << physical_memory << ';';
if (scanType & eProfileMemory) {
-#if defined(HOST_VM_INFO64_COUNT) && defined(_VM_PAGE_SIZE_H_)
static vm_size_t pagesize = vm_kernel_page_size;
-#else
- static vm_size_t pagesize;
- static bool calculated = false;
- if (!calculated) {
-calculated = true;
-pagesize = PageSize();
- }
-#endif
-/* Unused values. Optimized out for transfer performance.
-profile_data_stream << "wired:" << vminfo.wire_count * pagesize << ';';
-profile_data_stream << "active:" << vminfo.active_count * pagesize << ';';
-profile_data_stream << "inactive:" << vminfo.inactive_count * pagesize << ';';
- */
-#if defined(HOST_VM_INFO64_COUNT)
// This mimicks Activity Monitor.
uint64_t total_used_count =
(physical_memory / pagesize) -
(vminfo.free_count - vminfo.speculative_count) -
vminfo.external_page_count - vminfo.purgeable_count;
-#else
- uint64_t total_used_count =
- vminfo.wire_count + vminfo.inactive_count + vminfo.active_count;
-#endif
profile_data_stream << "used:" << total_used_count * pagesize << ';';
- /* Unused values. Optimized out for transfer performance.
- profile_data_stream << "free:" << vminfo.free_count * pagesize << ';';
- */
-
- profile_data_stream << "rprvt:" << rprvt << ';';
- /* Unused values. Optimized out for transfer performance.
- profile_data_stream << "rsize:" << rsize << ';';
- profile_data_stream << "vprvt:" << vprvt << ';';
- profile_data_stream << "vsize:" << vsize << ';';
- */
-
- if (scanType & eProfileMemoryDirtyPage)
-profile_data_stream << "dirty:" << dirty_size << ';';
if (scanType & eProfileMemoryAnonymous) {
-profile_data_stream << "purgeable:" << purgeable << ';';
profile_data_stream << "anonymous:" << anonymous << ';';
}
+
+ profile_data_stream << "phys_footprint:" << phys_footprint << ';';
}
// proc_pid_rusage pm_sample_task_and_pid pm_energy_im
[Lldb-commits] [lldb] r324019 - Create a marker for Spotlight to never index $BUILD_DIR.
Author: adrian
Date: Thu Feb 1 14:18:02 2018
New Revision: 324019
URL: http://llvm.org/viewvc/llvm-project?rev=324019&view=rev
Log:
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.
This should fix the broken green dragon bots.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/dotest.py
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=324019&r1=324018&r2=324019&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Thu Feb 1 14:18:02 2018
@@ -1196,8 +1196,17 @@ def run_suite():
# Set up the working directory.
# Note that it's not dotest's job to clean this directory.
-try: os.makedirs(configuration.test_build_dir)
-except: pass
+import lldbsuite.test.lldbutil as lldbutil
+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]
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r324019 - Create a marker for Spotlight to never index $BUILD_DIR.
On Thu, Feb 1, 2018 at 2:18 PM, Adrian Prantl via lldb-commits wrote: > Author: adrian > Date: Thu Feb 1 14:18:02 2018 > New Revision: 324019 > > URL: http://llvm.org/viewvc/llvm-project?rev=324019&view=rev > Log: > 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. > > This should fix the broken green dragon bots. > Thanks! -- Davide ___ 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
jasonmolenda created this revision.
jasonmolenda added a reviewer: labath.
jasonmolenda added a project: LLDB.
Herald added a subscriber: llvm-commits.
I caught this while running the testsuite against lldb built with address
sanitizer (ASAN) enabled - it found one problem when running the
TestLinuxCore.py test. The ELFLinuxPrPsInfo structure has two fixed width
strings in it, pr_fname (16 chars) and pr_psargs (80 chars). They are not
required to be nul (\0) terminated, and in the case of ppc64le, pr_fname is not
-
(lldb) p prpsinfo
(ELFLinuxPrPsInfo) $1 = {
pr_fname = {
[0] = 'l'
[1] = 'i'
[2] = 'n'
[3] = 'u'
[4] = 'x'
[5] = '-'
[6] = 'p'
[7] = 'p'
[8] = 'c'
[9] = '6'
[10] = '4'
[11] = 'l'
[12] = 'e'
[13] = '.'
[14] = 'o'
[15] = 'u'
}
When we copy this into a std::string,
thread_data.name = prpsinfo.pr_fname;
the read goes off the end of the array. It goes into the next element on the
structure, pr_psargs, so it's unlikely to crash, but it's an easy one to fix so
I think we should.
TestLinuxCore.py's do_test() could also get passed in the expected thread name
and verify that it was set correctly), that would have caught this without
using ASAN. But given that ASAN did catch it, I'm pretty happy with it as-is.
Repository:
rL LLVM
https://reviews.llvm.org/D42828
Files:
source/Plugins/Process/elf-core/ProcessElfCore.cpp
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ 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, sizeof (prpsinfo.pr_fname));
SetID(prpsinfo.pr_pid);
break;
}
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ 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, 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
Re: [Lldb-commits] [lldb] r324019 - Create a marker for Spotlight to never index $BUILD_DIR.
I'm afraid this still wasn't enough to placate the bots. http://lab.llvm.org:8080/green/view/LLDB/job/lldb-xcode/4647/ Can you please take another look? -- Davide On Thu, Feb 1, 2018 at 2:27 PM, Davide Italiano wrote: > On Thu, Feb 1, 2018 at 2:18 PM, Adrian Prantl via lldb-commits > wrote: >> Author: adrian >> Date: Thu Feb 1 14:18:02 2018 >> New Revision: 324019 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=324019&view=rev >> Log: >> 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. >> >> This should fix the broken green dragon bots. >> > > Thanks! > > -- > Davide ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [lldb] r324019 - Create a marker for Spotlight to never index $BUILD_DIR.
Might try adding this patch to the test case: Index: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_case_sensitivity/TestBreakpointCaseSensitivity.py === --- packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_case_sensitivity/TestBreakpointCaseSensitivity.py (revision 324047) +++ packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_case_sensitivity/TestBreakpointCaseSensitivity.py (working copy) @@ -82,6 +82,10 @@ desc = ' file %s set by %s' % ( file, 'regex' if source_regex else 'location') +exe_module = self.target.FindModule(self.target.GetExecutable()) +sym_file = exe_module.GetSymbolFileSpec() +desc += " SymFile: %s/%s"%(sym_file.GetDirectory(), sym_file.GetFilename()) + if source_regex: breakpoint = self.target.BreakpointCreateBySourceRegex( self.BREAKPOINT_TEXT, lldb.SBFileSpec(file)) That way the error message will print out where we're getting symbols. If we're getting them from some weird unexpected place, this will show that. Jim > On Feb 1, 2018, at 5:40 PM, Davide Italiano via lldb-commits > wrote: > > I'm afraid this still wasn't enough to placate the bots. > http://lab.llvm.org:8080/green/view/LLDB/job/lldb-xcode/4647/ > > Can you please take another look? > > -- > Davide > > On Thu, Feb 1, 2018 at 2:27 PM, Davide Italiano wrote: >> On Thu, Feb 1, 2018 at 2:18 PM, Adrian Prantl via lldb-commits >> wrote: >>> Author: adrian >>> Date: Thu Feb 1 14:18:02 2018 >>> New Revision: 324019 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=324019&view=rev >>> Log: >>> 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. >>> >>> This should fix the broken green dragon bots. >>> >> >> Thanks! >> >> -- >> Davide > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
