Re: [Lldb-commits] [lldb] r335431 - Make testcase classnames unique

[Pavel Labath via lldb-commits]

BTW, the format of the test trace file names is configurable with a
command line option (--session-file-format). Since we already have
code which enforces that every test file name is unique, we might want
to swap the default format to use the file name instead of the class
name.
On Sun, 24 Jun 2018 at 11:41, Jan Kratochvil via lldb-commits
 wrote:
>
> Author: jankratochvil
> Date: Sun Jun 24 03:36:44 2018
> New Revision: 335431
>
> URL: http://llvm.org/viewvc/llvm-project?rev=335431&view=rev
> Log:
> Make testcase classnames unique
>
> Filenames with test results contain only the class name which makes it more
> difficult to find it if the same class name is present in multiple *.py files.
>
> packages/Python/lldbsuite/test/functionalities/step-avoids-no-debug/TestStepNoDebug.py
> -class ReturnValueTestCase(TestBase):
> +class StepAvoidsNoDebugTestCase(TestBase):
> as ReturnValueTestCase is already present in:
> packages/Python/lldbsuite/test/functionalities/return-value/TestReturnValue.py
>
> packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
> -class CreateDuringStepTestCase(TestBase):
> +class CrashDuringStepTestCase(TestBase):
> as CreateDuringStepTestCase is already present in:
> packages/Python/lldbsuite/test/functionalities/thread/create_during_step/TestCreateDuringStep.py
>
> packages/Python/lldbsuite/test/functionalities/thread/step_until/TestStepUntil.py
> -class TestCStepping(TestBase):
> +class StepUntilTestCase(TestBase):
> as TestCStepping is already present in:
> packages/Python/lldbsuite/test/lang/c/stepping/TestStepAndBreakpoints.py
>
> Modified:
> 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/step-avoids-no-debug/TestStepNoDebug.py
> 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
> 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/step_until/TestStepUntil.py
>
> Modified: 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/step-avoids-no-debug/TestStepNoDebug.py
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/step-avoids-no-debug/TestStepNoDebug.py?rev=335431&r1=335430&r2=335431&view=diff
> ==
> --- 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/step-avoids-no-debug/TestStepNoDebug.py
>  (original)
> +++ 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/step-avoids-no-debug/TestStepNoDebug.py
>  Sun Jun 24 03:36:44 2018
> @@ -15,7 +15,7 @@ from lldbsuite.test.lldbtest import *
>  from lldbsuite.test import lldbutil
>
>
> -class ReturnValueTestCase(TestBase):
> +class StepAvoidsNoDebugTestCase(TestBase):
>
>  mydir = TestBase.compute_mydir(__file__)
>
>
> Modified: 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py?rev=335431&r1=335430&r2=335431&view=diff
> ==
> --- 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
>  (original)
> +++ 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/crash_during_step/TestCrashDuringStep.py
>  Sun Jun 24 03:36:44 2018
> @@ -12,7 +12,7 @@ from lldbsuite.test.lldbtest import *
>  from lldbsuite.test import lldbutil
>
>
> -class CreateDuringStepTestCase(TestBase):
> +class CrashDuringStepTestCase(TestBase):
>
>  mydir = TestBase.compute_mydir(__file__)
>
>
> Modified: 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/step_until/TestStepUntil.py
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/step_until/TestStepUntil.py?rev=335431&r1=335430&r2=335431&view=diff
> ==
> --- 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/step_until/TestStepUntil.py
>  (original)
> +++ 
> lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/step_until/TestStepUntil.py
>  Sun Jun 24 03:36:44 2018
> @@ -11,7 +11,7 @@ from lldbsuite.test.lldbtest import *
>  from lldbsuite.test import lldbutil
>
>
> -class TestCStepping(TestBase):
> +class StepUntilTestCase(TestBase):
>
>  mydir = TestBase.compute_mydir(__file__)
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r335432 - [FileSpec] Always normalize

[Pavel Labath via lldb-commits]

I believe the raison d'être of the needsNormalization function was the
normalization process is not exactly cheap (it constructs a vector of
StringRefs and whatnot), and measurements showed that this actually is
important for performance of lldb
.
On Sun, 24 Jun 2018 at 14:36, Jonas Devlieghere via lldb-commits
 wrote:
