[Lldb-commits] [PATCH] D68188: allow arbitrary python streams to be converted to SBFile

2019-10-10 Thread Jan Kratochvil via Phabricator via lldb-commits 
jankratochvil added a comment.

Fixed now by rL374322 , there was a 
regression - Linux Fedora 30 x86_64:

  ==
  ERROR: test_exceptions (TestFileHandle.FileHandleTestCase)
  --
  Traceback (most recent call last):
File 
"/home/jkratoch/redhat/llvm-monorepo2/lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py",
 line 483, in test_exceptions
  self.assertRaises(TypeError, lldb.SBFile, None)
File 
"/home/jkratoch/redhat/llvm-monorepo2/lldb/third_party/Python/module/unittest2/unittest2/case.py",
 line 534, in assertRaises
  callableObj(*args, **kwargs)
File 
"/home/jkratoch/redhat/llvm-monorepo2-clangassert/lib/python2.7/site-packages/lldb/__init__.py",
 line 5334, in __init__
  this = _lldb.new_SBFile(*args)
  NotImplementedError: Wrong number or type of arguments for overloaded 
function 'new_SBFile'.
Possible C/C++ prototypes are:
  lldb::SBFile::SBFile()
  lldb::SBFile::SBFile(int,char const *,bool)
  lldb::SBFile::SBFile(lldb::FileSP)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68188/new/

https://reviews.llvm.org/D68188



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r374322 - TestFileHandle.py: relax exception type checks

2019-10-10 Thread Pavel Labath via lldb-commits 
Author: labath
Date: Thu Oct 10 05:07:30 2019
New Revision: 374322

URL: http://llvm.org/viewvc/llvm-project?rev=374322=rev
Log:
TestFileHandle.py: relax exception type checks

the exceptions returned differ between swig4 (TypeError) and swig<=3
(NotImplementedError). Just check for the base Exception class instead.

Theoretically we could switch on the swig version and expect the precise
type directly, but checking the exact type does not seem that important.

Thanks to Raphael for helping me figure this out.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py?rev=374322=374321=374322=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
 Thu Oct 10 05:07:30 2019
@@ -667,10 +667,10 @@ class FileHandleTestCase(lldbtest.TestBa
 
 @add_test_categories(['pyapi'])
 def test_exceptions(self):
-self.assertRaises(TypeError, lldb.SBFile, None)
-self.assertRaises(TypeError, lldb.SBFile, "ham sandwich")
+self.assertRaises(Exception, lldb.SBFile, None)
+self.assertRaises(Exception, lldb.SBFile, "ham sandwich")
 if sys.version_info[0] < 3:
-self.assertRaises(TypeError, lldb.SBFile, ReallyBadIO())
+self.assertRaises(Exception, lldb.SBFile, ReallyBadIO())
 else:
 self.assertRaises(OhNoe, lldb.SBFile, ReallyBadIO())
 error, n = lldb.SBFile(BadIO()).Write(b"FOO")


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68696: [lldb][NFC] Remove strange bool parameter from Searcher::SearchCallback

2019-10-10 Thread Raphael Isemann via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG95e264fc8a93: [lldb][NFC] Remove strange bool parameter from 
Searcher::SearchCallback (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68696/new/

https://reviews.llvm.org/D68696

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverAddress.h
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Breakpoint/BreakpointResolverFileRegex.h
  lldb/include/lldb/Breakpoint/BreakpointResolverName.h
  lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h
  lldb/include/lldb/Core/AddressResolverFileLine.h
  lldb/include/lldb/Core/AddressResolverName.h
  lldb/include/lldb/Core/FileLineResolver.h
  lldb/include/lldb/Core/SearchFilter.h
  lldb/include/lldb/Interpreter/CommandCompletions.h
  lldb/source/Breakpoint/BreakpointResolverAddress.cpp
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Breakpoint/BreakpointResolverScripted.cpp
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Core/AddressResolverFileLine.cpp
  lldb/source/Core/AddressResolverName.cpp
  lldb/source/Core/FileLineResolver.cpp
  lldb/source/Core/SearchFilter.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
  lldb/source/Target/LanguageRuntime.cpp

Index: lldb/source/Target/LanguageRuntime.cpp
===
--- lldb/source/Target/LanguageRuntime.cpp
+++ lldb/source/Target/LanguageRuntime.cpp
@@ -111,12 +111,11 @@
   ~ExceptionBreakpointResolver() override = default;
 
   Searcher::CallbackReturn SearchCallback(SearchFilter ,
-  SymbolContext , Address *addr,
-  bool containing) override {
+  SymbolContext ,
+  Address *addr) override {
 
 if (SetActualResolver())
-  return m_actual_resolver_sp->SearchCallback(filter, context, addr,
-  containing);
+  return m_actual_resolver_sp->SearchCallback(filter, context, addr);
 else
   return eCallbackReturnStop;
   }
Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
@@ -67,8 +67,8 @@
   void Dump(Stream *s) const override {}
 
   Searcher::CallbackReturn SearchCallback(SearchFilter ,
-  SymbolContext , Address *addr,
-  bool containing) override;
+  SymbolContext ,
+  Address *addr) override;
 
   lldb::SearchDepth GetDepth() override { return lldb::eSearchDepthModule; }
 
@@ -117,8 +117,8 @@
   void Dump(Stream *s) const override {}
 
   Searcher::CallbackReturn SearchCallback(SearchFilter ,
-  SymbolContext , Address *addr,
-  bool containing) override;
+  SymbolContext ,
+  Address *addr) override;
 
   lldb::SearchDepth GetDepth() override { return lldb::eSearchDepthModule; }
 
