[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D47801#1123051, @sas wrote:

> Don't we risk creating circular dependencies with this? What happens when a 
> tool depends on liblldb? you'll have theTool depend on liblldb and liblldb 
> depend on theTool.


The same thought has occurred to me, but I believe this should be fine. No 
existing INCLUDE_IN_FRAMEWORK tool links to liblldb, just it's individual 
components, which is fine. And this is probably a state that we should maintain 
for the future.

What I am thinking about is how to reconcile this with the code in 
tools/driver/CMakeLists.txt which tries to do a very similar thing by adding 
dependencies from lldb->{lldb-server,debugserver}. Ideally, we shouldn't need 
to do this sort of thing twice. And indeed, since lldb driver depends on 
liblldb, with this patch the branches we are adding to lldb would already be 
present implicitly. The only catch there is we would need to make this 
dependency unconditional, but that is probably a good thing anyway, as it makes 
the dependency graph more predictable.

So, how about we do this instead:

- rename INCLUDE_IN_FRAMEWORK to something more neutral (USED_BY_LIBLLDB or 
whatever)
- make the liblldb -> tool dependency not conditioned by LLDB_BUILD_FRAMEWORK
- remove the lldb->tool dependencies altogether

WDYT?


https://reviews.llvm.org/D47801



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


[Lldb-commits] [PATCH] D47797: [lldb-mi] Re-implement MI -exec-next command.

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: tools/lldb-mi/MICmdCmdExec.cpp:384
   if (nThreadId != UINT64_MAX)
-strCmd += CMIUtilString::Format(" %llu", nThreadId);
-  rDebugger.GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult,
-  false);
+rSessionInfo.GetProcess().GetThreadByID(nThreadId).StepOver();
+  else

It looks like this branch is not covered by the test. Also, do we care about 
what happens when the client specifies an invalid thread id?
(I'm mainly pointing these out because these seem like they would be hard to 
test in the current framework, the thread ids being unpredictable and all.)


https://reviews.llvm.org/D47797



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


[Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

2018-06-06 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov added a comment.

Thank you!

But it looks like the binary files (*.pdb and *.exe) have not been committed 
properly (they have zero size).


https://reviews.llvm.org/D47708



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


[Lldb-commits] [PATCH] D47812: [lldb] [lit] Do not run Python tests w/ LLDB_DISABLE_PYTHON

2018-06-06 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: JDevlieghere, labath.

Skip all Python-based tests as unsupported when LLDB_DISABLE_PYTHON is
enabled.  Otherwise, those tests simply fail being unable to import lldb
module.


https://reviews.llvm.org/D47812

Files:
  lit/CMakeLists.txt
  lit/Suite/lit.site.cfg.in
  lit/Suite/lldbtest.py


Index: lit/Suite/lldbtest.py
===
--- lit/Suite/lldbtest.py
+++ lit/Suite/lldbtest.py
@@ -37,6 +37,9 @@
 if litConfig.noExecute:
 return lit.Test.PASS, ''
 
+if test.config.lldb_disable_python:
+return (lit.Test.UNSUPPORTED, 'Python module disabled')
+
 if test.config.unsupported:
 return (lit.Test.UNSUPPORTED, 'Test is unsupported')
 
Index: lit/Suite/lit.site.cfg.in
===
--- lit/Suite/lit.site.cfg.in
+++ lit/Suite/lit.site.cfg.in
@@ -13,6 +13,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
 config.dotest_args_str = "@LLDB_DOTEST_ARGS@"
+config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
 
 
 # Additional dotest arguments can be passed to lit by providing a
Index: lit/CMakeLists.txt
===
--- lit/CMakeLists.txt
+++ lit/CMakeLists.txt
@@ -46,6 +46,7 @@
 
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
+  LLDB_DISABLE_PYTHON
   LLVM_ENABLE_ZLIB)
 
 configure_lit_site_cfg(


Index: lit/Suite/lldbtest.py
===
--- lit/Suite/lldbtest.py
+++ lit/Suite/lldbtest.py
@@ -37,6 +37,9 @@
 if litConfig.noExecute:
 return lit.Test.PASS, ''
 
+if test.config.lldb_disable_python:
+return (lit.Test.UNSUPPORTED, 'Python module disabled')
+
 if test.config.unsupported:
 return (lit.Test.UNSUPPORTED, 'Test is unsupported')
 
Index: lit/Suite/lit.site.cfg.in
===
--- lit/Suite/lit.site.cfg.in
+++ lit/Suite/lit.site.cfg.in
@@ -13,6 +13,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
 config.dotest_args_str = "@LLDB_DOTEST_ARGS@"
+config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
 
 
 # Additional dotest arguments can be passed to lit by providing a
Index: lit/CMakeLists.txt
===
--- lit/CMakeLists.txt
+++ lit/CMakeLists.txt
@@ -46,6 +46,7 @@
 
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
+  LLDB_DISABLE_PYTHON
   LLVM_ENABLE_ZLIB)
 
 configure_lit_site_cfg(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D47812: [lldb] [lit] Do not run Python tests w/ LLDB_DISABLE_PYTHON

2018-06-06 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.


https://reviews.llvm.org/D47812



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


[Lldb-commits] [lldb] r334076 - Revert "PDB support of function-level linking and splitted functions"

2018-06-06 Thread Pavel Labath via lldb-commits
Author: labath
Date: Wed Jun  6 02:16:00 2018
New Revision: 334076

URL: http://llvm.org/viewvc/llvm-project?rev=334076&view=rev
Log:
Revert "PDB support of function-level linking and splitted functions"

This reverts commit r334030 because it adds a broken test.

Removed:

lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-function-level-linking.cpp

lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-function-level-linking.exe

lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-function-level-linking.pdb
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-splitted-function.cpp
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-splitted-function.exe
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-splitted-function.pdb
Modified:
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/trunk/unittests/SymbolFile/PDB/CMakeLists.txt
lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp?rev=334076&r1=334075&r2=334076&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Wed Jun  6 
02:16:00 2018
@@ -1571,9 +1571,6 @@ bool SymbolFilePDB::ParseCompileUnitLine
 line_table->AppendLineEntryToSequence(
 sequence.get(), prev_addr + prev_length, prev_line, 0,
 prev_source_idx, false, false, false, false, true);
-
-line_table->InsertSequence(sequence.release());
-sequence.reset(line_table->CreateLineSequenceContainer());
   }
 
   if (ShouldAddLine(match_line, lno, length)) {

Modified: lldb/trunk/unittests/SymbolFile/PDB/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/CMakeLists.txt?rev=334076&r1=334075&r2=334076&view=diff
==
--- lldb/trunk/unittests/SymbolFile/PDB/CMakeLists.txt (original)
+++ lldb/trunk/unittests/SymbolFile/PDB/CMakeLists.txt Wed Jun  6 02:16:00 2018
@@ -17,10 +17,6 @@ add_lldb_unittest(SymbolFilePDBTests
 set(test_inputs
test-pdb.exe
test-pdb.pdb
-   test-pdb-function-level-linking.exe
-   test-pdb-function-level-linking.pdb
-   test-pdb-splitted-function.exe
-   test-pdb-splitted-function.pdb
test-pdb-types.exe
test-pdb-types.pdb)
 

Removed: 
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-function-level-linking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-function-level-linking.cpp?rev=334075&view=auto
==
--- 
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-function-level-linking.cpp 
(original)
+++ 
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-function-level-linking.cpp 
(removed)
@@ -1,17 +0,0 @@
-// Compile with "cl /c /ZI /sdl /EHsc /MTd /permissive-
-// test-pdb-function-level-linking.cpp"
-// Link with "link /debug:full test-pdb-function-level-linking.obj"
-
-#include 
-#include 
-
-std::string foo()
-{
-return "Hello!";
-}
-
-int main()
-{
-auto x = foo();
-return 0;
-}

Removed: 
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-function-level-linking.exe
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-function-level-linking.exe?rev=334075&view=auto
==
(empty)

Removed: 
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-function-level-linking.pdb
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-function-level-linking.pdb?rev=334075&view=auto
==
(empty)

Removed: 
lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-splitted-function.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-splitted-function.cpp?rev=334075&view=auto
==
--- lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-splitted-function.cpp 
(original)
+++ lldb/trunk/unittests/SymbolFile/PDB/Inputs/test-pdb-splitted-function.cpp 
(removed)
@@ -1,27 +0,0 @@
-// Compile with "cl /c /Zi /GL /O2 /EHsc /MTd test-pdb-splitted-function.cpp"
-// Link with "link /debug:full /LTCG /GENPROFILE
-//   test-pdb-splitted-function.obj"
-// Run several times
-// Link with "link /debug:full /LTCG /USEPROFILE
-//   test-pdb-splitted-function.obj"
-
-#include 
-#include 
-
-int main()
-{
-auto b = false;
-for (auto i = 1; i <= 1024; i++)
-{
-if (b)
-{
-std::cout << "Unreachable code" << std::endl;
-   

[Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I have reverted this because of the broken tests.

However, I have to also ask: isn't there better way to test this? (one that 
does not depend on opaque checked-in binaries). On linux, I could check-in a .s 
file which has the line table exactly as I want it and then have the test 
assert that. Is there some suitable intermediate form here?

It seems that lld tests use some yaml form to store pdb's. Is there any chance 
we could do the same?


https://reviews.llvm.org/D47708



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


[Lldb-commits] [lldb] r334080 - [lit] Do not run Python tests w/ LLDB_DISABLE_PYTHON

2018-06-06 Thread Michal Gorny via lldb-commits
Author: mgorny
Date: Wed Jun  6 02:44:14 2018
New Revision: 334080

URL: http://llvm.org/viewvc/llvm-project?rev=334080&view=rev
Log:
[lit] Do not run Python tests w/ LLDB_DISABLE_PYTHON

Skip all Python-based tests as unsupported when LLDB_DISABLE_PYTHON is
enabled.  Otherwise, those tests simply fail being unable to import lldb
module.

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

Modified:
lldb/trunk/lit/CMakeLists.txt
lldb/trunk/lit/Suite/lit.site.cfg.in
lldb/trunk/lit/Suite/lldbtest.py

Modified: lldb/trunk/lit/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/CMakeLists.txt?rev=334080&r1=334079&r2=334080&view=diff
==
--- lldb/trunk/lit/CMakeLists.txt (original)
+++ lldb/trunk/lit/CMakeLists.txt Wed Jun  6 02:44:14 2018
@@ -46,6 +46,7 @@ endif(BUILD_SHARED_LIBS)
 
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
+  LLDB_DISABLE_PYTHON
   LLVM_ENABLE_ZLIB)
 
 configure_lit_site_cfg(

Modified: lldb/trunk/lit/Suite/lit.site.cfg.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lit.site.cfg.in?rev=334080&r1=334079&r2=334080&view=diff
==
--- lldb/trunk/lit/Suite/lit.site.cfg.in (original)
+++ lldb/trunk/lit/Suite/lit.site.cfg.in Wed Jun  6 02:44:14 2018
@@ -13,6 +13,7 @@ config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
 config.dotest_args_str = "@LLDB_DOTEST_ARGS@"
+config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
 
 
 # Additional dotest arguments can be passed to lit by providing a

Modified: lldb/trunk/lit/Suite/lldbtest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Suite/lldbtest.py?rev=334080&r1=334079&r2=334080&view=diff
==
--- lldb/trunk/lit/Suite/lldbtest.py (original)
+++ lldb/trunk/lit/Suite/lldbtest.py Wed Jun  6 02:44:14 2018
@@ -37,6 +37,9 @@ class LLDBTest(TestFormat):
 if litConfig.noExecute:
 return lit.Test.PASS, ''
 
+if test.config.lldb_disable_python:
+return (lit.Test.UNSUPPORTED, 'Python module disabled')
+
 if test.config.unsupported:
 return (lit.Test.UNSUPPORTED, 'Test is unsupported')
 


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


[Lldb-commits] [PATCH] D47812: [lldb] [lit] Do not run Python tests w/ LLDB_DISABLE_PYTHON

2018-06-06 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334080: [lit] Do not run Python tests w/ LLDB_DISABLE_PYTHON 
(authored by mgorny, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47812?vs=150088&id=150096#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47812

Files:
  lldb/trunk/lit/CMakeLists.txt
  lldb/trunk/lit/Suite/lit.site.cfg.in
  lldb/trunk/lit/Suite/lldbtest.py


Index: lldb/trunk/lit/CMakeLists.txt
===
--- lldb/trunk/lit/CMakeLists.txt
+++ lldb/trunk/lit/CMakeLists.txt
@@ -46,6 +46,7 @@
 
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
+  LLDB_DISABLE_PYTHON
   LLVM_ENABLE_ZLIB)
 
 configure_lit_site_cfg(
Index: lldb/trunk/lit/Suite/lldbtest.py
===
--- lldb/trunk/lit/Suite/lldbtest.py
+++ lldb/trunk/lit/Suite/lldbtest.py
@@ -37,6 +37,9 @@
 if litConfig.noExecute:
 return lit.Test.PASS, ''
 
+if test.config.lldb_disable_python:
+return (lit.Test.UNSUPPORTED, 'Python module disabled')
+
 if test.config.unsupported:
 return (lit.Test.UNSUPPORTED, 'Test is unsupported')
 
Index: lldb/trunk/lit/Suite/lit.site.cfg.in
===
--- lldb/trunk/lit/Suite/lit.site.cfg.in
+++ lldb/trunk/lit/Suite/lit.site.cfg.in
@@ -13,6 +13,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
 config.dotest_args_str = "@LLDB_DOTEST_ARGS@"
+config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
 
 
 # Additional dotest arguments can be passed to lit by providing a


Index: lldb/trunk/lit/CMakeLists.txt
===
--- lldb/trunk/lit/CMakeLists.txt
+++ lldb/trunk/lit/CMakeLists.txt
@@ -46,6 +46,7 @@
 
 # the value is not canonicalized within LLVM
 llvm_canonicalize_cmake_booleans(
+  LLDB_DISABLE_PYTHON
   LLVM_ENABLE_ZLIB)
 
 configure_lit_site_cfg(
Index: lldb/trunk/lit/Suite/lldbtest.py
===
--- lldb/trunk/lit/Suite/lldbtest.py
+++ lldb/trunk/lit/Suite/lldbtest.py
@@ -37,6 +37,9 @@
 if litConfig.noExecute:
 return lit.Test.PASS, ''
 
+if test.config.lldb_disable_python:
+return (lit.Test.UNSUPPORTED, 'Python module disabled')
+
 if test.config.unsupported:
 return (lit.Test.UNSUPPORTED, 'Test is unsupported')
 
Index: lldb/trunk/lit/Suite/lit.site.cfg.in
===
--- lldb/trunk/lit/Suite/lit.site.cfg.in
+++ lldb/trunk/lit/Suite/lit.site.cfg.in
@@ -13,6 +13,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.dotest_path = "@LLDB_SOURCE_DIR@/test/dotest.py"
 config.dotest_args_str = "@LLDB_DOTEST_ARGS@"
+config.lldb_disable_python = @LLDB_DISABLE_PYTHON@
 
 
 # Additional dotest arguments can be passed to lit by providing a
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r334088 - [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

2018-06-06 Thread Pavel Labath via lldb-commits
Author: labath
Date: Wed Jun  6 04:35:23 2018
New Revision: 334088

URL: http://llvm.org/viewvc/llvm-project?rev=334088&view=rev
Log:
[DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

Summary:
This patch adds the skeleton for implementing the DWARF v5 name index
class. All of the methods are stubbed out and will be implemented in
subsequent patches. The interesting part of the patch is the addition of
a "ignore-file-indexes" setting to the dwarf plugin which enables a
user to force using manual indexing path in lldb (for example as a
debugging aid). I have also added a test that verifies that file indexes
are used by default.

Reviewers: JDevlieghere, clayborg, jingham

Subscribers: mgorny, mehdi_amini, aprantl, lldb-commits

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

Added:
lldb/trunk/lit/SymbolFile/DWARF/apple-index-is-used.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
Modified:
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Symbol/SymbolVendor.cpp

Modified: lldb/trunk/include/lldb/Symbol/SymbolFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolFile.h?rev=334088&r1=334087&r2=334088&view=diff
==
--- lldb/trunk/include/lldb/Symbol/SymbolFile.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolFile.h Wed Jun  6 04:35:23 2018
@@ -200,6 +200,8 @@ public:
   //--
   virtual void SectionFileAddressesChanged() {}
 
+  virtual void Dump(Stream &s) {}
+
 protected:
   ObjectFile *m_obj_file; // The object file that symbols can be extracted 
from.
   uint32_t m_abilities;

Added: lldb/trunk/lit/SymbolFile/DWARF/apple-index-is-used.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/apple-index-is-used.cpp?rev=334088&view=auto
==
--- lldb/trunk/lit/SymbolFile/DWARF/apple-index-is-used.cpp (added)
+++ lldb/trunk/lit/SymbolFile/DWARF/apple-index-is-used.cpp Wed Jun  6 04:35:23 
2018
@@ -0,0 +1,8 @@
+// Test that we use the apple indexes.
+// RUN: clang %s -g -c -o %t --target=x86_64-apple-macosx
+// RUN: lldb-test symbols %t | FileCheck %s
+
+// CHECK: .apple_names index present
+// CHECK: .apple_types index present
+
+int foo;

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp?rev=334088&r1=334087&r2=334088&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp Wed Jun  6 
04:35:23 2018
@@ -236,5 +236,13 @@ void AppleDWARFIndex::ReportInvalidDIEOf
 }
 
 void AppleDWARFIndex::Dump(Stream &s) {
-  // TODO: Implement dumping.
+  if (m_apple_names_up)
+s.PutCString(".apple_names index present\n");
+  if (m_apple_namespaces_up)
+s.PutCString(".apple_namespaces index present\n");
+  if (m_apple_types_up)
+s.PutCString(".apple_types index present\n");
+  if (m_apple_objc_up)
+s.PutCString(".apple_objc index present\n");
+  // TODO: Dump index contents
 }

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt?rev=334088&r1=334087&r2=334088&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt Wed Jun  6 
04:35:23 2018
@@ -1,5 +1,6 @@
 add_lldb_library(lldbPluginSymbolFileDWARF PLUGIN
   AppleDWARFIndex.cpp
+  DebugNamesDWARFIndex.cpp
   DIERef.cpp
   DWARFAbbreviationDeclaration.cpp
   DWARFASTParserClang.cpp

Added: lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp?rev=334088&view=auto
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp (added)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp Wed Jun 
 6 04:35:23 2018
@@ -0,0 +1,33 @@
+//===-- DebugNamesDWARFIndex.cpp ---*- C++ -*-===//
+//

[Lldb-commits] [PATCH] D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334088: [DWARF] Add (empty) DebugNamesDWARFIndex class and a 
setting to control its use (authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47629

Files:
  lldb/trunk/include/lldb/Symbol/SymbolFile.h
  lldb/trunk/lit/SymbolFile/DWARF/apple-index-is-used.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/trunk/source/Symbol/SymbolVendor.cpp

Index: lldb/trunk/source/Symbol/SymbolVendor.cpp
===
--- lldb/trunk/source/Symbol/SymbolVendor.cpp
+++ lldb/trunk/source/Symbol/SymbolVendor.cpp
@@ -392,6 +392,8 @@
   }
 }
 s->EOL();
+if (m_sym_file_ap)
+  m_sym_file_ap->Dump(*s);
 s->IndentMore();
 m_type_list.Dump(s, show_context);
 
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_library(lldbPluginSymbolFileDWARF PLUGIN
   AppleDWARFIndex.cpp
+  DebugNamesDWARFIndex.cpp
   DIERef.cpp
   DWARFAbbreviationDeclaration.cpp
   DWARFASTParserClang.cpp
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -64,6 +64,7 @@
 #include "DWARFDeclContext.h"
 #include "DWARFFormValue.h"
 #include "DWARFUnit.h"
+#include "DebugNamesDWARFIndex.h"
 #include "LogChannelDWARF.h"
 #include "ManualDWARFIndex.h"
 #include "SymbolFileDWARFDebugMap.h"
@@ -111,11 +112,20 @@
 
 PropertyDefinition g_properties[] = {
 {"comp-dir-symlink-paths", OptionValue::eTypeFileSpecList, true, 0, nullptr,
- nullptr, "If the DW_AT_comp_dir matches any of these paths the symbolic "
-  "links will be resolved at DWARF parse time."},
-{nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, nullptr, nullptr}};
+ nullptr,
+ "If the DW_AT_comp_dir matches any of these paths the symbolic "
+ "links will be resolved at DWARF parse time."},
+{"ignore-file-indexes", OptionValue::eTypeBoolean, true, 0, nullptr,
+ nullptr,
+ "Ignore indexes present in the object files and always index DWARF "
+ "manually."},
+{nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, nullptr, nullptr},
+};
 
-enum { ePropertySymLinkPaths };
+enum {
+  ePropertySymLinkPaths,
+  ePropertyIgnoreIndexes,
+};
 
 class PluginProperties : public Properties {
 public:
@@ -135,6 +145,11 @@
 assert(option_value);
 return option_value->GetCurrentValue();
   }
+
+  bool IgnoreFileIndexes() const {
+return m_collection_sp->GetPropertyAtIndexAsBoolean(
+nullptr, ePropertyIgnoreIndexes, false);
+  }
 };
 
 typedef std::shared_ptr SymbolFileDWARFPropertiesSP;
@@ -432,6 +447,7 @@
 }
 
 void SymbolFileDWARF::InitializeObject() {
+  Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
   ModuleSP module_sp(m_obj_file->GetModule());
   if (module_sp) {
 const SectionList *section_list = module_sp->GetSectionList();
@@ -442,19 +458,38 @@
   m_obj_file->ReadSectionData(section, m_dwarf_data);
   }
 
-  DWARFDataExtractor apple_names, apple_namespaces, apple_types, apple_objc;
-  LoadSectionData(eSectionTypeDWARFAppleNames, apple_names);
-  LoadSectionData(eSectionTypeDWARFAppleNamespaces, apple_namespaces);
-  LoadSectionData(eSectionTypeDWARFAppleTypes, apple_types);
-  LoadSectionData(eSectionTypeDWARFAppleObjC, apple_objc);
-
-  m_index = AppleDWARFIndex::Create(*GetObjectFile()->GetModule(), apple_names,
-apple_namespaces, apple_types, apple_objc,
-get_debug_str_data());
-
-  if (!m_index)
-m_index = llvm::make_unique(*GetObjectFile()->GetModule(),
-  DebugInfo());
+  if (!GetGlobalPluginProperties()->IgnoreFileIndexes()) {
+DWARFDataExtractor apple_names, apple_namespaces, apple_types, apple_objc;
+LoadSectionData(eSectionTypeDWARFAppleNames, apple_names);
+LoadSectionData(eSectionTypeDWARFAppleNamespaces, apple_namespaces);
+LoadSectionData(eSectionTypeDWARFAppleTypes, apple_types);
+LoadSectionData(eSectionTypeDWARFAppleObjC, apple_objc);
+
+m_index = A

[Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

2018-06-06 Thread Aaron Smith via Phabricator via lldb-commits
asmith added a comment.

In https://reviews.llvm.org/D47708#1123286, @aleksandr.urakov wrote:

> Thank you!
>
> But it looks like the binary files (*.pdb and *.exe) have not been committed 
> properly (they have zero size).


I've seen that issue before. I think you have to specify --binary when 
generating the diff.

git diff --no-prefix --binary branch master > patch.file


https://reviews.llvm.org/D47708



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


[Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

2018-06-06 Thread Aleksandr Urakov via Phabricator via lldb-commits
aleksandr.urakov added a comment.

> I think you have to specify --binary when generating the diff.

I have specified --binary when I was generating the patch, and there is encoded 
binaries in the patch file, may be they was somehow cutted out during an upload 
to the Phabricator... I have attached the original patch to this message. 
F6318577: patch.diff 


https://reviews.llvm.org/D47708



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


[Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

2018-06-06 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment.

The problem is that Phabricator doesn't preserve binary in diffs


https://reviews.llvm.org/D47708



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


[Lldb-commits] [PATCH] D47797: [lldb-mi] Re-implement MI -exec-next command.

2018-06-06 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex updated this revision to Diff 150137.
polyakov.alex added a reviewer: labath.
polyakov.alex added a comment.

Added some checks for invalid thread id.


https://reviews.llvm.org/D47797

Files:
  lit/tools/lldb-mi/exec/exec-next.test
  lit/tools/lldb-mi/exec/inputs/main.c
  tools/lldb-mi/MICmdCmdExec.cpp


Index: tools/lldb-mi/MICmdCmdExec.cpp
===
--- tools/lldb-mi/MICmdCmdExec.cpp
+++ tools/lldb-mi/MICmdCmdExec.cpp
@@ -22,6 +22,7 @@
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBThread.h"
 #include "lldb/lldb-enumerations.h"
 
 // In-house headers:
@@ -378,12 +379,18 @@
 
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
   CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBDebugger &rDebugger = rSessionInfo.GetDebugger();
-  CMIUtilString strCmd("thread step-over");
-  if (nThreadId != UINT64_MAX)
-strCmd += CMIUtilString::Format(" %llu", nThreadId);
-  rDebugger.GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult,
-  false);
+
+  if (nThreadId != UINT64_MAX) {
+lldb::SBThread sbThread = 
rSessionInfo.GetProcess().GetThreadByID(nThreadId);
+if (sbThread.IsValid())
+  sbThread.StepOver();
+else {
+  SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_THREAD_INVALID),
+ m_cmdData.strMiCmd.c_str(),
+ m_constStrArgThread.c_str()));
+  return MIstatus::failure;
+}
+  } else rSessionInfo.GetProcess().GetSelectedThread().StepOver();
 
   return MIstatus::success;
 }
Index: lit/tools/lldb-mi/exec/inputs/main.c
===
--- /dev/null
+++ lit/tools/lldb-mi/exec/inputs/main.c
@@ -0,0 +1,4 @@
+int main(void) {
+  int x = 0;
+  return x;
+}
Index: lit/tools/lldb-mi/exec/exec-next.test
===
--- /dev/null
+++ lit/tools/lldb-mi/exec/exec-next.test
@@ -0,0 +1,26 @@
+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# RUN: %cc -o %t %p/inputs/main.c -g
+# RUN: %lldbmi %t < %s | FileCheck %s
+
+# Test lldb-mi -exec-next command.
+
+# Check that we have a valid target created via '%lldbmi %t'.
+# CHECK: ^done
+
+-break-insert main
+# CHECK: ^done,bkpt={number="1"
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
+-exec-next --thread 0
+# Check that exec-next can process the case of invalid thread ID.
+# CHECK: ^error,msg="Command 'exec-next'. Thread ID invalid"
+
+-exec-next
+# Check that exec-next can step over in a selected thread.
+# CHECK: ^running
+# CHECK: *stopped,reason="end-stepping-range"


Index: tools/lldb-mi/MICmdCmdExec.cpp
===
--- tools/lldb-mi/MICmdCmdExec.cpp
+++ tools/lldb-mi/MICmdCmdExec.cpp
@@ -22,6 +22,7 @@
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBThread.h"
 #include "lldb/lldb-enumerations.h"
 
 // In-house headers:
@@ -378,12 +379,18 @@
 
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
   CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBDebugger &rDebugger = rSessionInfo.GetDebugger();
-  CMIUtilString strCmd("thread step-over");
-  if (nThreadId != UINT64_MAX)
-strCmd += CMIUtilString::Format(" %llu", nThreadId);
-  rDebugger.GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult,
-  false);
+
+  if (nThreadId != UINT64_MAX) {
+lldb::SBThread sbThread = rSessionInfo.GetProcess().GetThreadByID(nThreadId);
+if (sbThread.IsValid())
+  sbThread.StepOver();
+else {
+  SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_THREAD_INVALID),
+ m_cmdData.strMiCmd.c_str(),
+ m_constStrArgThread.c_str()));
+  return MIstatus::failure;
+}
+  } else rSessionInfo.GetProcess().GetSelectedThread().StepOver();
 
   return MIstatus::success;
 }