>
> Author: jdevlieghere
> Date: Sun Jun 24 06:31:44 2018
> New Revision: 335432
>
> URL: http://llvm.org/viewvc/llvm-project?rev=335432&view=rev
> Log:
> [FileSpec] Always normalize
>
> Removing redundant components from the path seems pretty harmless.
> Rather than checking whether this is necessary and then actually doing
> so, always invoke remove_dots to start with a normalized path.
>
> Modified:
> lldb/trunk/source/Utility/FileSpec.cpp
>
> Modified: lldb/trunk/source/Utility/FileSpec.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/FileSpec.cpp?rev=335432&r1=335431&r2=335432&view=diff
> ==
> --- lldb/trunk/source/Utility/FileSpec.cpp (original)
> +++ lldb/trunk/source/Utility/FileSpec.cpp Sun Jun 24 06:31:44 2018
> @@ -119,100 +119,6 @@ FileSpec::FileSpec(const FileSpec *rhs)
>  //--
>  FileSpec::~FileSpec() {}
>
> -namespace {
> -//--
> -/// Safely get a character at the specified index.
> -///
> -/// @param[in] path
> -/// A full, partial, or relative path to a file.
> -///
> -/// @param[in] i
> -/// An index into path which may or may not be valid.
> -///
> -/// @return
> -///   The character at index \a i if the index is valid, or 0 if
> -///   the index is not valid.
> -//--
> -inline char safeCharAtIndex(const llvm::StringRef &path, size_t i) {
> -  if (i < path.size())
> -return path[i];
> -  return 0;
> -}
> -
> -//--
> -/// Check if a path needs to be normalized.
> -///
> -/// Check if a path needs to be normalized. We currently consider a
> -/// path to need normalization if any of the following are true
> -///  - path contains "/./"
> -///  - path contains "/../"
> -///  - path contains "//"
> -///  - path ends with "/"
> -/// Paths that start with "./" or with "../" are not considered to
> -/// need normalization since we aren't trying to resolve the path,
> -/// we are just trying to remove redundant things from the path.
> -///
> -/// @param[in] path
> -/// A full, partial, or relative path to a file.
> -///
> -/// @return
> -///   Returns \b true if the path needs to be normalized.
> -//--
> -bool needsNormalization(const llvm::StringRef &path) {
> -  if (path.empty())
> -return false;
> -  // We strip off leading "." values so these paths need to be normalized
> -  if (path[0] == '.')
> -return true;
> -  for (auto i = path.find_first_of("\\/"); i != llvm::StringRef::npos;
> -   i = path.find_first_of("\\/", i + 1)) {
> -const auto next = safeCharAtIndex(path, i+1);
> -switch (next) {
> -  case 0:
> -// path separator char at the end of the string which should be
> -// stripped unless it is the one and only character
> -return i > 0;
> -  case '/':
> -  case '\\':
> -// two path separator chars in the middle of a path needs to be
> -// normalized
> -if (i > 0)
> -  return true;
> -++i;
> -break;
> -
> -  case '.': {
> -  const auto next_next = safeCharAtIndex(path, i+2);
> -  switch (next_next) {
> -default: break;
> -case 0: return true; // ends with "/."
> -case '/':
> -case '\\':
> -  return true; // contains "/./"
> -case '.': {
> -  const auto next_next_next = safeCharAtIndex(path, i+3);
> -  switch (next_next_next) {
> -default: break;
> -case 0: return true; // ends with "/.."
> -case '/':
> -case '\\':
> -  return true; // contains "/../"
> -  }
> -  break;
> -}
> -  }
> -}
> -break;
> -
> -  default:
> -break;
> -}
> -  }
> -  return false;
> -}
> -
> -
> -}
>  //--
>  // Assignment operator.
>  //--
> @@ -252,8 +158,7 @@ void FileSpec::SetFile(llvm::StringRef p
>}
>
>// Normalize the path by removing ".", ".." and other redundant components.
> -  if (needsNormalization(resolved))
> -llvm::sys::path::remove_dots(resolved, true, m_style);
> +  llvm::sys::path::remove_dots(resolved

[Lldb-commits] [lldb] r335448 - Revert "[FileSpec] Always normalize"

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Jun 25 03:11:53 2018
New Revision: 335448

URL: http://llvm.org/viewvc/llvm-project?rev=335448&view=rev
Log:
Revert "[FileSpec] Always normalize"

This reverts r335432 because remove_dots() is expensive and measuring
its impact showed an observable performance regression
(https://reviews.llvm.org/D45977#1078510).

Modified:
lldb/trunk/source/Utility/FileSpec.cpp

Modified: lldb/trunk/source/Utility/FileSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/FileSpec.cpp?rev=335448&r1=335447&r2=335448&view=diff
==
--- lldb/trunk/source/Utility/FileSpec.cpp (original)
+++ lldb/trunk/source/Utility/FileSpec.cpp Mon Jun 25 03:11:53 2018
@@ -119,6 +119,100 @@ FileSpec::FileSpec(const FileSpec *rhs)
 //--
 FileSpec::~FileSpec() {}
 
+namespace {
+//--
+/// Safely get a character at the specified index.
+///
+/// @param[in] path
+/// A full, partial, or relative path to a file.
+///
+/// @param[in] i
+/// An index into path which may or may not be valid.
+///
+/// @return
+///   The character at index \a i if the index is valid, or 0 if
+///   the index is not valid.
+//--
+inline char safeCharAtIndex(const llvm::StringRef &path, size_t i) {
+  if (i < path.size())
+return path[i];
+  return 0;
+}
+
+//--
+/// Check if a path needs to be normalized.
+///
+/// Check if a path needs to be normalized. We currently consider a
+/// path to need normalization if any of the following are true
+///  - path contains "/./"
+///  - path contains "/../"
+///  - path contains "//"
+///  - path ends with "/"
+/// Paths that start with "./" or with "../" are not considered to
+/// need normalization since we aren't trying to resolve the path,
+/// we are just trying to remove redundant things from the path.
+///
+/// @param[in] path
+/// A full, partial, or relative path to a file.
+///
+/// @return
+///   Returns \b true if the path needs to be normalized.
+//--
+bool needsNormalization(const llvm::StringRef &path) {
+  if (path.empty())
+return false;
+  // We strip off leading "." values so these paths need to be normalized
+  if (path[0] == '.')
+return true;
+  for (auto i = path.find_first_of("\\/"); i != llvm::StringRef::npos;
+   i = path.find_first_of("\\/", i + 1)) {
+const auto next = safeCharAtIndex(path, i+1);
+switch (next) {
+  case 0:
+// path separator char at the end of the string which should be
+// stripped unless it is the one and only character
+return i > 0;
+  case '/':
+  case '\\':
+// two path separator chars in the middle of a path needs to be
+// normalized
+if (i > 0)
+  return true;
+++i;
+break;
+
+  case '.': {
+  const auto next_next = safeCharAtIndex(path, i+2);
+  switch (next_next) {
+default: break;
+case 0: return true; // ends with "/."
+case '/':
+case '\\':
+  return true; // contains "/./"
+case '.': {
+  const auto next_next_next = safeCharAtIndex(path, i+3);
+  switch (next_next_next) {
+default: break;
+case 0: return true; // ends with "/.."
+case '/':
+case '\\':
+  return true; // contains "/../"
+  }
+  break;
+}
+  }
+}
+break;
+
+  default:
+break;
+}
+  }
+  return false;
+}
+
+
+}
 //--
 // Assignment operator.
 //--
@@ -158,7 +252,8 @@ void FileSpec::SetFile(llvm::StringRef p
   }
 
   // Normalize the path by removing ".", ".." and other redundant components.
-  llvm::sys::path::remove_dots(resolved, true, m_style);
+  if (needsNormalization(resolved))
+llvm::sys::path::remove_dots(resolved, true, m_style);
 
   // Normalize back slashes to forward slashes
   if (m_style == Style::windows)
@@ -175,10 +270,10 @@ void FileSpec::SetFile(llvm::StringRef p
   // Split path into filename and directory. We rely on the underlying char
   // pointer to be nullptr when the components are empty.
   llvm::StringRef filename = llvm::sys::path::filename(resolved, m_style);
-  if (!filename.empty())
+  if(!filename.empty())
 m_filename.SetString(filename);
   llvm::StringRef directory = llvm::sys::path::parent_path(resolved, m_style);
-  if (!directory.empty())
+  if(!directory.empty())
 m_directory.SetString(directory);
 

Re: [Lldb-commits] [lldb] r335432 - [FileSpec] Always normalize

[Jonas Devlieghere via lldb-commits]

Thanks. Reverted in r335448.

On Mon, Jun 25, 2018 at 9:59 AM Pavel Labath  wrote:

> I believe the raison d'être of the needsNormalization function was the
> normalization process is not exactly cheap (it constructs a vector of
> StringRefs and whatnot), and measurements showed that this actually is
> important for performance of lldb
> .
> On Sun, 24 Jun 2018 at 14:36, Jonas Devlieghere via lldb-commits
>  wrote:
> >
> > Author: jdevlieghere
> > Date: Sun Jun 24 06:31:44 2018
> > New Revision: 335432
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=335432&view=rev
> > Log:
> > [FileSpec] Always normalize
> >
> > Removing redundant components from the path seems pretty harmless.
> > Rather than checking whether this is necessary and then actually doing
> > so, always invoke remove_dots to start with a normalized path.
> >
> > Modified:
> > lldb/trunk/source/Utility/FileSpec.cpp
> >
> > Modified: lldb/trunk/source/Utility/FileSpec.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/FileSpec.cpp?rev=335432&r1=335431&r2=335432&view=diff
> >
> ==
> > --- lldb/trunk/source/Utility/FileSpec.cpp (original)
> > +++ lldb/trunk/source/Utility/FileSpec.cpp Sun Jun 24 06:31:44 2018
> > @@ -119,100 +119,6 @@ FileSpec::FileSpec(const FileSpec *rhs)
> >  //--
> >  FileSpec::~FileSpec() {}
> >
> > -namespace {
> > -//--
> > -/// Safely get a character at the specified index.
> > -///
> > -/// @param[in] path
> > -/// A full, partial, or relative path to a file.
> > -///
> > -/// @param[in] i
> > -/// An index into path which may or may not be valid.
> > -///
> > -/// @return
> > -///   The character at index \a i if the index is valid, or 0 if
> > -///   the index is not valid.
> > -//--
> > -inline char safeCharAtIndex(const llvm::StringRef &path, size_t i) {
> > -  if (i < path.size())
> > -return path[i];
> > -  return 0;
> > -}
> > -
> > -//--
> > -/// Check if a path needs to be normalized.
> > -///
> > -/// Check if a path needs to be normalized. We currently consider a
> > -/// path to need normalization if any of the following are true
> > -///  - path contains "/./"
> > -///  - path contains "/../"
> > -///  - path contains "//"
> > -///  - path ends with "/"
> > -/// Paths that start with "./" or with "../" are not considered to
> > -/// need normalization since we aren't trying to resolve the path,
> > -/// we are just trying to remove redundant things from the path.
> > -///
> > -/// @param[in] path
> > -/// A full, partial, or relative path to a file.
> > -///
> > -/// @return
> > -///   Returns \b true if the path needs to be normalized.
> > -//--
> > -bool needsNormalization(const llvm::StringRef &path) {
> > -  if (path.empty())
> > -return false;
> > -  // We strip off leading "." values so these paths need to be
> normalized
> > -  if (path[0] == '.')
> > -return true;
> > -  for (auto i = path.find_first_of("\\/"); i != llvm::StringRef::npos;
> > -   i = path.find_first_of("\\/", i + 1)) {
> > -const auto next = safeCharAtIndex(path, i+1);
> > -switch (next) {
> > -  case 0:
> > -// path separator char at the end of the string which should be
> > -// stripped unless it is the one and only character
> > -return i > 0;
> > -  case '/':
> > -  case '\\':
> > -// two path separator chars in the middle of a path needs to be
> > -// normalized
> > -if (i > 0)
> > -  return true;
> > -++i;
> > -break;
> > -
> > -  case '.': {
> > -  const auto next_next = safeCharAtIndex(path, i+2);
> > -  switch (next_next) {
> > -default: break;
> > -case 0: return true; // ends with "/."
> > -case '/':
> > -case '\\':
> > -  return true; // contains "/./"
> > -case '.': {
> > -  const auto next_next_next = safeCharAtIndex(path, i+3);
> > -  switch (next_next_next) {
> > -default: break;
> > -case 0: return true; // ends with "/.."
> > -case '/':
> > -case '\\':
> > -  return true; // contains "/../"
> > -  }
> > -  break;
> > -}
> > -  }
> > -}
> > -break;
> > -
> > -  default:
> > -break;
> > -}
> > -  }
> > -  return false;
> > -}
> > -
> > -
> > -}
> >  //--
> >  // Assignment operator.
> >  //--

Re: [Lldb-commits] [lldb] r335431 - Make testcase classnames unique

[Jan Kratochvil via lldb-commits]

On Mon, 25 Jun 2018 10:52:57 +0200, Pavel Labath wrote:
> BTW, the format of the test trace file names is configurable with a
> command line option (--session-file-format). Since we already have
> code which enforces that every test file name is unique, we might want
> to swap the default format to use the file name instead of the class
> name.

I have tried to change it but my patch has no effect, thanks for the advice
but I have now a bigger fish to fry.


Thanks,
Jan
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 463c147ff..3dcce5e86 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -41,7 +41,7 @@ set(LLDB_TEST_COMMON_ARGS
   ${CMAKE_BINARY_DIR}/lldb-test-traces
   --build-dir
   ${CMAKE_BINARY_DIR}/lldb-test-build.noindex
-  -S nm
+  -S fm
   -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] [PATCH] D47992: [lldb-mi] Clean up and update a few MI commands.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: tools/lldb-mi/MICmdCmdExec.cpp:139
+  const CMIUtilString 
&rErrMsg(CMIDriver::Instance().GetErrorDescription());
+  this->SetError(CMIUtilString::Format(
+  MIRSRC(IDS_CMD_ERR_SET_NEW_DRIVER_STATE),

any reason for the "this->"? Remove it?



Comment at: tools/lldb-mi/MICmdCmdExec.cpp:141
+  MIRSRC(IDS_CMD_ERR_SET_NEW_DRIVER_STATE),
+  this->m_cmdData.strMiCmd.c_str(),
+  rErrMsg.c_str()));

any reason for the "this->"? Remove it?



Comment at: tools/lldb-mi/MICmdCmdExec.cpp:244
   const CMIUtilString 
&rErrMsg(CMIDriver::Instance().GetErrorDescription());
-  SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_SET_NEW_DRIVER_STATE),
- m_cmdData.strMiCmd.c_str(),
- rErrMsg.c_str()));
+  this->SetError(CMIUtilString::Format(
+  MIRSRC(IDS_CMD_ERR_SET_NEW_DRIVER_STATE),

any reason for the "this->"? Remove it?



Comment at: tools/lldb-mi/MICmdCmdExec.cpp:246
+  MIRSRC(IDS_CMD_ERR_SET_NEW_DRIVER_STATE),
+  this->m_cmdData.strMiCmd.c_str(),
+  rErrMsg.c_str()));