@@ -262,8 +262,8 @@
   void Dump(Stream *s) const override {}
 
   Searcher::CallbackReturn SearchCallback(SearchFilter ,
-  SymbolContext , Address *addr,
-  bool containing) override;
+  SymbolContext ,
+  Address *addr) override;
 
   lldb::SearchDepth GetDepth() override { return lldb::eSearchDepthModule; }
 
Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -790,7 +790,7 @@
 // symbol.
 Searcher::CallbackReturn
 RSBreakpointResolver::SearchCallback(SearchFilter ,
- SymbolContext , Address *, bool) {
+ SymbolContext , Address *) {
   ModuleSP module = context.module_sp;
 
   if (!module || !IsRenderScriptScriptModule(module))
@@

[Lldb-commits] [lldb] r374313 - [lldb][NFC] Remove strange bool parameter from Searcher::SearchCallback

2019-10-10 Thread Raphael Isemann via lldb-commits 
Author: teemperor
Date: Thu Oct 10 04:26:51 2019
New Revision: 374313

URL: http://llvm.org/viewvc/llvm-project?rev=374313=rev
Log:
[lldb][NFC] Remove strange bool parameter from Searcher::SearchCallback

Summary:
The SearchCallback has a bool parameter that we always set to false, we never 
use in any callback implementation and that also changes its name
from one file to the other (either `containing` and `complete`). It was added 
in the original LLDB check in, so there isn't any history what
this was supposed to be, so let's just remove it.

Reviewers: jingham, JDevlieghere, labath

Reviewed By: jingham, labath

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D68696

Modified:
lldb/trunk/include/lldb/Breakpoint/BreakpointResolverAddress.h
lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h
lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileRegex.h
lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h
lldb/trunk/include/lldb/Breakpoint/BreakpointResolverScripted.h
lldb/trunk/include/lldb/Core/AddressResolverFileLine.h
lldb/trunk/include/lldb/Core/AddressResolverName.h
lldb/trunk/include/lldb/Core/FileLineResolver.h
lldb/trunk/include/lldb/Core/SearchFilter.h
lldb/trunk/include/lldb/Interpreter/CommandCompletions.h
lldb/trunk/source/Breakpoint/BreakpointResolverAddress.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverFileRegex.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
lldb/trunk/source/Breakpoint/BreakpointResolverScripted.cpp
lldb/trunk/source/Commands/CommandCompletions.cpp
lldb/trunk/source/Core/AddressResolverFileLine.cpp
lldb/trunk/source/Core/AddressResolverName.cpp
lldb/trunk/source/Core/FileLineResolver.cpp
lldb/trunk/source/Core/SearchFilter.cpp

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp

lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
lldb/trunk/source/Target/LanguageRuntime.cpp

Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverAddress.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolverAddress.h?rev=374313=374312=374313=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointResolverAddress.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolverAddress.h Thu Oct 10 
04:26:51 2019
@@ -41,8 +41,8 @@ public:
   ModuleList ) override;
 
   Searcher::CallbackReturn SearchCallback(SearchFilter ,
-  SymbolContext , Address 
*addr,
-  bool containing) override;
+  SymbolContext ,
+  Address *addr) override;
 
   lldb::SearchDepth GetDepth() override;
 

Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h?rev=374313=374312=374313=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileLine.h Thu Oct 10 
04:26:51 2019
@@ -35,8 +35,8 @@ public:
   ~BreakpointResolverFileLine() override;
 
   Searcher::CallbackReturn SearchCallback(SearchFilter ,
-  SymbolContext , Address 
*addr,
-  bool containing) override;
+  SymbolContext ,
+  Address *addr) override;
 
   lldb::SearchDepth GetDepth() override;
 

Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileRegex.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileRegex.h?rev=374313=374312=374313=diff
==
--- lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileRegex.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointResolverFileRegex.h Thu Oct 10 
04:26:51 2019
@@ -37,8 +37,8 @@ public:
   ~BreakpointResolverFileRegex() override;
 
   Searcher::CallbackReturn SearchCallback(SearchFilter ,
-  SymbolContext , Address 
*addr,
-  bool containing) override;
+  SymbolContext ,
+  Address *addr) override;
 
   lldb::SearchDepth GetDepth() override;
 

Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointResolverName.h
URL:

[Lldb-commits] [PATCH] D68773: [lldb] Fix out of bounds read in DataExtractor::GetCStr and add unit test that function.

2019-10-10 Thread Raphael Isemann via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG067bb1f546ef: [lldb] Fix out of bounds read in 
DataExtractor::GetCStr and add unit test that… (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68773/new/

https://reviews.llvm.org/D68773

Files:
  lldb/source/Utility/DataExtractor.cpp
  lldb/unittests/Utility/DataExtractorTest.cpp

Index: lldb/unittests/Utility/DataExtractorTest.cpp
===
--- lldb/unittests/Utility/DataExtractorTest.cpp
+++ lldb/unittests/Utility/DataExtractorTest.cpp
@@ -49,6 +49,51 @@
   EXPECT_EQ(nullptr, E.PeekData(4, 1));
 }
 