Index: lit/tools/lldb-mi/exec/inputs/main.c
===
--- /dev/null
+++ lit/tools/lldb-mi/exec/inputs/main.c
@@ -0,0 +1,4 @@
+int main(void) {
+  int x = 0;
+  return x;
+}
Index: lit/tools/lldb-mi/exec/exec-next.test
===
--- /dev/null
+++ lit/tools/lldb-mi/exec/exec-next.test
@@ -0,0 +1,26 @@
+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# RUN: %cc -o %t %p/inputs/main.c -g
+# RUN: %lldbmi %t < %s | FileCheck %s
+
+# Test lldb-mi -exec-next command.
+
+# Check that we have a valid target created via '%lldbmi %t'.
+# CHECK: ^done
+
+-break-insert main
+# CHECK: ^done,bkpt={number="1"
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
+-exec-next --thread 0
+# Che

[Lldb-commits] [PATCH] D47797: [lldb-mi] Re-implement MI -exec-next command.

2018-06-06 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added inline comments.



Comment at: tools/lldb-mi/MICmdCmdExec.cpp:384
+  if (nThreadId != UINT64_MAX) {
+lldb::SBThread sbThread = 
rSessionInfo.GetProcess().GetThreadByID(nThreadId);
+if (sbThread.IsValid())

We can't test this branch until we have a way to get thread ID and then store 
it in some variable.


https://reviews.llvm.org/D47797



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


[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 150149.
labath added a comment.

Ok, so it turns out we **were** passing qualified names into the
GetGlobalVariables function. In the apple-tables case we were then searching
based on the basename and completely ignoring the context. This resulted in
incorrect matches being returned (I believe this is the bug Jonas is
investigating right now). In the manual-index case, we weren't doing the search
based on the basename, but things worked because our index also contained
demangled names.

This version of the patch does the following:

- remove context handling from the index classes. The GetGlobalVariables 
function now takes a basename like you suggested.
- in particular, this means that the manual index can now stop storing 
demangled names of global variables. This makes it consistent with the other 
indexes and with the function case, where we removed demangled names a couple 
of weeks ago.
- puts the context extraction into SymbolFileDWARF, so that it is available to 
all indexes.
- implements context filtering ("fixing" Jonas's bug), also in SymbolFileDWARF. 
I am not particularly proud of how this is implemented (substring search), but 
this is the same way that function context pruning is done (cf. 
Module::LookupInfo::Prune). A more sensible framework for context handling 
would be in order, but I consider that out of scope for this patch.


https://reviews.llvm.org/D47781

Files:
  lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp
  lit/SymbolFile/DWARF/find-basic-variable.cpp
  lit/SymbolFile/DWARF/find-qualified-variable.cpp
  source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -27,6 +27,7 @@
 #include "lldb/Utility/Timer.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h"
+#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
@@ -2024,14 +2025,25 @@
   // Remember how many variables are in the list before we search.
   const uint32_t original_size = variables.GetSize();
 
+  llvm::StringRef basename;
+  llvm::StringRef context;
+
+  if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name.GetCString(),
+  context, basename))
+basename = name.GetStringRef();
+
   DIEArray die_offsets;
-  m_index->GetGlobalVariables(name, die_offsets);
+  m_index->GetGlobalVariables(ConstString(basename), die_offsets);
   const size_t num_die_matches = die_offsets.size();
   if (num_die_matches) {
 SymbolContext sc;
 sc.module_sp = m_obj_file->GetModule();
 assert(sc.module_sp);
 
+// Loop invariant: Variables up to this index have been checked for context
+// matches.
+uint32_t pruned_idx = original_size;
+
 bool done = false;
 for (size_t i = 0; i < num_die_matches && !done; ++i) {
   const DIERef &die_ref = die_offsets[i];
@@ -2062,6 +2074,13 @@
 
   ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false,
  &variables);
+  while (pruned_idx < variables.GetSize()) {
+VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
+if (var_sp->GetName().GetStringRef().contains(name.GetStringRef()))
+  ++pruned_idx;
+else
+  variables.RemoveVariableAtIndex(pruned_idx);
+  }
 
   if (variables.GetSize() - original_size >= max_matches)
 done = true;
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -21,7 +21,7 @@
 
   void Preload() override { Index(); }
 
-  void GetGlobalVariables(ConstString name, DIEArray &offsets) override;
+  void GetGlobalVariables(ConstString basename, DIEArray &offsets) override;
   void GetGlobalVariables(const RegularExpression ®ex,
   DIEArray &offsets) override;
   void GetGlobalVariables(const DWARFUnit &cu, DIEArray &offsets) override;
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -345,12 +345,8 @@
 // entries
 if (ma

[Lldb-commits] [PATCH] D47832: DebugNamesDWARFIndex: Add support for partial indexes

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: JDevlieghere, clayborg.
Herald added a subscriber: aprantl.

It possible that a single module has indexed and non-indexed compile
units. In this case, we can use the fast indexed lookup for the first
ones and fall back to the manual index for the others.

This patch implements this functionality by adding a units_to_avoid
argument to the ManualDWARFIndex constructor. Any units present in that
list will be ignored for the purposes of manual index. Individual
DebugNamesDWARFIndex then always consult both the manual fallback index
as well as the index in the .debug_names section.


https://reviews.llvm.org/D47832

Files:
  lit/SymbolFile/DWARF/dwarf5-partial-index.cpp
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h

Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -12,12 +12,15 @@
 
 #include "Plugins/SymbolFile/DWARF/DWARFIndex.h"
 #include "Plugins/SymbolFile/DWARF/NameToDIE.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace lldb_private {
 class ManualDWARFIndex : public DWARFIndex {
 public:
-  ManualDWARFIndex(Module &module, DWARFDebugInfo *debug_info)
-  : DWARFIndex(module), m_debug_info(debug_info) {}
+  ManualDWARFIndex(Module &module, DWARFDebugInfo *debug_info,
+   llvm::DenseSet units_to_avoid = {})
+  : DWARFIndex(module), m_debug_info(debug_info),
+m_units_to_avoid(std::move(units_to_avoid)) {}
 
   void Preload() override { Index(); }
 
@@ -62,6 +65,8 @@
 
   /// Non-null value means we haven't built the index yet.
   DWARFDebugInfo *m_debug_info;
+  /// Which dwarf units should we skip while building the index.
+  llvm::DenseSet m_units_to_avoid;
 
   IndexSet m_set;
 };
Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -32,28 +32,33 @@
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, "%p", static_cast(&debug_info));
 
-  const uint32_t num_compile_units = debug_info.GetNumCompileUnits();
-  if (num_compile_units == 0)
+  std::vector units_to_index;
+  units_to_index.reserve(debug_info.GetNumCompileUnits());
+  for (size_t U = 0; U < debug_info.GetNumCompileUnits(); ++U) {
+DWARFUnit *unit = debug_info.GetCompileUnitAtIndex(U);
+if (!unit)
+  continue;
+if (m_units_to_avoid.find(unit->GetOffset()) != m_units_to_avoid.end())
+  continue;
+units_to_index.push_back(unit);
+  }
+  if (units_to_index.empty())
 return;
 
-  std::vector sets(num_compile_units);
+  std::vector sets(units_to_index.size());
 
   //--
   // Keep memory down by clearing DIEs for any compile units if indexing
   // caused us to load the compile unit's DIEs.
   //--
-  std::vector>
-  clear_cu_dies(num_compile_units);
+  std::vector> clear_cu_dies(
+  units_to_index.size());
   auto parser_fn = [&](size_t cu_idx) {
-DWARFUnit *dwarf_cu = debug_info.GetCompileUnitAtIndex(cu_idx);
-if (dwarf_cu)
-  IndexUnit(*dwarf_cu, sets[cu_idx]);
+IndexUnit(*units_to_index[cu_idx], sets[cu_idx]);
   };
 
-  auto extract_fn = [&debug_info, &clear_cu_dies](size_t cu_idx) {
-DWARFUnit *dwarf_cu = debug_info.GetCompileUnitAtIndex(cu_idx);
-if (dwarf_cu)
-  clear_cu_dies[cu_idx] = dwarf_cu->ExtractDIEsScoped();
+  auto extract_fn = [&units_to_index, &clear_cu_dies](size_t cu_idx) {
+clear_cu_dies[cu_idx] = units_to_index[cu_idx]->ExtractDIEsScoped();
   };
 
   // Create a task runner that extracts dies for each DWARF compile unit in a
@@ -66,12 +71,12 @@
   // to wait until all compile units have been indexed in case a DIE in one
   // compile unit refers to another and the indexes accesses those DIEs.
   //--
-  TaskMapOverInt(0, num_compile_units, extract_fn);
+  TaskMapOverInt(0, units_to_index.size(), extract_fn);
 
   // Now create a task runner that can index each DWARF compile unit in a
   // separate thread so we can index quickly.
 
-  TaskMapOverInt(0, num_compile_units, parser_fn);
+  TaskMapOverInt(0, units_to_index.size(), parser_fn);
 
   auto finalize_fn = [this, &sets](NameToDIE(IndexSet::*index)) {
 NameToDIE &result = m_set.*index;
Index: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
===
--- source/Plugins/Sym

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

2018-06-06 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Much better. Anything we can do to clearly define and simplify the indexers is 
great stuff.


https://reviews.llvm.org/D47781



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


[Lldb-commits] [PATCH] D47832: DebugNamesDWARFIndex: Add support for partial indexes

2018-06-06 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Just one nit you can choose to fix or not.




Comment at: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:39-41
+if (!unit)
+  continue;
+if (m_units_to_avoid.find(unit->GetOffset()) != m_units_to_avoid.end())

Simpler code:
```
if (!unit || m_units_to_avoid.count(unit->GetOffset())
  continue;
```


https://reviews.llvm.org/D47832



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


[Lldb-commits] [PATCH] D47838: [lldb-mi] Re-implement MI -exec-step command.

2018-06-06 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex created this revision.
polyakov.alex added reviewers: aprantl, clayborg, labath, stella.stamenova.
Herald added a subscriber: ki.stfu.

Now -exec-step uses SB API instead of HandleCommand hack.


https://reviews.llvm.org/D47838

Files:
  lit/tools/lldb-mi/exec/exec-step.test
  tools/lldb-mi/MICmdCmdExec.cpp


Index: tools/lldb-mi/MICmdCmdExec.cpp
===
--- tools/lldb-mi/MICmdCmdExec.cpp
+++ tools/lldb-mi/MICmdCmdExec.cpp
@@ -510,14 +510,28 @@
 
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
   CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBDebugger &rDebugger = rSessionInfo.GetDebugger();
-  CMIUtilString strCmd("thread step-in");
-  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().GetThreadByID(nThreadId);
+if (sbThread.IsValid())
+  sbThread.StepInto(nullptr, LLDB_INVALID_LINE_NUMBER, error);
+else {
+  SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_THREAD_INVALID),
+ m_cmdData.strMiCmd.c_str(),
+ m_constStrArgThread.c_str()));
+  return MIstatus::failure;
+}
+  } else rSessionInfo.GetProcess().GetSelectedThread().StepInto(
+ nullptr, LLDB_INVALID_LINE_NUMBER, error);
+
+  if (error.Success())
+return MIstatus::success;
+
+  SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_FNFAILED),
+ m_cmdData.strMiCmd.c_str(),
+ error.GetCString()));
+  return MIstatus::failure;
 }
 
 //++