any reason for the "this->"? Remove it?


https://reviews.llvm.org/D47992



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

[Lldb-commits] [PATCH] D48520: [lldb-mi] Re-implement a few MI commands.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

LGTM. Ok when Adrian is happy with testing changes.


https://reviews.llvm.org/D48520



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

[Lldb-commits] [lldb] r335476 - Fix TestThreadExit for gcc&libc++ combo

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Jun 25 07:28:38 2018
New Revision: 335476

URL: http://llvm.org/viewvc/llvm-project?rev=335476&view=rev
Log:
Fix TestThreadExit for gcc&libc++ combo

pseudo_barrier_wait() begins by decrementing an atomic variable. Since
these are always_inline in libc++, there is no line table anchor to
break on before we decrement it. This meant that on gcc we stopped after
the variable has been decremented, which meant that thread2 could have
exited, violating the test setup. On clang this wasn't a problem
because it generated some line table entries for the do{}while(0) loop
in the macro, so we still ended up stopping, before we touched the
variable.

I fix this by adding a dummy statement before the pseudo_barrier_wait()
command and setting the breakpoint there.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_exit/main.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_exit/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_exit/main.cpp?rev=335476&r1=335475&r2=335476&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_exit/main.cpp
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/thread/thread_exit/main.cpp
 Mon Jun 25 07:28:38 2018
@@ -64,7 +64,8 @@ int main ()
 thread_1.join();
 
 // Synchronize with the remaining thread
-pseudo_barrier_wait(g_barrier3);  // Set third breakpoint 
here
+int dummy = 47;   // Set third breakpoint here
+pseudo_barrier_wait(g_barrier3);
 
 // Wait for the second thread to finish
 thread_2.join();


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

[Lldb-commits] [PATCH] D47992: [lldb-mi] Clean up and update a few MI commands.

[Alexander Polyakov via Phabricator via lldb-commits]

apolyakov updated this revision to Diff 152694.
apolyakov added a comment.

Removed unnecessary `this->`.


https://reviews.llvm.org/D47992

Files:
  tools/lldb-mi/MICmdCmdExec.cpp
  tools/lldb-mi/MICmdCmdExec.h

Index: tools/lldb-mi/MICmdCmdExec.h
===
--- tools/lldb-mi/MICmdCmdExec.h
+++ tools/lldb-mi/MICmdCmdExec.h
@@ -66,7 +66,6 @@
 private:
   const CMIUtilString m_constStrArgStart; // StopAtEntry - run to first
   // instruction or main() if specified
-  lldb::SBCommandReturnObject m_lldbResult;
 };
 
 //++
@@ -91,7 +90,6 @@
   bool Acknowledge() override;
   // From CMICmnBase
   /* dtor */ ~CMICmdCmdExecContinue() override;
-
   // Attributes:
 };
 
@@ -121,7 +119,6 @@
 
   // Attributes:
 private:
-  lldb::SBCommandReturnObject m_lldbResult;
   const CMIUtilString m_constStrArgNumber; // Not specified in MI spec but
// Eclipse gives this option
 };
Index: tools/lldb-mi/MICmdCmdExec.cpp
===
--- tools/lldb-mi/MICmdCmdExec.cpp
+++ tools/lldb-mi/MICmdCmdExec.cpp
@@ -126,21 +126,25 @@
   }
 
   lldb::SBProcess process = rSessionInfo.GetTarget().Launch(launchInfo, error);
-  if ((!process.IsValid()) || (error.Fail())) {
+  if (!process.IsValid()) {
 SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_PROCESS),
m_cmdData.strMiCmd.c_str(),
errMsg.GetData()));
 return MIstatus::failure;
   }
 
-  if (!CMIDriver::Instance().SetDriverStateRunningDebugging()) {
-const CMIUtilString &rErrMsg(CMIDriver::Instance().GetErrorDescription());
-SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_SET_NEW_DRIVER_STATE),
-   m_cmdData.strMiCmd.c_str(),
-   rErrMsg.c_str()));
-return MIstatus::failure;
-  }
-  return MIstatus::success;
+  const auto successHandler = [this] {
+if (!CMIDriver::Instance().SetDriverStateRunningDebugging()) {
+  const CMIUtilString &rErrMsg(CMIDriver::Instance().GetErrorDescription());
+  SetError(CMIUtilString::Format(
+  MIRSRC(IDS_CMD_ERR_SET_NEW_DRIVER_STATE),
+  m_cmdData.strMiCmd.c_str(), rErrMsg.c_str()));
+  return MIstatus::failure;
+}
+return MIstatus::success;
+  };
+
+  return HandleSBErrorWithSuccess(error, successHandler);
 }
 
 //++
