date:20210422

[Lldb-commits] [PATCH] D100340: [lldb-vscode] Add postRunCommands

2021-04-22 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

It has caused a regression for Fedora buildbot (which is so far a silent one):
https://lab.llvm.org/staging/#/builders/16/builds/4936

  Failed Tests (2):
lldb-api :: tools/lldb-vscode/attach/TestVSCode_attach.py
lldb-api :: tools/lldb-vscode/launch/TestVSCode_launch.py

I will check it more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100340

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

[Lldb-commits] [lldb] 00764c3 - [lldb] Add support for evaluating expressions in static member functions

2021-04-22 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2021-04-22T12:14:31+02:00
New Revision: 00764c36edf88ae9806e8d57a6addb782e6ceae8

URL: 
https://github.com/llvm/llvm-project/commit/00764c36edf88ae9806e8d57a6addb782e6ceae8
DIFF: 
https://github.com/llvm/llvm-project/commit/00764c36edf88ae9806e8d57a6addb782e6ceae8.diff

LOG: [lldb] Add support for evaluating expressions in static member functions

At the moment the expression parser doesn't support evaluating expressions in
static member functions and just pretends the expression is evaluated within a
non-member function. This causes that all static members are inaccessible when
doing unqualified name lookup.

This patch adds support for evaluating in static member functions. It
essentially just does the same setup as what LLDB is already doing for
non-static member functions (i.e., wrapping the expression in a fake member
function) with the difference that we now mark the wrapping function as static
(to prevent access to non-static members).

Reviewed By: shafik, jarin

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

Added: 
lldb/test/API/lang/cpp/stopped_in_static_member_function/Makefile