Index: lit/tools/lldb-mi/exec/exec-step.test
===
--- /dev/null
+++ lit/tools/lldb-mi/exec/exec-step.test
@@ -0,0 +1,26 @@
+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# RUN: %cc -o %t %p/inputs/main.c -g
+# RUN: %lldbmi %t < %s | FileCheck %s
+
+# Test lldb-mi -exec-step command.
+
+# Check that we have a valid target created via '%lldbmi %t'.
+# CHECK: ^done
+
+-break-insert main
+# CHECK: ^done,bkpt={number="1"
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
+-exec-step --thread 0
+# Check that exec-step can process the case of invalid thread ID.
+# CHECK: ^error,msg="Command 'exec-step'. Thread ID invalid"
+
+-exec-step
+# Check that exec-step can step-in in a selected thread.
+# CHECK: ^running
+# CHECK: *stopped,reason="end-stepping-range"


Index: tools/lldb-mi/MICmdCmdExec.cpp
===
--- tools/lldb-mi/MICmdCmdExec.cpp
+++ tools/lldb-mi/MICmdCmdExec.cpp
@@ -510,14 +510,28 @@
 
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
   CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBDebugger &rDebugger = rSessionInfo.GetDebugger();
-  CMIUtilString strCmd("thread step-in");
-  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().GetThreadByID(nThreadId);
+if (sbThread.IsValid())
+  sbThread.StepInto(nullptr, LLDB_INVALID_LINE_NUMBER, error);
+else {
+  SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_THREAD_INVALID),
+ m_cmdData.strMiCmd.c_str(),
+ m_constStrArgThread.c_str()));
+  return MIstatus::failure;
+}
+  } else rSessionInfo.GetProcess().GetSelectedThread().StepInto(
+ nullptr, LLDB_INVALID_LINE_NUMBER, error);
+
+  if (error.Success())
+return MIstatus::success;
+
+  SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_FNFAILED),
+ m_cmdData.strMiCmd.c_str(),
+ error.GetCString()));
+  return MIstatus::failure;
 }
 
 //++