@@ -156,9 +160,8 @@
 // Throws:  None.
 //--
 bool CMICmdCmdExecRun::Acknowledge() {
-  const CMICmnMIResultRecord miRecordResult(
+  m_miResultRecord = CMICmnMIResultRecord(
   m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Running);
-  m_miResultRecord = miRecordResult;
 
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
   CMICmnLLDBDebugSessionInfo::Instance());
@@ -233,23 +236,21 @@
 // Throws:  None.
 //--
 bool CMICmdCmdExecContinue::Execute() {
-  lldb::SBError error =
-  CMICmnLLDBDebugSessionInfo::Instance().GetProcess().Continue();
- 
-  if (error.Success()) {
+  const auto successHandler = [this] {
 // CODETAG_DEBUG_SESSION_RUNNING_PROG_RECEIVED_SIGINT_PAUSE_PROGRAM
 if (!CMIDriver::Instance().SetDriverStateRunningDebugging()) {
   const CMIUtilString &rErrMsg(CMIDriver::Instance().GetErrorDescription());
-  SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_SET_NEW_DRIVER_STATE),
- m_cmdData.strMiCmd.c_str(),
- rErrMsg.c_str()));
+  SetError(CMIUtilString::Format(
+  MIRSRC(IDS_CMD_ERR_SET_NEW_DRIVER_STATE),
+  m_cmdData.strMiCmd.c_str(), rErrMsg.c_str()));
   return MIstatus::failure;
 }
 return MIstatus::success;
-  }
+  };
 
-  SetError(error.GetCString());
-  return MIstatus::failure;
+  return HandleSBErrorWithSuccess(
+  CMICmnLLDBDebugSessionInfo::Instance().GetProcess().Continue(),
+  successHandler);
 }
 
 //++
@@ -264,9 +265,8 @@
 // Throws:  None.
 //--
 bool CMICmdCmdExecContinue::Acknowledge() {
-  const CMICmnMIResultRecord miRecordResult(
+  m_miResultRecord = CMICmnMIResultRecord(
   m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Running);
-  m_miResultRecord = miRecordResult;
   return MIstatus::success;
 }
 
@@ -358,18 +358,21 @@
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
   CMICmnLLDBDebugSessionInfo::Instance());
 
+  lldb::SBError error;
   if (nThreadId != UINT64_MAX) {
 lldb::SBThread sbThread = rSessionInfo.GetProcess().GetThreadByIndexID(nThreadId);
 if (!sbThread.IsValid()) {
   SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_THREAD_INVALID),
  m_cmdData.strMiCmd.c_str(),
  m_constStrArgThread.c_str()));
   return MIstatus::failure;
 }
-sbThread.StepOver();
-  } else rSessionInfo.GetProcess().GetSelectedThread().StepOver();
+sbThread.StepOve

[Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D48479#1141274, @clayborg wrote:

> Would love to remove the "accept_zeroes" argument everywhere. Too much 
> matching happens in LLDB and we can't have multiple shared libraries claiming 
> zeros as their UUID


A zero UUID is a problem only if it someone put it there to mean "I don't 
know". "Real" UUIDs seem to acknowledge the concept of a nil value 
https://en.wikipedia.org/wiki/Universally_unique_identifier#Nil_UUID, and I 
guess that's what Mac is following. But we're using this class to represent 
other things besides the "real" UUIDs, and for those a zero value is as valid 
as any other number (it's extremely unlikely that a 20-byte hash will come out 
all-zero, but that isn't so much the case for a 32-bit CRC). I think the best 
we can do here is make the choice between the two treatments as explicit as 
possible. I hope the explicitly named constructor functions will be enough to 
achieve that.


https://reviews.llvm.org/D48479



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

[Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 152699.
labath added a comment.

- Use static factory functions instead of the extra argument (the best names I 
could come up with is fromData and fromOptionalData).


https://reviews.llvm.org/D48479

Files:
  include/lldb/Utility/UUID.h
  source/API/SBModuleSpec.cpp
  source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  source/Plugins/Process/minidump/MinidumpParser.cpp
  source/Utility/DataExtractor.cpp
  source/Utility/UUID.cpp
  unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
  unittests/Utility/UUIDTest.cpp

Index: unittests/Utility/UUIDTest.cpp
===
--- unittests/Utility/UUIDTest.cpp
+++ unittests/Utility/UUIDTest.cpp
@@ -15,10 +15,10 @@
 
 TEST(UUIDTest, RelationalOperators) {
   UUID empty;
-  UUID a16("1234567890123456", 16);
-  UUID b16("1234567890123457", 16);
-  UUID a20("12345678901234567890", 20);
-  UUID b20("12345678900987654321", 20);
+  UUID a16 = UUID::fromData("1234567890123456", 16);
+  UUID b16 = UUID::fromData("1234567890123457", 16);
+  UUID a20 = UUID::fromData("12345678901234567890", 20);
+  UUID b20 = UUID::fromData("12345678900987654321", 20);
 
   EXPECT_EQ(empty, empty);
   EXPECT_EQ(a16, a16);
@@ -34,3 +34,40 @@
   EXPECT_LT(a16, b16);
   EXPECT_GT(a20, b20);
 }
+
+TEST(UUIDTest, Validity) {
+  UUID empty;
+  std::vector zeroes(20, 0);
+  UUID a16 = UUID::fromData(zeroes.data(), 16);
+  UUID a20 = UUID::fromData(zeroes.data(), 20);
+  UUID a16_0 = UUID::fromOptionalData(zeroes.data(), 16);
+  UUID a20_0 = UUID::fromOptionalData(zeroes.data(), 20);
+  EXPECT_FALSE(empty);
+  EXPECT_TRUE(a16);
+  EXPECT_TRUE(a20);
+  EXPECT_FALSE(a16_0);
+  EXPECT_FALSE(a20_0);
+}
+
+TEST(UUIDTest, SetFromStringRef) {
+  UUID u;
+  EXPECT_EQ(32, u.SetFromStringRef("404142434445464748494a4b4c4d4e4f"));
+  EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u);
+
+  EXPECT_EQ(36, u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f"));
+  EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u);
+
+  EXPECT_EQ(45, u.SetFromStringRef(
+"40-41-42-43-4445464748494a4b4c4d4e4f-50515253", 20));
+  EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNOPQRS", 20), u);
+
+  EXPECT_EQ(0, u.SetFromStringRef("40-41-42-43-4445464748494a4b4c4d4e4f", 20));
+  EXPECT_EQ(0, u.SetFromStringRef("40x"));
+  EXPECT_EQ(0, u.SetFromStringRef(""));
+  EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNOPQRS", 20), u)
+  << "uuid was changed by failed parse calls";
+
+  EXPECT_EQ(
+  32, u.SetFromStringRef("404142434445464748494a4b4c4d4e4f-50515253", 16));
+  EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16), u);
+}
Index: unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -194,7 +194,8 @@
   ASSERT_EQ(1u, result->size());
   EXPECT_EQ("/foo/bar.so", result.getValue()[0].GetFileSpec().GetPath());
   EXPECT_EQ(triple, result.getValue()[0].GetArchitecture().GetTriple());
-  EXPECT_EQ(UUID("@ABCDEFGHIJKLMNO", 16), result.getValue()[0].GetUUID());
+  EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNO", 16),
+result.getValue()[0].GetUUID());
   EXPECT_EQ(0u, result.getValue()[0].GetObjectOffset());
   EXPECT_EQ(1234u, result.getValue()[0].GetObjectSize());
 }
@@ -218,7 +219,8 @@
   ASSERT_EQ(1u, result->size());
   EXPECT_EQ("/foo/bar.so", result.getValue()[0].GetFileSpec().GetPath());
   EXPECT_EQ(triple, result.getValue()[0].GetArchitecture().GetTriple());
-  EXPECT_EQ(UUID("@ABCDEFGHIJKLMNOPQRS", 20), result.getValue()[0].GetUUID());
+  EXPECT_EQ(UUID::fromData("@ABCDEFGHIJKLMNOPQRS", 20),
+result.getValue()[0].GetUUID());
   EXPECT_EQ(0u, result.getValue()[0].GetObjectOffset());
   EXPECT_EQ(1234u, result.getValue()[0].GetObjectSize());
 }
Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
@@ -21,29 +21,12 @@
 
 using namespace lldb_private;
 