lldb/test/API/lang/cpp/stopped_in_static_member_function/TestStoppedInStaticMemberFunction.py
lldb/test/API/lang/cpp/stopped_in_static_member_function/main.cpp

Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index 0c943bbb0b46..bfcfa0b90b15 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -810,7 +810,7 @@ void 
ClangExpressionDeclMap::LookUpLldbClass(NameSearchContext &context) {
 LLDB_LOG(log, "  CEDM::FEVD Adding type for $__lldb_class: {1}",
  class_qual_type.getAsString());
 
-AddContextClassType(context, class_user_type);
+AddContextClassType(context, class_user_type, method_decl);
 
 if (method_decl->isInstance()) {
   // self is a pointer to the object
@@ -1889,8 +1889,9 @@ void 
ClangExpressionDeclMap::AddOneFunction(NameSearchContext &context,
   }
 }
 
-void ClangExpressionDeclMap::AddContextClassType(NameSearchContext &context,
- const TypeFromUser &ut) {
+void ClangExpressionDeclMap::AddContextClassType(
+NameSearchContext &context, const TypeFromUser &ut,
+CXXMethodDecl *context_method) {
   CompilerType copied_clang_type = GuardedCopyType(ut);
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
@@ -1912,7 +1913,12 @@ void 
ClangExpressionDeclMap::AddContextClassType(NameSearchContext &context,
 void_clang_type, &void_ptr_clang_type, 1, false, 0);
 
 const bool is_virtual = false;
-const bool is_static = false;
+// If we evaluate an expression inside a static method, we also need to
+// make our lldb_expr method static so that Clang denies access to
+// non-static members.
+// If we don't have a context_method we are evaluating within a context
+// object and we can allow access to non-static members.
+const bool is_static = context_method ? context_method->isStatic() : false;
 const bool is_inline = false;
 const bool is_explicit = false;
 const bool is_attr_used = true;

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
index a9cd5d166b9d..f1a0e21452d9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
@@ -608,8 +608,13 @@ class ClangExpressionDeclMap : public ClangASTSource {
   ///
   /// \param[in] type
   /// The type of the class that serves as the evaluation context.
-  void AddContextClassType(NameSearchContext &context,
-   const TypeFromUser &type);
+  ///
+  /// \param[in] context_method
+  /// The member function declaration in which the expression is being
+  /// evaluated or null if the expression is not evaluated in the context
+  /// of a member function.
+  void AddContextClassType(NameSearchContext &context, const TypeFromUser 
&type,
+   clang::CXXMethodDecl *context_method = nullptr);
 
   /// Move a type out of the current ASTContext into another

[Lldb-commits] [PATCH] D81550: [lldb] Add support for evaluating expressions in static member functions

2021-04-22 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
teemperor marked an inline comment as done.
Closed by commit rG00764c36edf8: [lldb] Add support for evaluating expressions 
in static member functions (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D81550?vs=269792&id=339546#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81550

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
  lldb/test/API/lang/cpp/stopped_in_static_member_function/Makefile
  
lldb/test/API/lang/cpp/stopped_in_static_member_function/TestStoppedInStaticMemberFunction.py
  lldb/test/API/lang/cpp/stopped_in_static_member_function/main.cpp

Index: lldb/test/API/lang/cpp/stopped_in_static_member_function/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/stopped_in_static_member_function/main.cpp
@@ -0,0 +1,31 @@
+struct A {
+  int member_var = 1;
+  static int static_member_var;
+  static const int static_const_member_var;
+  static constexpr int static_constexpr_member_var = 4;
+  int member_func() { return 5; }
+  static int static_func() { return 6; }
+
+  static int context_static_func() {
+int i = static_member_var;
+i += static_func();
+return i; // break in static member function
+  }
+
+  int context_member_func() {
+int i = member_var;
+i += member_func();
+return i; // break in member function
+  }
+};
+
+int A::static_member_var = 2;
+const int A::static_const_member_var = 3;
+constexpr int A::static_constexpr_member_var;
+
+int main() {
+  int i = A::context_static_func();
+  A a;
+  a.context_member_func();
+  return i;
+}
Index: lldb/test/API/lang/cpp/stopped_in_static_member_function/TestStoppedInStaticMemberFunction.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/stopped_in_static_member_function/TestStoppedInStaticMemberFunction.py
@@ -0,0 +1,38 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break in static member function", lldb.SBFileSpec("main.cpp"))
+
+# Evaluate a static member and call a static member function.
+self.expect_expr("static_member_var", result_type="int", result_value="2")
+self.expect_expr("static_const_member_var", result_type="const int", result_value="3")
+self.expect_expr("static_constexpr_member_var", result_type="const int", result_value="4")
+self.expect_expr("static_func()", result_type="int", result_value="6")
+
+# Check that accessing non-static members just reports a diagnostic.
+self.expect("expr member_var", error=True,
+substrs=["invalid use of member 'member_var' in static member function"])
+self.expect("expr member_func()", error=True,
+substrs=["call to non-static member function without an object argument"])
+self.expect("expr this", error=True,
+substrs=["invalid use of 'this' outside of a non-static member function"])
+
+# Continue to a non-static member function of the same class and make
+# sure that evaluating non-static members now works.
+breakpoint = self.target().BreakpointCreateBySourceRegex(
+"// break in member function", lldb.SBFileSpec("main.cpp"))
+self.assertNotEqual(breakpoint.GetNumResolvedLocations(), 0)
+stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint)
+
+self.expect_expr("member_var", result_type="int", result_value="1")
+self.expect_expr("member_func()", result_type="int", result_value="5")
Index: lldb/test/API/lang/cpp/stopped_in_static_member_function/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/stopped_in_static_member_function/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
+++ lldb/source/Plugins/ExpressionPars

[Lldb-commits] [lldb] 034c73d - [lldb][NFC] Fix unsigned/signed cmp warning in MainLoopTest

2021-04-22 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2021-04-22T12:20:32+02:00
New Revision: 034c73d42e4655ae247fc1f72dc3d2d6c621ab23

URL: 
https://github.com/llvm/llvm-project/commit/034c73d42e4655ae247fc1f72dc3d2d6c621ab23
DIFF: 
https://github.com/llvm/llvm-project/commit/034c73d42e4655ae247fc1f72dc3d2d6c621ab23.diff

LOG: [lldb][NFC] Fix unsigned/signed cmp warning in MainLoopTest

The gtest checks compare all against unsigned int constants so this also needs
to be unsigned.

Added: 


Modified: 
lldb/unittests/Host/MainLoopTest.cpp

Removed: 




diff  --git a/lldb/unittests/Host/MainLoopTest.cpp 
b/lldb/unittests/Host/MainLoopTest.cpp
index 50a49b92d48b..890f6eb66197 100644
--- a/lldb/unittests/Host/MainLoopTest.cpp
+++ b/lldb/unittests/Host/MainLoopTest.cpp
@@ -158,8 +158,8 @@ TEST_F(MainLoopTest, UnmonitoredSignal) {
 TEST_F(MainLoopTest, TwoSignalCallbacks) {
   MainLoop loop;
   Status error;
-  int callback2_count = 0;
-  int callback3_count = 0;
+  unsigned callback2_count = 0;
+  unsigned callback3_count = 0;
 
   auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
   ASSERT_TRUE(error.Success());



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

[Lldb-commits] [lldb] edc869c - [lldb-vscode] Use a DenseMap to pacify overly aggressive linters

2021-04-22 Thread Benjamin Kramer via lldb-commits


Author: Benjamin Kramer
Date: 2021-04-22T13:07:39+02:00
New Revision: edc869cb57fb4cf999c8a388b48ae4ecd027bfe7

URL: 
https://github.com/llvm/llvm-project/commit/edc869cb57fb4cf999c8a388b48ae4ecd027bfe7
DIFF: 
https://github.com/llvm/llvm-project/commit/edc869cb57fb4cf999c8a388b48ae4ecd027bfe7.diff

LOG: [lldb-vscode] Use a DenseMap to pacify overly aggressive linters

Some linters get rather upset upon seeing
`std::unordered_map
 #include 
 #include 
-#include 
 #include 
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
@@ -2931,7 +2931,7 @@ void request_variables(const llvm::json::Object &request) 
{
 const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
 
 // We first find out which variable names are duplicated
-std::unordered_map variable_name_counts;
+llvm::DenseMap variable_name_counts;
 for (auto i = start_idx; i < end_idx; ++i) {
   lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(i);
   if (!variable.IsValid())



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

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

2021-04-22 Thread Raphael Isemann via Phabricator via lldb-commits

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

The test below reproduces the eager layout generation (and the resulting 
crashes) for me. Just apply it on top and then this is good to go. Thanks for 
tracking this down!

  diff --git 
a/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
 
b/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
  new file mode 100644
  index ..8b20bcb0
  --- /dev/null
  +++ 
b/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
  @@ -0,0 +1,3 @@
  +CXX_SOURCES := main.cpp
  +
  +include Makefile.rules
  diff --git 
a/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
 
b/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
  new file mode 100644
  index ..fd978158e716
  --- /dev/null
  +++ 
b/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
  @@ -0,0 +1,24 @@
  +import lldb
  +from lldbsuite.test.decorators import *
  +from lldbsuite.test.lldbtest import *
  +from lldbsuite.test import lldbutil
  +
  +class TestCase(TestBase):
  +
  +mydir = TestBase.compute_mydir(__file__)
  +
  +@no_debug_info_test
  +def test(self):
  +"""
  +This tests a pointer-to-member member which class part is the
  +surrounding class. LLDB should *not* try to generate the record 
layout
  +of the class when parsing pointer-to-member types while parsing debug
  +info (as the references class might not be complete when the type is
  +parsed).
  +"""
  +self.build()
  +self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
  +
  +# Force the record layout for 'ToLayout' to be generated by printing
  +# a value of it's type.
  +self.expect("target variable test_var")
  diff --git 
a/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp
 
b/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp
  new file mode 100644
  index ..4787691ac9f7
  --- /dev/null
  +++ 
b/lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp
  @@ -0,0 +1,35 @@
  +// This class just serves as an indirection between LLDB and Clang. LLDB 
might
  +// be tempted to check the member type of DependsOnParam2 for whether it's
  +// in some 'currently-loading' state before trying to produce the record 
layout.
  +// By inheriting from ToLayout this will make LLDB just check if
  +// DependsOnParam1 is currently being loaded (which it's not) but it won't
  +// check if all the types DependsOnParam2 is depending on for its layout are
  +// currently parsed.
  +template  struct DependsOnParam1 : ToLayoutParam {};
  +// This class forces the memory layout of it's type parameter to be created.
  +template  struct DependsOnParam2 {
  +  DependsOnParam1 m;
  +};
  +
  +// This is the class that LLDB has to generate the record layout for.
  +struct ToLayout {
  +  // The class part of this pointer-to-member type has a memory layout that
  +  // depends on the surrounding class. If LLDB eagerly tries to layout the
  +  // class part of a pointer-to-member type while parsing, then layouting 
this
  +  // type should cause a test failure (as we aren't done parsing ToLayout
  +  // at this point).
  +  int DependsOnParam2::* pointer_to_member_member;
  +  // Some dummy member variable. This is only there so that Clang can detect
  +  // that the record layout is inconsistent (i.e., the number of fields in 
the
  +  // layout doesn't fit to the fields in the declaration).
  +  int some_member;
  +};
  +
  +// Emit the definition of DependsOnParam2. It seems Clang won't
  +// emit the definition of a class template if it's only used in the class 
part
  +// of a pointer-to-member type.
  +DependsOnParam2 x;
  +
  +ToLayout test_var;
  +
  +int main() { return test_var.some_member; }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

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

[Lldb-commits] [lldb] d2223c7 - [lldb] XFAIL TestStoppedInStaticMemberFunction on Windows

2021-04-22 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2021-04-22T13:46:27+02:00
New Revision: d2223c7a49973a61cc2de62992662afa8d19065a

URL: 
https://github.com/llvm/llvm-project/commit/d2223c7a49973a61cc2de62992662afa8d19065a
DIFF: 
https://github.com/llvm/llvm-project/commit/d2223c7a49973a61cc2de62992662afa8d19065a.diff

LOG: [lldb] XFAIL TestStoppedInStaticMemberFunction on Windows

It seems we can't find the symbols of static members on Windows? The bug is not
 relevant to what this test is actually testing so let's just XFAIL it.

Added: 


Modified: 

lldb/test/API/lang/cpp/stopped_in_static_member_function/TestStoppedInStaticMemberFunction.py

Removed: 




diff  --git 
a/lldb/test/API/lang/cpp/stopped_in_static_member_function/TestStoppedInStaticMemberFunction.py
 
b/lldb/test/API/lang/cpp/stopped_in_static_member_function/TestStoppedInStaticMemberFunction.py
index e1cfa12305cc..b69263a00fd2 100644
--- 
a/lldb/test/API/lang/cpp/stopped_in_static_member_function/TestStoppedInStaticMemberFunction.py
+++ 
b/lldb/test/API/lang/cpp/stopped_in_static_member_function/TestStoppedInStaticMemberFunction.py
@@ -8,6 +8,9 @@ class TestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
+# On Windows we can lookup the declarations of static members but finding
+# up the underlying symbols doesn't work yet.
+@expectedFailureAll(oslist=["windows"])
 @no_debug_info_test
 def test(self):
 self.build()



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

[Lldb-commits] [PATCH] D99989: [lldb-vscode] Distinguish shadowed variables in the scopes request

2021-04-22 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor reopened this revision.
teemperor added inline comments.
This revision is now accepted and ready to land.
Herald added a subscriber: JDevlieghere.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2924
+// We first find out which variable names are duplicated
+std::unordered_map variable_name_repeats;
+for (auto i = start_idx; i < end_idx; ++i) {

clayborg wrote:
> This variable name can be improved to accurately describe what it is doing. 
> "variable_name_counts" would be my suggestion since that is what it is really 
> doing.
> 
> You can also just make the key of this map "const char *" since the variable 
> names come from ConstString values inside LLDB the pointers will be unique
Let's not. The last time someone did this whole "This `const char *` is 
actually coming from a ConstString so assume ConstString semantics" it just 
caused us a bunch of pain (see D88490 and the related revisions).

Either add an assert that the strings are coming from a ConstString (assuming 
we really care about the performance benefit here) or use some 
StringMap/std::string-key approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99989

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

[Lldb-commits] [PATCH] D100846: [lldb] Don't leak LineSequence in PDB parsers

2021-04-22 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe3dd82ae3c4e: [lldb] Don't leak LineSequence in PDB 
parsers (authored by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100846

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp


Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1813,7 +1813,7 @@
 sequence.get(), prev_addr + prev_length, prev_line, 0,
 prev_source_idx, false, false, false, false, true);
 
-line_table->InsertSequence(sequence.release());
+line_table->InsertSequence(sequence.get());
 sequence = line_table->CreateLineSequenceContainer();
   }
 
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1039,7 +1039,7 @@
   table.AppendLineEntryToSequence(seq.get(), base_addr + block.CodeSize,
   last_line, 0, file_number, false, false,
   false, false, true);
-  table.InsertSequence(seq.release());
+  table.InsertSequence(seq.get());
 }
 
 bool SymbolFileNativePDB::ParseLineTable(CompileUnit &comp_unit) {


Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1813,7 +1813,7 @@
 sequence.get(), prev_addr + prev_length, prev_line, 0,
 prev_source_idx, false, false, false, false, true);
 
-line_table->InsertSequence(sequence.release());
+line_table->InsertSequence(sequence.get());
 sequence = line_table->CreateLineSequenceContainer();
   }
 
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1039,7 +1039,7 @@
   table.AppendLineEntryToSequence(seq.get(), base_addr + block.CodeSize,
   last_line, 0, file_number, false, false,
   false, false, true);
-  table.InsertSequence(seq.release());
+  table.InsertSequence(seq.get());
 }
 
 bool SymbolFileNativePDB::ParseLineTable(CompileUnit &comp_unit) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e3dd82a - [lldb] Don't leak LineSequence in PDB parsers

2021-04-22 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2021-04-22T14:11:01+02:00
New Revision: e3dd82ae3c4edaeb3c8f835c29566375ee25023d

URL: 
https://github.com/llvm/llvm-project/commit/e3dd82ae3c4edaeb3c8f835c29566375ee25023d
DIFF: 
https://github.com/llvm/llvm-project/commit/e3dd82ae3c4edaeb3c8f835c29566375ee25023d.diff

LOG: [lldb] Don't leak LineSequence in PDB parsers

`InsertSequence` doesn't take ownership of the pointer so releasing this pointer
is just leaking memory.

Follow up to D100806 that was fixing other leak sanitizer test failures

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 24b4c64a91bcf..9f48f84f51ef6 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1039,7 +1039,7 @@ static void TerminateLineSequence(LineTable &table,
   table.AppendLineEntryToSequence(seq.get(), base_addr + block.CodeSize,
   last_line, 0, file_number, false, false,
   false, false, true);
-  table.InsertSequence(seq.release());
+  table.InsertSequence(seq.get());
 }
 
 bool SymbolFileNativePDB::ParseLineTable(CompileUnit &comp_unit) {

diff  --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp 
b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
index befc08158cea4..0f0757ebc9ced 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1813,7 +1813,7 @@ bool SymbolFilePDB::ParseCompileUnitLineTable(CompileUnit 
&comp_unit,
 sequence.get(), prev_addr + prev_length, prev_line, 0,
 prev_source_idx, false, false, false, false, true);
 
-line_table->InsertSequence(sequence.release());
+line_table->InsertSequence(sequence.get());
 sequence = line_table->CreateLineSequenceContainer();
   }
 



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

[Lldb-commits] [lldb] e5984a3 - [lldb/elf] Avoid side effects in function calls ParseUnwindSymbols

2021-04-22 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2021-04-22T14:31:00+02:00
New Revision: e5984a3680bef22d422beaafa73bf131d7197973

URL: 
https://github.com/llvm/llvm-project/commit/e5984a3680bef22d422beaafa73bf131d7197973
DIFF: 
https://github.com/llvm/llvm-project/commit/e5984a3680bef22d422beaafa73bf131d7197973.diff

LOG: [lldb/elf] Avoid side effects in function calls ParseUnwindSymbols

This addresses post-commit feedback to cd64273.

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 6e94ab97992a1..be73d38961ea6 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2918,20 +2918,21 @@ void ObjectFileELF::ParseUnwindSymbols(Symtab 
*symbol_table,
   if (section_sp) {
 addr_t offset = file_addr - section_sp->GetFileAddress();
 const char *symbol_name = GetNextSyntheticSymbolName().GetCString();
+uint64_t symbol_id = ++last_symbol_id;
 Symbol eh_symbol(
-++last_symbol_id, // Symbol table index.
-symbol_name,  // Symbol name.
-eSymbolTypeCode,  // Type of this symbol.
-true, // Is this globally visible?
-false,// Is this symbol debug info?
-false,// Is this symbol a trampoline?
-true, // Is this symbol artificial?
-section_sp, // Section in which this symbol is defined or null.
-offset, // Offset in section or symbol value.
-0,  // Size:  Don't specify the size as an FDE can
-false,  // Size is valid: cover multiple symbols.
-false,  // Contains linker annotations?
-0); // Symbol flags.
+symbol_id,   // Symbol table index.
+symbol_name, // Symbol name.
+eSymbolTypeCode, // Type of this symbol.
+true,// Is this globally visible?
+false,   // Is this symbol debug info?
+false,   // Is this symbol a trampoline?
+true,// Is this symbol artificial?
+section_sp,  // Section in which this symbol is defined or 
null.
+offset,  // Offset in section or symbol value.
+0, // Size:  Don't specify the size as an FDE can
+false, // Size is valid: cover multiple symbols.
+false, // Contains linker annotations?
+0);// Symbol flags.
 new_symbols.push_back(eh_symbol);
   }
 }



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

Re: [Lldb-commits] [lldb] cd64273 - [lldb/ELF] Fix IDs of synthetic eh_frame symbols

2021-04-22 Thread Pavel Labath via lldb-commits


On 21/04/2021 22:13, Shafik Yaghmour wrote:

Having side effects in the argument to a function call just feels like a bad 
idea in general even if it seems obviously correct here e.g.

   ++last_symbol_id,

Can’t we just do:

   uint64_t last_symbol_id =
   num_symbols ? symbol_table->SymbolAtIndex(num_symbols - 1)->GetID() + 1: 
1;


This would just change the pre-increment to a post-increment. The 
increment still needs to happen somewhere, as the function can add more 
than one symbol.


However, I don't have a problem with moving the increment out of the 
function call. That is what I've done in e5984a3.


Thanks for reviewing this,
Pavel
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98370: [lldb] Fix SBValue::Persist() for constant values

2021-04-22 Thread Andy Yankovsky via Phabricator via lldb-commits

werat added a comment.

In D98370#2705741 , @jingham wrote:

> Sure.  But but when I was poking around at it a little bit, it seems like the 
> other use cases already work, and the only one that was failing was the case 
> where you call persist on a persistent variable.  If that is really true, 
> then maybe we should fix the failing case directly.

Right now `Persist()` doesn't really work for values created via 
`CreateValueFromData`. You can read them, but can't modify:

  >>> data = lldb.SBData.CreateDataFromUInt64Array(lldb.process.GetByteOrder(), 
lldb.process.GetAddressByteSize(), [42])
  >>> v = lldb.target.CreateValueFromData('v', data, 
lldb.target.GetBasicType(lldb.eBasicTypeUnsignedLong))
  >>> v.value
  '42'
  >>> vp = v.Persist()
  >>> vp.name
  '$3'
  >>> lldb.frame.EvaluateExpression('$3').value
  '42'
  >>> lldb.frame.EvaluateExpression('++$3 + 1').value
  >>> lldb.frame.EvaluateExpression('++$3 + 1').error.GetCString()
  "error: supposed to interpret, but failed: Interpreter couldn't read from 
memory\n"

However I realized my patch doesn't completely fixes it either:

  >>> data = lldb.SBData.CreateDataFromUInt64Array(lldb.process.GetByteOrder(), 
lldb.process.GetAddressByteSize(), [42])
  >>> v = lldb.target.CreateValueFromData('v', data, 
lldb.target.GetBasicType(lldb.eBasicTypeUnsignedLong))
  >>> vp = v.Persist()
  >>> vp.name
  '$0'
  >>> lldb.frame.EvaluateExpression('$0').value
  '42'
  >>> lldb.frame.EvaluateExpression('++$0').value
  '43'
  >>> lldb.frame.EvaluateExpression('++$0').value
  '44'
  >>> vp.value
  '42'



In D98370#2705741 , @jingham wrote:

> Not sure why?  The API is a request:  "I made a variable somehow, and I would 
> like you to make it persist so I can use its value later on even if the 
> underlying data has changed."  Why do you care whether you get a copy of an 
> already persistent or just a shared value?

You're right, I got confused by something else. I don't care if I get a new 
name/copy, as long as I can use it by the returned name it's fine. However I 
want to point out that the current API does generate a new name every time (but 
the it points to the same data):

  >>> x = lldb.frame.FindVariable('x')
  >>> x.value
  '1'
  >>> xp1 = x.Persist()
  >>> xp1.name
  '$0'
  >>> xp2 = x.Persist()
  >>> xp2.name
  '$1'
  >>> lldb.frame.EvaluateExpression('++$0 + ++$1').value
  '3'
  >>> xp1.value
  '3'
  >>> xp2.value
  '3'




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98370

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

[Lldb-commits] [PATCH] D100208: [lldb] [Process/Linux] Report fork/vfork stop reason

2021-04-22 Thread Michał Górny via Phabricator via lldb-commits

mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:932
+
+std::unique_ptr child_process_up{child_process};
+Extension expected_ext = is_vfork ? Extension::vfork : Extension::fork;

labath wrote:
> Store directly into unique_ptr above.
Heh, I suppose it started working because we no longer pass-by-reference to 
`NewSubprocess()`. Cool!



Comment at: lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp:401-402
+  m_stop_info.reason = StopReason::eStopReasonFork;
+  m_stop_info.details.fork.child_pid = child_pid;
+  m_stop_info.details.fork.child_tid = child_pid;
+}

labath wrote:
> duplicated
Sure but that's only on Linux. FreeBSD and NetBSD use distinct thread IDs.


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

https://reviews.llvm.org/D100208

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

[Lldb-commits] [lldb] d616a6b - [lldb] Fix that the expression commands --top-level flag overwrites --allow-jit false

2021-04-22 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2021-04-22T18:51:03+02:00
New Revision: d616a6bd107fcc0dc74f79e174e0b4fe27b26fe6

URL: 
https://github.com/llvm/llvm-project/commit/d616a6bd107fcc0dc74f79e174e0b4fe27b26fe6
DIFF: 
https://github.com/llvm/llvm-project/commit/d616a6bd107fcc0dc74f79e174e0b4fe27b26fe6.diff

LOG: [lldb] Fix that the expression commands --top-level flag overwrites 
--allow-jit false

The `--allow-jit` flag allows the user to force the IR interpreter to run the
provided expression.

The `--top-level` flag parses and injects the code as if its in the top level
scope of a source file.

Both flags just change the ExecutionPolicy of the expression:
* `--allow-jit true` -> doesn't change anything (its the default)
* `--allow-jit false` -> ExecutionPolicyNever
* `--top-level` -> ExecutionPolicyTopLevel

Passing `--allow-jit false` and `--top-level` currently causes the `--top-level`
to silently overwrite the ExecutionPolicy value that was set by `--allow-jit
false`. There isn't any ExecutionPolicy value that says "top-level but only
interpret", so I would say we reject this combination of flags until someone
finds time to refactor top-level feature out of the ExecutionPolicy enum.

The SBExpressionOptions suffer from a similar symptom as `SetTopLevel` and
`SetAllowJIT` just silently disable each other. But those functions don't have
any error handling, so not a lot we can do about this in the meantime.

Reviewed By: labath, kastiglione

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectExpression.cpp
lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectExpression.cpp 
b/lldb/source/Commands/CommandObjectExpression.cpp
index c7866f966569f..4f4318edc14fe 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -408,6 +408,13 @@ bool 
CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
   lldb::ValueObjectSP result_valobj_sp;
   StackFrame *frame = exe_ctx.GetFramePtr();
 
+  if (m_command_options.top_level && !m_command_options.allow_jit) {
+result.AppendErrorWithFormat(
+"Can't disable JIT compilation for top-level expressions.\n");
+result.SetStatus(eReturnStatusFailed);
+return false;
+  }
+
   const EvaluateExpressionOptions options = GetEvalOptions(target);
   ExpressionResults success = target.EvaluateExpression(
   expr, frame, result_valobj_sp, options, &m_fixed_expression);

diff  --git a/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py 
b/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
index ab1438479aab9..7bba9c8d2c8d7 100644
--- a/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
+++ b/lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
@@ -80,3 +80,16 @@ def expr_options_test(self):
 self.assertSuccess(result.GetError())
 self.assertEqual(result.GetValueAsSigned(), 18, "got the right value.")
 
+def test_allow_jit_with_top_level(self):
+"""Test combined --allow-jit and --top-level flags"""
+# Can't force interpreting for top-level expressions which are always
+# injected.
+self.expect("expr --allow-jit false --top-level -- int i;", error=True,
+substrs=["Can't disable JIT compilation for top-level 
expressions."])
+
+self.build()
+lldbutil.run_to_source_breakpoint(self, "Set a breakpoint here", 
lldb.SBFileSpec("main.c"))
+# Allowing JITing for top-level expressions is redundant but should 
work.
+self.expect("expr --allow-jit true --top-level -- int top_level_f() { 
return 2; }")
+# Make sure we actually declared a working top-level function.
+self.expect_expr("top_level_f()", result_value="2")



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

[Lldb-commits] [PATCH] D91780: [lldb] Fix that the expression commands --top-level flag overwrites --allow-jit false

2021-04-22 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd616a6bd107f: [lldb] Fix that the expression commands 
--top-level flag overwrites --allow-jit… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91780

Files:
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py


Index: lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
===
--- lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
+++ lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
@@ -80,3 +80,16 @@
 self.assertSuccess(result.GetError())
 self.assertEqual(result.GetValueAsSigned(), 18, "got the right value.")
 
+def test_allow_jit_with_top_level(self):
+"""Test combined --allow-jit and --top-level flags"""
+# Can't force interpreting for top-level expressions which are always
+# injected.
+self.expect("expr --allow-jit false --top-level -- int i;", error=True,
+substrs=["Can't disable JIT compilation for top-level 
expressions."])
+
+self.build()
+lldbutil.run_to_source_breakpoint(self, "Set a breakpoint here", 
lldb.SBFileSpec("main.c"))
+# Allowing JITing for top-level expressions is redundant but should 
work.
+self.expect("expr --allow-jit true --top-level -- int top_level_f() { 
return 2; }")
+# Make sure we actually declared a working top-level function.
+self.expect_expr("top_level_f()", result_value="2")
Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -408,6 +408,13 @@
   lldb::ValueObjectSP result_valobj_sp;
   StackFrame *frame = exe_ctx.GetFramePtr();
 
+  if (m_command_options.top_level && !m_command_options.allow_jit) {
+result.AppendErrorWithFormat(
+"Can't disable JIT compilation for top-level expressions.\n");
+result.SetStatus(eReturnStatusFailed);
+return false;
+  }
+
   const EvaluateExpressionOptions options = GetEvalOptions(target);
   ExpressionResults success = target.EvaluateExpression(
   expr, frame, result_valobj_sp, options, &m_fixed_expression);


Index: lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
===
--- lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
+++ lldb/test/API/commands/expression/dont_allow_jit/TestAllowJIT.py
@@ -80,3 +80,16 @@
 self.assertSuccess(result.GetError())
 self.assertEqual(result.GetValueAsSigned(), 18, "got the right value.")
 
+def test_allow_jit_with_top_level(self):
+"""Test combined --allow-jit and --top-level flags"""
+# Can't force interpreting for top-level expressions which are always
+# injected.
+self.expect("expr --allow-jit false --top-level -- int i;", error=True,
+substrs=["Can't disable JIT compilation for top-level expressions."])
+
+self.build()
+lldbutil.run_to_source_breakpoint(self, "Set a breakpoint here", lldb.SBFileSpec("main.c"))
+# Allowing JITing for top-level expressions is redundant but should work.
+self.expect("expr --allow-jit true --top-level -- int top_level_f() { return 2; }")
+# Make sure we actually declared a working top-level function.
+self.expect_expr("top_level_f()", result_value="2")
Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -408,6 +408,13 @@
   lldb::ValueObjectSP result_valobj_sp;
   StackFrame *frame = exe_ctx.GetFramePtr();
 
+  if (m_command_options.top_level && !m_command_options.allow_jit) {
+result.AppendErrorWithFormat(
+"Can't disable JIT compilation for top-level expressions.\n");
+result.SetStatus(eReturnStatusFailed);
+return false;
+  }
+
   const EvaluateExpressionOptions options = GetEvalOptions(target);
   ExpressionResults success = target.EvaluateExpression(
   expr, frame, result_valobj_sp, options, &m_fixed_expression);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

2021-04-22 Thread Emre Kultursay via Phabricator via lldb-commits

emrekultursay updated this revision to Diff 339694.
emrekultursay added a comment.

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
  lldb/test/API/functionalities/lazy-loading/main.cpp
  
lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
  
lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
  
lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp

Index: lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/main.cpp
@@ -0,0 +1,35 @@
+// This class just serves as an indirection between LLDB and Clang. LLDB might
+// be tempted to check the member type of DependsOnParam2 for whether it's
+// in some 'currently-loading' state before trying to produce the record layout.
+// By inheriting from ToLayout this will make LLDB just check if
+// DependsOnParam1 is currently being loaded (which it's not) but it won't
+// check if all the types DependsOnParam2 is depending on for its layout are
+// currently parsed.
+template  struct DependsOnParam1 : ToLayoutParam {};
+// This class forces the memory layout of it's type parameter to be created.
+template  struct DependsOnParam2 {
+  DependsOnParam1 m;
+};
+
+// This is the class that LLDB has to generate the record layout for.
+struct ToLayout {
+  // The class part of this pointer-to-member type has a memory layout that
+  // depends on the surrounding class. If LLDB eagerly tries to layout the
+  // class part of a pointer-to-member type while parsing, then layouting this
+  // type should cause a test failure (as we aren't done parsing ToLayout
+  // at this point).
+  int DependsOnParam2::* pointer_to_member_member;
+  // Some dummy member variable. This is only there so that Clang can detect
+  // that the record layout is inconsistent (i.e., the number of fields in the
+  // layout doesn't fit to the fields in the declaration).
+  int some_member;
+};
+
+// Emit the definition of DependsOnParam2. It seems Clang won't
+// emit the definition of a class template if it's only used in the class part
+// of a pointer-to-member type.
+DependsOnParam2 x;
+
+ToLayout test_var;
+
+int main() { return test_var.some_member; }
Index: lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py
@@ -0,0 +1,24 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test(self):
+"""
+This tests a pointer-to-member member which class part is the
+surrounding class. LLDB should *not* try to generate the record layout
+of the class when parsing pointer-to-member types while parsing debug
+info (as the references class might not be complete when the type is
+parsed).
+"""
+self.build()
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+# Force the record layout for 'ToLayout' to be generated by printing
+# a value of it's type.
+self.expect("target variable test_var")
Index: lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/pointer_to_member_type_depending_on_parent_size/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/functionalities/lazy-loading/main.cpp
===
--- lldb/test/API/functionalities/lazy-loading/main.cpp
+++ lldb/test/API/functionalities/lazy-loading/main.cpp
@@ -26,6 +26,7 @@
 struct StaticClassMember { int i; };
 struct UnusedClassMember { int i; };
 struct UnusedClassMemberPtr { int i; };
+struct PointerToMember { int i; };
 
 namespace NS {
 class ClassInNamespace {
@@ -36,6 +37,7 @@
   int dummy; // Prevent bug where LLDB always completes first member.
   ClassMember member;
   static StaticClassMember static_member;
+  int (PointerToMember::*ptr_to_member);
   UnusedClassMember unused_member;
   UnusedClassMemberPtr *unused_member_ptr;
   int enteredFunction() {
Index: lldb/test/API/functionalities/lazy-loading/TestLazyLoading.py
=

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

2021-04-22 Thread Emre Kultursay via Phabricator via lldb-commits

emrekultursay added a comment.

> DependsOnParam2 x;

This must have been the magic glue I was missing, Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

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

[Lldb-commits] [PATCH] D101086: [lldb] [Process/elf-core] Fix reading FPRs from FreeBSD/i386 cores

2021-04-22 Thread Michał Górny via Phabricator via lldb-commits

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
Herald added subscribers: pengfei, arichardson.
mgorny requested review of this revision.

The FreeBSD coredumps from i386 systems contain only FSAVE-style
NT_FPREGSET.  Since we do not really support reading that kind of data
anymore, just use NT_X86_XSTATE to get FXSAVE-style data when available.


https://reviews.llvm.org/D101086

Files:
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/test/Shell/Register/Core/x86-32-freebsd-fp.test


Index: lldb/test/Shell/Register/Core/x86-32-freebsd-fp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-32-freebsd-fp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-32-freebsd.core | FileCheck 
%p/Inputs/x86-fp.check
+
+register read --all
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -96,6 +96,9 @@
 llvm::ArrayRef RegsetDescs);
 
 constexpr RegsetDesc FPR_Desc[] = {
+// FreeBSD/i386 core NT_FPREGSET is x87 FSAVE result but the XSAVE dump
+// starts with FXSAVE struct, so use that instead if available.
+{llvm::Triple::FreeBSD, llvm::Triple::x86, llvm::ELF::NT_X86_XSTATE},
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_FPREGSET},
 // In a i386 core file NT_FPREGSET is present, but it's not the result
 // of the FXSAVE instruction like in 64 bit files.


Index: lldb/test/Shell/Register/Core/x86-32-freebsd-fp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-32-freebsd-fp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-32-freebsd.core | FileCheck %p/Inputs/x86-fp.check
+
+register read --all
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -96,6 +96,9 @@
 llvm::ArrayRef RegsetDescs);
 
 constexpr RegsetDesc FPR_Desc[] = {
+// FreeBSD/i386 core NT_FPREGSET is x87 FSAVE result but the XSAVE dump
+// starts with FXSAVE struct, so use that instead if available.
+{llvm::Triple::FreeBSD, llvm::Triple::x86, llvm::ELF::NT_X86_XSTATE},
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_FPREGSET},
 // In a i386 core file NT_FPREGSET is present, but it's not the result
 // of the FXSAVE instruction like in 64 bit files.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D101091: [lldb] [Process/elf-core] Fix reading NetBSD/i386 core dumps

2021-04-22 Thread Michał Górny via Phabricator via lldb-commits

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
mgorny requested review of this revision.

Add support for extracting basic data from NetBSD/i386 core dumps.
FPU registers are not supported at the moment.


https://reviews.llvm.org/D101091

Files:
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
  lldb/test/Shell/Register/Core/x86-32-netbsd-addr.test
  lldb/test/Shell/Register/Core/x86-32-netbsd-gp.test


Index: lldb/test/Shell/Register/Core/x86-32-netbsd-gp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-32-netbsd-gp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-32-netbsd.core | FileCheck 
%p/Inputs/x86-32-gp.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-32-netbsd-addr.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-32-netbsd-addr.test
@@ -0,0 +1,13 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-32-netbsd.core | FileCheck %s
+
+register read --all
+# CHECK-DAG: eip = 0x08048955
+# CHECK-DAG: eflags = 0x00010282
+# CHECK-DAG: cs = 0x0037
+# CHECK-DAG: fs = 0x004f
+# CHECK-DAG: gs = 0x008b
+# CHECK-DAG: ss = 0x004f
+# CHECK-DAG: ds = 0x004f
+# CHECK-DAG: es = 0x004f
+
+# TODO: fix reading fp registers
Index: lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -20,6 +20,7 @@
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_s390x.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
+#include "Plugins/Process/Utility/RegisterContextNetBSD_i386.h"
 #include "Plugins/Process/Utility/RegisterContextNetBSD_x86_64.h"
 #include "Plugins/Process/Utility/RegisterContextOpenBSD_i386.h"
 #include "Plugins/Process/Utility/RegisterContextOpenBSD_x86_64.h"
@@ -107,6 +108,9 @@
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
 break;
+  case llvm::Triple::x86:
+reg_interface = new RegisterContextNetBSD_i386(arch);
+break;
   case llvm::Triple::x86_64:
 reg_interface = new RegisterContextNetBSD_x86_64(arch);
 break;
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -51,7 +51,7 @@
 enum { NT_REGS = 32, NT_FPREGS = 34 };
 }
 
-namespace AMD64 {
+namespace X86 {
 enum { NT_REGS = 33, NT_FPREGS = 35 };
 }
 
@@ -106,7 +106,8 @@
 {llvm::Triple::Linux, llvm::Triple::x86, llvm::ELF::NT_PRXFPREG},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, llvm::ELF::NT_FPREGSET},
 {llvm::Triple::NetBSD, llvm::Triple::aarch64, NETBSD::AARCH64::NT_FPREGS},
-{llvm::Triple::NetBSD, llvm::Triple::x86_64, NETBSD::AMD64::NT_FPREGS},
+{llvm::Triple::NetBSD, llvm::Triple::x86, NETBSD::X86::NT_FPREGS},
+{llvm::Triple::NetBSD, llvm::Triple::x86_64, NETBSD::X86::NT_FPREGS},
 {llvm::Triple::OpenBSD, llvm::Triple::UnknownArch, OPENBSD::NT_FPREGS},
 };
 
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -641,9 +641,10 @@
   thread_data.notes.push_back(note);
 }
   } break;
+  case llvm::Triple::x86:
   case llvm::Triple::x86_64: {
 // Assume order PT_GETREGS, PT_GETFPREGS
-if (note.info.n_type == NETBSD::AMD64::NT_REGS) {
+if (note.info.n_type == NETBSD::X86::NT_REGS) {
   // If this is the next thread, push the previous one first.
   if (had_nt_regs) {
 m_thread_data.push_back(thread_data);
@@ -658,7 +659,7 @@
 "Could not find general purpose registers note in core file.",
 llvm::inconvertibleErrorCode());
   had_nt_regs = true;
-} else if (note.info.n_type == NETBSD::AMD64::NT_FPREGS) {
+} else if (note.info.n_type == NETBSD::X86::NT_FPREGS) {
   if (!had_nt_regs || tid != thread_data.tid)
 return llvm::make_error(
 "Error parsing NetBSD core(5) notes: Unexpected order "


Index: lldb/test/Shell/Register/Core/x86-32-netbsd-gp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-32-netbsd-gp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-32-netbsd.core | FileCheck %

[Lldb-commits] [PATCH] D101086: [lldb] [Process/elf-core] Fix reading FPRs from FreeBSD/i386 cores

2021-04-22 Thread Michał Górny via Phabricator via lldb-commits

mgorny updated this revision to Diff 339719.
mgorny added a comment.

Also add FPU-specific checks to `x86-32-freebsd-addr.test`.


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

https://reviews.llvm.org/D101086

Files:
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/test/Shell/Register/Core/x86-32-freebsd-addr.test
  lldb/test/Shell/Register/Core/x86-32-freebsd-fp.test


Index: lldb/test/Shell/Register/Core/x86-32-freebsd-fp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-32-freebsd-fp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-32-freebsd.core | FileCheck 
%p/Inputs/x86-fp.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-32-freebsd-addr.test
===
--- lldb/test/Shell/Register/Core/x86-32-freebsd-addr.test
+++ lldb/test/Shell/Register/Core/x86-32-freebsd-addr.test
@@ -10,4 +10,7 @@
 # CHECK-DAG: ds = 0x003b
 # CHECK-DAG: es = 0x003b
 
-# TODO: fix reading fp registers
+# CHECK-DAG: fiseg = 0x
+# CHECK-DAG: fioff = 0x00401c52
+# CHECK-DAG: foseg = 0x
+# CHECK-DAG: fooff = 0xd8b8
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -96,6 +96,9 @@
 llvm::ArrayRef RegsetDescs);
 
 constexpr RegsetDesc FPR_Desc[] = {
+// FreeBSD/i386 core NT_FPREGSET is x87 FSAVE result but the XSAVE dump
+// starts with FXSAVE struct, so use that instead if available.
+{llvm::Triple::FreeBSD, llvm::Triple::x86, llvm::ELF::NT_X86_XSTATE},
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_FPREGSET},
 // In a i386 core file NT_FPREGSET is present, but it's not the result
 // of the FXSAVE instruction like in 64 bit files.


Index: lldb/test/Shell/Register/Core/x86-32-freebsd-fp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-32-freebsd-fp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-32-freebsd.core | FileCheck %p/Inputs/x86-fp.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-32-freebsd-addr.test
===
--- lldb/test/Shell/Register/Core/x86-32-freebsd-addr.test
+++ lldb/test/Shell/Register/Core/x86-32-freebsd-addr.test
@@ -10,4 +10,7 @@
 # CHECK-DAG: ds = 0x003b
 # CHECK-DAG: es = 0x003b
 
-# TODO: fix reading fp registers
+# CHECK-DAG: fiseg = 0x
+# CHECK-DAG: fioff = 0x00401c52
+# CHECK-DAG: foseg = 0x
+# CHECK-DAG: fooff = 0xd8b8
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -96,6 +96,9 @@
 llvm::ArrayRef RegsetDescs);
 
 constexpr RegsetDesc FPR_Desc[] = {
+// FreeBSD/i386 core NT_FPREGSET is x87 FSAVE result but the XSAVE dump
+// starts with FXSAVE struct, so use that instead if available.
+{llvm::Triple::FreeBSD, llvm::Triple::x86, llvm::ELF::NT_X86_XSTATE},
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_FPREGSET},
 // In a i386 core file NT_FPREGSET is present, but it's not the result
 // of the FXSAVE instruction like in 64 bit files.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D101094: lldb/Instrumentation: NFC-ish use GetFrameCodeAddressForSymbolication()

2021-04-22 Thread Frederic Riss via Phabricator via lldb-commits

friss created this revision.
friss added reviewers: jasonmolenda, JDevlieghere.
friss requested review of this revision.
Herald added a project: LLDB.

A couple of our Instrumentation runtimes were gathering backtraces,
storing it in a StructuredData array and later creating a HistoryThread
using this data. By deafult HistoryThread will consider the history PCs
as return addresses and thus will substract 1 from them to go to the
call address.

This is usually correct, but it's also wasteful as when we gather the
backtraces ourselves, we have much better information to decide how
to backtrace and symbolicate. This patch uses the new
GetFrameCodeAddressForSymbolication() to gather the PCs that should
be used for symbolication and configures the HistoryThread to just
use those PCs as-is.

(The MTC plugin was actaully applying a -1 itself and then the
HistoryThread would do it again, so this actaully fixes a bug there.)

rdar://77027680


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101094

Files:
  
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp


Index: 
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
===
--- 
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
+++ 
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
@@ -150,8 +150,8 @@
   StructuredData::Array *trace = new StructuredData::Array();
   auto trace_sp = StructuredData::ObjectSP(trace);
   for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
-const Address FCA =
-thread_sp->GetStackFrameAtIndex(I)->GetFrameCodeAddress();
+const Address FCA = thread_sp->GetStackFrameAtIndex(I)
+->GetFrameCodeAddressForSymbolication();
 if (FCA.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
   continue;
 
@@ -324,7 +324,11 @@
   info->GetObjectForDotSeparatedPath("tid");
   tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs);
+  // We gather symbolication addresses above, so no need for HistoryThread to
+  // try to infer the call addresses.
+  bool pcs_are_call_addresses = true;
+  HistoryThread *history_thread =
+  new HistoryThread(*process_sp, tid, PCs, pcs_are_call_addresses);
   ThreadSP new_thread_sp(history_thread);
   std::string stop_reason_description = GetStopReasonDescription(info);
   new_thread_sp->SetName(stop_reason_description.c_str());
Index: 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
===
--- 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
+++ 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
@@ -127,7 +127,7 @@
   StackFrameSP responsible_frame;
   for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
 StackFrameSP frame = thread_sp->GetStackFrameAtIndex(I);
-Address addr = frame->GetFrameCodeAddress();
+Address addr = frame->GetFrameCodeAddressForSymbolication();
 if (addr.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
   continue;
 
@@ -135,11 +135,6 @@
 if (!responsible_frame)
   responsible_frame = frame;
 
-// First frame in stacktrace should point to a real PC, not return address.
-if (I != 0 && trace->GetSize() == 0) {
-  addr.Slide(-1);
-}
-
 lldb::addr_t PC = addr.GetLoadAddress(&target);
 trace->AddItem(StructuredData::ObjectSP(new StructuredData::Integer(PC)));
   }
@@ -271,7 +266,11 @@
   info->GetObjectForDotSeparatedPath("tid");
   tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs);
+  // We gather symbolication addresses above, so no need for HistoryThread to
+  // try to infer the call addresses.
+  bool pcs_are_call_addresses = true;
+  HistoryThread *history_thread =
+  new HistoryThread(*process_sp, tid, PCs, pcs_are_call_addresses);
   ThreadSP new_thread_sp(history_thread);
 
   // Save this in the Process' ExtendedThreadList so a strong pointer retains


Index: lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
===
--- lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
@@ -150,8 +150,8 @@
   StructuredData::Array *trace = new StructuredData::Array();
   auto trace_sp = StructuredData::ObjectSP(trace);
   for (unsign

[Lldb-commits] [PATCH] D99989: [lldb-vscode] Distinguish shadowed variables in the scopes request

2021-04-22 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added inline comments.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2934
+// We first find out which variable names are duplicated
+std::unordered_map variable_name_counts;
+for (auto i = start_idx; i < end_idx; ++i) {

We are getting a variable name via the public API which returns a "const char 
*". The only way this works is if the string comes from a ConstString otherwise 
there are lifetime issues. So this is and should be ok. The diff you are 
talking about was from internal LLDB code. For internal LLDB code we can 
enforce this by ensuring that people use ConstString, but externally through 
the API, we assume any "const char *" that comes out of it is uniqued and comes 
form a ConstString.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99989

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

[Lldb-commits] [lldb] a62cbd9 - [lldb] Include thread name in crashlog.py output

2021-04-22 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-04-22T11:38:53-07:00
New Revision: a62cbd9a0211d08bace8794b435996890feb44d4

URL: 
https://github.com/llvm/llvm-project/commit/a62cbd9a0211d08bace8794b435996890feb44d4
DIFF: 
https://github.com/llvm/llvm-project/commit/a62cbd9a0211d08bace8794b435996890feb44d4.diff

LOG: [lldb] Include thread name in crashlog.py output

Update the JSON parser to include the thread name in the Thread object.

rdar://76677320

Added: 


Modified: 
lldb/examples/python/crashlog.py
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 1f26739f60e16..3fde19e0895b9 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -418,11 +418,15 @@ def parse(self):
 self.parse_images(self.data['usedImages'])
 self.parse_threads(self.data['threads'])
 thread = self.crashlog.threads[self.crashlog.crashed_thread_idx]
-thread.reason = self.parse_crash_reason(self.data['exception'])
+reason = self.parse_crash_reason(self.data['exception'])
+if thread.reason:
+thread.reason = '{} {}'.format(thread.reason, reason)
+else:
+thread.reason = reason
 except (KeyError, ValueError, TypeError) as e:
-   raise CrashLogParseException(
-   'Failed to parse JSON crashlog: {}: {}'.format(
-   type(e).__name__, e))
+raise CrashLogParseException(
+'Failed to parse JSON crashlog: {}: {}'.format(
+type(e).__name__, e))
 
 return self.crashlog
 
@@ -480,6 +484,8 @@ def parse_threads(self, json_threads):
 idx = 0
 for json_thread in json_threads:
 thread = self.crashlog.Thread(idx, False)
+if 'name' in json_thread:
+thread.reason = json_thread['name']
 if json_thread.get('triggered', False):
 self.crashlog.crashed_thread_idx = idx
 self.registers = self.parse_thread_registers(

diff  --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
index 02a7a037afe44..5446d0d9973a4 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
@@ -40,6 +40,7 @@
   {
 "triggered": true,
 "id": 6152004,
+"name": "Crashing Thread Name",
 "threadState": {
   "r13": {
 "value": 0

diff  --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
index fc95f489c3b4c..46c37132fc47a 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
@@ -3,7 +3,7 @@
 # RUN: python %S/patch-crashlog.py --binary %t.out --crashlog %t.crash 
--offsets '{"main":20, "bar":9, "foo":16}' --json
 # RUN: %lldb %t.out -o 'command script import lldb.macosx.crashlog' -o 
'crashlog %t.crash' 2>&1 | FileCheck %s
 
-# CHECK: Thread[0] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 
0x)
+# CHECK: Thread[0] Crashing Thread Name EXC_BAD_ACCESS (SIGSEGV) 
(KERN_INVALID_ADDRESS at 0x)
 # CHECK: [  0] {{.*}}out`foo + 16 at test.c
 # CHECK: [  1] {{.*}}out`bar + 8 at test.c
 # CHECK: [  2] {{.*}}out`main + 19 at test.c



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

[Lldb-commits] [PATCH] D101094: lldb/Instrumentation: NFC-ish use GetFrameCodeAddressForSymbolication()

2021-04-22 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp:272-274
+  HistoryThread *history_thread =
+  new HistoryThread(*process_sp, tid, PCs, pcs_are_call_addresses);
   ThreadSP new_thread_sp(history_thread);




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101094

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

[Lldb-commits] [PATCH] D100962: [lldb/Utility] Add SourceLocationSpec class (NFC)

2021-04-22 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib marked 9 inline comments as done.
mib added inline comments.



Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:160
+  /// file_spec and line number. An empty \a pattern matches everything.
+  static bool Match(const SourceLocationSpec &pattern,
+const SourceLocationSpec &location);

JDevlieghere wrote:
> Would it make sense to merge this into Equal and replace the bool with an 
> enum value that represents 
> 
>  - file + line must match, 
>  - file + line + col must match,
>  - file + line + col must match if lhs has it?
I simplified the API removing the `Match` method since it's the same as calling 
`Exact` with `bool full = false`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100962

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

[Lldb-commits] [PATCH] D100962: [lldb/Utility] Add SourceLocationSpec class (NFC)

2021-04-22 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 339743.
mib marked an inline comment as done.
mib edited the summary of this revision.
mib added a comment.

Addressed @JDevlieghere comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100962

Files:
  lldb/include/lldb/Utility/SourceLocationSpec.h
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/SourceLocationSpec.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/SourceLocationSpecTest.cpp

Index: lldb/unittests/Utility/SourceLocationSpecTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/SourceLocationSpecTest.cpp
@@ -0,0 +1,211 @@
+//===-- SourceLocationSpecTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Utility/LLDBAssert.h"
+#include "lldb/Utility/SourceLocationSpec.h"
+
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+
+TEST(SourceLocationSpecTest, CreateSourceLocationSpec) {
+  FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  const uint32_t line = 19;
+  const uint32_t column = 4;
+  const bool inlined = false;
+  const bool exact_match = true;
+
+  // Invalid FileSpec
+  ASSERT_THAT_EXPECTED(
+  SourceLocationSpec::Create(inlined, exact_match, FileSpec(), line),
+  llvm::Failed());
+  // Null Line
+  ASSERT_THAT_EXPECTED(SourceLocationSpec::Create(inlined, exact_match, fs, 0),
+   llvm::Failed());
+  // Invalid Line
+  ASSERT_THAT_EXPECTED(SourceLocationSpec::Create(inlined, exact_match, fs,
+  LLDB_INVALID_LINE_NUMBER),
+   llvm::Failed());
+  // Invalid FileSpec & Line
+  ASSERT_THAT_EXPECTED(
+  SourceLocationSpec::Create(inlined, exact_match, FileSpec(), 0),
+  llvm::Failed());
+
+  // Valid FileSpec & Line
+  ASSERT_THAT_EXPECTED(
+  SourceLocationSpec::Create(inlined, exact_match, fs, line),
+  llvm::Succeeded());
+  // Valid FileSpec, Line & Column
+  ASSERT_THAT_EXPECTED(
+  SourceLocationSpec::Create(inlined, exact_match, fs, line, column),
+  llvm::Succeeded());
+}
+
+TEST(SourceLocationSpecTest, FileLineColumnComponents) {
+  FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  const uint32_t line = 19;
+  const uint16_t column = 4;
+
+  auto location_spec = SourceLocationSpec::Create(false, true, fs, line);
+  ASSERT_THAT_EXPECTED(location_spec, llvm::Succeeded());
+  SourceLocationSpec without_column = *location_spec;
+  EXPECT_EQ(fs, without_column.GetFileSpec());
+  EXPECT_EQ(line, without_column.GetLine());
+  EXPECT_FALSE(without_column.HasColumn());
+  EXPECT_EQ(llvm::None, without_column.GetColumn());
+  EXPECT_FALSE(without_column.IsInlined());
+  EXPECT_TRUE(without_column.IsExactMatch());
+  EXPECT_STREQ("/foo/bar:19", without_column.GetCString());
+
+  location_spec = SourceLocationSpec::Create(true, false, fs, line, column);
+  ASSERT_THAT_EXPECTED(location_spec, llvm::Succeeded());
+  SourceLocationSpec with_column = *location_spec;
+  EXPECT_TRUE(with_column.HasColumn());
+  EXPECT_EQ(column, with_column.GetColumn());
+  EXPECT_TRUE(with_column.IsInlined());
+  EXPECT_FALSE(with_column.IsExactMatch());
+  EXPECT_STREQ("/foo/bar:19:4", with_column.GetCString());
+}
+
+TEST(SourceLocationSpecTest, Equal) {
+  auto Create = [](bool inlined, bool exact_match, FileSpec fs, uint32_t line,
+   uint16_t column = 0) -> SourceLocationSpec {
+auto location =
+SourceLocationSpec::Create(inlined, exact_match, fs, line, column);
+lldbassert(location && "Invalid SourceLocationSpec.");
+return *location;
+  };
+
+  auto Equal = [](SourceLocationSpec lhs, SourceLocationSpec rhs, bool full) {
+return SourceLocationSpec::Equal(lhs, rhs, full);
+  };
+
+  const FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  const FileSpec other_fs("/foo/baz", FileSpec::Style::posix);
+
+  // mutating FileSpec + const Inlined, ExactMatch, Line
+  EXPECT_TRUE(
+  Equal(Create(false, false, fs, 4), Create(false, false, fs, 4), true));
+  EXPECT_TRUE(
+  Equal(Create(true, true, fs, 4), Create(true, true, fs, 4), false));
+  EXPECT_FALSE(Equal(Create(false, false, fs, 4),
+ Create(false, false, other_fs, 4), true));
+  EXPECT_FALSE(
+  Equal(Create(true, true, fs, 4), Create(true, true, other_fs, 4), false));
+
+  // mutating FileSpec + const Inlined, ExactMatch, Line, Column
+  EXPECT_TRUE(Equal(Create(false, false, fs, 4, 19),
+Create(false, false, fs, 4, 19), true));
+  EXPECT_TRUE(Equal(Create(true, true, fs, 4,

[Lldb-commits] [lldb] 007158a - Skip unreliable LLDB tests when running under asan

2021-04-22 Thread Adrian Prantl via lldb-commits


Author: Adrian Prantl
Date: 2021-04-22T11:55:43-07:00
New Revision: 007158ac42c785cd59aab8bc4b4b39085b960a58

URL: 
https://github.com/llvm/llvm-project/commit/007158ac42c785cd59aab8bc4b4b39085b960a58
DIFF: 
https://github.com/llvm/llvm-project/commit/007158ac42c785cd59aab8bc4b4b39085b960a58.diff

LOG: Skip unreliable LLDB tests when running under asan

Added: 


Modified: 
lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py

Removed: 




diff  --git 
a/lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py 
b/lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
index 86a1693721774..a39fc0fc99df5 100644
--- 
a/lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
+++ 
b/lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
@@ -58,6 +58,7 @@ def get_pid(self):
 @skipIfWindows
 @expectedFailureNetBSD
 @expectedFailureDarwin # No signals delivered
+@skipIfAsan # Times out under asan
 def test_signal_process_without_tid(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -69,6 +70,7 @@ def test_signal_process_without_tid(self):
 
 @skipUnlessPlatform(["netbsd"])
 @expectedFailureNetBSD
+@skipIfAsan # Times out under asan
 def test_signal_one_thread(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -82,6 +84,7 @@ def test_signal_one_thread(self):
 @skipIfWindows
 @expectedFailureNetBSD
 @expectedFailureDarwin # Only one signal delivered
+@skipIfAsan # Times out under asan
 def test_signal_all_threads(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -97,6 +100,7 @@ def test_signal_all_threads(self):
 @skipIfWindows
 @expectedFailureNetBSD
 @add_test_categories(["llgs"])
+@skipIfAsan # Times out under asan
 def test_signal_process_by_pid(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -111,6 +115,7 @@ def test_signal_process_by_pid(self):
 @skipIfWindows
 @expectedFailureNetBSD
 @add_test_categories(["llgs"])
+@skipIfAsan # Times out under asan
 def test_signal_process_minus_one(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -136,6 +141,7 @@ def test_signal_minus_one(self):
 @skipIfWindows
 @expectedFailureNetBSD
 @add_test_categories(["llgs"])
+@skipIfAsan # Times out under asan
 def test_signal_all_threads_by_pid(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -152,6 +158,7 @@ def test_signal_all_threads_by_pid(self):
 @skipIfWindows
 @expectedFailureNetBSD
 @add_test_categories(["llgs"])
+@skipIfAsan # Times out under asan
 def test_signal_minus_one_by_pid(self):
 self.build()
 self.set_inferior_startup_launch()
@@ -166,6 +173,7 @@ def test_signal_minus_one_by_pid(self):
 @skipIfWindows
 @expectedFailureNetBSD
 @add_test_categories(["llgs"])
+@skipIfAsan # Times out under asan
 def test_signal_minus_one_by_minus_one(self):
 self.build()
 self.set_inferior_startup_launch()



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

[Lldb-commits] [PATCH] D101094: lldb/Instrumentation: NFC-ish use GetFrameCodeAddressForSymbolication()

2021-04-22 Thread Frederic Riss via Phabricator via lldb-commits

friss updated this revision to Diff 339746.
friss added a comment.

Use make_shared


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101094

Files:
  
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp


Index: 
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
===
--- 
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
+++ 
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
@@ -150,8 +150,8 @@
   StructuredData::Array *trace = new StructuredData::Array();
   auto trace_sp = StructuredData::ObjectSP(trace);
   for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
-const Address FCA =
-thread_sp->GetStackFrameAtIndex(I)->GetFrameCodeAddress();
+const Address FCA = thread_sp->GetStackFrameAtIndex(I)
+->GetFrameCodeAddressForSymbolication();
 if (FCA.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
   continue;
 
@@ -324,8 +324,11 @@
   info->GetObjectForDotSeparatedPath("tid");
   tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs);
-  ThreadSP new_thread_sp(history_thread);
+  // We gather symbolication addresses above, so no need for HistoryThread to
+  // try to infer the call addresses.
+  bool pcs_are_call_addresses = true;
+  ThreadSP new_thread_sp = std::make_shared(
+  *process_sp, tid, PCs, pcs_are_call_addresses);
   std::string stop_reason_description = GetStopReasonDescription(info);
   new_thread_sp->SetName(stop_reason_description.c_str());
 
Index: 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
===
--- 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
+++ 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
@@ -127,7 +127,7 @@
   StackFrameSP responsible_frame;
   for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
 StackFrameSP frame = thread_sp->GetStackFrameAtIndex(I);
-Address addr = frame->GetFrameCodeAddress();
+Address addr = frame->GetFrameCodeAddressForSymbolication();
 if (addr.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
   continue;
 
@@ -135,11 +135,6 @@
 if (!responsible_frame)
   responsible_frame = frame;
 
-// First frame in stacktrace should point to a real PC, not return address.
-if (I != 0 && trace->GetSize() == 0) {
-  addr.Slide(-1);
-}
-
 lldb::addr_t PC = addr.GetLoadAddress(&target);
 trace->AddItem(StructuredData::ObjectSP(new StructuredData::Integer(PC)));
   }
@@ -271,8 +266,11 @@
   info->GetObjectForDotSeparatedPath("tid");
   tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs);
-  ThreadSP new_thread_sp(history_thread);
+  // We gather symbolication addresses above, so no need for HistoryThread to
+  // try to infer the call addresses.
+  bool pcs_are_call_addresses = true;
+  ThreadSP new_thread_sp = std::make_shared(
+  *process_sp, tid, PCs, pcs_are_call_addresses);
 
   // Save this in the Process' ExtendedThreadList so a strong pointer retains
   // the object


Index: lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
===
--- lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
@@ -150,8 +150,8 @@
   StructuredData::Array *trace = new StructuredData::Array();
   auto trace_sp = StructuredData::ObjectSP(trace);
   for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
-const Address FCA =
-thread_sp->GetStackFrameAtIndex(I)->GetFrameCodeAddress();
+const Address FCA = thread_sp->GetStackFrameAtIndex(I)
+->GetFrameCodeAddressForSymbolication();
 if (FCA.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
   continue;
 
@@ -324,8 +324,11 @@
   info->GetObjectForDotSeparatedPath("tid");
   tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs);
-  ThreadSP new_thread_sp(history_thread);
+  // We gather symbolication addresses above, so no need for HistoryThread to
+  // try to infer the call addresses.
+  bool pcs_are_call_addresses = true;

[Lldb-commits] [PATCH] D101094: lldb/Instrumentation: NFC-ish use GetFrameCodeAddressForSymbolication()

2021-04-22 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Thanks for addressing my critical feedback. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101094

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

[Lldb-commits] [PATCH] D98370: [lldb] Fix SBValue::Persist() for constant values

2021-04-22 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

In D98370#2708672 , @werat wrote:

> In D98370#2705741 , @jingham wrote:
>
>> Sure.  But but when I was poking around at it a little bit, it seems like 
>> the other use cases already work, and the only one that was failing was the 
>> case where you call persist on a persistent variable.  If that is really 
>> true, then maybe we should fix the failing case directly.
>
> Right now `Persist()` doesn't really work for values created via 
> `CreateValueFromData`. You can read them, but can't modify:
>
>   >>> data = 
> lldb.SBData.CreateDataFromUInt64Array(lldb.process.GetByteOrder(), 
> lldb.process.GetAddressByteSize(), [42])
>   >>> v = lldb.target.CreateValueFromData('v', data, 
> lldb.target.GetBasicType(lldb.eBasicTypeUnsignedLong))
>   >>> v.value
>   '42'
>   >>> vp = v.Persist()
>   >>> vp.name
>   '$3'
>   >>> lldb.frame.EvaluateExpression('$3').value
>   '42'
>   >>> lldb.frame.EvaluateExpression('++$3 + 1').value
>   >>> lldb.frame.EvaluateExpression('++$3 + 1').error.GetCString()
>   "error: supposed to interpret, but failed: Interpreter couldn't read from 
> memory\n"
>
> However I realized my patch doesn't completely fixes it either:
>
>   >>> data = 
> lldb.SBData.CreateDataFromUInt64Array(lldb.process.GetByteOrder(), 
> lldb.process.GetAddressByteSize(), [42])
>   >>> v = lldb.target.CreateValueFromData('v', data, 
> lldb.target.GetBasicType(lldb.eBasicTypeUnsignedLong))
>   >>> vp = v.Persist()
>   >>> vp.name
>   '$0'
>   >>> lldb.frame.EvaluateExpression('$0').value
>   '42'
>   >>> lldb.frame.EvaluateExpression('++$0').value
>   '43'
>   >>> lldb.frame.EvaluateExpression('++$0').value
>   '44'
>   >>> vp.value
>   '42'
>
>
>
> In D98370#2705741 , @jingham wrote:
>
>> Not sure why?  The API is a request:  "I made a variable somehow, and I 
>> would like you to make it persist so I can use its value later on even if 
>> the underlying data has changed."  Why do you care whether you get a copy of 
>> an already persistent or just a shared value?
>
> You're right, I got confused by something else. I don't care if I get a new 
> name/copy, as long as I can use it by the returned name it's fine. However I 
> want to point out that the current API does generate a new name every time 
> (but the it points to the same data):
>
>   >>> x = lldb.frame.FindVariable('x')
>   >>> x.value
>   '1'
>   >>> xp1 = x.Persist()
>   >>> xp1.name
>   '$0'
>   >>> xp2 = x.Persist()
>   >>> xp2.name
>   '$1'
>   >>> lldb.frame.EvaluateExpression('++$0 + ++$1').value
>   '3'
>   >>> xp1.value
>   '3'
>   >>> xp2.value
>   '3'

Be careful here.  There are really two kinds of persistent variables: 
"expression results" and variables created in the expression parser.  The 
former are by design constant.  The idea is that you can use them to checkpoint 
the state of a value and refer to it later.  You can use their values in 
expressions, but they aren't supposed to be modifiable.  Those are the ones 
called $NUM.

The ones you make in an expression, like:

(lldb) expr int $myVar = 20

on the other hand are modifiable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98370

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

[Lldb-commits] [PATCH] D99989: [lldb-vscode] Distinguish shadowed variables in the scopes request

2021-04-22 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added inline comments.

Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2934
+// We first find out which variable names are duplicated
+std::unordered_map variable_name_counts;
+for (auto i = start_idx; i < end_idx; ++i) {

clayborg wrote:
> We are getting a variable name via the public API which returns a "const char 
> *". The only way this works is if the string comes from a ConstString 
> otherwise there are lifetime issues. So this is and should be ok. The diff 
> you are talking about was from internal LLDB code. For internal LLDB code we 
> can enforce this by ensuring that people use ConstString, but externally 
> through the API, we assume any "const char *" that comes out of it is uniqued 
> and comes form a ConstString.
I don't think there is any promise that every `const char *` coming from the SB 
API is generated from a ConstString. If there was such a promise a good chunk 
of the SB API would already violate it.

That the `const char *` needs to have a reasonable life time (e.g., the same 
life time as the SB API object that returns it) seems like a basic foundation 
to have the API work at all. But promising that they are uniquified really 
serves essentially no purpose? I sure hope that simply returning string 
literals doesn't suddenly become an API violation (or that we need to forever 
store every generated error message that we return from the SB API).

Also I think my example is spot on: An unverified assumption that doesn't hold 
true. And once we change the underlying code someone has to spend a week 
tracking down bogus string comparisons in the year 2021.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99989

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

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

2021-04-22 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

I assume you don't have a commit access, so I'm going to land this for you 
tomorrow. Thanks again for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

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

[Lldb-commits] [PATCH] D101091: [lldb] [Process/elf-core] Fix reading NetBSD/i386 core dumps

2021-04-22 Thread Michał Górny via Phabricator via lldb-commits

mgorny updated this revision to Diff 339768.
mgorny added a comment.

Use split code branches for i386 and amd64.


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

https://reviews.llvm.org/D101091

Files:
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
  lldb/test/Shell/Register/Core/x86-32-netbsd-addr.test
  lldb/test/Shell/Register/Core/x86-32-netbsd-gp.test

Index: lldb/test/Shell/Register/Core/x86-32-netbsd-gp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-32-netbsd-gp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-32-netbsd.core | FileCheck %p/Inputs/x86-32-gp.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-32-netbsd-addr.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-32-netbsd-addr.test
@@ -0,0 +1,13 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-32-netbsd.core | FileCheck %s
+
+register read --all
+# CHECK-DAG: eip = 0x08048955
+# CHECK-DAG: eflags = 0x00010282
+# CHECK-DAG: cs = 0x0037
+# CHECK-DAG: fs = 0x004f
+# CHECK-DAG: gs = 0x008b
+# CHECK-DAG: ss = 0x004f
+# CHECK-DAG: ds = 0x004f
+# CHECK-DAG: es = 0x004f
+
+# TODO: fix reading fp registers
Index: lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -20,6 +20,7 @@
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_s390x.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
+#include "Plugins/Process/Utility/RegisterContextNetBSD_i386.h"
 #include "Plugins/Process/Utility/RegisterContextNetBSD_x86_64.h"
 #include "Plugins/Process/Utility/RegisterContextOpenBSD_i386.h"
 #include "Plugins/Process/Utility/RegisterContextOpenBSD_x86_64.h"
@@ -107,6 +108,9 @@
   switch (arch.GetMachine()) {
   case llvm::Triple::aarch64:
 break;
+  case llvm::Triple::x86:
+reg_interface = new RegisterContextNetBSD_i386(arch);
+break;
   case llvm::Triple::x86_64:
 reg_interface = new RegisterContextNetBSD_x86_64(arch);
 break;
Index: lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -55,6 +55,10 @@
 enum { NT_REGS = 33, NT_FPREGS = 35 };
 }
 
+namespace I386 {
+enum { NT_REGS = 33, NT_FPREGS = 35 };
+}
+
 } // namespace NETBSD
 
 namespace OPENBSD {
@@ -106,6 +110,7 @@
 {llvm::Triple::Linux, llvm::Triple::x86, llvm::ELF::NT_PRXFPREG},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, llvm::ELF::NT_FPREGSET},
 {llvm::Triple::NetBSD, llvm::Triple::aarch64, NETBSD::AARCH64::NT_FPREGS},
+{llvm::Triple::NetBSD, llvm::Triple::x86, NETBSD::I386::NT_FPREGS},
 {llvm::Triple::NetBSD, llvm::Triple::x86_64, NETBSD::AMD64::NT_FPREGS},
 {llvm::Triple::OpenBSD, llvm::Triple::UnknownArch, OPENBSD::NT_FPREGS},
 };
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -641,6 +641,32 @@
   thread_data.notes.push_back(note);
 }
   } break;
+  case llvm::Triple::x86: {
+// Assume order PT_GETREGS, PT_GETFPREGS
+if (note.info.n_type == NETBSD::I386::NT_REGS) {
+  // If this is the next thread, push the previous one first.
+  if (had_nt_regs) {
+m_thread_data.push_back(thread_data);
+thread_data = ThreadData();
+had_nt_regs = false;
+  }
+
+  thread_data.gpregset = note.data;
+  thread_data.tid = tid;
+  if (thread_data.gpregset.GetByteSize() == 0)
+return llvm::make_error(
+"Could not find general purpose registers note in core file.",
+llvm::inconvertibleErrorCode());
+  had_nt_regs = true;
+} else if (note.info.n_type == NETBSD::I386::NT_FPREGS) {
+  if (!had_nt_regs || tid != thread_data.tid)
+return llvm::make_error(
+"Error parsing NetBSD core(5) notes: Unexpected order "
+"of NOTEs PT_GETFPREG before PT_GETREG",
+llvm::inconvertibleErrorCode());
+  thread_data.notes.push_back(note);
+}
+  } break;
   case llvm::Triple::x86_64: {
 // Assume order PT_GETREGS, PT_GETFPREGS
 if (note.info.n_type == NETBSD:

[Lldb-commits] [lldb] 18a8527 - [trace][intel-pt] Fix a crash on unconsumed Expected's Error

2021-04-22 Thread Jan Kratochvil via lldb-commits


Author: Jan Kratochvil
Date: 2021-04-22T22:27:08+02:00
New Revision: 18a85276426b9604a9ceca8c528b694964dd8d7c

URL: 
https://github.com/llvm/llvm-project/commit/18a85276426b9604a9ceca8c528b694964dd8d7c
DIFF: 
https://github.com/llvm/llvm-project/commit/18a85276426b9604a9ceca8c528b694964dd8d7c.diff

LOG: [trace][intel-pt] Fix a crash on unconsumed Expected's Error

Reproducible with build using libipt and -DLLVM_ENABLE_ASSERTIONS=ON:
(lldb) b main
(lldb) r
(lldb) process trace start

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/IntelPTManager.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTManager.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTManager.cpp
index 14b08a6aa93c0..00aed576eb1d5 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTManager.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTManager.cpp
@@ -542,7 +542,14 @@ IntelPTManager::GetBinaryData(const 
TraceGetBinaryDataRequest &request) const {
 
 void IntelPTManager::ClearProcessTracing() { m_process_trace = None; }
 
-bool IntelPTManager::IsSupported() { return (bool)GetOSEventType(); }
+bool IntelPTManager::IsSupported() {
+  Expected intel_pt_type = GetOSEventType();
+  if (!intel_pt_type) {
+llvm::consumeError(intel_pt_type.takeError());
+return false;
+  }
+  return true;
+}
 
 bool IntelPTManager::IsProcessTracingEnabled() const {
   return (bool)m_process_trace;



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

[Lldb-commits] [PATCH] D100340: [lldb-vscode] Add postRunCommands

2021-04-22 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

The regression was due to the `help process trace` command which surprisingly 
ends up in `GDBRemoteCommunicationClient::SendTraceSupported` which was 
crashing on `libipt`-enabled builds. Fixed by: 
rG18a85276426b9604a9ceca8c528b694964dd8d7c 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100340

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

[Lldb-commits] [PATCH] D91679: [trace][intel-pt] Implement trace start and trace stop

2021-04-22 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added a comment.

Checked in a regression fix: rG18a85276426b9604a9ceca8c528b694964dd8d7c 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91679

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

[Lldb-commits] [lldb] 91e90cf - lldb/Instrumentation: NFC-ish use GetFrameCodeAddressForSymbolication()

2021-04-22 Thread Fred Riss via lldb-commits


Author: Fred Riss
Date: 2021-04-22T13:32:43-07:00
New Revision: 91e90cf622074633009788a220a354043a609dee

URL: 
https://github.com/llvm/llvm-project/commit/91e90cf622074633009788a220a354043a609dee
DIFF: 
https://github.com/llvm/llvm-project/commit/91e90cf622074633009788a220a354043a609dee.diff

LOG: lldb/Instrumentation: NFC-ish use GetFrameCodeAddressForSymbolication()

A couple of our Instrumentation runtimes were gathering backtraces,
storing it in a StructuredData array and later creating a HistoryThread
using this data. By deafult HistoryThread will consider the history PCs
as return addresses and thus will substract 1 from them to go to the
call address.

This is usually correct, but it's also wasteful as when we gather the
backtraces ourselves, we have much better information to decide how
to backtrace and symbolicate. This patch uses the new
GetFrameCodeAddressForSymbolication() to gather the PCs that should
be used for symbolication and configures the HistoryThread to just
use those PCs as-is.

(The MTC plugin was actaully applying a -1 itself and then the
HistoryThread would do it again, so this actaully fixes a bug there.)

rdar://77027680

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

Added: 


Modified: 

lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp

lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
index 99784bd3dbd19..9a88b343878cc 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
@@ -127,7 +127,7 @@ InstrumentationRuntimeMainThreadChecker::RetrieveReportData(
   StackFrameSP responsible_frame;
   for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
 StackFrameSP frame = thread_sp->GetStackFrameAtIndex(I);
-Address addr = frame->GetFrameCodeAddress();
+Address addr = frame->GetFrameCodeAddressForSymbolication();
 if (addr.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
   continue;
 
@@ -135,11 +135,6 @@ 
InstrumentationRuntimeMainThreadChecker::RetrieveReportData(
 if (!responsible_frame)
   responsible_frame = frame;
 
-// First frame in stacktrace should point to a real PC, not return address.
-if (I != 0 && trace->GetSize() == 0) {
-  addr.Slide(-1);
-}
-
 lldb::addr_t PC = addr.GetLoadAddress(&target);
 trace->AddItem(StructuredData::ObjectSP(new StructuredData::Integer(PC)));
   }
@@ -271,8 +266,11 @@ 
InstrumentationRuntimeMainThreadChecker::GetBacktracesFromExtendedStopInfo(
   info->GetObjectForDotSeparatedPath("tid");
   tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs);
-  ThreadSP new_thread_sp(history_thread);
+  // We gather symbolication addresses above, so no need for HistoryThread to
+  // try to infer the call addresses.
+  bool pcs_are_call_addresses = true;
+  ThreadSP new_thread_sp = std::make_shared(
+  *process_sp, tid, PCs, pcs_are_call_addresses);
 
   // Save this in the Process' ExtendedThreadList so a strong pointer retains
   // the object

diff  --git 
a/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
index b60eb53f3d4a7..5f27da609f84b 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
@@ -150,8 +150,8 @@ StructuredData::ObjectSP 
InstrumentationRuntimeUBSan::RetrieveReportData(
   StructuredData::Array *trace = new StructuredData::Array();
   auto trace_sp = StructuredData::ObjectSP(trace);
   for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
-const Address FCA =
-thread_sp->GetStackFrameAtIndex(I)->GetFrameCodeAddress();
+const Address FCA = thread_sp->GetStackFrameAtIndex(I)
+->GetFrameCodeAddressForSymbolication();
 if (FCA.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
   continue;
 
@@ -324,8 +324,11 @@ 
InstrumentationRuntimeUBSan::GetBacktracesFromExtendedStopInfo(
   info->GetObjectForDotSeparatedPath("tid");
   tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs);
-  ThreadSP new_thread_sp(history_thread);
+  // We gather symbolication addres

[Lldb-commits] [PATCH] D101094: lldb/Instrumentation: NFC-ish use GetFrameCodeAddressForSymbolication()

2021-04-22 Thread Frederic Riss via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
friss marked an inline comment as done.
Closed by commit rG91e90cf62207: lldb/Instrumentation: NFC-ish use 
GetFrameCodeAddressForSymbolication() (authored by friss).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101094

Files:
  
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp


Index: 
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
===
--- 
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
+++ 
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
@@ -150,8 +150,8 @@
   StructuredData::Array *trace = new StructuredData::Array();
   auto trace_sp = StructuredData::ObjectSP(trace);
   for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
-const Address FCA =
-thread_sp->GetStackFrameAtIndex(I)->GetFrameCodeAddress();
+const Address FCA = thread_sp->GetStackFrameAtIndex(I)
+->GetFrameCodeAddressForSymbolication();
 if (FCA.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
   continue;
 
@@ -324,8 +324,11 @@
   info->GetObjectForDotSeparatedPath("tid");
   tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs);
-  ThreadSP new_thread_sp(history_thread);
+  // We gather symbolication addresses above, so no need for HistoryThread to
+  // try to infer the call addresses.
+  bool pcs_are_call_addresses = true;
+  ThreadSP new_thread_sp = std::make_shared(
+  *process_sp, tid, PCs, pcs_are_call_addresses);
   std::string stop_reason_description = GetStopReasonDescription(info);
   new_thread_sp->SetName(stop_reason_description.c_str());
 
Index: 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
===
--- 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
+++ 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
@@ -127,7 +127,7 @@
   StackFrameSP responsible_frame;
   for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
 StackFrameSP frame = thread_sp->GetStackFrameAtIndex(I);
-Address addr = frame->GetFrameCodeAddress();
+Address addr = frame->GetFrameCodeAddressForSymbolication();
 if (addr.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
   continue;
 
@@ -135,11 +135,6 @@
 if (!responsible_frame)
   responsible_frame = frame;
 
-// First frame in stacktrace should point to a real PC, not return address.
-if (I != 0 && trace->GetSize() == 0) {
-  addr.Slide(-1);
-}
-
 lldb::addr_t PC = addr.GetLoadAddress(&target);
 trace->AddItem(StructuredData::ObjectSP(new StructuredData::Integer(PC)));
   }
@@ -271,8 +266,11 @@
   info->GetObjectForDotSeparatedPath("tid");
   tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp, tid, PCs);
-  ThreadSP new_thread_sp(history_thread);
+  // We gather symbolication addresses above, so no need for HistoryThread to
+  // try to infer the call addresses.
+  bool pcs_are_call_addresses = true;
+  ThreadSP new_thread_sp = std::make_shared(
+  *process_sp, tid, PCs, pcs_are_call_addresses);
 
   // Save this in the Process' ExtendedThreadList so a strong pointer retains
   // the object


Index: lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
===
--- lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
@@ -150,8 +150,8 @@
   StructuredData::Array *trace = new StructuredData::Array();
   auto trace_sp = StructuredData::ObjectSP(trace);
   for (unsigned I = 0; I < thread_sp->GetStackFrameCount(); ++I) {
-const Address FCA =
-thread_sp->GetStackFrameAtIndex(I)->GetFrameCodeAddress();
+const Address FCA = thread_sp->GetStackFrameAtIndex(I)
+->GetFrameCodeAddressForSymbolication();
 if (FCA.GetModule() == runtime_module_sp) // Skip PCs from the runtime.
   continue;
 
@@ -324,8 +324,11 @@
   info->GetObjectForDotSeparatedPath("tid");
   tid_t tid = thread_id_obj ? thread_id_obj->GetIntegerValue() : 0;
 
-  HistoryThread *history_thread = new HistoryThread(*process_sp

[Lldb-commits] [PATCH] D101094: lldb/Instrumentation: NFC-ish use GetFrameCodeAddressForSymbolication()

2021-04-22 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added inline comments.



Comment at: 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp:273
+  ThreadSP new_thread_sp = std::make_shared(
+  *process_sp, tid, PCs, pcs_are_call_addresses);
 

A nitpick but you could have also done:

```
 ThreadSP new_thread_sp = std::make_shared(
  *process_sp, tid, PCs, true /*pcs_are_call_addresses*/);

```

Otherwise I would have made `pcs_are_call_addresses` a constant since you are 
using purely for documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101094

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

[Lldb-commits] [PATCH] D98370: [lldb] Fix SBValue::Persist() for constant values

2021-04-22 Thread Andy Yankovsky via Phabricator via lldb-commits

werat added a comment.

In D98370#2709828 , @jingham wrote:

> Be careful here.  There are really two kinds of persistent variables: 
> "expression results" and variables created in the expression parser.  The 
> former are by design constant.  The idea is that you can use them to 
> checkpoint the state of a value and refer to it later.  You can use their 
> values in expressions, but they aren't supposed to be modifiable.  Those are 
> the ones called $NUM.
>
> The ones you make in an expression, like:
>
> (lldb) expr int $myVar = 20
>
> on the other hand are modifiable.

What about the values created via `SBValue::Persist()` then? They have names 
like `$NUM`, but they can be either of the two you mentioned and more (values 
created via `CreateValueFromData` or values acquired via `FindVariable`):

  v = frame.EvaluateExpression("...")  # `v` is "$0" -- it's an "expression 
result" by your classification
  vp = v.Persist()  # `vp` is "$1" -- I assume it should point the same value 
as "$0" and be non-modifiable

  --

  v = frame.EvaluateExpression("$myVar = 20")  # `v` is "$0" -- it's an 
"expression result" by your classification
  vp = v.Persist()  # `vp` is "$1" -- I assume it should point the same value 
as "$0" and be non-modifiable

  myVar = frame.EvaluateExpression("$myVar")  # now `myVar` is "$myVar" -- it's 
a "variables created in the expression parser"
  myVarP= myVar.Persist()  # `myVarP` is "$2" -- ?? Should it point the same 
value as `myVar` and be modifiable?

  --

  v = target.CreateValueFromData(...)
  vp = v.Persist()  # `vp` is "$0" -- ?? Should it be modifiable? Should it 
point to the original `v`?

--

I think at this point I should explain the problem I'm solving from the 
beginning.

I have built an expression evaluator for LLDB (and based on LLDB) -- 
https://github.com/google/lldb-eval. It's much faster than LLDB's 
`EvaluateExpression`, but has some limitations -- it doesn't support 
user-defined function calls, doesn't allows to evaluate arbitrary C++ code, etc.
It is used in Stadia for Visual Studio debugger 
(https://github.com/googlestadia/vsi-lldb) complementing LLDB. Expressions that 
are not supported by `lldb-eval` are evaluated by `LLDB`, thus from user 
perspective everything "just works". In some situations we need to maintain 
some "state", which can be read/written by multiple consecutive expressions. 
Simplified example:

  EvaluateExpression("$index += 1");
  v1 = EvaluateExpression("array[$index]")

  EvaluateExpression("$index += 1");
  v2 = EvaluateExpression("array[$index]")

In case of `LLDB` we create `$index` via something like `expr $index = 0` and 
then the expressions can modify it. In `lldb-eval`, however, we don't want to 
use LLDB's persistent variables, so the state is represented via regular map of 
`SBValue` objects 
(https://werat.dev/blog/blazing-fast-expression-evaluation-for-c-in-lldb/#maintaning-state).
 Basically the API is something like this:

  state = {"index": some_sbvalue}

  EvaluateExpression("index += 1", state);
  v1 = EvaluateExpression("array[index]", state)

  EvaluateExpression("index += 1", state);
  v2 = EvaluateExpression("array[index]", state)

  some_sbvalue.GetValue()  # == 2

But as I mentioned before, our debugger shell is "smart" and can fallback to 
`LLDB` if `lldb-eval` fails to evaluate the expression. If there was a state 
involved, it needs to be converted to LLDB's persistent variables. Right now we 
workaround this issues by always allocating "state" values in LLDB 
(https://github.com/googlestadia/vsi-lldb/blob/0fcbe30a498fac70230efdb815125ee5f6f087bb/YetiVSI/DebugEngine/NatvisEngine/NatvisExpressionEvaluator.cs#L315)
 -- this way we don't need to convert it back. However it would be better to 
convert only if needed and for that I was hoping to use `SBValue::Persist()`.

The example below doesn't work, but that's what I would like to fix (code is 
simplified):

  // We can assume here the result is created via something like 
`CreateValueFromData()`
  // 
  lldb::SBValue counter = lldb_eval::EvaluateExpression("10");

  lldb_eval::ContextVariableList vars = { {"counter", &counter } };

  while (counter.GetValueAsUnsigned() > 5) {
lldb_eval::EvaluateExpression("--counter", vars);
  }

  lldb::SBValue p = counter.Persist();  // Assume `p.GetName()` is "$1"
  lldb::SBValue v1 = lldb::EvaluateExpression("$1");// v1.GetValue() == 5
  lldb::SBValue v2 = lldb::EvaluateExpression("--$1");  // v2.GetValue() == 4

Sorry for the long message, I hope it's more clear what I'm trying to do here :)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98370

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

[Lldb-commits] [PATCH] D101094: lldb/Instrumentation: NFC-ish use GetFrameCodeAddressForSymbolication()

2021-04-22 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added inline comments.



Comment at: 
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp:273
+  ThreadSP new_thread_sp = std::make_shared(
+  *process_sp, tid, PCs, pcs_are_call_addresses);
 

shafik wrote:
> A nitpick but you could have also done:
> 
> ```
>  ThreadSP new_thread_sp = std::make_shared(
>   *process_sp, tid, PCs, true /*pcs_are_call_addresses*/);
> 
> ```
> 
> Otherwise I would have made `pcs_are_call_addresses` a constant since you are 
> using purely for documentation.
I think the format `/*pc_are_call_addresses=*/true);` is what LLVM is using 
(with that format clang-tidy will check that the argument fits the parameter 
name).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101094

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

[Lldb-commits] [PATCH] D96236: [lldb] DWZ 1/9: Pass main DWARFUnit * along DWARFDIEs

2021-04-22 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

I would be fine with DWARFDie getting bigger to 24B. These objects are used 
temporarily and not used as part of the storage of all of a DWARFUnit's DIEs. 
DWARFUnit objects store an array of DWARFDebugInfoEntry objects and those are 
what we care about not getting bigger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96236

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

[Lldb-commits] [lldb] 91d3f73 - [lldb] Update register state parsing for JSON crashlogs

2021-04-22 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-04-22T16:40:59-07:00
New Revision: 91d3f73937b603b168a2be40f57a81efcc37da86

URL: 
https://github.com/llvm/llvm-project/commit/91d3f73937b603b168a2be40f57a81efcc37da86
DIFF: 
https://github.com/llvm/llvm-project/commit/91d3f73937b603b168a2be40f57a81efcc37da86.diff

LOG: [lldb] Update register state parsing for JSON crashlogs

 - The register encoding state in the JSON crashlog format changes.
   Update the parser accordingly.
 - Print the register state when printing the symbolicated thread.

Added: 


Modified: 
lldb/examples/python/crashlog.py
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 3fde19e0895b..45f7d01bc38a 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -97,7 +97,7 @@ def dump(self, prefix):
 if self.registers:
 print("%s  Registers:" % (prefix))
 for reg in self.registers.keys():
-print("%s%-5s = %#16.16x" % (prefix, reg, 
self.registers[reg]))
+print("%s%-8s = %#16.16x" % (prefix, reg, 
self.registers[reg]))
 
 def dump_symbolicated(self, crash_log, options):
 this_thread_crashed = self.app_specific_backtrace
@@ -156,6 +156,10 @@ def dump_symbolicated(self, crash_log, options):
 symbolicated_frame_address_idx += 1
 else:
 print(frame)
+if self.registers:
+print()
+for reg in self.registers.keys():
+print("%-8s = %#16.16x" % (reg, self.registers[reg]))
 
 def add_ident(self, ident):
 if ident not in self.idents:
@@ -488,7 +492,7 @@ def parse_threads(self, json_threads):
 thread.reason = json_thread['name']
 if json_thread.get('triggered', False):
 self.crashlog.crashed_thread_idx = idx
-self.registers = self.parse_thread_registers(
+thread.registers = self.parse_thread_registers(
 json_thread['threadState'])
 thread.queue = json_thread.get('queue')
 self.parse_frames(thread, json_thread.get('frames', []))
@@ -496,19 +500,13 @@ def parse_threads(self, json_threads):
 idx += 1
 
 def parse_thread_registers(self, json_thread_state):
-idx = 0
 registers = dict()
-for json_reg in json_thread_state.get('x', []):
-key = str('x{}'.format(idx))
-value = int(json_reg['value'])
-registers[key] = value
-idx += 1
-
-for register in ['lr', 'cpsr', 'fp', 'sp', 'esr', 'pc']:
-if register in json_thread_state:
-json_reg = json_thread_state[register]
-registers[register] = int(json_reg['value'])
-
+for key, state in json_thread_state.items():
+try:
+   value = int(state['value'])
+   registers[key] = value
+except (TypeError, ValueError):
+   pass
 return registers
 
 

diff  --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
index 46c37132fc47..a514b07fe9f8 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/json.test
@@ -7,3 +7,4 @@
 # CHECK: [  0] {{.*}}out`foo + 16 at test.c
 # CHECK: [  1] {{.*}}out`bar + 8 at test.c
 # CHECK: [  2] {{.*}}out`main + 19 at test.c
+# CHECK: rbp = 0x7ffeec22a530

diff  --git a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
index 7c6e1fc1d1da..44ee5cb80431 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/text.test
@@ -7,3 +7,4 @@
 # CHECK: [  0] {{.*}}out`foo + 16 at test.c
 # CHECK: [  1] {{.*}}out`bar + 8 at test.c
 # CHECK: [  2] {{.*}}out`main + 19 at test.c
+# CHECK: rbp = 0x7ffee42d8020



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

[Lldb-commits] [PATCH] D98370: [lldb] Fix SBValue::Persist() for constant values

2021-04-22 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

In D98370#2710058 , @werat wrote:

> In D98370#2709828 , @jingham wrote:
>
>> Be careful here.  There are really two kinds of persistent variables: 
>> "expression results" and variables created in the expression parser.  The 
>> former are by design constant.  The idea is that you can use them to 
>> checkpoint the state of a value and refer to it later.  You can use their 
>> values in expressions, but they aren't supposed to be modifiable.  Those are 
>> the ones called $NUM.
>>
>> The ones you make in an expression, like:
>>
>> (lldb) expr int $myVar = 20
>>
>> on the other hand are modifiable.
>
> What about the values created via `SBValue::Persist()` then? They have names 
> like `$NUM`, but they can be either of the two you mentioned and more (values 
> created via `CreateValueFromData` or values acquired via `FindVariable`):

I am guessing those names are chosen just because that was a handy routine for 
auto-generating a variable name.  It doesn't follow with our general naming 
conventions, we should probably distinguish them from result variables by 
giving them some other name (like $P0...) or something.

>   v = frame.EvaluateExpression("...")  # `v` is "$0" -- it's an "expression 
> result" by your classification
>   vp = v.Persist()  # `vp` is "$1" -- I assume it should point the same value 
> as "$0" and be non-modifiable
>   
>   --
>   
>   v = frame.EvaluateExpression("$myVar = 20")  # `v` is "$0" -- it's an 
> "expression result" by your classification
>   vp = v.Persist()  # `vp` is "$1" -- I assume it should point the same value 
> as "$0" and be non-modifiable
>   
>   myVar = frame.EvaluateExpression("$myVar")  # now `myVar` is "$myVar" -- 
> it's a "variables created in the expression parser"
>   myVarP= myVar.Persist()  # `myVarP` is "$2" -- ?? Should it point the same 
> value as `myVar` and be modifiable?
>   
>   --
>   
>   v = target.CreateValueFromData(...)
>   vp = v.Persist()  # `vp` is "$0" -- ?? Should it be modifiable? Should it 
> point to the original `v`?
>
>
>
> --
>
> I think at this point I should explain the problem I'm solving from the 
> beginning.
>
> I have built an expression evaluator for LLDB (and based on LLDB) -- 
> https://github.com/google/lldb-eval. It's much faster than LLDB's 
> `EvaluateExpression`, but has some limitations -- it doesn't support 
> user-defined function calls, doesn't allows to evaluate arbitrary C++ code, 
> etc.
> It is used in Stadia for Visual Studio debugger 
> (https://github.com/googlestadia/vsi-lldb) complementing LLDB. Expressions 
> that are not supported by `lldb-eval` are evaluated by `LLDB`, thus from user 
> perspective everything "just works". In some situations we need to maintain 
> some "state", which can be read/written by multiple consecutive expressions. 
> Simplified example:
>
>   EvaluateExpression("$index += 1");
>   v1 = EvaluateExpression("array[$index]")
>   
>   EvaluateExpression("$index += 1");
>   v2 = EvaluateExpression("array[$index]")
>
> In case of `LLDB` we create `$index` via something like `expr $index = 0` and 
> then the expressions can modify it. In `lldb-eval`, however, we don't want to 
> use LLDB's persistent variables, so the state is represented via regular map 
> of `SBValue` objects 
> (https://werat.dev/blog/blazing-fast-expression-evaluation-for-c-in-lldb/#maintaning-state).
>  Basically the API is something like this:
>
>   state = {"index": some_sbvalue}
>   
>   EvaluateExpression("index += 1", state);
>   v1 = EvaluateExpression("array[index]", state)
>   
>   EvaluateExpression("index += 1", state);
>   v2 = EvaluateExpression("array[index]", state)
>   
>   some_sbvalue.GetValue()  # == 2
>
> But as I mentioned before, our debugger shell is "smart" and can fallback to 
> `LLDB` if `lldb-eval` fails to evaluate the expression. If there was a state 
> involved, it needs to be converted to LLDB's persistent variables. Right now 
> we workaround this issues by always allocating "state" values in LLDB 
> (https://github.com/googlestadia/vsi-lldb/blob/0fcbe30a498fac70230efdb815125ee5f6f087bb/YetiVSI/DebugEngine/NatvisEngine/NatvisExpressionEvaluator.cs#L315)
>  -- this way we don't need to convert it back. However it would be better to 
> convert only if needed and for that I was hoping to use `SBValue::Persist()`.
>
> The example below doesn't work, but that's what I would like to fix (code is 
> simplified):
>
>   // We can assume here the result is created via something like 
> `CreateValueFromData()`
>   // 
>   lldb::SBValue counter = lldb_eval::EvaluateExpression("10");
>   
>   lldb_eval::ContextVariableList vars = { {"counter", &counter } };
>   
>   while (counter.GetValueAsUnsigned() > 5) {
> lldb_eval::EvaluateExpression("--counter", vars);
>   }
>   
>   lldb::SBValue p = counter.Persist();  // Assume `p.GetName()` is "$1"
>   lldb::SBValue v1 = lldb::Eval

[Lldb-commits] [PATCH] D99989: [lldb-vscode] Distinguish shadowed variables in the scopes request

2021-04-22 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.



Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2934
+// We first find out which variable names are duplicated
+std::unordered_map variable_name_counts;
+for (auto i = start_idx; i < end_idx; ++i) {

teemperor wrote:
> clayborg wrote:
> > We are getting a variable name via the public API which returns a "const 
> > char *". The only way this works is if the string comes from a ConstString 
> > otherwise there are lifetime issues. So this is and should be ok. The diff 
> > you are talking about was from internal LLDB code. For internal LLDB code 
> > we can enforce this by ensuring that people use ConstString, but externally 
> > through the API, we assume any "const char *" that comes out of it is 
> > uniqued and comes form a ConstString.
> I don't think there is any promise that every `const char *` coming from the 
> SB API is generated from a ConstString. If there was such a promise a good 
> chunk of the SB API would already violate it.
> 
> That the `const char *` needs to have a reasonable life time (e.g., the same 
> life time as the SB API object that returns it) seems like a basic foundation 
> to have the API work at all. But promising that they are uniquified really 
> serves essentially no purpose? I sure hope that simply returning string 
> literals doesn't suddenly become an API violation (or that we need to forever 
> store every generated error message that we return from the SB API).
> 
> Also I think my example is spot on: An unverified assumption that doesn't 
> hold true. And once we change the underlying code someone has to spend a week 
> tracking down bogus string comparisons in the year 2021.
The number of variables shouldn't be massive, so I'll change it to std::string 
just for safety. I don't want to spend a lot of time debugging this if at some 
point the assumption doensn't hold.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99989

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

[Lldb-commits] [PATCH] D101128: [lldb-vscode] only report long running progress events

2021-04-22 Thread walter erquinigo via Phabricator via lldb-commits

wallace created this revision.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When the number of shared libs is massive, there could be hundreds of
thousands of short lived progress events sent to the IDE, which makes it
irresponsive while it's processing all this data. As these small jobs
take less than a second to process, the user doesn't even see them,
because the IDE only display the progress of long operations. So it's
better not to send these events.

I'm fixing that by sending only the events that are taking longer than 5
seconds to process.
In a specific run, I got the number of events from ~500k to 100, because
there was only 1 big lib to parse.

I've tried this on several small and massive targets, and it seems to
work fine.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101128

Files:
  lldb/tools/lldb-vscode/ProgressEvent.cpp
  lldb/tools/lldb-vscode/ProgressEvent.h
  lldb/tools/lldb-vscode/VSCode.h

Index: lldb/tools/lldb-vscode/VSCode.h
===
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -125,10 +125,6 @@
   int64_t GetLineForPC(int64_t sourceReference, lldb::addr_t pc) const;
   ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
   ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
-  // Send the JSON in "json_str" to the "out" stream. Correctly send the
-  // "Content-Length:" field followed by the length, followed by the raw
-  // JSON bytes.
-  void SendJSON(const std::string &json_str);
 
   // Serialize the JSON value into a string and send the JSON packet to
   // the "out" stream.
@@ -208,6 +204,12 @@
   /// The callback to execute when the given request is triggered by the
   /// IDE.
   void RegisterRequestCallback(std::string request, RequestCallback callback);
+
+private:
+  // Send the JSON in "json_str" to the "out" stream. Correctly send the
+  // "Content-Length:" field followed by the length, followed by the raw
+  // JSON bytes.
+  void SendJSON(const std::string &json_str);
 };
 
 extern VSCode g_vsc;
Index: lldb/tools/lldb-vscode/ProgressEvent.h
===
--- lldb/tools/lldb-vscode/ProgressEvent.h
+++ lldb/tools/lldb-vscode/ProgressEvent.h
@@ -6,6 +6,10 @@
 //
 //===--===//
 
+#include 
+#include 
+#include 
+
 #include "VSCodeForward.h"
 
 #include "llvm/Support/JSON.h"
@@ -28,35 +32,99 @@
 
   llvm::json::Value ToJSON() const;
 
-  /// This operator returns \b true if two event messages
-  /// would result in the same event for the IDE, e.g.
-  /// same rounded percentage.
-  bool operator==(const ProgressEvent &other) const;
+  /// \return
+  ///   \b true if two event messages would result in the same event for the
+  ///   IDE, e.g. same rounded percentage.
+  bool EqualsForIDE(const ProgressEvent &other) const;
 
   const char *GetEventName() const;
 
+  ProgressEventType GetEventType() const;
+
   bool IsValid() const;
 
   uint64_t GetID() const;
 
 private:
   uint64_t m_progress_id;
-  const char *m_message;
+  std::string m_message;
   ProgressEventType m_event_type;
   llvm::Optional m_percentage;
 };
 
+/// Class that keeps the start event and its most recent update.
+/// It controls when the event should start being reported to the IDE.
+class TaskProgressEventQueue {
+public:
+  TaskProgressEventQueue(const ProgressEvent &start_event);
+
+  /// \return
+  /// \b true if enough seconds have passed since the start event,
+  /// which means the events should start being reported.
+  bool ShouldReport() const;
+
+  /// Report the start event, if not reported yet, and the most recent
+  /// update if \a ShouldReport() is \b true.
+  bool TryReport(std::function report_callback);
+
+  /// \return
+  /// \b true if the new event is effectively different to previous
+  /// updates.
+  bool SetUpdate(const ProgressEvent &event);
+
+  /// \return
+  /// \b true if a \a progressEnd event has been notified.
+  bool Finished() const;
+
+  /// \return
+  ///  The ID of the underlying start event.
+  uint64_t GetID() const;
+
+private:
+  ProgressEvent m_start_event;
+  llvm::Optional m_last_update_event;
+  std::chrono::duration m_start_event_timestamp;
+  bool m_waiting;
+  bool m_finished;
+};
+
 /// Class that filters out progress event messages that shouldn't be reported
 /// to the IDE, either because they are invalid or because they are too chatty.
+///
+/// We don't short lived events to be sent to the IDE, as they are rendered
+/// in the UI and end up spamming the DAP connection. The only events
+/// worth of being reported are the ones that take at least a few seconds to run
+/// and that carry new information.
 class ProgressEventFilterQueue {
 public:
-  ProgressEventFilterQue

[Lldb-commits] [PATCH] D70882: Add environment var LLDBVSCODE_SKIP_INIT_FILES to lldb-vscode

2021-04-22 Thread walter erquinigo via Phabricator via lldb-commits

wallace abandoned this revision.
wallace added a comment.

not needed anymore because of D99974 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70882

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

[Lldb-commits] [PATCH] D80659: [lldb-vscode] Redirect stderr and stdout to DAPs console message

2021-04-22 Thread walter erquinigo via Phabricator via lldb-commits

wallace abandoned this revision.
wallace added a comment.

not needed anymore because of D99974 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80659

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

[Lldb-commits] [PATCH] D99974: [lldb-vscode] redirect stderr/stdout to the IDE's console

2021-04-22 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py:939
 if log_file:
 adaptor_env['LLDBVSCODE_LOG'] = log_file
 self.process = subprocess.Popen([executable],

Should this be `LLDB_VSCODE_LOG`, similar to 
`LLDB_VSCODE_TEST_STDOUT_STDERR_REDIRECTION`? Most of our environment variables 
begin with `LLDB_` so I think that would be most consistent. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99974

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

[Lldb-commits] [PATCH] D101131: [lldb-vscode] Follow up of D99989 - store some strings more safely

2021-04-22 Thread walter erquinigo via Phabricator via lldb-commits

wallace created this revision.
wallace added reviewers: clayborg, teemperor.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

As a follow up of https://reviews.llvm.org/D99989#inline-953343, I'm now
storing std::string instead of char *. I know it might never break as char *,
but if it does, chasing that bug might be dauting.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101131

Files:
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2931,7 +2931,7 @@
 const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
 
 // We first find out which variable names are duplicated
-llvm::DenseMap variable_name_counts;
+std::map variable_name_counts;
 for (auto i = start_idx; i < end_idx; ++i) {
   lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(i);
   if (!variable.IsValid())


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2931,7 +2931,7 @@
 const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
 
 // We first find out which variable names are duplicated
-llvm::DenseMap variable_name_counts;
+std::map variable_name_counts;
 for (auto i = start_idx; i < end_idx; ++i) {
   lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(i);
   if (!variable.IsValid())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100962: [lldb/Utility] Add SourceLocationSpec class (NFC)

2021-04-22 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 339867.
mib edited the summary of this revision.
mib added a comment.

- Move boolean attributes to the end of the class.
- Fix deleted default constructor issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100962

Files:
  lldb/include/lldb/Utility/SourceLocationSpec.h
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/SourceLocationSpec.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/SourceLocationSpecTest.cpp

Index: lldb/unittests/Utility/SourceLocationSpecTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/SourceLocationSpecTest.cpp
@@ -0,0 +1,215 @@
+//===-- SourceLocationSpecTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Utility/LLDBAssert.h"
+#include "lldb/Utility/SourceLocationSpec.h"
+
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private;
+
+TEST(SourceLocationSpecTest, CreateSourceLocationSpec) {
+  FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  const uint32_t line = 19;
+  const uint32_t column = 4;
+  const bool check_inlines = false;
+  const bool exact_match = true;
+
+  // Invalid FileSpec
+  ASSERT_THAT_EXPECTED(
+  SourceLocationSpec::Create(FileSpec(), line, check_inlines, exact_match),
+  llvm::Failed());
+  // Null Line
+  ASSERT_THAT_EXPECTED(
+  SourceLocationSpec::Create(fs, 0, check_inlines, exact_match),
+  llvm::Failed());
+  // Invalid Line
+  ASSERT_THAT_EXPECTED(SourceLocationSpec::Create(fs, LLDB_INVALID_LINE_NUMBER,
+  check_inlines, exact_match),
+   llvm::Failed());
+  // Invalid FileSpec & Line
+  ASSERT_THAT_EXPECTED(
+  SourceLocationSpec::Create(FileSpec(), 0, check_inlines, exact_match),
+  llvm::Failed());
+
+  // Valid FileSpec & Line
+  ASSERT_THAT_EXPECTED(
+  SourceLocationSpec::Create(fs, line, check_inlines, exact_match),
+  llvm::Succeeded());
+  // Valid FileSpec, Line & Column
+  ASSERT_THAT_EXPECTED(
+  SourceLocationSpec::Create(fs, line, column, check_inlines, exact_match),
+  llvm::Succeeded());
+}
+
+TEST(SourceLocationSpecTest, FileLineColumnComponents) {
+  FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  const uint32_t line = 19;
+  const uint16_t column = 4;
+
+  auto location_spec = SourceLocationSpec::Create(
+  fs, line, LLDB_INVALID_COLUMN_NUMBER, false, true);
+  ASSERT_THAT_EXPECTED(location_spec, llvm::Succeeded());
+  SourceLocationSpec without_column = *location_spec;
+  EXPECT_EQ(fs, without_column.GetFileSpec());
+  EXPECT_EQ(line, without_column.GetLine());
+  EXPECT_FALSE(without_column.HasColumn());
+  EXPECT_EQ(llvm::None, without_column.GetColumn());
+  EXPECT_FALSE(without_column.GetCheckInlines());
+  EXPECT_TRUE(without_column.GetExactMatch());
+  EXPECT_STREQ("/foo/bar:19", without_column.GetCString());
+
+  location_spec = SourceLocationSpec::Create(fs, line, column, true, false);
+  ASSERT_THAT_EXPECTED(location_spec, llvm::Succeeded());
+  SourceLocationSpec with_column = *location_spec;
+  EXPECT_TRUE(with_column.HasColumn());
+  EXPECT_EQ(column, with_column.GetColumn());
+  EXPECT_TRUE(with_column.GetCheckInlines());
+  EXPECT_FALSE(with_column.GetExactMatch());
+  EXPECT_STREQ("/foo/bar:19:4", with_column.GetCString());
+}
+
+TEST(SourceLocationSpecTest, Equal) {
+  auto Create =
+  [](bool check_inlines, bool exact_match, FileSpec fs, uint32_t line,
+ uint16_t column = LLDB_INVALID_COLUMN_NUMBER) -> SourceLocationSpec {
+auto location = SourceLocationSpec::Create(fs, line, column, check_inlines,
+   exact_match);
+lldbassert(location && "Invalid SourceLocationSpec.");
+return *location;
+  };
+
+  auto Equal = [](SourceLocationSpec lhs, SourceLocationSpec rhs, bool full) {
+return SourceLocationSpec::Equal(lhs, rhs, full);
+  };
+
+  const FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  const FileSpec other_fs("/foo/baz", FileSpec::Style::posix);
+
+  // mutating FileSpec + const Inlined, ExactMatch, Line
+  EXPECT_TRUE(
+  Equal(Create(false, false, fs, 4), Create(false, false, fs, 4), true));
+  EXPECT_TRUE(
+  Equal(Create(true, true, fs, 4), Create(true, true, fs, 4), false));
+  EXPECT_FALSE(Equal(Create(false, false, fs, 4),
+ Create(false, false, other_fs, 4), true));
+  EXPECT_FALSE(
+  Equal(Create(true, true, fs, 4), Create(true, true, other_fs, 4), false));
+
+  // mutating FileSpec + const Inlined, ExactMatch,

[Lldb-commits] [PATCH] D99974: [lldb-vscode] redirect stderr/stdout to the IDE's console

2021-04-22 Thread walter erquinigo via Phabricator via lldb-commits

wallace added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py:939
 if log_file:
 adaptor_env['LLDBVSCODE_LOG'] = log_file
 self.process = subprocess.Popen([executable],

JDevlieghere wrote:
> Should this be `LLDB_VSCODE_LOG`, similar to 
> `LLDB_VSCODE_TEST_STDOUT_STDERR_REDIRECTION`? Most of our environment 
> variables begin with `LLDB_` so I think that would be most consistent. 
I agree with you, but it might be hard to change it, as that might break some 
users' flow. The env var is also used inside of lldb-vscode.cpp itself


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99974

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

[Lldb-commits] [PATCH] D100965: [lldb] Refactor (FileSpec+Line) to SourceLocationSpec (NFCI)

2021-04-22 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 339878.
mib marked 3 inline comments as done.
mib added a comment.

Address @JDevlieghere & @jingham feedbacks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100965

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Core/AddressResolverFileLine.h
  lldb/include/lldb/Symbol/CompileUnit.h
  lldb/include/lldb/Symbol/LineTable.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/API/SBThread.cpp
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
  lldb/source/Core/AddressResolverFileLine.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/LineTable.cpp
  lldb/source/Symbol/SymbolFile.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Symbol/TestLineEntry.cpp

Index: lldb/unittests/Symbol/TestLineEntry.cpp
===
--- lldb/unittests/Symbol/TestLineEntry.cpp
+++ lldb/unittests/Symbol/TestLineEntry.cpp
@@ -64,9 +64,10 @@
   if (sc_comp_units.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No comp unit found on the test object.");
+
   sc_comp_units[0].comp_unit->ResolveSymbolContext(
-  file_spec, line, check_inlines, exact, eSymbolContextLineEntry,
-  sc_line_entries);
+  SourceLocationSpec(file_spec, line), check_inlines, exact,
+  eSymbolContextLineEntry, sc_line_entries);
   if (sc_line_entries.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No line entry found on the test object.");
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -371,9 +371,13 @@
   if (move_to_nearest_code == eLazyBoolCalculate)
 move_to_nearest_code = GetMoveToNearestCode() ? eLazyBoolYes : eLazyBoolNo;
 
+  auto location_spec =
+  SourceLocationSpec::Create(remapped_file, line_no, column, check_inlines,
+ !static_cast(move_to_nearest_code));
+  lldbassert(location_spec && "Invalid SourceLocationSpec.");
+
   BreakpointResolverSP resolver_sp(new BreakpointResolverFileLine(
-  nullptr, remapped_file, line_no, column, offset, check_inlines,
-  skip_prologue, !static_cast(move_to_nearest_code)));
+  nullptr, offset, skip_prologue, *location_spec));
   return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true);
 }
 
Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -97,10 +97,10 @@
   return type_system_or_err;
 }
 
-uint32_t SymbolFile::ResolveSymbolContext(const FileSpec &file_spec,
-  uint32_t line, bool check_inlines,
-  lldb::SymbolContextItem resolve_scope,
-  SymbolContextList &sc_list) {
+uint32_t
+SymbolFile::ResolveSymbolContext(const SourceLocationSpec &src_location_spec,
+ lldb::SymbolContextItem resolve_scope,
+ SymbolContextList &sc_list) {
   return 0;
 }
 
Index: lldb/source/Symbol/LineTable.cpp
===
--- lldb/source/Symbol/LineTable.cpp
+++ lldb/source/Symbol/LineTable.cpp
@@ -304,7 +304,7 @@
 
 uint32_t LineTable::FindLineEntryIndexByFileIndex(
 uint32_t start_idx, const std::vector &file_indexes,
-uint32_t line, bool exact, LineEntry *line_entry_ptr) {
+const SourceLocationSpec &src_location_spec, LineEntry *line_entry_ptr) {
 
   const size_t count = m_entries.size();
   size_t best_match = UINT32_MAX;
@@ -324,13 +324,15 @@
 // after and
 // if they're not in the same function, don't return a match.
 
+uint32_t line = src_location_spec.GetLine();
+
 if (m_entries[idx].line < line) {
   continue;
 } else if (m_entries[idx].line == line) {
   if (line_entry_ptr)
 ConvertEntryAtIndexToLine

[Lldb-commits] [PATCH] D100965: [lldb] Refactor argument group by SourceLocationSpec (NFCI)

2021-04-22 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 339881.
mib added a comment.

Update `LineEntry` unit test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100965

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Core/AddressResolverFileLine.h
  lldb/include/lldb/Symbol/CompileUnit.h
  lldb/include/lldb/Symbol/LineTable.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/API/SBThread.cpp
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
  lldb/source/Core/AddressResolverFileLine.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/LineTable.cpp
  lldb/source/Symbol/SymbolFile.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Symbol/TestLineEntry.cpp

Index: lldb/unittests/Symbol/TestLineEntry.cpp
===
--- lldb/unittests/Symbol/TestLineEntry.cpp
+++ lldb/unittests/Symbol/TestLineEntry.cpp
@@ -53,8 +53,10 @@
 }
 
 llvm::Expected LineEntryTest::GetLineEntryForLine(uint32_t line) {
-  bool check_inlines = true;
-  bool exact = true;
+  // TODO: Handle SourceLocationSpec column information
+  const llvm::Optional column = llvm::None;
+  const bool check_inlines = true;
+  const bool exact_match = true;
   SymbolContextList sc_comp_units;
   SymbolContextList sc_line_entries;
   FileSpec file_spec("inlined-functions.cpp");
@@ -64,9 +66,13 @@
   if (sc_comp_units.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No comp unit found on the test object.");
+
+  auto location_spec = SourceLocationSpec::Create(file_spec, line, column,
+  check_inlines, exact_match);
+  lldbassert(location_spec && "Invalid SourceLocationSpec.");
+
   sc_comp_units[0].comp_unit->ResolveSymbolContext(
-  file_spec, line, check_inlines, exact, eSymbolContextLineEntry,
-  sc_line_entries);
+  *location_spec, eSymbolContextLineEntry, sc_line_entries);
   if (sc_line_entries.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No line entry found on the test object.");
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -371,9 +371,13 @@
   if (move_to_nearest_code == eLazyBoolCalculate)
 move_to_nearest_code = GetMoveToNearestCode() ? eLazyBoolYes : eLazyBoolNo;
 
+  auto location_spec =
+  SourceLocationSpec::Create(remapped_file, line_no, column, check_inlines,
+ !static_cast(move_to_nearest_code));
+  lldbassert(location_spec && "Invalid SourceLocationSpec.");
+
   BreakpointResolverSP resolver_sp(new BreakpointResolverFileLine(
-  nullptr, remapped_file, line_no, column, offset, check_inlines,
-  skip_prologue, !static_cast(move_to_nearest_code)));
+  nullptr, offset, skip_prologue, *location_spec));
   return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true);
 }
 
Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -97,10 +97,10 @@
   return type_system_or_err;
 }
 
-uint32_t SymbolFile::ResolveSymbolContext(const FileSpec &file_spec,
-  uint32_t line, bool check_inlines,
-  lldb::SymbolContextItem resolve_scope,
-  SymbolContextList &sc_list) {
+uint32_t
+SymbolFile::ResolveSymbolContext(const SourceLocationSpec &src_location_spec,
+ lldb::SymbolContextItem resolve_scope,
+ SymbolContextList &sc_list) {
   return 0;
 }
 
Index: lldb/source/Symbol/LineTable.cpp
===
--- lldb/source/Symbol/LineTable.cpp
+++ lldb/source/Symbol/LineTable.cpp
@@ -304,7 +304,7 @@
 
 uint32_t LineTable::FindLineEntryIndexByFileIndex(
 uint32_t start_idx, const std::vector &f

[Lldb-commits] [PATCH] D100965: [lldb] Refactor argument group by SourceLocationSpec (NFCI)

2021-04-22 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 339882.
mib added a comment.

Update BreakpointResolver column default value and fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100965

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Core/AddressResolverFileLine.h
  lldb/include/lldb/Symbol/CompileUnit.h
  lldb/include/lldb/Symbol/LineTable.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/API/SBThread.cpp
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
  lldb/source/Core/AddressResolverFileLine.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/LineTable.cpp
  lldb/source/Symbol/SymbolFile.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Symbol/TestLineEntry.cpp

Index: lldb/unittests/Symbol/TestLineEntry.cpp
===
--- lldb/unittests/Symbol/TestLineEntry.cpp
+++ lldb/unittests/Symbol/TestLineEntry.cpp
@@ -53,8 +53,10 @@
 }
 
 llvm::Expected LineEntryTest::GetLineEntryForLine(uint32_t line) {
-  bool check_inlines = true;
-  bool exact = true;
+  // TODO: Handle SourceLocationSpec column information
+  const llvm::Optional column = llvm::None;
+  const bool check_inlines = true;
+  const bool exact_match = true;
   SymbolContextList sc_comp_units;
   SymbolContextList sc_line_entries;
   FileSpec file_spec("inlined-functions.cpp");
@@ -64,9 +66,13 @@
   if (sc_comp_units.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No comp unit found on the test object.");
+
+  auto location_spec = SourceLocationSpec::Create(file_spec, line, column,
+  check_inlines, exact_match);
+  lldbassert(location_spec && "Invalid SourceLocationSpec.");
+
   sc_comp_units[0].comp_unit->ResolveSymbolContext(
-  file_spec, line, check_inlines, exact, eSymbolContextLineEntry,
-  sc_line_entries);
+  *location_spec, eSymbolContextLineEntry, sc_line_entries);
   if (sc_line_entries.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No line entry found on the test object.");
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -371,9 +371,13 @@
   if (move_to_nearest_code == eLazyBoolCalculate)
 move_to_nearest_code = GetMoveToNearestCode() ? eLazyBoolYes : eLazyBoolNo;
 
+  auto location_spec =
+  SourceLocationSpec::Create(remapped_file, line_no, column, check_inlines,
+ !static_cast(move_to_nearest_code));
+  lldbassert(location_spec && "Invalid SourceLocationSpec.");
+
   BreakpointResolverSP resolver_sp(new BreakpointResolverFileLine(
-  nullptr, remapped_file, line_no, column, offset, check_inlines,
-  skip_prologue, !static_cast(move_to_nearest_code)));
+  nullptr, offset, skip_prologue, *location_spec));
   return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true);
 }
 
Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -97,10 +97,10 @@
   return type_system_or_err;
 }
 
-uint32_t SymbolFile::ResolveSymbolContext(const FileSpec &file_spec,
-  uint32_t line, bool check_inlines,
-  lldb::SymbolContextItem resolve_scope,
-  SymbolContextList &sc_list) {
+uint32_t
+SymbolFile::ResolveSymbolContext(const SourceLocationSpec &src_location_spec,
+ lldb::SymbolContextItem resolve_scope,
+ SymbolContextList &sc_list) {
   return 0;
 }
 
Index: lldb/source/Symbol/LineTable.cpp
===
--- lldb/source/Symbol/LineTable.cpp
+++ lldb/source/Symbol/LineTable.cpp
@@ -304,7 +304,7 @@
 
 uint32_t LineTable::FindLineEntryIndexByFileIndex(
 uint32_t s

56 matches

Mail list logo