Index: lit/tools/lldb-mi/exec/exec-step.test
===
--- /dev/null
+++ lit/tools/lldb-mi/exec/exec-step.test
@@ -0,0 +1,26 @@
+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# RUN: %cc -o %t %p/inputs/main.c -g
+# RUN: %lldbmi %t < %s | FileCheck %s
+
+# Test lldb-mi -exec-step command.
+
+# Check that we have a valid target created via '%lldbmi %t'.
+# CHECK: ^done
+
+-break-insert main
+# CHECK: ^done,bkpt={number="1"
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
+-exec-step --thread 0
+# Check that exec-step can process the case of invalid thread ID.
+# CHECK: ^error,msg="Command 'exec-step'. Thread ID 

[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

2018-06-06 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

> No existing INCLUDE_IN_FRAMEWORK tool links to liblldb, just it's individual 
> components, which is fine. And this is probably a state that we should 
> maintain for the future.

I think I agree with you here.

> So, how about we do this instead:
> 
> - rename INCLUDE_IN_FRAMEWORK to something more neutral (USED_BY_LIBLLDB or 
> whatever)
> - make the liblldb -> tool dependency not conditioned by LLDB_BUILD_FRAMEWORK
> - remove the lldb->tool dependencies altogether
> 
> WDYT?

I think that this a good short-term solution. My concern is your suggestion 
doesn't reflect the actual dependencies, but conveniently errs on the side of 
"build it if we think we might need it". I'm not against your suggestion, but 
I'd like to see if we can come up with something better.
One idea I had was to introduce another target for the framwork itself, e.g. 
lldbFramework, which gets built if LLDB_BUILD_FRAMEWORK is set. It would depend 
on liblldb and all the necessary tools, headers, etc, that the framework would 
need. That way liblldb can depend only on what it needs to build instead of 
treating it as both the library and the entire framework. How do you feel about 
this?


https://reviews.llvm.org/D47801



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


[Lldb-commits] [PATCH] D47838: [lldb-mi] Re-implement MI -exec-step command.

2018-06-06 Thread Alexander Polyakov via Phabricator via lldb-commits
polyakov.alex added a comment.

I'm a little bit confused about situation with thread id before my changes. For 
example:

  (gdb)
  thread list
  ~"Process 31642 stopped\n* thread #1: tid = 31642, 0x0041eed0 
bash`main, name = 'bash', stop reason = breakpoint 1.1\n"
  ^done

What we should use as a thread ID? Thread number or tid? The main problem that 
before my changes exec-like commands used thread number, but SBThread uses tid, 
as I know.


https://reviews.llvm.org/D47838



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


[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

2018-06-06 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

> One idea I had was to introduce another target for the framwork itself, e.g. 
> lldbFramework, which gets built if LLDB_BUILD_FRAMEWORK is set. It would 
> depend on liblldb and all the necessary tools, headers, etc, that the 
> framework would need. That way liblldb can depend only on what it needs to 
> build instead of treating it as both the library and the entire framework. 
> How do you feel about this?

I think this is the cleanest way of handling this. If @labath agrees and if 
it's not too much of a pain to implement in cmake, we should do this.


https://reviews.llvm.org/D47801



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


[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

2018-06-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D47801#1123895, @xiaobai wrote:

> > - rename INCLUDE_IN_FRAMEWORK to something more neutral (USED_BY_LIBLLDB or 
> > whatever)
> > - make the liblldb -> tool dependency not conditioned by 
> > LLDB_BUILD_FRAMEWORK
> > - remove the lldb->tool dependencies altogether
> > 
> > WDYT?
>
> I think that this a good short-term solution. My concern is your suggestion 
> doesn't reflect the actual dependencies, but conveniently errs on the side of 
> "build it if we think we might need it". I'm not against your suggestion, but 
> I'd like to see if we can come up with something better.
>  One idea I had was to introduce another target for the framwork itself, e.g. 
> lldbFramework, which gets built if LLDB_BUILD_FRAMEWORK is set. It would 
> depend on liblldb and all the necessary tools, headers, etc, that the 
> framework would need. That way liblldb can depend only on what it needs to 
> build instead of treating it as both the library and the entire framework. 
> How do you feel about this?


It all comes down to how do you want to view the "liblldb" cmake target. 
Whether it is just "things that are needed to build liblldb.so binary itself", 
or whether "everything needed to make liblldb functional". In the second POV, 
it is natural to have liblldb depend on lldb-server, since lldb-server is 
necessary to have a fully functional liblldb. However, I can see reasons 
against that too, so I am fine with your proposed solution.

The thing I would suggest, if it is not too much trouble, is again to make the 
new target framework-agnostic. I.e., in a LLDB_BUILD_FRAMEWORK build, it would 
build everything that goes into a framework and package it. Otherwise, it would 
just be a convenient way to refer to liblldb and everything that is needed to 
make it functional. The reason I'm suggesting that is that then we could remove 
the additional dependency management when building lldb driver by just making 
the driver depend on this new target.


https://reviews.llvm.org/D47801



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


[Lldb-commits] [PATCH] D47792: Fix up Info.plist when building LLDB.framework with CMake

2018-06-06 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added inline comments.



Comment at: source/API/CMakeLists.txt:184
+  set(EXECUTABLE_NAME "LLDB")
+  set(CURRENT_PROJECT_VERSION "360.99.0")
   set_target_properties(liblldb PROPERTIES

clayborg wrote:
> sas wrote:
> > clayborg wrote:
> > > Currently the apple generic versioning is used and the value for this is 
> > > grabbed from CURRENT_PROJECT_VERSION in the Xcode project in 
> > > lldb.xcodeproj/project.pbxproj. We will need to let people know that they 
> > > will need to update this variable from now on.
> > Hardcoding this here isn't ideal. Where did you get this number from? Is 
> > there a way to fetch it from a single location?
> It was hardcoded into the Xcode project before. The numbers would be manually 
> bumped by using "agvtool bump -all". That would update the Xcode project and 
> a few plist files. Feel free to try running that from the root LLDB directory 
> to see all files that get updated, then revert once you see what changed.
In the meantime, is hardcoding this version an acceptable thing to do? 
Additionally, this value is actually hardcoded in the Info.plist as 
CFBundleVersion, but filled in with CURRENT_PROJECT_VERSION in 
CFBundleShortVersionString. That seems kind of clowny.


https://reviews.llvm.org/D47792



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


[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

2018-06-06 Thread Alex Langford via Phabricator via lldb-commits
xiaobai added a comment.

In https://reviews.llvm.org/D47801#1123933, @labath wrote:

> In https://reviews.llvm.org/D47801#1123895, @xiaobai wrote:
>
> > > - rename INCLUDE_IN_FRAMEWORK to something more neutral (USED_BY_LIBLLDB 
> > > or whatever)
> > > - make the liblldb -> tool dependency not conditioned by 
> > > LLDB_BUILD_FRAMEWORK
> > > - remove the lldb->tool dependencies altogether
> > > 
> > > WDYT?
> >
> > I think that this a good short-term solution. My concern is your suggestion 
> > doesn't reflect the actual dependencies, but conveniently errs on the side 
> > of "build it if we think we might need it". I'm not against your 
> > suggestion, but I'd like to see if we can come up with something better.
> >  One idea I had was to introduce another target for the framwork itself, 
> > e.g. lldbFramework, which gets built if LLDB_BUILD_FRAMEWORK is set. It 
> > would depend on liblldb and all the necessary tools, headers, etc, that the 
> > framework would need. That way liblldb can depend only on what it needs to 
> > build instead of treating it as both the library and the entire framework. 
> > How do you feel about this?
>
>
> The thing I would suggest, if it is not too much trouble, is again to make 
> the new target framework-agnostic. I.e., in a LLDB_BUILD_FRAMEWORK build, it 
> would build everything that goes into a framework and package it. Otherwise, 
> it would just be a convenient way to refer to liblldb and everything that is 
> needed to make it functional. The reason I'm suggesting that is that then we 
> could remove the additional dependency management when building lldb driver 
> by just making the driver depend on this new target.


I think I can try to make that happen.


https://reviews.llvm.org/D47801



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


[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

2018-06-06 Thread Alex Langford via Phabricator via lldb-commits
xiaobai abandoned this revision.
xiaobai added a comment.

Going to go with the direction we discussed here. This shouldn't be needed.


https://reviews.llvm.org/D47801



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


[Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

2018-06-06 Thread Aaron Smith via Phabricator via lldb-commits
asmith added a comment.

In https://reviews.llvm.org/D47708#1123334, @labath wrote:

> I have reverted this because of the broken tests.
>
> However, I have to also ask: isn't there better way to test this? (one that 
> does not depend on opaque checked-in binaries). On linux, I could check-in a 
> .s file which has the line table exactly as I want it and then have the test 
> assert that. Is there some suitable intermediate form here?
>
> It seems that lld tests use some yaml form to store pdb's. Is there any 
> chance we could do the same?


I wish these binaries didn’t exist too. Im not sure if the lld approach works 
because I think we need the VS generated PDBs for function level testing. Maybe 
Zachary as a better idea.


https://reviews.llvm.org/D47708



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


Re: [Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

2018-06-06 Thread Zachary Turner via lldb-commits
Do you just need a pdb, or does it really need to be a vs pdb? lld can
generate high quality pdbs now. So it might be possible to use lld to link
and produce a pdb when you run the test.

Pavel’s suggestion is equally viable, you can dump a pdb to yaml and
convert it back to a pdb at test time.

The real problem is the exe. It’s harder to generate exes at test time
because we have to ensure that dependent libraries are present on the
system.

If it has to be an msvc generated pdb, can you elaborate on why? Tbh I’m
not really against checking in pdbs. Exes I’d like to find a way to avoid
checking in wherever possible though. And even then, sometimes I don’t have
any better ideas other than compile and link before running the test
On Wed, Jun 6, 2018 at 2:20 PM Aaron Smith via Phabricator <
revi...@reviews.llvm.org> wrote:

> asmith added a comment.
>
> In https://reviews.llvm.org/D47708#1123334, @labath wrote:
>
> > I have reverted this because of the broken tests.
> >
> > However, I have to also ask: isn't there better way to test this? (one
> that does not depend on opaque checked-in binaries). On linux, I could
> check-in a .s file which has the line table exactly as I want it and then
> have the test assert that. Is there some suitable intermediate form here?
> >
> > It seems that lld tests use some yaml form to store pdb's. Is there any
> chance we could do the same?
>
>
> I wish these binaries didn’t exist too. Im not sure if the lld approach
> works because I think we need the VS generated PDBs for function level
> testing. Maybe Zachary as a better idea.
>
>
> https://reviews.llvm.org/D47708
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

2018-06-06 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a subscriber: aleksandr.urakov.
zturner added a comment.

Do you just need a pdb, or does it really need to be a vs pdb? lld can
generate high quality pdbs now. So it might be possible to use lld to link
and produce a pdb when you run the test.

Pavel’s suggestion is equally viable, you can dump a pdb to yaml and
convert it back to a pdb at test time.

The real problem is the exe. It’s harder to generate exes at test time
because we have to ensure that dependent libraries are present on the
system.

If it has to be an msvc generated pdb, can you elaborate on why? Tbh I’m
not really against checking in pdbs. Exes I’d like to find a way to avoid
checking in wherever possible though. And even then, sometimes I don’t have
any better ideas other than compile and link before running the test


https://reviews.llvm.org/D47708



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


[Lldb-commits] [PATCH] D47797: [lldb-mi] Re-implement MI -exec-next command.

2018-06-06 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl accepted this revision.
aprantl added inline comments.



Comment at: lit/tools/lldb-mi/exec/exec-next.test:19
+
+-exec-next --thread 0
+# Check that exec-next can process the case of invalid thread ID.

0 feels like it might actually be a valid thread id on some systems.. perhaps 
use really high number instead?


https://reviews.llvm.org/D47797



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


[Lldb-commits] [PATCH] D47838: [lldb-mi] Re-implement MI -exec-step command.

2018-06-06 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

Did the old implementation come with a testcase? Perhaps I'm misunderstanding 
the question, but it would probably be best to preserve the old behavior.




Comment at: tools/lldb-mi/MICmdCmdExec.cpp:515
+  lldb::SBError error;
+  if (nThreadId != UINT64_MAX) {
+lldb::SBThread sbThread = 
rSessionInfo.GetProcess().GetThreadByID(nThreadId);

Usually we prefer early exits in LLVM code 
(https://www.llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code)

```
if (!sbThread.IsValid)
  return failure;
sbThread.StepInto();
```
  


https://reviews.llvm.org/D47838



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