-UUID::UUID() { Clear(); }
+UUID::UUID(llvm::ArrayRef bytes) {
+  if (bytes.size() != 20 && bytes.size() != 16)
+bytes = {};
 
-UUID::UUID(const UUID &rhs) {
-  SetBytes(rhs.m_uuid, rhs.m_num_uuid_bytes);
-}
-
-UUID::UUID(const void *uuid_bytes, uint32_t num_uuid_bytes) {
-  SetBytes(uuid_bytes, num_uuid_bytes);
-}
-
-const UUID &UUID::operator=(const UUID &rhs) {
-  if (this != &rhs) {
-m_num_uuid_bytes = rhs.m_num_uuid_bytes;
-::memcpy(m_uuid, rhs.m_uuid, sizeof(m_uuid));
-  }
-  return *this;
-}
-
-UUID::~UUID() {}
-
-void UUID::Clear() {
-  m_num_uuid_bytes = 16;
-  ::memset(m_uuid, 0, sizeof(m_uuid));
+  m_nu

[Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

We should allow 4 and 8 byte UUIDs as pointed out by inlined comments. This 
means we should probably modify the UUID dumper to handle those cases as well.




Comment at: include/lldb/Utility/UUID.h:109
+
+  uint32_t m_num_uuid_bytes = 0; // Should be 0, 16 or 20
   ValueType m_uuid;

Do we need this comment here? We currently take a 4 byte debug info CRC and 
call it a 16 byte UUID for no reason. Can we remove the need for this comment 
and allow any length?



Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:735-736
   // Use 4 bytes of crc from the .gnu_debuglink section.
   uint32_t uuidt[4] = {gnu_debuglink_crc, 0, 0, 0};
-  uuid.SetBytes(uuidt, sizeof(uuidt));
+  uuid = UUID::fromData(uuidt, sizeof(uuidt));
 } else if (core_notes_crc) {

Should we just save the UUId is 4 bytes long here?



Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:741-742
   // of note segments crc.
   uint32_t uuidt[4] = {g_core_uuid_magic, core_notes_crc, 0, 0};
-  uuid.SetBytes(uuidt, sizeof(uuidt));
+  uuid = UUID::fromData(uuidt, sizeof(uuidt));
 }

Should we just save the UUId is 8 bytes long here?



Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:929
   uint32_t uuidt[4] = {g_core_uuid_magic, core_notes_crc, 0, 0};
-  m_uuid.SetBytes(uuidt, sizeof(uuidt));
+  m_uuid = UUID::fromData(uuidt, sizeof(uuidt));
 }

Should we just save the UUId is 8 bytes long here?



Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:938
   uint32_t uuidt[4] = {m_gnu_debuglink_crc, 0, 0, 0};
-  m_uuid.SetBytes(uuidt, sizeof(uuidt));
+  m_uuid = UUID::fromData(uuidt, sizeof(uuidt));
 }

Should we just save the UUId is 4 bytes long here?



