Re: [Lldb-commits] [PATCH] D42443: [SymbolFilePDB] Add support for function symbols

2018-02-02 Thread Pavel Labath via lldb-commits
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

2018-02-02 Thread Pavel Labath via Phabricator via lldb-commits
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

2018-02-02 Thread Pavel Labath via Phabricator via lldb-commits
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()

2018-02-02 Thread Pavel Labath via Phabricator via lldb-commits
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

2018-02-02 Thread Jonas Devlieghere via Phabricator via lldb-commits
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()

2018-02-02 Thread Pavel Labath via Phabricator via lldb-commits
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

2018-02-02 Thread Pavel Labath via Phabricator via lldb-commits
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

2018-02-02 Thread Greg Clayton via Phabricator via lldb-commits
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()

2018-02-02 Thread Adrian Prantl via Phabricator via lldb-commits
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

2018-02-02 Thread Jan Kratochvil via Phabricator via lldb-commits
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()

2018-02-02 Thread Pavel Labath via Phabricator via lldb-commits
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

2018-02-02 Thread Jan Kratochvil via Phabricator via lldb-commits
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

2018-02-02 Thread Greg Clayton via Phabricator via lldb-commits
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

2018-02-02 Thread Greg Clayton via Phabricator via lldb-commits
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.

2018-02-02 Thread Adrian Prantl via lldb-commits
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

2018-02-02 Thread Jim Ingham via lldb-commits
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

2018-02-02 Thread Jim Ingham via Phabricator via lldb-commits
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()

2018-02-02 Thread Adrian Prantl via Phabricator via lldb-commits
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

2018-02-02 Thread Owen Shaw via Phabricator via lldb-commits
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

2018-02-02 Thread Jan Kratochvil via Phabricator via lldb-commits
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

2018-02-02 Thread Jan Kratochvil via Phabricator via lldb-commits
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

2018-02-02 Thread Kamil Rytarowski via Phabricator via lldb-commits
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

2018-02-02 Thread Kamil Rytarowski via Phabricator via lldb-commits
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

2018-02-02 Thread Kamil Rytarowski via Phabricator via lldb-commits
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

2018-02-02 Thread Kamil Rytarowski via Phabricator via lldb-commits
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

2018-02-02 Thread Kamil Rytarowski via Phabricator via lldb-commits
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

2018-02-02 Thread Jason Molenda via lldb-commits
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

2018-02-02 Thread Phabricator via Phabricator via lldb-commits
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

2018-02-02 Thread Kamil Rytarowski via Phabricator via lldb-commits
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

2018-02-02 Thread Davide Italiano via Phabricator via lldb-commits
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

2018-02-02 Thread Kamil Rytarowski via Phabricator via lldb-commits
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

2018-02-02 Thread Greg Clayton via Phabricator via lldb-commits
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

2018-02-02 Thread Greg Clayton via Phabricator via lldb-commits
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

2018-02-02 Thread Davide Italiano via Phabricator via lldb-commits
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

2018-02-02 Thread Kamil Rytarowski via Phabricator via lldb-commits
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

2018-02-02 Thread Davide Italiano via Phabricator via lldb-commits
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

2018-02-02 Thread Jim Ingham via Phabricator via lldb-commits
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

2018-02-02 Thread Jason Molenda via lldb-commits
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

2018-02-02 Thread Jason Molenda via lldb-commits
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