+TEST(DataExtractorTest, GetCStr) {
+  uint8_t buffer[] = {'X', 'f', 'o', 'o', '\0'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_STREQ("foo", E.GetCStr());
+  EXPECT_EQ(5U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrEmpty) {
+  uint8_t buffer[] = {'X', '\0'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_STREQ("", E.GetCStr());
+  EXPECT_EQ(2U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrUnterminated) {
+  uint8_t buffer[] = {'X', 'f', 'o', 'o'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_EQ(nullptr, E.GetCStr());
+  EXPECT_EQ(1U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrAtEnd) {
+  uint8_t buffer[] = {'X'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_EQ(nullptr, E.GetCStr());
+  EXPECT_EQ(1U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrAtNullOffset) {
+  uint8_t buffer[] = {'f', 'o', 'o', '\0'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 0;
+  EXPECT_STREQ("foo", E.GetCStr());
+  EXPECT_EQ(4U, offset);
+}
+
 TEST(DataExtractorTest, GetMaxU64) {
   uint8_t buffer[] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08};
   DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
Index: lldb/source/Utility/DataExtractor.cpp
===
--- lldb/source/Utility/DataExtractor.cpp
+++ lldb/source/Utility/DataExtractor.cpp
@@ -816,26 +816,25 @@
 // non-zero and there aren't enough available bytes, nullptr will be returned
 // and "offset_ptr" will not be updated.
 const char *DataExtractor::GetCStr(offset_t *offset_ptr) const {
-  const char *cstr = reinterpret_cast(PeekData(*offset_ptr, 1));
-  if (cstr) {
-const char *cstr_end = cstr;
-const char *end = reinterpret_cast(m_end);
-while (cstr_end < end && *cstr_end)
-  ++cstr_end;
-
-// Now we are either at the end of the data or we point to the
-// NULL C string terminator with cstr_end...
-if (*cstr_end == '\0') {
-  // Advance the offset with one extra byte for the NULL terminator
-  *offset_ptr += (cstr_end - cstr + 1);
-  return cstr;
-}
+  const char *start = reinterpret_cast(PeekData(*offset_ptr, 1));
+  // Already at the end of the data.
+  if (!start)
+return nullptr;
 
-// We reached the end of the data without finding a NULL C string
-// terminator. Fall through and return nullptr otherwise anyone that would
-// have used the result as a C string can wander into unknown memory...
-  }
-  return nullptr;
+  const char *end = reinterpret_cast(m_end);
+
+  // Check all bytes for a null terminator that terminates a C string.
+  const char *terminator_or_end = std::find(start, end, '\0');
+
+  // We didn't find a null terminator, so return nullptr to indicate that there
+  // is no valid C string at that offset.
+  if (terminator_or_end == end)
+return nullptr;
+
+  // Update offset_ptr for the caller to point to the data behind the
+  // terminator (which is 1 byte long).
+  *offset_ptr += (terminator_or_end - start + 1UL);
+  return start;
 }
 
 // Extracts a NULL terminated C string from the fixed length field of length
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-10 Thread Fangrui Song via Phabricator via lldb-commits 
MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:111
+
+  explicit MemoryInfoListStream(std::vector Infos)
+  : Stream(StreamKind::MemoryInfoList,

grimar wrote:
> Maybe be more explicit here, i.e.
> 
> ```
> std::vector &
> ```
> ?
Or let the constructor take `iterator_range` 
as the argument, and change the call site below.



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:116
+
+  MemoryInfoListStream()
+  : Stream(StreamKind::MemoryInfoList,

Move default constructor above.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68645/new/

https://reviews.llvm.org/D68645



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r374311 - [lldb] Fix out of bounds read in DataExtractor::GetCStr and add unit test that function.

2019-10-10 Thread Raphael Isemann via lldb-commits 
Author: teemperor
Date: Thu Oct 10 04:15:38 2019
New Revision: 374311

URL: http://llvm.org/viewvc/llvm-project?rev=374311=rev
Log:
[lldb] Fix out of bounds read in DataExtractor::GetCStr and add unit test that 
function.

Summary:
The `if (*cstr_end == '\0')` in the previous code checked if the previous loop 
terminated because it
found a null terminator or because it reached the end of the data. However, in 
the case that we hit
the end of the data before finding a null terminator, `cstr_end` points behind 
the last byte in our
data and `*cstr_end` reads the memory behind the array (which may be 
uninitialised)

This patch just rewrites that function use `std::find` and adds the relevant 
unit tests.

Reviewers: labath

Reviewed By: labath

Subscribers: abidh, JDevlieghere, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D68773

Modified:
lldb/trunk/source/Utility/DataExtractor.cpp
lldb/trunk/unittests/Utility/DataExtractorTest.cpp

Modified: lldb/trunk/source/Utility/DataExtractor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/DataExtractor.cpp?rev=374311=374310=374311=diff
==
--- lldb/trunk/source/Utility/DataExtractor.cpp (original)
+++ lldb/trunk/source/Utility/DataExtractor.cpp Thu Oct 10 04:15:38 2019
@@ -816,26 +816,25 @@ DataExtractor::CopyByteOrderedData(offse
 // non-zero and there aren't enough available bytes, nullptr will be returned
 // and "offset_ptr" will not be updated.
 const char *DataExtractor::GetCStr(offset_t *offset_ptr) const {
-  const char *cstr = reinterpret_cast(PeekData(*offset_ptr, 1));
-  if (cstr) {
-const char *cstr_end = cstr;
-const char *end = reinterpret_cast(m_end);
-while (cstr_end < end && *cstr_end)
-  ++cstr_end;
+  const char *start = reinterpret_cast(PeekData(*offset_ptr, 1));
+  // Already at the end of the data.
+  if (!start)
+return nullptr;
 
-// Now we are either at the end of the data or we point to the
-// NULL C string terminator with cstr_end...
-if (*cstr_end == '\0') {
-  // Advance the offset with one extra byte for the NULL terminator
-  *offset_ptr += (cstr_end - cstr + 1);
-  return cstr;
-}
+  const char *end = reinterpret_cast(m_end);
 
-// We reached the end of the data without finding a NULL C string
-// terminator. Fall through and return nullptr otherwise anyone that would
-// have used the result as a C string can wander into unknown memory...
-  }
-  return nullptr;
+  // Check all bytes for a null terminator that terminates a C string.
+  const char *terminator_or_end = std::find(start, end, '\0');
+
+  // We didn't find a null terminator, so return nullptr to indicate that there
+  // is no valid C string at that offset.
+  if (terminator_or_end == end)
+return nullptr;
+
+  // Update offset_ptr for the caller to point to the data behind the
+  // terminator (which is 1 byte long).
+  *offset_ptr += (terminator_or_end - start + 1UL);
+  return start;
 }
 
 // Extracts a NULL terminated C string from the fixed length field of length

Modified: lldb/trunk/unittests/Utility/DataExtractorTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/DataExtractorTest.cpp?rev=374311=374310=374311=diff
==
--- lldb/trunk/unittests/Utility/DataExtractorTest.cpp (original)
+++ lldb/trunk/unittests/Utility/DataExtractorTest.cpp Thu Oct 10 04:15:38 2019
@@ -49,6 +49,51 @@ TEST(DataExtractorTest, PeekData) {
   EXPECT_EQ(nullptr, E.PeekData(4, 1));
 }
 
+TEST(DataExtractorTest, GetCStr) {
+  uint8_t buffer[] = {'X', 'f', 'o', 'o', '\0'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_STREQ("foo", E.GetCStr());
+  EXPECT_EQ(5U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrEmpty) {
+  uint8_t buffer[] = {'X', '\0'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_STREQ("", E.GetCStr());
+  EXPECT_EQ(2U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrUnterminated) {
+  uint8_t buffer[] = {'X', 'f', 'o', 'o'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_EQ(nullptr, E.GetCStr());
+  EXPECT_EQ(1U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrAtEnd) {
+  uint8_t buffer[] = {'X'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_EQ(nullptr, E.GetCStr());
+  EXPECT_EQ(1U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrAtNullOffset) {
+  uint8_t buffer[] = {'f', 'o', 'o', '\0'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 0;
+  EXPECT_STREQ("foo", E.GetCStr());
+  EXPECT_EQ(4U, offset);
+}
+
 TEST(DataExtractorTest, GetMaxU64) {
   uint8_t buffer[] = {0x01,

[Lldb-commits] [lldb] r374307 - [lldb][NFC] Use llvm::all_of instead of std::all_of in CppModuleConfiguration

2019-10-10 Thread Raphael Isemann via lldb-commits 
Author: teemperor
Date: Thu Oct 10 03:56:12 2019
New Revision: 374307

URL: http://llvm.org/viewvc/llvm-project?rev=374307=rev
Log:
[lldb][NFC] Use llvm::all_of instead of std::all_of in CppModuleConfiguration

Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp

Modified: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp?rev=374307=374306=374307=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp 
Thu Oct 10 03:56:12 2019
@@ -63,9 +63,9 @@ bool CppModuleConfiguration::hasValidCon
 CppModuleConfiguration::CppModuleConfiguration(
 const FileSpecList _files) {
   // Analyze all files we were given to build the configuration.
-  bool error = !std::all_of(support_files.begin(), support_files.end(),
-std::bind(::analyzeFile,
-  this, std::placeholders::_1));
+  bool error = !llvm::all_of(support_files,
+ std::bind(::analyzeFile,
+   this, std::placeholders::_1));
   // If we have a valid configuration at this point, set the
   // include directories and module list that should be used.
   if (!error && hasValidConfig()) {


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68696: [lldb][NFC] Remove strange bool parameter from Searcher::SearchCallback

2019-10-10 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor added a comment.

Thanks for the explanation! Doesn't seem like removing this breaks Swift, so 
I'll land this.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68696/new/

https://reviews.llvm.org/D68696



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread David CARLIER via Phabricator via lldb-commits 
devnexen added a comment.

In D68723#1703379 , @MaskRay wrote:

> In D68723#1703322 , @devnexen wrote:
>
> > @dim I LGTMed this but would it a real issue for you to take on a less 
> > disruptive approach proposed by MaskRay ?
>
>
> I think we should do what @labath suggested (changing Expected to Optional) 
> if that is not very difficult.


Good point, I missed it. Should be alright to.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68723/new/

https://reviews.llvm.org/D68723



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68773: [lldb] Fix out of bounds read in DataExtractor::GetCStr and add actually unit test that function.

2019-10-10 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor created this revision.
teemperor added a reviewer: labath.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.
Herald added a project: LLDB.

The `if (*cstr_end == '\0')` in the previous code checked if the previous loop 
terminated because it
found a null terminator or because it reached the end of the data. However, in 
the case that we hit
the end of the data before finding a null terminator, `cstr_end` points behind 
the last byte in our
data and `*cstr_end` reads the memory behind the array (which may be 
uninitialised)

This patch just rewrites that function use `std::find` and adds the relevant 
unit tests.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68773

Files:
  lldb/source/Utility/DataExtractor.cpp
  lldb/unittests/Utility/DataExtractorTest.cpp

Index: lldb/unittests/Utility/DataExtractorTest.cpp
===
--- lldb/unittests/Utility/DataExtractorTest.cpp
+++ lldb/unittests/Utility/DataExtractorTest.cpp
@@ -49,6 +49,51 @@
   EXPECT_EQ(nullptr, E.PeekData(4, 1));
 }
 
+TEST(DataExtractorTest, GetCStr) {
+  uint8_t buffer[] = {'X', 'f', 'o', 'o', '\0'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_STREQ("foo", E.GetCStr());
+  EXPECT_EQ(5U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrEmpty) {
+  uint8_t buffer[] = {'X', '\0'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_STREQ("", E.GetCStr());
+  EXPECT_EQ(2U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrUnterminated) {
+  uint8_t buffer[] = {'X', 'f', 'o', 'o'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_EQ(nullptr, E.GetCStr());
+  EXPECT_EQ(1U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrAtEnd) {
+  uint8_t buffer[] = {'X'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 1;
+  EXPECT_EQ(nullptr, E.GetCStr());
+  EXPECT_EQ(1U, offset);
+}
+
+TEST(DataExtractorTest, GetCStrAtNullOffset) {
+  uint8_t buffer[] = {'f', 'o', 'o', '\0'};
+  DataExtractor E(buffer, sizeof buffer, lldb::eByteOrderLittle, 4);
+
+  lldb::offset_t offset = 0;
+  EXPECT_STREQ("foo", E.GetCStr());
+  EXPECT_EQ(4U, offset);
+}
+
 TEST(DataExtractorTest, GetMaxU64) {
   uint8_t buffer[] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08};
   DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
Index: lldb/source/Utility/DataExtractor.cpp
===
--- lldb/source/Utility/DataExtractor.cpp
+++ lldb/source/Utility/DataExtractor.cpp
@@ -816,26 +816,25 @@
 // non-zero and there aren't enough available bytes, nullptr will be returned
 // and "offset_ptr" will not be updated.
 const char *DataExtractor::GetCStr(offset_t *offset_ptr) const {
-  const char *cstr = reinterpret_cast(PeekData(*offset_ptr, 1));
-  if (cstr) {
-const char *cstr_end = cstr;
-const char *end = reinterpret_cast(m_end);
-while (cstr_end < end && *cstr_end)
-  ++cstr_end;
-
-// Now we are either at the end of the data or we point to the
-// NULL C string terminator with cstr_end...
-if (*cstr_end == '\0') {
-  // Advance the offset with one extra byte for the NULL terminator
-  *offset_ptr += (cstr_end - cstr + 1);
-  return cstr;
-}
+  const char *start = reinterpret_cast(PeekData(*offset_ptr, 1));
+  // Already at the end of the data.
+  if (!start)
+return nullptr;
 
-// We reached the end of the data without finding a NULL C string
-// terminator. Fall through and return nullptr otherwise anyone that would
-// have used the result as a C string can wander into unknown memory...
-  }
-  return nullptr;
+  const char *end = reinterpret_cast(m_end);
+
+  // Check all bytes for a null terminator that terminates a C string.
+  const char *terminator_or_end = std::find(start, end, '\0');
+
+  // We didn't find a null terminator, so return nullptr to indicate that there
+  // is no valid C string at that offset.
+  if (terminator_or_end == end)
+return nullptr;
+
+  // Update offset_ptr for the caller to point to the data behind the
+  // terminator (which is 1 byte long).
+  *offset_ptr += (terminator_or_end - start + 1UL);
+  return start;
 }
 
 // Extracts a NULL terminated C string from the fixed length field of length
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread Fangrui Song via Phabricator via lldb-commits 
MaskRay added a comment.

In D68723#1703322 , @devnexen wrote:

> @dim I LGTMed this but would it a real issue for you to take on a less 
> disruptive approach proposed by MaskRay ?


I think we should do what @labath suggested (changing Expected to Optional) if 
that is not very difficult.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68723/new/

https://reviews.llvm.org/D68723



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r374299 - s/@expectedFailure/@expectedFailureAll in TestFileHandle

2019-10-10 Thread Pavel Labath via lldb-commits 
Author: labath
Date: Thu Oct 10 02:52:32 2019
New Revision: 374299

URL: http://llvm.org/viewvc/llvm-project?rev=374299=rev
Log:
s/@expectedFailure/@expectedFailureAll in TestFileHandle

The test isn't using @expectedFailure correctly, which causes weird
errors, at least with python2, at least with linux. Possibly that
function shouldn't even be public as it's main use is as a backed for
other decorators.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py?rev=374299=374298=374299=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
 Thu Oct 10 02:52:32 2019
@@ -12,8 +12,7 @@ from contextlib import contextmanager
 
 import lldb
 from lldbsuite.test import  lldbtest
-from lldbsuite.test.decorators import (
-add_test_categories, skipIf, skipIfWindows, expectedFailure)
+from lldbsuite.test.decorators import *
 
 class OhNoe(Exception):
 pass
@@ -400,7 +399,7 @@ class FileHandleTestCase(lldbtest.TestBa
 
 
 @add_test_categories(['pyapi'])
-@expectedFailure # FIXME IOHandler still using FILE*
+@expectedFailureAll() # FIXME IOHandler still using FILE*
 def test_string_inout(self):
 inf = io.StringIO("help help\n")
 outf = io.StringIO()
@@ -416,7 +415,7 @@ class FileHandleTestCase(lldbtest.TestBa
 
 
 @add_test_categories(['pyapi'])
-@expectedFailure # FIXME IOHandler still using FILE*
+@expectedFailureAll() # FIXME IOHandler still using FILE*
 def test_bytes_inout(self):
 inf = io.BytesIO(b"help help\nhelp b\n")
 outf = io.BytesIO()
@@ -462,7 +461,7 @@ class FileHandleTestCase(lldbtest.TestBa
 
 
 @add_test_categories(['pyapi'])
-@expectedFailure #FIXME bug in ScriptInterpreterPython
+@expectedFailureAll() #FIXME bug in ScriptInterpreterPython
 def test_replace_stdout_with_nonfile(self):
 debugger = self.debugger
 f = io.StringIO()
@@ -550,7 +549,7 @@ class FileHandleTestCase(lldbtest.TestBa
 
 @add_test_categories(['pyapi'])
 @skipIf(py_version=['<', (3,)])
-@expectedFailure # fixme multiple problems with this
+@expectedFailureAll() # fixme multiple problems with this
 def test_string_out(self):
 f = io.StringIO()
 status = self.debugger.SetOutputFile(f)
@@ -560,7 +559,7 @@ class FileHandleTestCase(lldbtest.TestBa
 
 
 @add_test_categories(['pyapi'])
-@expectedFailure # FIXME need FileSP version of SBDebugger::SetErrorFile
+@expectedFailureAll() # FIXME need FileSP version of 
SBDebugger::SetErrorFile
 @skipIf(py_version=['<', (3,)])
 def test_string_error(self):
 f = io.StringIO()
@@ -631,7 +630,7 @@ class FileHandleTestCase(lldbtest.TestBa
 
 
 @add_test_categories(['pyapi'])
-@expectedFailure # FIXME need FileSP version of SBDebugger::SetErrorFile
+@expectedFailureAll() # FIXME need FileSP version of 
SBDebugger::SetErrorFile
 @skipIf(py_version=['<', (3,)])
 def test_file_out(self):
 with open(self.out_filename, 'w') as f:
@@ -655,7 +654,7 @@ class FileHandleTestCase(lldbtest.TestBa
 
 
 @add_test_categories(['pyapi'])
-@expectedFailure # FIXME need FileSP version of SBDebugger::SetErrorFile
+@expectedFailureAll() # FIXME need FileSP version of 
SBDebugger::SetErrorFile
 def test_file_error(self):
 with open(self.out_filename, 'w') as f:
 status = self.debugger.SetErrorFile(f)
@@ -747,7 +746,7 @@ class FileHandleTestCase(lldbtest.TestBa
 
 
 @add_test_categories(['pyapi'])
-@expectedFailure # FIXME need FileSP version of SBDebugger::SetOutputFile
+@expectedFailureAll() # FIXME need FileSP version of 
SBDebugger::SetOutputFile
 def test_close(self):
 debugger = self.debugger
 with open(self.out_filename, 'w') as f:
@@ -768,7 +767,7 @@ class FileHandleTestCase(lldbtest.TestBa
 
 @add_test_categories(['pyapi'])
 @skipIf(py_version=['<', (3,)])
-@expectedFailure # FIXME need FileSP version of SBDebugger::SetOutputFile
+@expectedFailureAll() # FIXME need FileSP version of 
SBDebugger::SetOutputFile
 def test_stdout(self):
 f = io.StringIO()
 status = self.debugger.SetOutputFile(f)
@@ -778,7 +777,7 @@ class FileHandleTestCase(lldbtest.TestBa
 
 
 @add_test_categories(['pyapi'])
-@expectedFailure # FIXME implement SBFile::GetFile
+@expectedFailureAll() # FIXME implement SBFile::GetFile
 @skipIf(py_version=['<', (3,)])
 def test_identity(self):
 


___
lldb-commits mailing list

[Lldb-commits] [PATCH] D68770: [LLDB] [Driver] Use llvm::InitLLVM to do unicode argument conversion on Windows

2019-10-10 Thread Martin Storsjö via Phabricator via lldb-commits 
mstorsjo created this revision.
mstorsjo added reviewers: labath, amccarth, rnk.
Herald added a project: LLDB.

This avoids the currently MSVC specific codepath of using the wchar entry point 
and converting that to utf8.

In the main lldb driver, we had this MSVC specific codepath, but in other 
potentially important entry points (like lldb-server), there's none at all. So 
if this is fine, we should probably add the same InitLLVM call to other lldb 
entry points as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68770

Files:
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -19,8 +19,8 @@
 #include "lldb/API/SBStringList.h"
 
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
@@ -807,23 +807,9 @@
   return llvm::None;
 }
 
-int
-#ifdef _MSC_VER
-wmain(int argc, wchar_t const *wargv[])
-#else
-main(int argc, char const *argv[])
-#endif
+int main(int argc, char const *argv[])
 {
-#ifdef _MSC_VER
-  // Convert wide arguments to UTF-8
-  std::vector argvStrings(argc);
-  std::vector argvPointers(argc);
-  for (int i = 0; i != argc; ++i) {
-llvm::convertWideToUTF8(wargv[i], argvStrings[i]);
-argvPointers[i] = argvStrings[i].c_str();
-  }
-  const char **argv = argvPointers.data();
-#endif
+  llvm::InitLLVM IL(argc, argv);
 
   // Print stack trace on crash.
   llvm::StringRef ToolName = llvm::sys::path::filename(argv[0]);


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -19,8 +19,8 @@
 #include "lldb/API/SBStringList.h"
 
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Process.h"
@@ -807,23 +807,9 @@
   return llvm::None;
 }
 
-int
-#ifdef _MSC_VER
-wmain(int argc, wchar_t const *wargv[])
-#else
-main(int argc, char const *argv[])
-#endif
+int main(int argc, char const *argv[])
 {
-#ifdef _MSC_VER
-  // Convert wide arguments to UTF-8
-  std::vector argvStrings(argc);
-  std::vector argvPointers(argc);
-  for (int i = 0; i != argc; ++i) {
-llvm::convertWideToUTF8(wargv[i], argvStrings[i]);
-argvPointers[i] = argvStrings[i].c_str();
-  }
-  const char **argv = argvPointers.data();
-#endif
+  llvm::InitLLVM IL(argc, argv);
 
   // Print stack trace on crash.
   llvm::StringRef ToolName = llvm::sys::path::filename(argv[0]);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread David CARLIER via Phabricator via lldb-commits 
devnexen added a comment.

@dim I LGTMed this but would it a real issue for you to take on a less 
disruptive approach proposed by MaskRay ?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68723/new/

https://reviews.llvm.org/D68723



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread David CARLIER via Phabricator via lldb-commits 
devnexen added a comment.

In D68723#1703098 , @MaskRay wrote:

> In D68723#1703051 , @devnexen wrote:
>
> > LGTM otherwise.
>
>
> I don't think this change should be reverted. It can just be repaired by 
> adding some `(void)!xxx;`




In D68723#1703098 , @MaskRay wrote:

> In D68723#1703051 , @devnexen wrote:
>
> > LGTM otherwise.
>
>
> I don't think this change should be reverted. It can just be repaired by 
> adding some `(void)!xxx;`


I would prefer this approach indeed but either way I m in that s a real issue 
at the moment since LLVM runtime check is on in the FreeBSD's system.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68723/new/

https://reviews.llvm.org/D68723



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68734: update SBDebugger::SetInputFile() etc to work on native Files

2019-10-10 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 fine to me. The public-consumptionness of the FileSP overloads was also 
discussed in D68546 , and I currently do not 
see a better solution than this one. The way I've resolved this issue for 
myself is to look at the FileSP argument as a placeholder for some hypothetical 
future feature which would allow C++ users to pass in custom file objects into 
lldb (i.e. do the same thing that python can do now). If/when we have that, we 
can replace FileSP with the new thing, and have the python objects typemap to 
that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68734/new/

https://reviews.llvm.org/D68734



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

I don't think having Expected members is a reasonable thing to do. If we 
want to keep the notion of validity explicit (which is consistent with the 
direction lldb is moving in), then I think the members should be Optional. 
The initialization code can take the Expected result, do something with the 
error, and then emplace the result into the Optional member in case of 
success.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68723/new/

https://reviews.llvm.org/D68723



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68454: Fix the unwinding plan augmentation from x86 assembly

2019-10-10 Thread Jaroslav Sevcik via Phabricator via lldb-commits 
jarin added a comment.

Thank you for the review! Could you also possibly commit the change for me?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68454/new/

https://reviews.llvm.org/D68454



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r374289 - [lldb][NFC] Use unique_ptr in DiagnosticManager to express ownership

2019-10-10 Thread Raphael Isemann via lldb-commits 
Author: teemperor
Date: Thu Oct 10 01:30:10 2019
New Revision: 374289

URL: http://llvm.org/viewvc/llvm-project?rev=374289=rev
Log:
[lldb][NFC] Use unique_ptr in DiagnosticManager to express ownership

Modified:
lldb/trunk/include/lldb/Expression/DiagnosticManager.h
lldb/trunk/source/Expression/DiagnosticManager.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/trunk/unittests/Expression/DiagnosticManagerTest.cpp

Modified: lldb/trunk/include/lldb/Expression/DiagnosticManager.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/DiagnosticManager.h?rev=374289=374288=374289=diff
==
--- lldb/trunk/include/lldb/Expression/DiagnosticManager.h (original)
+++ lldb/trunk/include/lldb/Expression/DiagnosticManager.h Thu Oct 10 01:30:10 
2019
@@ -12,6 +12,7 @@
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-types.h"
 
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 
 #include 
@@ -87,7 +88,7 @@ protected:
   uint32_t m_compiler_id; // Compiler-specific diagnostic ID
 };
 
-typedef std::vector DiagnosticList;
+typedef std::vector> DiagnosticList;
 
 class DiagnosticManager {
 public:
@@ -96,33 +97,24 @@ public:
 m_fixed_expression.clear();
   }
 
-  // The diagnostic manager holds a list of diagnostics, which are owned by the
-  // manager.
   const DiagnosticList () { return m_diagnostics; }
 
-  ~DiagnosticManager() {
-for (Diagnostic *diag : m_diagnostics) {
-  delete diag;
-}
-  }
-
   bool HasFixIts() const {
-for (Diagnostic *diag : m_diagnostics) {
-  if (diag->HasFixIts())
-return true;
-}
-return false;
+return llvm::any_of(m_diagnostics,
+[](const std::unique_ptr ) {
+  return diag->HasFixIts();
+});
   }
 
   void AddDiagnostic(llvm::StringRef message, DiagnosticSeverity severity,
  DiagnosticOrigin origin,
  uint32_t compiler_id = LLDB_INVALID_COMPILER_ID) {
-m_diagnostics.push_back(
-new Diagnostic(message, severity, origin, compiler_id));
+m_diagnostics.emplace_back(
+std::make_unique(message, severity, origin, compiler_id));
   }
 
-  void AddDiagnostic(Diagnostic *diagnostic) {
-m_diagnostics.push_back(diagnostic);
+  void AddDiagnostic(std::unique_ptr diagnostic) {
+m_diagnostics.push_back(std::move(diagnostic));
   }
 
   size_t Printf(DiagnosticSeverity severity, const char *format, ...)

Modified: lldb/trunk/source/Expression/DiagnosticManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/DiagnosticManager.cpp?rev=374289=374288=374289=diff
==
--- lldb/trunk/source/Expression/DiagnosticManager.cpp (original)
+++ lldb/trunk/source/Expression/DiagnosticManager.cpp Thu Oct 10 01:30:10 2019
@@ -47,7 +47,7 @@ static const char *StringForSeverity(Dia
 std::string DiagnosticManager::GetString(char separator) {
   std::string ret;
 
-  for (const Diagnostic *diagnostic : Diagnostics()) {
+  for (const auto  : Diagnostics()) {
 ret.append(StringForSeverity(diagnostic->GetSeverity()));
 ret.append(diagnostic->GetMessage());
 ret.push_back(separator);

Modified: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=374289=374288=374289=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
Thu Oct 10 01:30:10 2019
@@ -208,9 +208,8 @@ public:
   // around them.
   std::string stripped_output = llvm::StringRef(m_output).trim();
 
-  ClangDiagnostic *new_diagnostic =
-  new ClangDiagnostic(stripped_output, severity, Info.getID());
-  m_manager->AddDiagnostic(new_diagnostic);
+  auto new_diagnostic = std::make_unique(
+  stripped_output, severity, Info.getID());
 
   // Don't store away warning fixits, since the compiler doesn't have
   // enough context in an expression for the warning to be useful.
@@ -224,6 +223,8 @@ public:
 new_diagnostic->AddFixitHint(fixit);
 }
   }
+
+  m_manager->AddDiagnostic(std::move(new_diagnostic));
 }
   }
 
@@ -1100,8 +1101,8 @@ bool ClangExpressionParser::RewriteExpre
   if (num_diags == 0)
 return false;
 
-  for (const Diagnostic *diag : diagnostic_manager.Diagnostics()) {
-const ClangDiagnostic *diagnostic = llvm::dyn_cast(diag);
+  for (const auto  : diagnostic_manager.Diagnostics()) {
+const auto *diagnostic = llvm::dyn_cast(diag.get());
 if (diagnostic &&

[Lldb-commits] [PATCH] D68755: [test] Use a different module cache for Shell and API tests.

2019-10-10 Thread Pavel Labath via Phabricator via lldb-commits 
labath added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/make/Makefile.rules:316
 
-# Use a shared module cache when building in the default test build directory.
-CLANG_MODULE_CACHE_DIR := $(shell echo "$(BUILDDIR)" | sed 
$(QUOTE)s/lldb-test-build.noindex.*/lldb-test-build.noindex\/module-cache-clang/$(QUOTE))

JDevlieghere wrote:
> We should have a fallback here that defaults to `$(BUILDDIR)/module-cache` if 
> the variable is not set in the environment.
Is that wise, given D68731 tries to move away from autodetecting stuff in 
`make`?

What I think would be more useful is to pass this variable via the command like 
instead of the environment. That way it will be visible in the `make` 
invocation that dotest prints and the exact make invocation can be reproduced 
by copy-pasting. It shouldn't be even hard to do that -- you'd just need 
builder_base.py to fetch this from the `configuration` object and inject it 
into the make arguments.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68755/new/

https://reviews.llvm.org/D68755



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68731: Remove CC autodetection from Makefile.rules

2019-10-10 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.

Yay


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68731/new/

https://reviews.llvm.org/D68731



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68750: Implement serialization and deserialization of scripted points

2019-10-10 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor added a comment.

LGTM, thanks a lot!


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68750/new/

https://reviews.llvm.org/D68750



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68737: SBFile::GetFile: convert SBFile back into python native files.

2019-10-10 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

It looks pretty straight-forward. The two main comments I have are:

- implement rtti to get rid of the `GetPythonObject` method on the base class
- it looks like the implementations of new apis you're adding (GetOptions, 
mainly) have to go out of their way to ignore errors, only for their callers to 
synthesize new Error objects. It seems like it would be better to just use 
Expecteds throughout.




Comment at: lldb/include/lldb/Host/File.h:56
   static bool DescriptorIsValid(int descriptor) { return descriptor >= 0; };
+  static const char *GetStreamOpenModeFromOptions(uint32_t options);
 

change the argument type to OpenOptions. For some reason, lldb has historically 
used integer types for passing enumerations around, but we're now slowly 
changing that..



Comment at: lldb/include/lldb/Host/File.h:316
+  ///The PyObject* that this File wraps, or NULL.
+  virtual void *GetPythonObject() const;
+

We've spend a lot of time removing python from the lowest layers, and this 
let's it back in through the back door. It would be way better to add support 
for llvm-style rtti to this class, so that the python code can `dyn_cast` to a 
`PythonFile` when it needs to (re)construct the python object. You can take a 
look at CPPLanguageRuntime::classof  (and friend) for how to implement this in 
a plugin-friendly manner.



Comment at: lldb/include/lldb/Host/File.h:328
+  ///OpenOptions flags for this file, or 0 if unknown.
+  virtual uint32_t GetOptions() const;
+

How about returning Expected from here?



Comment at: lldb/include/lldb/Host/File.h:330
+
+  const char *GetOpenMode() const {
+return GetStreamOpenModeFromOptions(GetOptions());

and Expected from here?



Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:776
 @add_test_categories(['pyapi'])
-@expectedFailure # FIXME implement SBFile::GetFile
 @skipIf(py_version=['<', (3,)])

So, if I understand correctly, these tests check that you can feed a file 
opened by python into lldb, and then pump it back out to python. Are there any 
tests which do the opposite (have lldb open a file, move it to python and then 
reinject it into lldb)?



Comment at: lldb/scripts/interface/SBFile.i:81
+
+%feature("docstring", "convert this SBFile into a python io.IOBase file 
object");
+FileSP GetFile();

I am somewhat surprised that there's no ownership handling in this patch 
(whereas ownership transfer was a fairly big chunk of the conversion in the 
other direction). Could you maybe explain what are the ownership expectations 
of the file returned through here (and what happens when we close it for 
instance)?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:674-682
+  PythonFile(File , const char *mode = nullptr) {
+auto f = FromFile(file, mode);
+if (f)
+  *this = std::move(f.get());
+else {
+  Reset();
+  llvm::consumeError(f.takeError());

It looks like there's just one caller of this constructor (the "out" typemap 
for FILE*). Can we just inline this stuff there and delete this constructor?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68737/new/

https://reviews.llvm.org/D68737



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68738: update TestRunCommandInterpreterAPI to use SBFile

2019-10-10 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.

Makes sense to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68738/new/

https://reviews.llvm.org/D68738



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread Fangrui Song via Phabricator via lldb-commits 
MaskRay added a comment.

In D68723#1703051 , @devnexen wrote:

> LGTM otherwise.


I don't think this change should be reverted. It can just be repaired by adding 
some `(void)!xxx;`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68723/new/

https://reviews.llvm.org/D68723



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68727: Allow pexpect tests to work in remote testsuite runs

2019-10-10 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.

I don't see harm in adding this, but I am curious about what are you planning 
to use this for. My takeaway from all of the pexpect discussions we've had in 
the past was that pexpect should only be used for stuff that really, really 
needs an interactive terminal to work (i.e., stuff like batch mode, the "gui" 
command, interactive command line editing/libedit stuff, etc.). All of these 
seem like innately host-related stuff, and I am having trouble imagining a 
feature/test that requires an interactive terminal, but only works on remote 
targets.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68727/new/

https://reviews.llvm.org/D68727



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread David CARLIER via Phabricator via lldb-commits 
devnexen accepted this revision.
devnexen added a comment.
This revision is now accepted and ready to land.

LGTM otherwise.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68723/new/

https://reviews.llvm.org/D68723



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread David CARLIER via Phabricator via lldb-commits 
devnexen added inline comments.



Comment at: source/Plugins/Process/FreeBSD/ProcessMonitor.cpp:733
   // Finally, start monitoring the child process for change in state.
-  m_monitor_thread = Host::StartMonitoringChildProcess(
+  auto monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),

nit: Would prefer obvious typing but can go along with it, just a detail.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68723/new/

https://reviews.llvm.org/D68723



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68762: [LLDB] Fix when building with strict LLVM assertion checking on FreeBSD

2019-10-10 Thread David CARLIER via Phabricator via lldb-commits 
devnexen abandoned this revision.
devnexen added a comment.

Just realised there was an existing PR addressing it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68762/new/

https://reviews.llvm.org/D68762



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68762: [LLDB] Fix when building with strict LLVM assertion checking on FreeBSD

2019-10-10 Thread David CARLIER via Phabricator via lldb-commits 
devnexen updated this revision to Diff 224256.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68762/new/

https://reviews.llvm.org/D68762

Files:
  lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp


Index: lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -730,10 +730,12 @@
   }
 
   // Finally, start monitoring the child process for change in state.
+  if (m_monitor_thread)
+m_monitor_thread->Reset();
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread->IsJoinable()) {
+  if (m_monitor_thread && !m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process launch failed.");
 return;
@@ -771,7 +773,7 @@
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread->IsJoinable()) {
+  if (m_monitor_thread && !m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process attach failed.");
 return;
@@ -784,9 +786,11 @@
 void ProcessMonitor::StartLaunchOpThread(LaunchArgs *args, Status ) {
   static const char *g_thread_name = "lldb.process.freebsd.operation";
 
-  if (m_operation_thread->IsJoinable())
+  if (m_operation_thread && m_operation_thread->IsJoinable())
 return;
 
+  if (m_operation_thread)
+m_operation_thread->Reset();
   m_operation_thread =
   ThreadLauncher::LaunchThread(g_thread_name, LaunchOpThread, args);
   if (!m_operation_thread)
@@ -1412,7 +1416,7 @@
 bool ProcessMonitor::WaitForInitialTIDStop(lldb::tid_t tid) { return true; }
 
 void ProcessMonitor::StopOpThread() {
-  if (!m_operation_thread->IsJoinable())
+  if (m_operation && !m_operation_thread->IsJoinable())
 return;
 
   m_operation_thread->Cancel();


Index: lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -730,10 +730,12 @@
   }
 
   // Finally, start monitoring the child process for change in state.
+  if (m_monitor_thread)
+m_monitor_thread->Reset();
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread->IsJoinable()) {
+  if (m_monitor_thread && !m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process launch failed.");
 return;
@@ -771,7 +773,7 @@
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread->IsJoinable()) {
+  if (m_monitor_thread && !m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process attach failed.");
 return;
@@ -784,9 +786,11 @@
 void ProcessMonitor::StartLaunchOpThread(LaunchArgs *args, Status ) {
   static const char *g_thread_name = "lldb.process.freebsd.operation";
 
-  if (m_operation_thread->IsJoinable())
+  if (m_operation_thread && m_operation_thread->IsJoinable())
 return;
 
+  if (m_operation_thread)
+m_operation_thread->Reset();
   m_operation_thread =
   ThreadLauncher::LaunchThread(g_thread_name, LaunchOpThread, args);
   if (!m_operation_thread)
@@ -1412,7 +1416,7 @@
 bool ProcessMonitor::WaitForInitialTIDStop(lldb::tid_t tid) { return true; }
 
 void ProcessMonitor::StopOpThread() {
-  if (!m_operation_thread->IsJoinable())
+  if (m_operation && !m_operation_thread->IsJoinable())
 return;
 
   m_operation_thread->Cancel();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68723: Fix process launch failure on FreeBSD after r365761

2019-10-10 Thread Fangrui Song via Phabricator via lldb-commits 
MaskRay added a comment.

This was probably due to:

Expected(Error Err)
: HasError(true)
  #if LLVM_ENABLE_ABI_BREAKING_CHECKS
  // Expected is unchecked upon construction in Debug builds.
  , Unchecked(true)
  #endif
{
  assert(Err && "Cannot create Expected from Error success value.");
  new (getErrorStorage()) error_type(Err.takePayload());
}

The member initialization makes the members unchecked 
`m_operation_thread(nullptr)`. Adding something like 
`(void)!m_operation_thread` before assignment to `m_operation_thread` is 
probably better.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68723/new/

https://reviews.llvm.org/D68723



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68762: [LLDB] Fix when building with strict LLVM assertion checking on FreeBSD

2019-10-10 Thread David CARLIER via Phabricator via lldb-commits 
devnexen added a comment.

With this I do not get segfault anymore. @dim I just looked up the patch on 
FreeBSD, if this approach is preferred, I ll go along.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68762/new/

https://reviews.llvm.org/D68762



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68762: [LLDB] Fix when building with strict LLVM assertion checking on FreeBSD

2019-10-10 Thread David CARLIER via Phabricator via lldb-commits 
devnexen created this revision.
devnexen added reviewers: dim, jbeich.
devnexen created this object with visibility "All Users".
Herald added subscribers: lldb-commits, JDevlieghere, emaste.
Herald added a project: LLDB.
devnexen added a comment.

With this I do not get segfault anymore. @dim I just looked up the patch on 
FreeBSD, if this approach is preferred, I ll go along.


- For reference https://reviews.llvm.org/rL365761
- Plan is to backport to 9.x version


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68762

Files:
  lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp


Index: lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -730,10 +730,12 @@
   }
 
   // Finally, start monitoring the child process for change in state.
+  if (m_monitor_thread)
+m_monitor_thread->Reset();
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread->IsJoinable()) {
+  if (m_monitor_thread && !m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process launch failed.");
 return;
@@ -771,7 +773,7 @@
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread->IsJoinable()) {
+  if (m_monitor_thread && !m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process attach failed.");
 return;
@@ -784,9 +786,11 @@
 void ProcessMonitor::StartLaunchOpThread(LaunchArgs *args, Status ) {
   static const char *g_thread_name = "lldb.process.freebsd.operation";
 
-  if (m_operation_thread->IsJoinable())
+  if (m_operation_thread && m_operation_thread->IsJoinable())
 return;
 
+  if (m_operation_thread)
+m_operation_thread->Reset();
   m_operation_thread =
   ThreadLauncher::LaunchThread(g_thread_name, LaunchOpThread, args);
   if (!m_operation_thread)


Index: lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -730,10 +730,12 @@
   }
 
   // Finally, start monitoring the child process for change in state.
+  if (m_monitor_thread)
+m_monitor_thread->Reset();
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread->IsJoinable()) {
+  if (m_monitor_thread && !m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process launch failed.");
 return;
@@ -771,7 +773,7 @@
   m_monitor_thread = Host::StartMonitoringChildProcess(
   std::bind(::MonitorCallback, this, _1, _2, _3, _4),
   GetPID(), true);
-  if (!m_monitor_thread->IsJoinable()) {
+  if (m_monitor_thread && !m_monitor_thread->IsJoinable()) {
 error.SetErrorToGenericError();
 error.SetErrorString("Process attach failed.");
 return;
@@ -784,9 +786,11 @@
 void ProcessMonitor::StartLaunchOpThread(LaunchArgs *args, Status ) {
   static const char *g_thread_name = "lldb.process.freebsd.operation";
 
-  if (m_operation_thread->IsJoinable())
+  if (m_operation_thread && m_operation_thread->IsJoinable())
 return;
 
+  if (m_operation_thread)
+m_operation_thread->Reset();
   m_operation_thread =
   ThreadLauncher::LaunchThread(g_thread_name, LaunchOpThread, args);
   if (!m_operation_thread)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

<    1   2