Comment at: source/Utility/DataExtractor.cpp:1101
 //--
 void DataExtractor::DumpUUID(Stream *s, offset_t offset) const {
   if (s) {

This should take a UUID byte size as a second parameter and callers should be 
required to specify it explicitly


https://reviews.llvm.org/D48479



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

[Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Factory methods make things much clearer.


https://reviews.llvm.org/D48479



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

[Lldb-commits] [PATCH] D48500: [DWARFASTParser] Remove special cases for `llvm-gcc`

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I suppose this is fine. My main worry is whether llvm-gcc is the *only* 
situation where we rely on these tweaks, but I guess the best way to find out 
is to try removing them.


https://reviews.llvm.org/D48500



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

[Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: include/lldb/Utility/UUID.h:109
+
+  uint32_t m_num_uuid_bytes = 0; // Should be 0, 16 or 20
   ValueType m_uuid;

clayborg wrote:
> Do we need this comment here? We currently take a 4 byte debug info CRC and 
> call it a 16 byte UUID for no reason. Can we remove the need for this comment 
> and allow any length?
I will do that in a follow-up patch. I wanted to keep this one for dealing with 
the whole zero issue.



Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:735-736
   // Use 4 bytes of crc from the .gnu_debuglink section.
   uint32_t uuidt[4] = {gnu_debuglink_crc, 0, 0, 0};
-  uuid.SetBytes(uuidt, sizeof(uuidt));
+  uuid = UUID::fromData(uuidt, sizeof(uuidt));
 } else if (core_notes_crc) {

clayborg wrote:
> Should we just save the UUId is 4 bytes long here?
I will fix this and the other issues in the next patch.



Comment at: source/Utility/DataExtractor.cpp:1101
 //--
 void DataExtractor::DumpUUID(Stream *s, offset_t offset) const {
   if (s) {

clayborg wrote:
> This should take a UUID byte size as a second parameter and callers should be 
> required to specify it explicitly
Hmm.. this is actually unused. Can I just remove it?


https://reviews.llvm.org/D48479



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

Re: [Lldb-commits] [PATCH] D48393: Make DWARFParsing more thread-safe

[Pavel Labath via lldb-commits]

On Fri, 22 Jun 2018 at 18:42, Jim Ingham  wrote:
>
>
>
> > On Jun 22, 2018, at 4:05 AM, Pavel Labath  wrote:
> >
> > On Wed, 20 Jun 2018 at 23:21, Jim Ingham  wrote:
> >>
> >> It is not uncommon that you would be parsing the DWARF for module A and 
> >> find a type that is only known as a forward declaration.  In that case, 
> >> lldb will look through the other Modules' debug info for a real 
> >> definition, parse that and import it into module A.  So you would need to 
> >> suspend one task, start another and wait on its completion.
> >>
> >
> > Taking a step back, I was wondering what are the situations when we do
> > this kind of cross-module debug info importing? I was under the
> > impression that we don't do this kind importing precisely because the
> > module can be shared between multiple targets (and we don't want
> > information from a module which is not loaded in a given target to
> > leak into it just because we happen to have a different target with
> > that module around). I think these kinds of things came up during the
> > discussions about why lldb behaves poorly under -fno-standalone-debug.
> >
> > Am I misunderstanding something here? Because if this is true (parsing
> > debug info in a given module does not access other modules), then
> > maybe we could solve this by just taking some kind of a module lock
> > when parsing debug info in the given module.
>
> We only do this when we are parsing DWARF on behalf of the expression 
> parser's AST context.  That context is not shared among targets, so it 
> doesn't cause the sort of problems you are worried about.  The target AST 
> context looks in itself for definitions and then dispatches to the module's 
> AST contexts to look for, parse and import into itself definitions it doesn't 
> know or hasn't completed yet.  I don't remember whether we pause mid-stream 
> to find unknown types in this case, or do it as a cleanup.  I haven't looked 
> at that code in years.  Greg probably remembers it better than I do.
>
> Note, it is in fact a longstanding bug that, if you are in a frame of a 
> module that only has a forward declaration of type Foo, which exists in full 
> in another module, "expr" will helpfully print the full type, but "frame var" 
> will not.  That's tricky to solve for the reasons you specify.  The target 
> would need to keep a per-module side table of types that augment the ones in 
> the modules and use that somehow.  It's not clear how to make this work, and 
> the bug has languished for most of the life of lldb...  But it is not good 
> for the debugger to have two ways to print values - both of which are in 
> common usage because the "frame variable" path is how all GUI debuggers 
> present local variables - which give different answers for the same value.  
> So I still think the bug is important, and I don't want to make this more 
> difficult.  OTOH, however we solve it, the target-inferred extra types can't 
> go into the Module based AST contexts for the reasons you outline, so it 
> seems like a good assumption that if you are parsing module A, that can't 
> cause you to parse module B.  But we might have to introduce some more 
> distinctions between how we ingest DWARF for the scratch AST contexts and for 
> the module AST contexts.

Thanks for the detailed explanation!

I think this sounds like a promising direction then. Having a
per-module lock for a per-module AST context should be much more
understandable than some kind of multithreaded deadlock-free lazy
evaluation system.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r335489 - [LLDB] Select helper sign comparison fix

[David Carlier via lldb-commits]

Author: devnexen
Date: Mon Jun 25 09:10:20 2018
New Revision: 335489

URL: http://llvm.org/viewvc/llvm-project?rev=335489&view=rev
Log:
[LLDB] Select helper sign comparison fix

The constant could be unsigned thus explicit cast to silent compilation warnings

Reviewers: aprantl

Reviewed By: aprantl

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

Modified:
lldb/trunk/source/Utility/SelectHelper.cpp

Modified: lldb/trunk/source/Utility/SelectHelper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/SelectHelper.cpp?rev=335489&r1=335488&r2=335489&view=diff
==
--- lldb/trunk/source/Utility/SelectHelper.cpp (original)
+++ lldb/trunk/source/Utility/SelectHelper.cpp Mon Jun 25 09:10:20 2018
@@ -109,8 +109,8 @@ lldb_private::Status SelectHelper::Selec
 pair.second.PrepareForSelect();
 const lldb::socket_t fd = pair.first;
 #if !defined(__APPLE__) && !defined(_MSC_VER)
-lldbassert(fd < FD_SETSIZE);
-if (fd >= FD_SETSIZE) {
+lldbassert(fd < static_cast(FD_SETSIZE));
+if (fd >= static_cast(FD_SETSIZE)) {
   error.SetErrorStringWithFormat("%i is too large for select()", fd);
   return error;
 }


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

[Lldb-commits] [PATCH] D48479: Represent invalid UUIDs as UUIDs with length zero

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Patch is good. Feel free to remove the DataExtractor::DumpUUID() in a separate 
NFC commit or just remove it in this patch




Comment at: source/Utility/DataExtractor.cpp:1101
 //--
 void DataExtractor::DumpUUID(Stream *s, offset_t offset) const {
   if (s) {

labath wrote:
> clayborg wrote:
> > This should take a UUID byte size as a second parameter and callers should 
> > be required to specify it explicitly
> Hmm.. this is actually unused. Can I just remove it?
Yes, please remove.


https://reviews.llvm.org/D48479



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

[Lldb-commits] [PATCH] D48520: [lldb-mi] Re-implement a few MI commands.

[Alexander Polyakov via Phabricator via lldb-commits]

apolyakov updated this revision to Diff 152715.
apolyakov added a comment.

Removed duplicated and error prone(without --synchronous option) testing.


https://reviews.llvm.org/D48520

Files:
  lit/tools/lldb-mi/exec/exec-finish.test
  lit/tools/lldb-mi/exec/exec-interrupt.test
  lit/tools/lldb-mi/exec/exec-next-instruction.test
  lit/tools/lldb-mi/exec/exec-step-instruction.test
  lit/tools/lldb-mi/exec/inputs/main.c
  packages/Python/lldbsuite/test/tools/lldb-mi/control/TestMiExec.py
  tools/lldb-mi/MICmdCmdExec.cpp
  tools/lldb-mi/MICmdCmdExec.h

Index: tools/lldb-mi/MICmdCmdExec.h
===
--- tools/lldb-mi/MICmdCmdExec.h
+++ tools/lldb-mi/MICmdCmdExec.h
@@ -33,8 +33,6 @@
 #pragma once
 
 // Third party headers:
-#include "lldb/API/SBCommandReturnObject.h"
-
 // In-house headers:
 #include "MICmdBase.h"
 
@@ -90,7 +88,6 @@
   bool Acknowledge() override;
   // From CMICmnBase
   /* dtor */ ~CMICmdCmdExecContinue() override;
-  // Attributes:
 };
 
 //++
@@ -179,7 +176,6 @@
 
   // Attributes:
 private:
-  lldb::SBCommandReturnObject m_lldbResult;
   const CMIUtilString m_constStrArgNumber; // Not specified in MI spec but
// Eclipse gives this option
 };
@@ -210,7 +206,6 @@
 
   // Attributes:
 private:
-  lldb::SBCommandReturnObject m_lldbResult;
   const CMIUtilString m_constStrArgNumber; // Not specified in MI spec but
// Eclipse gives this option
 };
@@ -238,10 +233,6 @@
   bool ParseArgs() override;
   // From CMICmnBase
   /* dtor */ ~CMICmdCmdExecFinish() override;
-
-  // Attributes:
-private:
-  lldb::SBCommandReturnObject m_lldbResult;
 };
 
 // CODETAG_DEBUG_SESSION_RUNNING_PROG_RECEIVED_SIGINT_PAUSE_PROGRAM
@@ -270,10 +261,6 @@
   bool Acknowledge() override;
   // From CMICmnBase
   /* dtor */ ~CMICmdCmdExecInterrupt() override;
-
-  // Attributes:
-private:
-  lldb::SBCommandReturnObject m_lldbResult;
 };
 
 //++
Index: tools/lldb-mi/MICmdCmdExec.cpp
===
--- tools/lldb-mi/MICmdCmdExec.cpp
+++ tools/lldb-mi/MICmdCmdExec.cpp
@@ -599,14 +599,23 @@
 
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
   CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBDebugger &rDebugger = rSessionInfo.GetDebugger();
-  CMIUtilString strCmd("thread step-inst-over");
-  if (nThreadId != UINT64_MAX)
-strCmd += CMIUtilString::Format(" %llu", nThreadId);
-  rDebugger.GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult,
-  false);
 
-  return MIstatus::success;
+  lldb::SBError error;
+  if (nThreadId != UINT64_MAX) {
+lldb::SBThread sbThread =
+rSessionInfo.GetProcess().GetThreadByIndexID(nThreadId);
+if (!sbThread.IsValid()) {
+  SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_THREAD_INVALID),
+ m_cmdData.strMiCmd.c_str(),
+ m_constStrArgThread.c_str()));
+  return MIstatus::failure;
+}
+sbThread.StepInstruction(true, error);
+  } else
+rSessionInfo.GetProcess().GetSelectedThread().StepInstruction(
+true, error);
+
+  return HandleSBError(error);
 }
 
 //++
@@ -621,21 +630,8 @@
 // Throws:  None.
 //--
 bool CMICmdCmdExecNextInstruction::Acknowledge() {
-  if (m_lldbResult.GetErrorSize() > 0) {
-const char *pLldbErr = m_lldbResult.GetError();
-MIunused(pLldbErr);
-const CMICmnMIValueConst miValueConst(m_lldbResult.GetError());
-const CMICmnMIValueResult miValueResult("message", miValueConst);
-const CMICmnMIResultRecord miRecordResult(
-m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Error,
-miValueResult);
-m_miResultRecord = miRecordResult;
-  } else {
-const CMICmnMIResultRecord miRecordResult(
-m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Running);
-m_miResultRecord = miRecordResult;
-  }
-
+  m_miResultRecord = CMICmnMIResultRecord(
+  m_cmdData.strMiCmdToken, CMICmnMIResultRecord::eResultClass_Running);
   return MIstatus::success;
 }
 
@@ -727,14 +723,23 @@
 
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
   CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBDebugger &rDebugger = rSessionInfo.GetDebugger();
-  CMIUtilString strCmd("thread step-inst");
-  if (nThreadId != UINT64_MAX)
-strCmd += CMIUtilString::Format(" %llu", nThreadId);
-  rDebugger.GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult,
-  false);
 
-  return MIstatus::success;
+  lldb::SBError error;
+  if (nThreadId != UINT64_MAX) {
+lldb::SBThread sbThread =
+rSessionInfo.GetProcess().GetThreadByIndexID(nThreadId);
+if (!sbThread.IsValid()) {
+  SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_THREAD_INVALID),
+ m_cmdData.strMiCmd.c_str(),

[Lldb-commits] [PATCH] D48520: [lldb-mi] Re-implement a few MI commands.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: 
packages/Python/lldbsuite/test/tools/lldb-mi/control/TestMiExec.py:228
-# Test that --thread is optional
-self.runCmd("-exec-next-instruction --frame 0")
-self.expect("\^running")

Is this combination of --exec-next and --frame tested anywhere in the new tests?


https://reviews.llvm.org/D48520



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

[Lldb-commits] [PATCH] D48520: [lldb-mi] Re-implement a few MI commands.

[Alexander Polyakov via Phabricator via lldb-commits]

apolyakov added inline comments.



Comment at: 
packages/Python/lldbsuite/test/tools/lldb-mi/control/TestMiExec.py:228
-# Test that --thread is optional
-self.runCmd("-exec-next-instruction --frame 0")
-self.expect("\^running")

aprantl wrote:
> Is this combination of --exec-next and --frame tested anywhere in the new 
> tests?
No. `-exec-...` commands don't have the `--frame` option at all as I know.


https://reviews.llvm.org/D48520



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

[Lldb-commits] [lldb] r335541 - Implement new methods for handling an error in MI commands.

[Alexander Polyakov via lldb-commits]

Author: apolyakov
Date: Mon Jun 25 15:01:44 2018
New Revision: 335541

URL: http://llvm.org/viewvc/llvm-project?rev=335541&view=rev
Log:
Implement new methods for handling an error in MI commands.

Summary:
The new methods take SBError object and call handler,
specified by user, depending on SBError status.

Reviewers: aprantl, clayborg, labath

Reviewed By: aprantl, clayborg

Subscribers: ki.stfu, lldb-commits

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

Modified:
lldb/trunk/tools/lldb-mi/MICmdBase.cpp
lldb/trunk/tools/lldb-mi/MICmdBase.h

Modified: lldb/trunk/tools/lldb-mi/MICmdBase.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdBase.cpp?rev=335541&r1=335540&r2=335541&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmdBase.cpp (original)
+++ lldb/trunk/tools/lldb-mi/MICmdBase.cpp Mon Jun 25 15:01:44 2018
@@ -214,6 +214,64 @@ void CMICmdBase::SetError(const CMIUtilS
 
 //++
 
//
+// Details: Short cut function to check MI command's execute status and
+//  set an error in case of failure.
+// Type:Method.
+// Args:error - (R) Error description object.
+//  successHandler - (R) function describing actions to execute
+//  in case of success state of passed SBError object.
+//  errorHandler - (R) function describing actions to execute
+//  in case of fail status of passed SBError object.
+// Return:  bool.
+// Throws:  None.
+//--
+bool CMICmdBase::HandleSBError(const lldb::SBError &error,
+   const std::function &successHandler,
+   const std::function &errorHandler) {
+  if (error.Success())
+return successHandler();
+
+  SetError(error.GetCString());
+  errorHandler();
+  return MIstatus::failure;
+}
+
+//++
+//
+// Details: Short cut function to check MI command's execute status and
+//  call specified handler function for success case.
+// Type:Method.
+// Args:error - (R) Error description object.
+//  successHandler - (R) function describing actions to execute
+//  in case of success state of passed SBError object.
+// Return:  bool.
+// Throws:  None.
+//--
+bool CMICmdBase::HandleSBErrorWithSuccess(
+const lldb::SBError &error,
+const std::function &successHandler) {
+  return HandleSBError(error, successHandler);
+}
+
+//++
+//
+// Details: Short cut function to check MI command's execute status and
+//  call specified handler function for error case.
+// Type:Method.
+// Args:error - (R) Error description object.
+//  errorHandler - (R) function describing actions to execute
+//  in case of fail status of passed SBError object.
+// Return:  bool.
+// Throws:  None.
+//--
+bool CMICmdBase::HandleSBErrorWithFailure(
+const lldb::SBError &error,
+const std::function &errorHandler) {
+  return HandleSBError(error, [] { return MIstatus::success; }, errorHandler);
+}
+
+//++
+//
 // Details: Ask a command to provide its unique identifier.
 // Type:Method.
 // Args:A unique identifier for this command class.

Modified: lldb/trunk/tools/lldb-mi/MICmdBase.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdBase.h?rev=335541&r1=335540&r2=335541&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmdBase.h (original)
+++ lldb/trunk/tools/lldb-mi/MICmdBase.h Mon Jun 25 15:01:44 2018
@@ -12,6 +12,8 @@
 // C Includes
 // C++ Includes
 // Other libraries and framework includes
+#include "lldb/API/SBError.h"
+
 // Project includes
 #include "MICmdArgSet.h"
 #include "MICmdData.h"
@@ -80,6 +82,14 @@ public:
   // Methods:
 protected:
   void SetError(const CMIUtilString &rErrMsg);
+  bool HandleSBError(const lldb::SBError &error,
+ const std::function &successHandler =
+ [] { return MIstatus::success; },
+ const std::function &errorHandler = [] {});
+  bool HandleSBErrorWithSuccess(const lldb::SBError &error,
+const std::function &successHandler);
+  bool HandleSBErrorWithFailure(const lldb::SBError &error,
+const std::function &errorHandler);
   template  T *GetOption(const CMIUtilString &vStrOptionName);
   bool ParseValidateCmdOptions();
 


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

[Lldb-commits] [PATCH] D48295: Implement new methods for handling an error in MI commands.

[Alexander Polyakov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL335541: Implement new methods for handling an error in MI 
commands. (authored by apolyakov, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48295?vs=152339&id=152785#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48295

Files:
  lldb/trunk/tools/lldb-mi/MICmdBase.cpp
  lldb/trunk/tools/lldb-mi/MICmdBase.h


Index: lldb/trunk/tools/lldb-mi/MICmdBase.cpp
===
--- lldb/trunk/tools/lldb-mi/MICmdBase.cpp
+++ lldb/trunk/tools/lldb-mi/MICmdBase.cpp
@@ -214,6 +214,64 @@
 
 //++
 
//
+// Details: Short cut function to check MI command's execute status and
+//  set an error in case of failure.
+// Type:Method.
+// Args:error - (R) Error description object.
+//  successHandler - (R) function describing actions to execute
+//  in case of success state of passed SBError object.
+//  errorHandler - (R) function describing actions to execute
+//  in case of fail status of passed SBError object.
+// Return:  bool.
+// Throws:  None.
+//--
+bool CMICmdBase::HandleSBError(const lldb::SBError &error,
+   const std::function &successHandler,
+   const std::function &errorHandler) {
+  if (error.Success())
+return successHandler();
+
+  SetError(error.GetCString());
+  errorHandler();
+  return MIstatus::failure;
+}
+
+//++
+//
+// Details: Short cut function to check MI command's execute status and
+//  call specified handler function for success case.
+// Type:Method.
+// Args:error - (R) Error description object.
+//  successHandler - (R) function describing actions to execute
+//  in case of success state of passed SBError object.
+// Return:  bool.
+// Throws:  None.
+//--
+bool CMICmdBase::HandleSBErrorWithSuccess(
+const lldb::SBError &error,
+const std::function &successHandler) {
+  return HandleSBError(error, successHandler);
+}
+
+//++
+//
+// Details: Short cut function to check MI command's execute status and
+//  call specified handler function for error case.
+// Type:Method.
+// Args:error - (R) Error description object.
+//  errorHandler - (R) function describing actions to execute
+//  in case of fail status of passed SBError object.
+// Return:  bool.
+// Throws:  None.
+//--
+bool CMICmdBase::HandleSBErrorWithFailure(
+const lldb::SBError &error,
+const std::function &errorHandler) {
+  return HandleSBError(error, [] { return MIstatus::success; }, errorHandler);
+}
+
+//++
+//
 // Details: Ask a command to provide its unique identifier.
 // Type:Method.
 // Args:A unique identifier for this command class.
Index: lldb/trunk/tools/lldb-mi/MICmdBase.h
===
--- lldb/trunk/tools/lldb-mi/MICmdBase.h
+++ lldb/trunk/tools/lldb-mi/MICmdBase.h
@@ -12,6 +12,8 @@
 // C Includes
 // C++ Includes
 // Other libraries and framework includes
+#include "lldb/API/SBError.h"
+
 // Project includes
 #include "MICmdArgSet.h"
 #include "MICmdData.h"
@@ -80,6 +82,14 @@
   // Methods:
 protected:
   void SetError(const CMIUtilString &rErrMsg);
+  bool HandleSBError(const lldb::SBError &error,
+ const std::function &successHandler =
+ [] { return MIstatus::success; },
+ const std::function &errorHandler = [] {});
+  bool HandleSBErrorWithSuccess(const lldb::SBError &error,
+const std::function &successHandler);
+  bool HandleSBErrorWithFailure(const lldb::SBError &error,
+const std::function &errorHandler);
   template  T *GetOption(const CMIUtilString &vStrOptionName);
   bool ParseValidateCmdOptions();
 


Index: lldb/trunk/tools/lldb-mi/MICmdBase.cpp
===
--- lldb/trunk/tools/lldb-mi/MICmdBase.cpp
+++ lldb/trunk/tools/lldb-mi/MICmdBase.cpp
@@ -214,6 +214,64 @@
 
 //++
 //
+// Details: Short cut function to check MI command's execute status and
+//  set an error in case of failure.
+// Type:Method.
+// Args:error - (R) Error description object.
+//  successHandler - (R) function describing actions to execute
+//  in case of success state of passed SBError object.
+//  errorHandler - (R) function describing actions to ex

[Lldb-commits] [lldb] r335552 - Fix windows build for r335541.

[Alexander Polyakov via lldb-commits]

Author: apolyakov
Date: Mon Jun 25 16:29:52 2018
New Revision: 335552

URL: http://llvm.org/viewvc/llvm-project?rev=335552&view=rev
Log:
Fix windows build for r335541.

I didn't include  header and used std::function.

Modified:
lldb/trunk/tools/lldb-mi/MICmdBase.h

Modified: lldb/trunk/tools/lldb-mi/MICmdBase.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-mi/MICmdBase.h?rev=335552&r1=335551&r2=335552&view=diff
==
--- lldb/trunk/tools/lldb-mi/MICmdBase.h (original)
+++ lldb/trunk/tools/lldb-mi/MICmdBase.h Mon Jun 25 16:29:52 2018
@@ -11,6 +11,8 @@
 
 // C Includes
 // C++ Includes
+#include  // for function
+
 // Other libraries and framework includes
 #include "lldb/API/SBError.h"
 


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

[Lldb-commits] [lldb] r335556 - A little cleanup in ObjectFileMachO::GetSDKVersion.

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Mon Jun 25 16:45:39 2018
New Revision: 335556

URL: http://llvm.org/viewvc/llvm-project?rev=335556&view=rev
Log:
A little cleanup in ObjectFileMachO::GetSDKVersion.  
This method does one of two things:

1. finds a minimum os deployment version # in a Mach-O load
command and saves the three parts in the m_sdk_version, or

2. finds no valid min os version # load command, pushes a 
sentinel value on the m_sdk_version vector so we don't search
the same load commands multiple times.

There was a little bug when we found a load command with 
a version of 0.0.0 - the method would not add anything to
the m_sdk_version vector but would declare that a success.
It would not push the sentinel value to the vector.
There was code later in the method which assumed that
the vector always had a sentinel value, at least, and that
code could crash when this method was called back when
evaluating a Swift expression.  (these version #'s are 
fetched lazily so it wouldn't happen when the object file
was parsed, only when doing an expression that needed
the version #).

 

Modified:
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Modified: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp?rev=335556&r1=33&r2=335556&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Mon Jun 25 
16:45:39 2018
@@ -5757,20 +5757,29 @@ uint32_t ObjectFileMachO::GetSDKVersion(
 m_sdk_versions.push_back();
 m_sdk_versions.push_back(yy);
 m_sdk_versions.push_back(zz);
+success = true;
+  } else {
+GetModule()->ReportWarning(
+"minimum OS version load command with invalid (0) version 
found.");
   }
-  success = true;
 }
   }
   offset = load_cmd_offset + lc.cmdsize;
 }
 
 if (success == false) {
-  // Push an invalid value so we don't keep trying to
+  // Push an invalid value so we don't try to find
+  // the version # again on the next call to this
+  // method.
   m_sdk_versions.push_back(UINT32_MAX);
 }
   }
 
-  if (m_sdk_versions.size() > 1 || m_sdk_versions[0] != UINT32_MAX) {
+  // Legitimate version numbers will have 3 entries pushed
+  // on to m_sdk_versions.  If we only have one value, it's
+  // the sentinel value indicating that this object file
+  // does not have a valid minimum os version #.
+  if (m_sdk_versions.size() > 1) {
 if (versions != NULL && num_versions > 0) {
   for (size_t i = 0; i < num_versions; ++i) {
 if (i < m_sdk_versions.size())


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

[Lldb-commits] Adding a test for split-dwarf

[Puyan Lotfi via lldb-commits]

This is a first draft. Just trying to do some basic breakpoint and line
number checks on a split dwarf compiled binary for now.


Also, is there an LLDB phabricator? I didn't notice it in the project
listing in reviews.llvm.org.

PL
commit 901971ff8bfcc42679eb8113b5f2a251bc0b9094
Author: Puyan Lotfi 
Date:   Mon Jun 25 20:55:36 2018 -0700

Adding some additional testing for split-dwarf.

diff --git a/packages/Python/lldbsuite/test/linux/split-dwo-basics/Makefile b/packages/Python/lldbsuite/test/linux/split-dwo-basics/Makefile
new file mode 100644
index 0..8d0ca6ea3
--- /dev/null
+++ b/packages/Python/lldbsuite/test/linux/split-dwo-basics/Makefile
@@ -0,0 +1,11 @@
+LEVEL := ../../make
+
+C_SOURCES := a.c b.c
+a.o: CFLAGS_EXTRAS += -gsplit-dwarf
+b.o: CFLAGS_EXTRAS += -gsplit-dwarf
+
+include $(LEVEL)/Makefile.rules
+
+.PHONY: clean
+clean::
+	$(RM) -f a.dwo a.o b.o main
diff --git a/packages/Python/lldbsuite/test/linux/split-dwo-basics/TestSplitDwarfBinary.py b/packages/Python/lldbsuite/test/linux/split-dwo-basics/TestSplitDwarfBinary.py
new file mode 100644
index 0..44b131569
--- /dev/null
+++ b/packages/Python/lldbsuite/test/linux/split-dwo-basics/TestSplitDwarfBinary.py
@@ -0,0 +1,63 @@
+""" Testing debugging of a binary with split dwarf (with fission). """
+import os
+import lldb
+import sys
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestSplitDwarfBinary(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+TestBase.setUp(self)
+
+@no_debug_info_test  # Prevent the genaration of the dwarf version of this test
+@add_test_categories(["dwo"])
+@skipUnlessPlatform(["linux"])
+def test_split_dwarf(self):
+"""Test that 'frame variable' works
+for the executable built from two source files compiled
+with/whithout -gsplit-dwarf correspondingly."""
+
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+main_bp = self.target.BreakpointCreateByName("g", "a.out")
+self.assertTrue(main_bp, VALID_BREAKPOINT)
+
+y_bp = self.set_breakpoint('b.c', line_number('b.c', '// line 4'))
+x_bp = self.set_breakpoint('b.c', line_number('b.c', '// line 5'))
+self.assertTrue(y_bp, VALID_BREAKPOINT)
+self.assertTrue(x_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)
+
+frame = self.process.GetThreadAtIndex(0).GetFrameAtIndex(0)
+x = frame.FindVariable("x")
+self.assertTrue(x.IsValid(), "x is not valid")
+y = frame.FindVariable("y")
+self.assertTrue(y.IsValid(), "y is not valid")
+
+xDecl = x.GetDeclaration()
+yDecl = y.GetDeclaration()
+
+self.assertTrue(5 == xDecl.GetLine(), "x is not at expected line (5).")
+self.assertTrue(4 == yDecl.GetLine(), "x is not at expected line (4).")
+
+def set_breakpoint(self, filename, line):
+return lldbutil.run_break_set_by_file_and_line(self, filename, line, num_expected_locations=1, loc_exact=True)
+
+
diff --git a/packages/Python/lldbsuite/test/linux/split-dwo-basics/a.c b/packages/Python/lldbsuite/test/linux/split-dwo-basics/a.c
new file mode 100644
index 0..047e78a9b
--- /dev/null
+++ b/packages/Python/lldbsuite/test/linux/split-dwo-basics/a.c
@@ -0,0 +1,3 @@
+int f() {
+  return 1;
+}
diff --git a/packages/Python/lldbsuite/test/linux/split-dwo-basics/b.c b/packages/Python/lldbsuite/test/linux/split-dwo-basics/b.c
new file mode 100644
index 0..719c3d1e4
--- /dev/null
+++ b/packages/Python/lldbsuite/test/linux/split-dwo-basics/b.c
@@ -0,0 +1,11 @@
+extern int f();
+
+void g() {
+  int y = 14;  // line 4
+  int x = f(); // line 5
+}
+
+int main() {
+  g();
+  return 0;
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits