[Lldb-commits] [PATCH] D157960: [lldb][gui] Update TreeItem children's m_parent on move

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: clayborg, davide, k8stone.
kpdev42 added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added a subscriber: lldb-commits.

Before this patch, any time TreeItem is copied in Resize method, its
parent is not updated, which can cause crashes when, for example, thread
window with multiple hierarchy levels is updated. Makes TreeItem
move-only, removes TreeItem's m_delegate extra self-assignment by making
it a pointer, adds code to fix up children's parent on move constructor
and operator=

~~~

Huawei RRI, OS Lab


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157960

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/test/API/commands/gui/spawn-threads/Makefile
  lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
  lldb/test/API/commands/gui/spawn-threads/main.cpp

Index: lldb/test/API/commands/gui/spawn-threads/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/gui/spawn-threads/main.cpp
@@ -0,0 +1,25 @@
+#include 
+#include 
+#include 
+
+#include "pseudo_barrier.h"
+
+pseudo_barrier_t barrier_inside;
+
+void thread_func() { pseudo_barrier_wait(barrier_inside); }
+
+void test_thread() {
+  std::vector thrs;
+  for (int i = 0; i < 5; i++)
+thrs.push_back(std::thread(thread_func)); // break here
+
+  pseudo_barrier_wait(barrier_inside); // break before join
+  for (auto &t : thrs)
+t.join();
+}
+
+int main() {
+  pseudo_barrier_init(barrier_inside, 6);
+  test_thread();
+  return 0;
+}
Index: lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
===
--- /dev/null
+++ lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
@@ -0,0 +1,47 @@
+"""
+Test that 'gui' does not crash when adding new threads, which
+populate TreeItem's children and may be reallocated elsewhere.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+import sys
+
+class TestGuiSpawnThreadsTest(PExpectTest):
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipIfCursesSupportMissing
+def test_gui(self):
+self.build()
+
+self.launch(executable=self.getBuildArtifact('a.out'), dimensions=(100, 500))
+self.expect(
+'breakpoint set -f main.cpp -p "break here"', substrs=['Breakpoint 1', 'address =']
+)
+self.expect(
+'breakpoint set -f main.cpp -p "before join"', substrs=['Breakpoint 2', 'address =']
+)
+self.expect("run", substrs=["stop reason ="])
+
+escape_key = chr(27).encode()
+
+# Start the GUI
+self.child.sendline("gui")
+self.child.expect_exact("Threads")
+self.child.expect_exact(f"thread #1: tid =")
+
+for i in range(5):
+# Stopped at the breakpoit, continue over the thread creation
+self.child.send("c")
+# Check the newly created thread
+self.child.expect_exact(f"thread #{i + 2}: tid =")
+
+# Exit GUI.
+self.child.send(escape_key)
+self.expect_prompt()
+
+self.quit()
Index: lldb/test/API/commands/gui/spawn-threads/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/gui/spawn-threads/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+ENABLE_THREADS := YES
+include Makefile.rules
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -4614,30 +4614,48 @@
 
 typedef std::shared_ptr TreeDelegateSP;
 
-class TreeItem {
+struct TreeItemData {
+  TreeItemData(TreeItem *parent, TreeDelegate &delegate,
+   bool might_have_children, bool is_expanded)
+  : m_parent(parent), m_delegate(&delegate),
+m_might_have_children(might_have_children), m_is_expanded(is_expanded) {
+  }
+
+protected:
+  TreeItem *m_parent;
+  TreeDelegate *m_delegate;
+  void *m_user_data = nullptr;
+  uint64_t m_identifier = 0;
+  std::string m_text;
+  int m_row_idx = -1; // Zero based visible row index, -1 if not visible or for
+  // the root item
+  bool m_might_have_children;
+  bool m_is_expanded = false;
+};
+
+class TreeItem : public TreeItemData {
 public:
   TreeItem(TreeItem *parent, TreeDelegate &delegate, bool might_have_children)
-  : m_parent(parent), m_delegate(delegate), m_children(),
-m_might_have_children(might_have_children) {
-if (m_parent == nullptr)
-  m_is_expanded = m_delegate.TreeDelegateExpandRootByDefault();
-  }
+  : TreeItemData(parent, delegate, m

[Lldb-commits] [PATCH] D157960: [lldb][gui] Update TreeItem children's m_parent on move

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG83695d45d621: [lldb][gui] Update TreeItem children's 
m_parent on move (authored by kpdev42).

Changed prior to commit:
  https://reviews.llvm.org/D157960?vs=550240&id=550655#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157960

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/test/API/commands/gui/spawn-threads/Makefile
  lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
  lldb/test/API/commands/gui/spawn-threads/main.cpp

Index: lldb/test/API/commands/gui/spawn-threads/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/gui/spawn-threads/main.cpp
@@ -0,0 +1,25 @@
+#include 
+#include 
+#include 
+
+#include "pseudo_barrier.h"
+
+pseudo_barrier_t barrier_inside;
+
+void thread_func() { pseudo_barrier_wait(barrier_inside); }
+
+void test_thread() {
+  std::vector thrs;
+  for (int i = 0; i < 5; i++)
+thrs.push_back(std::thread(thread_func)); // break here
+
+  pseudo_barrier_wait(barrier_inside); // break before join
+  for (auto &t : thrs)
+t.join();
+}
+
+int main() {
+  pseudo_barrier_init(barrier_inside, 6);
+  test_thread();
+  return 0;
+}
Index: lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
===
--- /dev/null
+++ lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
@@ -0,0 +1,47 @@
+"""
+Test that 'gui' does not crash when adding new threads, which
+populate TreeItem's children and may be reallocated elsewhere.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+import sys
+
+class TestGuiSpawnThreadsTest(PExpectTest):
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipIfCursesSupportMissing
+def test_gui(self):
+self.build()
+
+self.launch(executable=self.getBuildArtifact('a.out'), dimensions=(100, 500))
+self.expect(
+'breakpoint set -f main.cpp -p "break here"', substrs=['Breakpoint 1', 'address =']
+)
+self.expect(
+'breakpoint set -f main.cpp -p "before join"', substrs=['Breakpoint 2', 'address =']
+)
+self.expect("run", substrs=["stop reason ="])
+
+escape_key = chr(27).encode()
+
+# Start the GUI
+self.child.sendline("gui")
+self.child.expect_exact("Threads")
+self.child.expect_exact(f"thread #1: tid =")
+
+for i in range(5):
+# Stopped at the breakpoint, continue over the thread creation
+self.child.send("c")
+# Check the newly created thread
+self.child.expect_exact(f"thread #{i + 2}: tid =")
+
+# Exit GUI.
+self.child.send(escape_key)
+self.expect_prompt()
+
+self.quit()
Index: lldb/test/API/commands/gui/spawn-threads/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/gui/spawn-threads/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+ENABLE_THREADS := YES
+include Makefile.rules
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -4614,30 +4614,48 @@
 
 typedef std::shared_ptr TreeDelegateSP;
 
-class TreeItem {
+struct TreeItemData {
+  TreeItemData(TreeItem *parent, TreeDelegate &delegate,
+   bool might_have_children, bool is_expanded)
+  : m_parent(parent), m_delegate(&delegate),
+m_might_have_children(might_have_children), m_is_expanded(is_expanded) {
+  }
+
+protected:
+  TreeItem *m_parent;
+  TreeDelegate *m_delegate;
+  void *m_user_data = nullptr;
+  uint64_t m_identifier = 0;
+  std::string m_text;
+  int m_row_idx = -1; // Zero based visible row index, -1 if not visible or for
+  // the root item
+  bool m_might_have_children;
+  bool m_is_expanded = false;
+};
+
+class TreeItem : public TreeItemData {
 public:
   TreeItem(TreeItem *parent, TreeDelegate &delegate, bool might_have_children)
-  : m_parent(parent), m_delegate(delegate), m_children(),
-m_might_have_children(might_have_children) {
-if (m_parent == nullptr)
-  m_is_expanded = m_delegate.TreeDelegateExpandRootByDefault();
-  }
+  : TreeItemData(parent, delegate, might_have_children,
+ parent == nullptr
+ ? delegate.TreeDelegateExpandRootByDefault()
+ : false),
+m_children() {}
+
+  TreeItem(const TreeItem &) = delete;
+  TreeItem &operator=(const TreeItem &rhs) = delete;
 
-  TreeItem &operator=(const Tr

[Lldb-commits] [PATCH] D126359: [LLDB] Add basic floating point ops to IR interpreter

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 440109.
kpdev42 added a comment.
Herald added a subscriber: Michael137.

Address review notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126359

Files:
  lldb/source/Expression/IRInterpreter.cpp
  lldb/test/API/lang/c/fpeval/Makefile
  lldb/test/API/lang/c/fpeval/TestFPEval.py
  lldb/test/API/lang/c/fpeval/main.c

Index: lldb/test/API/lang/c/fpeval/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/main.c
@@ -0,0 +1,6 @@
+int main (int argc, char const *argv[])
+{
+double a = 42.0;
+double b = 2.0;
+return 0;  Set break point at this line.
+}
Index: lldb/test/API/lang/c/fpeval/TestFPEval.py
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/TestFPEval.py
@@ -0,0 +1,54 @@
+"""Tests IR interpreter handling of basic floating point operations (fadd, fsub, etc)."""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class FPEvalTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Find the line number to break inside main().
+self.line = line_number('main.c', '// Set break point at this line.')
+
+def test(self):
+"""Test floating point expressions while jitter is disabled."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+# Break inside the main.
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("expr --allow-jit false  -- a + b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])
+self.expect("expr --allow-jit false  -- a - b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '40'])
+self.expect("expr --allow-jit false  -- a / b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '21'])
+self.expect("expr --allow-jit false  -- a * b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '84'])
+self.expect("expr --allow-jit false  -- a + 2", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])
+self.expect("expr --allow-jit false  -- a > b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- a >= b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- a < b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a <= b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a == b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a != b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+
Index: lldb/test/API/lang/c/fpeval/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Expression/IRInterpreter.cpp
===
--- lldb/source/Expression/IRInterpreter.cpp
+++ lldb/source/Expression/IRInterpreter.cpp
@@ -167,6 +167,18 @@
 const Constant *constant = dyn_cast(value);
 
 if (constant) {
+  if (constant->getValueID() == Value::ConstantFPVal) {
+if (auto *cfp = dyn_cast(constant)) {
+  if (cfp->getType()->isDoubleTy())
+scalar = cfp->getValueAPF().convertToDouble();
+  else if (cfp->getType()->isFloatTy())
+scalar = cfp->getValueAPF().convertToFloat();
+  else
+return false;
+  return true;
+}
+return false;
+  }
   APInt value_apint;
 
   if (!ResolveConstantValue(value_apint, constant))
@@ -189,9 +201,18 @@
 
 lldb::offset_t offset = 0;
 if (value_size <= 8) {
-  uint64_t u64value = value_extractor.GetMaxU64(&offset, value_size);
-  return AssignToMatchType(scalar, llvm::APInt(64, u64value),
-   value->getType());
+  Type *ty = value->getType();
+  if (ty->isDoubleTy()) {
+scalar = value_extractor.GetDouble(&offset);
+return true;
+  } else if (ty->isFloatTy()) {
+scalar = value_extractor.GetFloat(&offset);
+return true;
+  } else {
+uint64_t u64value = val

[Lldb-commits] [PATCH] D130062: [lldb][DWARF5] Enable macro evaluation

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: clayborg, davide, k8stone, DavidSpickett.
kpdev42 added a project: LLDB.
Herald added subscribers: Enna1, JDevlieghere, mgorny.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, Sanitizers, sstefan1.
Herald added a project: Sanitizers.

Patch enables handing of DWARFv5 DW_MACRO_define_strx and DW_MACRO_undef_strx


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130062

Files:
  compiler-rt/lib/fuzzer/CMakeLists.txt
  lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/API/lang/c/macro/Makefile
  lldb/test/API/lang/c/macro/TestMacro.py
  lldb/test/API/lang/c/macro/main.c

Index: lldb/test/API/lang/c/macro/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/macro/main.c
@@ -0,0 +1,7 @@
+#define DM 10
+#define DF(x) (42 + (x))
+
+int main (int argc, char const *argv[])
+{
+return 0;  Set break point at this line.
+}
Index: lldb/test/API/lang/c/macro/TestMacro.py
===
--- /dev/null
+++ lldb/test/API/lang/c/macro/TestMacro.py
@@ -0,0 +1,31 @@
+"""Tests lldb macro evaluation."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MacroTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Find the line number to break inside main().
+self.line = line_number('main.c', '// Set break point at this line.')
+
+def test(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+# Break inside the main.
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("expr DM + DF(10)", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['int', '62'])
+
Index: lldb/test/API/lang/c/macro/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/c/macro/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -fdebug-macro
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -55,6 +55,8 @@
 class SymbolFileDWARFDwo;
 class SymbolFileDWARFDwp;
 
+struct DWARFStrOffsetsInfo;
+
 #define DIE_IS_BEING_PARSED ((lldb_private::Type *)1)
 
 class SymbolFileDWARF : public lldb_private::SymbolFileCommon,
@@ -244,7 +246,9 @@
 
   bool Supports_DW_AT_APPLE_objc_complete_type(DWARFUnit *cu);
 
-  lldb_private::DebugMacrosSP ParseDebugMacros(lldb::offset_t *offset);
+  lldb_private::DebugMacrosSP
+  ParseDebugMacros(lldb::offset_t *offset,
+   const DWARFStrOffsetsInfo &str_offsets_info);
 
   static DWARFDIE GetParentSymbolContextDIE(const DWARFDIE &die);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1174,7 +1174,8 @@
 }
 
 lldb_private::DebugMacrosSP
-SymbolFileDWARF::ParseDebugMacros(lldb::offset_t *offset) {
+SymbolFileDWARF::ParseDebugMacros(lldb::offset_t *offset,
+  const DWARFStrOffsetsInfo &str_offsets_info) {
   auto iter = m_debug_macros_map.find(*offset);
   if (iter != m_debug_macros_map.end())
 return iter->second;
@@ -1190,8 +1191,8 @@
   const DWARFDebugMacroHeader &header =
   DWARFDebugMacroHeader::ParseHeader(debug_macro_data, offset);
   DWARFDebugMacroEntry::ReadMacroEntries(
-  debug_macro_data, m_context.getOrLoadStrData(), header.OffsetIs64Bit(),
-  offset, this, debug_macros_sp);
+  debug_macro_data, m_context.getOrLoadStrData(), str_offsets_info,
+  header.OffsetIs64Bit(), offset, this, debug_macros_sp);
 
   return debug_macros_sp;
 }
@@ -1199,7 +1200,7 @@
 bool SymbolFileDWARF::ParseDebugMacros(CompileUnit &comp_unit) {
   std::lock_guard guard(GetModuleMutex());
 
-  DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
+  DWARFUnit *dwarf_cu = &GetDWARFCompileUnit(&comp_unit)->GetNonSkeletonUnit();
   if (dwarf_cu == nullptr)
 return false;
 
@@ -1215,8 +1216,16 @@

[Lldb-commits] [PATCH] D126359: [LLDB] Add basic floating point ops to IR interpreter

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added inline comments.



Comment at: lldb/test/API/lang/c/fpeval/TestFPEval.py:32
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("expr --allow-jit false  -- a + b", 
VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])

clayborg wrote:
> You might be able to get away with not actually even creating a target or 
> running to main if you define variables in your expression:
> 
> ```
> self.expect('expr --allow-jit false  -- float x=2.2; float y=4.4; x+y', ...
> ```
Actually I wanted the test to read floating point values from process memory, 
checking if they're passed correctly to IR along the way. This might be 
redundant, however your expression still needs a target to be executed.



Comment at: lldb/test/API/lang/c/fpeval/TestFPEval.py:32-33
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("expr --allow-jit false  -- a + b", 
VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])
+self.expect("expr --allow-jit false  -- a - b", 
VARIABLES_DISPLAYED_CORRECTLY,

clayborg wrote:
> if we want to verify if the above "a+b" works as expected compared to JITed 
> code, you can also run an expression like:
> 
> ```
> expr eval(a, b, add)
> ```
> Then then we would want to compare the expression results to make sure the 
> binary answer matches exactly. To do this, we will want to use the LLDB 
> native APIs:
> ```
> no_jit_options = lldb.SBExpressionOptions()
> no_jit_options = expt_options.SetAllowJIT(False)
> jit_options = lldb.SBExpressionOptions()
> jit_options = expt_options.SetAllowJIT(True)
> no_jit_value = frame.EvaluateExpression("a+b", no_jit_options)
> jit_value = frame.EvaluateExpression("eval(a, b, add)", jit_options)
> no_jit_data = no_jit_value.GetData()
> jit_data = no_jit_value.GetData()
> ```
> Then we need to compare the data byte for byte.
> 
> 
> 
IMO this will not work, because lldb always tries interpreter first and jitter 
is used only if interpreter fails. This may be circumvented by generating 
expression which interpreter cannot handle (function call?), however I don't 
see a point in doing it. Why not just check result value?



Comment at: lldb/test/API/lang/c/fpeval/main.c:3-4
+{
+double a = 42.0;
+double b = 2.0;
+return 0;  Set break point at this line.

clayborg wrote:
> We should be testing "float" and "long double" as well to verify those work.
Long double is not supported by this patch, but can be added. However long 
double is platform dependent type, so it makes no sense at all comparing JIT 
and interpreted results


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126359

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

[Lldb-commits] [PATCH] D126359: [LLDB] Add basic floating point ops to IR interpreter

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 450244.
kpdev42 added a comment.

Update test case so it compares JIT’ed  and interpreted FP division results and 
also check operations on float type. Patch doesn’t implement long double, 
because IR interpreter currently doesn’t support instruction argument size of 
more than 64 bit which includes both fp128 and int128. This deserves a separate 
patch, I think


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126359

Files:
  lldb/source/Expression/IRInterpreter.cpp
  lldb/test/API/lang/c/fpeval/Makefile
  lldb/test/API/lang/c/fpeval/TestFPEval.py
  lldb/test/API/lang/c/fpeval/main.c

Index: lldb/test/API/lang/c/fpeval/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/main.c
@@ -0,0 +1,16 @@
+double eval(double a, double b, int op) {
+  switch (op) {
+  case 0: return a+b;
+  case 1: return a-b;
+  case 2: return a/b;
+  case 3: return a*b;
+  default: return 0;
+  }
+}
+
+int main (int argc, char const *argv[])
+{
+double a = 42.0, b = 2.0;
+float f = 42.0, q = 2.0;
+return 0;  Set break point at this line.
+}
Index: lldb/test/API/lang/c/fpeval/TestFPEval.py
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/TestFPEval.py
@@ -0,0 +1,101 @@
+"""Tests IR interpreter handling of basic floating point operations (fadd, fsub, etc)."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class FPEvalTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+self.jit_opts = lldb.SBExpressionOptions()
+self.jit_opts.SetAllowJIT(True)
+self.no_jit_opts = lldb.SBExpressionOptions()
+self.no_jit_opts.SetAllowJIT(False)
+# Find the line number to break inside main().
+self.line = line_number('main.c', '// Set break point at this line.')
+
+def test(self):
+"""Test floating point expressions while jitter is disabled."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+# Break inside the main.
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+
+value = self.frame().EvaluateExpression("a + b", self.no_jit_opts)
+
+self.runCmd("run", RUN_SUCCEEDED)
+# test double
+self.expect("expr --allow-jit false  -- a + b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])
+self.expect("expr --allow-jit false  -- a - b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '40'])
+self.expect("expr --allow-jit false  -- a / b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '21'])
+self.expect("expr --allow-jit false  -- a * b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '84'])
+self.expect("expr --allow-jit false  -- a + 2", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])
+self.expect("expr --allow-jit false  -- a > b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- a >= b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- a < b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a <= b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a == b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a != b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+
+# test single
+self.expect("expr --allow-jit false  -- f + q", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['float', '44'])
+self.expect("expr --allow-jit false  -- f - q", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['float', '40'])
+self.expect("expr --allow-jit false  -- f / q", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['float', '21'])
+self.expect("expr --allow-jit false  -- f * q", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['float', '84'])
+self.expect("expr --allow-jit false  -- f + 2", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['float', '44'])
+self.expect("expr --allow-jit false  -- f > q", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("e

[Lldb-commits] [PATCH] D130062: [lldb][DWARF5] Enable macro evaluation

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 451364.
kpdev42 added a comment.

Address review notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130062

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/API/commands/expression/macros/Makefile
  lldb/test/API/commands/expression/macros/TestMacros.py
  lldb/test/API/lang/c/macro/Makefile
  lldb/test/API/lang/c/macro/TestMacro.py
  lldb/test/API/lang/c/macro/main.c

Index: lldb/test/API/lang/c/macro/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/macro/main.c
@@ -0,0 +1,7 @@
+#define DM 10
+#define DF(x) (42 + (x))
+
+int main (int argc, char const *argv[])
+{
+return 0;  Set break point at this line.
+}
Index: lldb/test/API/lang/c/macro/TestMacro.py
===
--- /dev/null
+++ lldb/test/API/lang/c/macro/TestMacro.py
@@ -0,0 +1,31 @@
+"""Tests lldb macro evaluation."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MacroTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Find the line number to break inside main().
+self.line = line_number('main.c', '// Set break point at this line.')
+
+def test(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+# Break inside the main.
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("expr DM + DF(10)", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['int', '62'])
+
Index: lldb/test/API/lang/c/macro/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/c/macro/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -fdebug-macro
+
+include Makefile.rules
Index: lldb/test/API/commands/expression/macros/TestMacros.py
===
--- lldb/test/API/commands/expression/macros/TestMacros.py
+++ lldb/test/API/commands/expression/macros/TestMacros.py
@@ -8,12 +8,6 @@
 
 class TestMacros(TestBase):
 
-@expectedFailureAll(
-compiler="clang",
-bugnumber="clang does not emit .debug_macro[.dwo] sections.")
-@expectedFailureAll(
-debug_info="dwo",
-bugnumber="GCC produces multiple .debug_macro.dwo sections and the spec is unclear as to what it means")
 @expectedFailureAll(
 hostoslist=["windows"],
 compiler="gcc",
Index: lldb/test/API/commands/expression/macros/Makefile
===
--- lldb/test/API/commands/expression/macros/Makefile
+++ lldb/test/API/commands/expression/macros/Makefile
@@ -5,5 +5,6 @@
 # GCC produces incorrect .debug_macro section when "-include" option is used:
 # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93075.
 NO_TEST_COMMON_H := 1
+CFLAGS_EXTRAS := -fdebug-macro
 
 include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -55,6 +55,8 @@
 class SymbolFileDWARFDwo;
 class SymbolFileDWARFDwp;
 
+class DWARFStrOffsetsInfo;
+
 #define DIE_IS_BEING_PARSED ((lldb_private::Type *)1)
 
 class SymbolFileDWARF : public lldb_private::SymbolFileCommon,
@@ -243,7 +245,9 @@
 
   bool Supports_DW_AT_APPLE_objc_complete_type(DWARFUnit *cu);
 
-  lldb_private::DebugMacrosSP ParseDebugMacros(lldb::offset_t *offset);
+  lldb_private::DebugMacrosSP
+  ParseDebugMacros(lldb::offset_t *offset,
+   const DWARFStrOffsetsInfo *str_offsets_info);
 
   static DWARFDIE GetParentSymbolContextDIE(const DWARFDIE &die);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1194,7 +1194,8 @@
 }
 
 lldb_private::DebugMacrosSP
-SymbolFileDWARF::ParseDebugMacros(lldb::offset_t *offset) {
+SymbolFileDWARF::ParseDebugMacros(lldb::offset_t *offset,
+  const DWARFStrOffsetsInfo *str_offsets_info) {
   auto iter = m_debug_ma

[Lldb-commits] [PATCH] D126359: [LLDB] Add basic floating point ops to IR interpreter

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf63e2cfb7f52: [LLDB] Add basic floating point ops to IR 
interpreter (authored by kpdev42).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126359

Files:
  lldb/source/Expression/IRInterpreter.cpp
  lldb/test/API/lang/c/fpeval/Makefile
  lldb/test/API/lang/c/fpeval/TestFPEval.py
  lldb/test/API/lang/c/fpeval/main.c

Index: lldb/test/API/lang/c/fpeval/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/main.c
@@ -0,0 +1,16 @@
+double eval(double a, double b, int op) {
+  switch (op) {
+  case 0: return a+b;
+  case 1: return a-b;
+  case 2: return a/b;
+  case 3: return a*b;
+  default: return 0;
+  }
+}
+
+int main (int argc, char const *argv[])
+{
+double a = 42.0, b = 2.0;
+float f = 42.0, q = 2.0;
+return 0;  Set break point at this line.
+}
Index: lldb/test/API/lang/c/fpeval/TestFPEval.py
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/TestFPEval.py
@@ -0,0 +1,101 @@
+"""Tests IR interpreter handling of basic floating point operations (fadd, fsub, etc)."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class FPEvalTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+self.jit_opts = lldb.SBExpressionOptions()
+self.jit_opts.SetAllowJIT(True)
+self.no_jit_opts = lldb.SBExpressionOptions()
+self.no_jit_opts.SetAllowJIT(False)
+# Find the line number to break inside main().
+self.line = line_number('main.c', '// Set break point at this line.')
+
+def test(self):
+"""Test floating point expressions while jitter is disabled."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+# Break inside the main.
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+
+value = self.frame().EvaluateExpression("a + b", self.no_jit_opts)
+
+self.runCmd("run", RUN_SUCCEEDED)
+# test double
+self.expect("expr --allow-jit false  -- a + b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])
+self.expect("expr --allow-jit false  -- a - b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '40'])
+self.expect("expr --allow-jit false  -- a / b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '21'])
+self.expect("expr --allow-jit false  -- a * b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '84'])
+self.expect("expr --allow-jit false  -- a + 2", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])
+self.expect("expr --allow-jit false  -- a > b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- a >= b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- a < b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a <= b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a == b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a != b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+
+# test single
+self.expect("expr --allow-jit false  -- f + q", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['float', '44'])
+self.expect("expr --allow-jit false  -- f - q", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['float', '40'])
+self.expect("expr --allow-jit false  -- f / q", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['float', '21'])
+self.expect("expr --allow-jit false  -- f * q", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['float', '84'])
+self.expect("expr --allow-jit false  -- f + 2", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['float', '44'])
+self.expect("expr --allow-jit false  -- f > q", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- f >= q", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- f < q", VARIABLES_DISPLAYED_CORRECTLY,
+subst

[Lldb-commits] [PATCH] D131783: [LLDB][JIT] Set processor for ARM architecture

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: clayborg, davide, k8stone, DavidSpickett, 
granata.enrico.
kpdev42 added a project: LLDB.
Herald added subscribers: omjavaid, JDevlieghere, kristof.beyls.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added a subscriber: lldb-commits.

Patch sets ARM cpu, before compiling JIT code. This enables FastISel for armv6 
and higher CPUs and allows using hardware FPU

~~~

OS Laboratory. Huawei RRI. Saint-Petersburg


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131783

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/API/lang/c/fpeval/TestFPEval.py


Index: lldb/test/API/lang/c/fpeval/TestFPEval.py
===
--- lldb/test/API/lang/c/fpeval/TestFPEval.py
+++ lldb/test/API/lang/c/fpeval/TestFPEval.py
@@ -19,7 +19,6 @@
 # Find the line number to break inside main().
 self.line = line_number('main.c', '// Set break point at this line.')
 
-@skipIf(archs=no_match(['amd64', 'x86_64', 'arm64'])) # lldb jitter 
incorrectly evals function with FP args on 32 bit arm
 def test(self):
 """Test floating point expressions while jitter is disabled."""
 self.build()
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -583,7 +583,6 @@
 
 std::string ArchSpec::GetClangTargetCPU() const {
   std::string cpu;
-
   if (IsMIPS()) {
 switch (m_core) {
 case ArchSpec::eCore_mips32:
@@ -630,6 +629,9 @@
   break;
 }
   }
+
+  if (GetTriple().isARM())
+cpu = GetTriple().getARMCPUForArch("").str();
   return cpu;
 }
 


Index: lldb/test/API/lang/c/fpeval/TestFPEval.py
===
--- lldb/test/API/lang/c/fpeval/TestFPEval.py
+++ lldb/test/API/lang/c/fpeval/TestFPEval.py
@@ -19,7 +19,6 @@
 # Find the line number to break inside main().
 self.line = line_number('main.c', '// Set break point at this line.')
 
-@skipIf(archs=no_match(['amd64', 'x86_64', 'arm64'])) # lldb jitter incorrectly evals function with FP args on 32 bit arm
 def test(self):
 """Test floating point expressions while jitter is disabled."""
 self.build()
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -583,7 +583,6 @@
 
 std::string ArchSpec::GetClangTargetCPU() const {
   std::string cpu;
-
   if (IsMIPS()) {
 switch (m_core) {
 case ArchSpec::eCore_mips32:
@@ -630,6 +629,9 @@
   break;
 }
   }
+
+  if (GetTriple().isARM())
+cpu = GetTriple().getARMCPUForArch("").str();
   return cpu;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131783: [LLDB][JIT] Set processor for ARM architecture

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGff9efe240c47: [LLDB][JIT] Set processor for ARM architecture 
(authored by kpdev42).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131783

Files:
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/API/lang/c/fpeval/TestFPEval.py


Index: lldb/test/API/lang/c/fpeval/TestFPEval.py
===
--- lldb/test/API/lang/c/fpeval/TestFPEval.py
+++ lldb/test/API/lang/c/fpeval/TestFPEval.py
@@ -19,7 +19,6 @@
 # Find the line number to break inside main().
 self.line = line_number('main.c', '// Set break point at this line.')
 
-@skipIf(archs=no_match(['amd64', 'x86_64', 'arm64'])) # lldb jitter 
incorrectly evals function with FP args on 32 bit arm
 def test(self):
 """Test floating point expressions while jitter is disabled."""
 self.build()
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -583,7 +583,6 @@
 
 std::string ArchSpec::GetClangTargetCPU() const {
   std::string cpu;
-
   if (IsMIPS()) {
 switch (m_core) {
 case ArchSpec::eCore_mips32:
@@ -630,6 +629,9 @@
   break;
 }
   }
+
+  if (GetTriple().isARM())
+cpu = GetTriple().getARMCPUForArch("").str();
   return cpu;
 }
 


Index: lldb/test/API/lang/c/fpeval/TestFPEval.py
===
--- lldb/test/API/lang/c/fpeval/TestFPEval.py
+++ lldb/test/API/lang/c/fpeval/TestFPEval.py
@@ -19,7 +19,6 @@
 # Find the line number to break inside main().
 self.line = line_number('main.c', '// Set break point at this line.')
 
-@skipIf(archs=no_match(['amd64', 'x86_64', 'arm64'])) # lldb jitter incorrectly evals function with FP args on 32 bit arm
 def test(self):
 """Test floating point expressions while jitter is disabled."""
 self.build()
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -583,7 +583,6 @@
 
 std::string ArchSpec::GetClangTargetCPU() const {
   std::string cpu;
-
   if (IsMIPS()) {
 switch (m_core) {
 case ArchSpec::eCore_mips32:
@@ -630,6 +629,9 @@
   break;
 }
   }
+
+  if (GetTriple().isARM())
+cpu = GetTriple().getARMCPUForArch("").str();
   return cpu;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126359: [LLDB] Add basic floating point ops to IR interpreter

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: clayborg, granata.enrico, davide, k8stone.
kpdev42 added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
kpdev42 requested review of this revision.

Patch adds support for fadd, fsub, fdiv, fmul and fcmp to IR interpreter.

~~~

OS Laboratory. Huawei RRI. Saint-Petersburg


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126359

Files:
  lldb/source/Expression/IRInterpreter.cpp
  lldb/test/API/lang/c/fpeval/Makefile
  lldb/test/API/lang/c/fpeval/TestFPEval.py
  lldb/test/API/lang/c/fpeval/main.c

Index: lldb/test/API/lang/c/fpeval/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/main.c
@@ -0,0 +1,10 @@
+#include 
+#include 
+#include 
+
+int main (int argc, char const *argv[])
+{
+double a = 42.0;
+double b = 2.0;
+return (long)(a + b);  Set break point at this line.
+}
Index: lldb/test/API/lang/c/fpeval/TestFPEval.py
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/TestFPEval.py
@@ -0,0 +1,52 @@
+"""Show bitfields and check that they display correctly."""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class FPEvalTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Find the line number to break inside main().
+self.line = line_number('main.c', '// Set break point at this line.')
+
+def test_and_run_command(self):
+"""Test 'frame variable ...' on a variable with bitfields."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+# Break inside the main.
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("expr --allow-jit false  -- a + b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])
+self.expect("expr --allow-jit false  -- a - b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '40'])
+self.expect("expr --allow-jit false  -- a / b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '21'])
+self.expect("expr --allow-jit false  -- a * b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '84'])
+self.expect("expr --allow-jit false  -- a + 2", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['double', '44'])
+self.expect("expr --allow-jit false  -- a > b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- a >= b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['true'])
+self.expect("expr --allow-jit false  -- a < b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a <= b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+self.expect("expr --allow-jit false  -- a == b", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['false'])
+
Index: lldb/test/API/lang/c/fpeval/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/c/fpeval/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Expression/IRInterpreter.cpp
===
--- lldb/source/Expression/IRInterpreter.cpp
+++ lldb/source/Expression/IRInterpreter.cpp
@@ -167,6 +167,18 @@
 const Constant *constant = dyn_cast(value);
 
 if (constant) {
+  if (constant->getValueID() == Value::ConstantFPVal) {
+if (auto *cfp = dyn_cast(constant)) {
+  if (cfp->getType()->isDoubleTy())
+scalar = cfp->getValueAPF().convertToDouble();
+  else if (cfp->getType()->isFloatTy())
+scalar = cfp->getValueAPF().convertToFloat();
+  else
+return false;
+  return true;
+}
+return false;
+  }
   APInt value_apint;
 
   if (!ResolveConstantValue(value_apint, constant))
@@ -189,9 +201,18 @@
 
 lldb::offset_t offset = 0;
 if (value_size <= 8) {
-  uint64_t u64value = value_extractor.GetMaxU64(&offset, value_size);
-  return AssignToMatchType(scalar, llvm::APInt(64, u64value),
-   value->getType());
+  Type *ty = value->getType();
+  if (ty->isDoubleTy()) {
+scalar = value_extractor.GetDouble(&offset);
+return true;
+  } else if (ty->isFloatTy()) {
+scalar = value_extractor.GetFloat(&offset

[Lldb-commits] [PATCH] D140368: [lldb] Consider all breakpoints in breakpoint detection

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: clayborg, davide, k8stone, DavidSpickett.
kpdev42 added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added a subscriber: lldb-commits.

Currently in some cases lldb reports stop reason as "step out" or "step over" 
(from thread plan completion) over "breakpoint", if the user breakpoint happens 
to be set on the same address.
The part of 
https://github.com/llvm/llvm-project/commit/f08f5c99262ff9eaa08956334accbb2614b0f7a2
 seems to overwrite internal breakpoint detection logic, so that only the last 
breakpoint for the current stop address is considered.
Together with step-out plans not clearing its breakpoint until they are 
destrouyed, this creates a situation when there is a user breakpoint set for 
address, but internal breakpoint makes lldb report a plan completion stop 
reason instead of breakpoint.
This patch reverts that internal breakpoint detection logic to consider all 
breakpoints


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140368

Files:
  lldb/source/Target/StopInfo.cpp
  lldb/test/API/functionalities/breakpoint/step_out_breakpoint/Makefile
  
lldb/test/API/functionalities/breakpoint/step_out_breakpoint/TestStepOutBreakpoint.py
  lldb/test/API/functionalities/breakpoint/step_out_breakpoint/main.cpp

Index: lldb/test/API/functionalities/breakpoint/step_out_breakpoint/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/step_out_breakpoint/main.cpp
@@ -0,0 +1,11 @@
+int func_1() { return 1; }
+
+int func_2() {
+  func_1(); // breakpoint_1
+  return 1 + func_1();  // breakpoint_2
+}
+
+int main(int argc, char const *argv[]) {
+  func_2();  // breakpoint_0
+  return 0;  // breakpoint_3
+}
Index: lldb/test/API/functionalities/breakpoint/step_out_breakpoint/TestStepOutBreakpoint.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/step_out_breakpoint/TestStepOutBreakpoint.py
@@ -0,0 +1,85 @@
+"""
+Test that breakpoints do not affect stepping.
+Check for correct StopReason when stepping to the line with breakpoint
+which should be eStopReasonBreakpoint in general,
+and eStopReasonPlanComplete when breakpoint's condition fails.
+"""
+
+
+import unittest2
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class StepOverBreakpointsTestCase(TestBase):
+
+def setUp(self):
+TestBase.setUp(self)
+
+self.build()
+exe = self.getBuildArtifact("a.out")
+src = lldb.SBFileSpec("main.cpp")
+
+# Create a target by the debugger.
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+# Setup four breakpoints
+self.lines = [line_number('main.cpp', "breakpoint_%i" % i) for i in range(4)]
+
+self.breakpoints = [self.target.BreakpointCreateByLocation(src, line) for line in self.lines]
+self.assertTrue(
+self.breakpoints[0] and self.breakpoints[0].GetNumLocations() == 1,
+VALID_BREAKPOINT)
+
+# Start debugging
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertIsNotNone(self.process, PROCESS_IS_VALID)
+self.thread = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[0])
+self.assertIsNotNone(self.thread, "Didn't stop at breakpoint 0.")
+
+def check_correct_stop_reason(self, breakpoint_idx, condition):
+self.assertState(self.process.GetState(), lldb.eStateStopped)
+if condition:
+# All breakpoints active, stop reason is breakpoint
+thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[breakpoint_idx])
+self.assertEquals(self.thread, thread1, "Didn't stop at breakpoint %i." % breakpoint_idx)
+else:
+# Breakpoints are inactive, stop reason is plan complete
+self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete,
+"Expected stop reason to be step into/over/out for inactive breakpoint %i line." % breakpoint_idx)
+
+def check_step_out_conditional(self, condition):
+conditionStr = 'true' if condition else 'false'
+
+self.breakpoints[1].GetLocationAtIndex(0).SetCondition(conditionStr)
+self.breakpoints[2].GetLocationAtIndex(0).SetCondition(conditionStr)
+self.breakpoints[3].GetLocationAtIndex(0).SetCondition(conditionStr)
+
+self.thread.StepInto()
+# We should be stopped at the breakpoint_1 line with the correct stop reason
+self.check_correct_stop_reason(1, condition)
+
+# This step-over creates a step-out from `f

[Lldb-commits] [PATCH] D140368: [lldb] Consider all breakpoints in breakpoint detection

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 484745.
kpdev42 added a comment.

Renamed the test, added more tests for unconditional (enabled/disabled) 
breakpoints and breakpoints with callbacks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140368

Files:
  lldb/source/Target/StopInfo.cpp
  lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/Makefile
  
lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
  lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp

Index: lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
@@ -0,0 +1,11 @@
+int func_1() { return 1; }
+
+int func_2() {
+  func_1(); // breakpoint_1
+  return 1 + func_1();  // breakpoint_2
+}
+
+int main(int argc, char const *argv[]) {
+  func_2();  // breakpoint_0
+  return 0;  // breakpoint_3
+}
Index: lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
@@ -0,0 +1,124 @@
+"""
+Test that breakpoints (reason = breakpoint) have more priority than
+plan completion (reason = step in/out/over) when reporting stop reason after step,
+in particular 'step out' and 'step over', and in addition 'step in'.
+Check for correct StopReason when stepping to the line with breakpoint,
+which should be eStopReasonBreakpoint in general,
+and eStopReasonPlanComplete when breakpoint's condition fails or it is disabled.
+"""
+
+
+import unittest2
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class ThreadPlanUserBreakpointsTestCase(TestBase):
+
+def setUp(self):
+TestBase.setUp(self)
+
+self.build()
+exe = self.getBuildArtifact("a.out")
+src = lldb.SBFileSpec("main.cpp")
+
+# Create a target by the debugger.
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+# Setup four breakpoints
+self.lines = [line_number('main.cpp', "breakpoint_%i" % i) for i in range(4)]
+
+self.breakpoints = [self.target.BreakpointCreateByLocation(src, line) for line in self.lines]
+self.assertTrue(
+self.breakpoints[0] and self.breakpoints[0].GetNumLocations() == 1,
+VALID_BREAKPOINT)
+
+# Start debugging
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertIsNotNone(self.process, PROCESS_IS_VALID)
+self.thread = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[0])
+self.assertIsNotNone(self.thread, "Didn't stop at breakpoint 0.")
+
+def check_correct_stop_reason(self, breakpoint_idx, condition):
+self.assertState(self.process.GetState(), lldb.eStateStopped)
+if condition:
+# All breakpoints active, stop reason is breakpoint
+thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[breakpoint_idx])
+self.assertEquals(self.thread, thread1, "Didn't stop at breakpoint %i." % breakpoint_idx)
+else:
+# Breakpoints are inactive, stop reason is plan complete
+self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete,
+"Expected stop reason to be step into/over/out for inactive breakpoint %i line." % breakpoint_idx)
+
+def check_thread_plan_user_breakpoint(self, condition, set_up_breakpoints_func):
+# Make breakpoints active/inactive in different ways
+set_up_breakpoints_func(condition)
+
+self.thread.StepInto()
+# We should be stopped at the breakpoint_1 line with the correct stop reason
+self.check_correct_stop_reason(1, condition)
+
+# This step-over creates a step-out from `func_1` plan
+self.thread.StepOver()
+# We should be stopped at the breakpoint_2 line with the correct stop reason
+self.check_correct_stop_reason(2, condition)
+
+# Check explicit step-out
+self.thread.StepOut()
+# We should be stopped at the breakpoint_3 line with the correct stop reason
+self.check_correct_stop_reason(3, condition)
+
+# Run the process until termination
+self.process.Continue()
+self.assertState(self.process.GetState(), lldb.eStateExited)
+
+def change_breakpoints(self, action):
+action(self.breakpoints[1])
+action(self.breakpoints[2])
+acti

[Lldb-commits] [PATCH] D140368: [lldb] Consider all breakpoints in breakpoint detection

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

In D140368#4007234 , @DavidSpickett 
wrote:

> The intent makes sense. We should stop and report user breakpoints triggered 
> while trying to execute some internal stepping plan, even if they overlap 
> what lldb was planning to do in the first place.
>
> Not totally sure how the change achieves that, this is quite the function. + 
> @jingham who wrote the original changes.
>
>> Currently in some cases lldb reports stop reason as "step out" or "step 
>> over" (from thread plan completion) over "breakpoint"
>
> This would be clearer if you said "(from thread plan completion) instead of 
> "breakpoint"". Took me a while to work out that it wasn't over meaning step 
> over a breakpoint.
>
> I think the test naming could be clearer. 
> `breakpoint/step_out_breakpoint/TestStepOutBreakpoint.py` implies it's just 
> about stepping out. How about 
> `breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py` ? 
> Something that is clear we're testing the interaction of automatic internal 
> stepping plans and breakpoints the user puts in.
>
> Is it worth checking that an unconditional user breakpoint is also reported?

Fixed, please take a look


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140368

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

[Lldb-commits] [PATCH] D140623: [LLDB] Fix for libc++ atomic allowing modification of contained value

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: DavidSpickett, clayborg, k8stone, davide.
kpdev42 added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140623

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
  lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile
  lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
  lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp


Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main()
+{
+std::atomic Q(1);
+return Q; // Set break point at this line.
+}
Index: 
lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
===
--- /dev/null
+++ 
lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
@@ -0,0 +1,48 @@
+"""
+Test change libc++ std::atomic values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxChangeValueTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@add_test_categories(["libc++"])
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772")
+def test(self):
+"""Test that we can change values of libc++ std::atomic."""
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), 
CURRENT_EXECUTABLE_SET)
+
+bkpt = self.target().FindBreakpointByID(
+lldbutil.run_break_set_by_source_regexp(
+self, "Set break point at this line."))
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# Get Frame #0.
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(
+process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint condition")
+frame0 = thread.GetFrameAtIndex(0)
+self.assertTrue(frame0.IsValid(), "Got a valid frame.")
+
+q_value = frame0.FindVariable("Q")
+self.assertTrue(q_value.IsValid(), "Got the SBValue for val")
+inner_val = q_value.GetChildAtIndex(0)
+self.assertTrue(inner_val.IsValid(), "Got the SBValue for inner atomic 
val")
+result = inner_val.SetValueFromCString("42")
+self.assertTrue(result, "Setting val returned True.")
+result = inner_val.GetValueAsUnsigned()
+self.assertTrue(result == 42, "Got correct value (42)")
Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile
@@ -0,0 +1,6 @@
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+
+CXXFLAGS_EXTRAS := -O0
+include Makefile.rules
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
@@ -139,7 +139,7 @@
 
 size_t lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::
 GetIndexOfChildWithName(ConstString name) {
-  return formatters::ExtractIndexFromString(name.GetCString());
+  return name == "Value" ? 0 : UINT32_MAX;
 }
 
 SyntheticChildrenFrontEnd *


Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main()
+{
+std::atomic Q(1);
+return Q; // Set break point at this line.
+}
Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
@@ -0,0 +1,48 @@
+"""
+Test change libc++ std::atomic values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxChangeValueTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@add_test_categories(["libc++"])
+@expectedFailureAll(oslist=["windows"], bugnumber="l

[Lldb-commits] [PATCH] D140624: [LLDB] Fix for libc++ map allowing modification of contained value

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: davide, DavidSpickett, clayborg, k8stone.
kpdev42 added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140624

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
  lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
  lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp

Index: lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main()
+{
+std::map M = {{1,1},{2,2}};
+return M[1]; // Set break point at this line.
+}
Index: lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
@@ -0,0 +1,51 @@
+"""
+Test change libc++ map values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxChangeValueTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@add_test_categories(["libc++"])
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772")
+def test(self):
+"""Test that we can change values of libc++ map."""
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+bkpt = self.target().FindBreakpointByID(
+lldbutil.run_break_set_by_source_regexp(
+self, "Set break point at this line."))
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# Get Frame #0.
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(
+process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint condition")
+frame0 = thread.GetFrameAtIndex(0)
+self.assertTrue(frame0.IsValid(), "Got a valid frame.")
+
+val_value = frame0.FindVariable("M")
+self.assertTrue(val_value.IsValid(), "Got the SBValue for val")
+pair0 = val_value.GetChildMemberWithName("[0]")
+self.assertTrue(pair0.IsValid(), "Got the SBValue for [0]")
+self.assertTrue(pair0.GetNumChildren() == 2, "Got 2 children")
+pair0_second = pair0.GetChildMemberWithName("second")
+self.assertTrue(pair0_second.IsValid(), "Got the SBValue for [0].second")
+result = pair0_second.SetValueFromCString("12345")
+self.assertTrue(result, "Setting val returned True.")
+result = pair0_second.GetValueAsUnsigned()
+self.assertTrue(result == 12345, "Got correct value (12345)")
Index: lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
@@ -0,0 +1,6 @@
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+
+CXXFLAGS_EXTRAS := -O0
+include Makefile.rules
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -397,18 +397,9 @@
   // at this point we have a valid
   // we need to copy current_sp into a new object otherwise we will end up with
   // all items named __value_
-  DataExtractor data;
-  Status error;
-  iterated_sp->GetData(data, error);
-  if (error.Fail()) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
-  }
   StreamString name;
   name.Printf("[%" PRIu64 "]", (uint64_t)idx);
-  auto potential_child_sp = CreateValueObjectFromData(
-  name.GetString(), data, m_backend.GetExecutionContextRef(),
-  m_element_type);
+  auto potential_child_sp = iterated_sp->Clone(ConstString(name.GetString()));
   if (potential_child_sp) {
 switch (potential_child_sp->GetNumChildren()) {
 case 1: {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140368: [lldb] Consider all breakpoints in breakpoint detection

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 485434.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140368

Files:
  lldb/source/Target/StopInfo.cpp
  lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/Makefile
  
lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
  lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp

Index: lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
@@ -0,0 +1,11 @@
+int func_1() { return 1; }
+
+int func_2() {
+  func_1(); // breakpoint_1
+  return 1 + func_1();  // breakpoint_2
+}
+
+int main(int argc, char const *argv[]) {
+  func_2();  // breakpoint_0
+  return 0;
+}
Index: lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
@@ -0,0 +1,130 @@
+"""
+Test that breakpoints (reason = breakpoint) have more priority than
+plan completion (reason = step in/out/over) when reporting stop reason after step,
+in particular 'step out' and 'step over', and in addition 'step in'.
+Check for correct StopReason when stepping to the line with breakpoint,
+which should be eStopReasonBreakpoint in general,
+and eStopReasonPlanComplete when breakpoint's condition fails or it is disabled.
+"""
+
+
+import unittest2
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class ThreadPlanUserBreakpointsTestCase(TestBase):
+
+def setUp(self):
+TestBase.setUp(self)
+
+self.build()
+exe = self.getBuildArtifact("a.out")
+src = lldb.SBFileSpec("main.cpp")
+
+# Create a target by the debugger.
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+
+# Setup three breakpoints
+self.lines = [line_number('main.cpp', "breakpoint_%i" % i) for i in range(3)]
+
+self.breakpoints = [self.target.BreakpointCreateByLocation(src, line) for line in self.lines]
+self.assertTrue(
+self.breakpoints[0] and self.breakpoints[0].GetNumLocations() == 1,
+VALID_BREAKPOINT)
+
+# Start debugging
+self.process = self.target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertIsNotNone(self.process, PROCESS_IS_VALID)
+self.thread = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[0])
+self.assertIsNotNone(self.thread, "Didn't stop at breakpoint 0.")
+
+def check_correct_stop_reason(self, breakpoint_idx, condition):
+self.assertState(self.process.GetState(), lldb.eStateStopped)
+if condition:
+# All breakpoints active, stop reason is breakpoint
+thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[breakpoint_idx])
+self.assertEquals(self.thread, thread1, "Didn't stop at breakpoint %i." % breakpoint_idx)
+else:
+# Breakpoints are inactive, stop reason is plan complete
+self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete,
+"Expected stop reason to be step into/over/out for inactive breakpoint %i line." % breakpoint_idx)
+
+def change_breakpoints(self, action):
+for i in range(1, len(self.breakpoints)):
+action(self.breakpoints[i])
+
+def check_thread_plan_user_breakpoint(self, condition, set_up_breakpoint_func):
+# Make breakpoints active/inactive in different ways
+self.change_breakpoints(lambda bp: set_up_breakpoint_func(condition, bp))
+
+self.thread.StepInto()
+# We should be stopped at the breakpoint_1 line with the correct stop reason
+self.check_correct_stop_reason(1, condition)
+
+# This step-over creates a step-out from `func_1` plan
+self.thread.StepOver()
+# We should be stopped at the breakpoint_2 line with the correct stop reason
+self.check_correct_stop_reason(2, condition)
+
+# Check explicit step-out
+# Make sure we install the breakpoint at the right address:
+# on some architectures (e.g, aarch64), step-out stops before the next source line
+return_addr = self.thread.GetFrameAtIndex(1).GetPC()
+step_out_breakpoint = self.target.BreakpointCreateByAddress(return_addr)
+set_up_breakpoint_func(condition, step_out_breakpoint)
+self.breakpoints.append(step_out_breakpoint)
+
+sel

[Lldb-commits] [PATCH] D140368: [lldb] Consider all breakpoints in breakpoint detection

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140368

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

[Lldb-commits] [PATCH] D140368: [lldb] Consider all breakpoints in breakpoint detection

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

@jingham
May I kindly ask you to take a look at this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140368

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

[Lldb-commits] [PATCH] D140368: [lldb] Consider all breakpoints in breakpoint detection

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 488154.
kpdev42 added a comment.

Renaming and cleanup according to review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140368

Files:
  lldb/source/Target/StopInfo.cpp
  lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/Makefile
  
lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
  lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp

Index: lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
@@ -0,0 +1,11 @@
+int func_1() { return 1; }
+
+int func_2() {
+  func_1(); // breakpoint_0
+  return 1 + func_1();  // breakpoint_1
+}
+
+int main(int argc, char const *argv[]) {
+  func_2();  // Start from here
+  return 0;
+}
Index: lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
@@ -0,0 +1,121 @@
+"""
+Test that breakpoints (reason = breakpoint) have more priority than
+plan completion (reason = step in/out/over) when reporting stop reason after step,
+in particular 'step out' and 'step over', and in addition 'step in'.
+Check for correct StopReason when stepping to the line with breakpoint,
+which should be eStopReasonBreakpoint in general,
+and eStopReasonPlanComplete when breakpoint's condition fails or it is disabled.
+"""
+
+
+import unittest2
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class ThreadPlanUserBreakpointsTestCase(TestBase):
+
+def setUp(self):
+TestBase.setUp(self)
+
+# Build and run to starting breakpoint
+self.build()
+src = lldb.SBFileSpec('main.cpp')
+(self.target, self.process, self.thread, _) = \
+lldbutil.run_to_source_breakpoint(self, '// Start from here', src)
+
+# Setup two more breakpoints
+self.breakpoints = [self.target.BreakpointCreateBySourceRegex('breakpoint_%i' % i, src)
+for i in range(2)]
+self.assertTrue(
+all(bp and bp.GetNumLocations() == 1 for bp in self.breakpoints),
+VALID_BREAKPOINT)
+
+def check_correct_stop_reason(self, breakpoint_idx, condition):
+self.assertState(self.process.GetState(), lldb.eStateStopped)
+if condition:
+# All breakpoints active, stop reason is breakpoint
+thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[breakpoint_idx])
+self.assertEquals(self.thread, thread1, "Didn't stop at breakpoint %i." % breakpoint_idx)
+else:
+# Breakpoints are inactive, stop reason is plan complete
+self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete,
+'Expected stop reason to be step into/over/out for inactive breakpoint %i line.' % breakpoint_idx)
+
+def change_breakpoints(self, action):
+for bp in self.breakpoints:
+action(bp)
+
+def check_thread_plan_user_breakpoint(self, condition, set_up_breakpoint_func):
+# Make breakpoints active/inactive in different ways
+self.change_breakpoints(lambda bp: set_up_breakpoint_func(condition, bp))
+
+self.thread.StepInto()
+# We should be stopped at the breakpoint_0 line with the correct stop reason
+self.check_correct_stop_reason(0, condition)
+
+# This step-over creates a step-out from `func_1` plan
+self.thread.StepOver()
+# We should be stopped at the breakpoint_1 line with the correct stop reason
+self.check_correct_stop_reason(1, condition)
+
+# Check explicit step-out
+# Make sure we install the breakpoint at the right address:
+# step-out might stop on different lines, if the compiler
+# did or did not emit more instructions after the return
+return_addr = self.thread.GetFrameAtIndex(1).GetPC()
+step_out_breakpoint = self.target.BreakpointCreateByAddress(return_addr)
+self.assertTrue(step_out_breakpoint, VALID_BREAKPOINT)
+set_up_breakpoint_func(condition, step_out_breakpoint)
+self.breakpoints.append(step_out_breakpoint)
+self.thread.StepOut()
+# We should be stopped somewhere in the main frame with the correct stop reason
+self.check_correct_stop_reason(2, condition)
+
+# Run the process until termination
+self.process.Continue()
+self.assertState(self.process.GetState(), lldb.eStateExited)

[Lldb-commits] [PATCH] D140623: [LLDB] Fix for libc++ atomic allowing modification of contained value

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGff9c31b23b76: [LLDB] Fix for libc++ atomic allowing 
modification of contained value (authored by kpdev42).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140623

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
  lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile
  lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
  lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp


Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main()
+{
+std::atomic Q(1);
+return Q; // Set break point at this line.
+}
Index: 
lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
===
--- /dev/null
+++ 
lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
@@ -0,0 +1,48 @@
+"""
+Test change libc++ std::atomic values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxChangeValueTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@add_test_categories(["libc++"])
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772")
+def test(self):
+"""Test that we can change values of libc++ std::atomic."""
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), 
CURRENT_EXECUTABLE_SET)
+
+bkpt = self.target().FindBreakpointByID(
+lldbutil.run_break_set_by_source_regexp(
+self, "Set break point at this line."))
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# Get Frame #0.
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(
+process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint condition")
+frame0 = thread.GetFrameAtIndex(0)
+self.assertTrue(frame0.IsValid(), "Got a valid frame.")
+
+q_value = frame0.FindVariable("Q")
+self.assertTrue(q_value.IsValid(), "Got the SBValue for val")
+inner_val = q_value.GetChildAtIndex(0)
+self.assertTrue(inner_val.IsValid(), "Got the SBValue for inner atomic 
val")
+result = inner_val.SetValueFromCString("42")
+self.assertTrue(result, "Setting val returned True.")
+result = inner_val.GetValueAsUnsigned()
+self.assertTrue(result == 42, "Got correct value (42)")
Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile
@@ -0,0 +1,6 @@
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+
+CXXFLAGS_EXTRAS := -O0
+include Makefile.rules
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
@@ -139,7 +139,7 @@
 
 size_t lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::
 GetIndexOfChildWithName(ConstString name) {
-  return formatters::ExtractIndexFromString(name.GetCString());
+  return name == "Value" ? 0 : UINT32_MAX;
 }
 
 SyntheticChildrenFrontEnd *


Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main()
+{
+std::atomic Q(1);
+return Q; // Set break point at this line.
+}
Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
@@ -0,0 +1,48 @@
+"""
+Test change libc++ std::atomic values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxChangeValueTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@add_test_categories(["libc++"])
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772

[Lldb-commits] [PATCH] D140624: [LLDB] Fixes summary formatter for libc++ map allowing modification of contained value

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG92f0e4ccafac: [LLDB] Fixes summary formatter for libc++ map 
allowing modification of… (authored by kpdev42).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140624

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
  lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
  lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp

Index: lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main()
+{
+std::map M = {{1,1},{2,2}};
+return M[1]; // Set break point at this line.
+}
Index: lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
@@ -0,0 +1,51 @@
+"""
+Test change libc++ map values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxChangeValueTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@add_test_categories(["libc++"])
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772")
+def test(self):
+"""Test that we can change values of libc++ map."""
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+bkpt = self.target().FindBreakpointByID(
+lldbutil.run_break_set_by_source_regexp(
+self, "Set break point at this line."))
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# Get Frame #0.
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(
+process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint condition")
+frame0 = thread.GetFrameAtIndex(0)
+self.assertTrue(frame0.IsValid(), "Got a valid frame.")
+
+val_value = frame0.FindVariable("M")
+self.assertTrue(val_value.IsValid(), "Got the SBValue for val")
+pair0 = val_value.GetChildMemberWithName("[0]")
+self.assertTrue(pair0.IsValid(), "Got the SBValue for [0]")
+self.assertTrue(pair0.GetNumChildren() == 2, "Got 2 children")
+pair0_second = pair0.GetChildMemberWithName("second")
+self.assertTrue(pair0_second.IsValid(), "Got the SBValue for [0].second")
+result = pair0_second.SetValueFromCString("12345")
+self.assertTrue(result, "Setting val returned True.")
+result = pair0_second.GetValueAsUnsigned()
+self.assertTrue(result == 12345, "Got correct value (12345)")
Index: lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
@@ -0,0 +1,6 @@
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+
+CXXFLAGS_EXTRAS := -O0
+include Makefile.rules
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -397,18 +397,9 @@
   // at this point we have a valid
   // we need to copy current_sp into a new object otherwise we will end up with
   // all items named __value_
-  DataExtractor data;
-  Status error;
-  iterated_sp->GetData(data, error);
-  if (error.Fail()) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
-  }
   StreamString name;
   name.Printf("[%" PRIu64 "]", (uint64_t)idx);
-  auto potential_child_sp = CreateValueObjectFromData(
-  name.GetString(), data, m_backend.GetExecutionContextRef(),
-  m_element_type);
+  auto potential_child_sp = iterated_sp->Clone(ConstString(name.GetString()));
   if (potential_child_sp) {
 switch (potential_child_sp->GetNumChildren()) {
 case 1: {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D140368: [lldb] Consider all breakpoints in breakpoint detection

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2af0a478eaee: [lldb] Consider all breakpoints in breakpoint 
detection (authored by kpdev42).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140368

Files:
  lldb/source/Target/StopInfo.cpp
  lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/Makefile
  
lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
  lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp

Index: lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/main.cpp
@@ -0,0 +1,11 @@
+int func_1() { return 1; }
+
+int func_2() {
+  func_1(); // breakpoint_0
+  return 1 + func_1();  // breakpoint_1
+}
+
+int main(int argc, char const *argv[]) {
+  func_2();  // Start from here
+  return 0;
+}
Index: lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/thread_plan_user_breakpoint/TestThreadPlanUserBreakpoint.py
@@ -0,0 +1,121 @@
+"""
+Test that breakpoints (reason = breakpoint) have more priority than
+plan completion (reason = step in/out/over) when reporting stop reason after step,
+in particular 'step out' and 'step over', and in addition 'step in'.
+Check for correct StopReason when stepping to the line with breakpoint,
+which should be eStopReasonBreakpoint in general,
+and eStopReasonPlanComplete when breakpoint's condition fails or it is disabled.
+"""
+
+
+import unittest2
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class ThreadPlanUserBreakpointsTestCase(TestBase):
+
+def setUp(self):
+TestBase.setUp(self)
+
+# Build and run to starting breakpoint
+self.build()
+src = lldb.SBFileSpec('main.cpp')
+(self.target, self.process, self.thread, _) = \
+lldbutil.run_to_source_breakpoint(self, '// Start from here', src)
+
+# Setup two more breakpoints
+self.breakpoints = [self.target.BreakpointCreateBySourceRegex('breakpoint_%i' % i, src)
+for i in range(2)]
+self.assertTrue(
+all(bp and bp.GetNumLocations() == 1 for bp in self.breakpoints),
+VALID_BREAKPOINT)
+
+def check_correct_stop_reason(self, breakpoint_idx, condition):
+self.assertState(self.process.GetState(), lldb.eStateStopped)
+if condition:
+# All breakpoints active, stop reason is breakpoint
+thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoints[breakpoint_idx])
+self.assertEquals(self.thread, thread1, "Didn't stop at breakpoint %i." % breakpoint_idx)
+else:
+# Breakpoints are inactive, stop reason is plan complete
+self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete,
+'Expected stop reason to be step into/over/out for inactive breakpoint %i line.' % breakpoint_idx)
+
+def change_breakpoints(self, action):
+for bp in self.breakpoints:
+action(bp)
+
+def check_thread_plan_user_breakpoint(self, condition, set_up_breakpoint_func):
+# Make breakpoints active/inactive in different ways
+self.change_breakpoints(lambda bp: set_up_breakpoint_func(condition, bp))
+
+self.thread.StepInto()
+# We should be stopped at the breakpoint_0 line with the correct stop reason
+self.check_correct_stop_reason(0, condition)
+
+# This step-over creates a step-out from `func_1` plan
+self.thread.StepOver()
+# We should be stopped at the breakpoint_1 line with the correct stop reason
+self.check_correct_stop_reason(1, condition)
+
+# Check explicit step-out
+# Make sure we install the breakpoint at the right address:
+# step-out might stop on different lines, if the compiler
+# did or did not emit more instructions after the return
+return_addr = self.thread.GetFrameAtIndex(1).GetPC()
+step_out_breakpoint = self.target.BreakpointCreateByAddress(return_addr)
+self.assertTrue(step_out_breakpoint, VALID_BREAKPOINT)
+set_up_breakpoint_func(condition, step_out_breakpoint)
+self.breakpoints.append(step_out_breakpoint)
+self.thread.StepOut()
+# We should be stopped somewhere in the main frame with the correct stop reason
+self.check_correct_stop_reason(2, condition)
+
+# Run the process until termination
+self.process.Continu

[Lldb-commits] [PATCH] D140624: [LLDB] Fixes summary formatter for libc++ map allowing modification of contained value

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

In D140624#4079306 , @omjavaid wrote:

> This rev has broken lldb-arm-ubuntu buildbot 
> https://lab.llvm.org/buildbot/#/builders/17/builds/33173

Sorry. I've added a small fix for it 
https://github.com/llvm/llvm-project/commit/4ae221f5a4d1977f316b7d5f033763f876b471e7
Could you please verify if it works now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140624

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

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: DavidSpickett, davide, clayborg, k8stone.
kpdev42 added projects: LLVM, LLDB.
Herald added subscribers: Michael137, JDevlieghere, martong.
Herald added a reviewer: shafik.
Herald added a reviewer: shafik.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The no_unique_address attribute is currently a part of C++20 standard and is 
being used in both libstdc++ and libc++. This causes problems when debugging 
C++ code with lldb because the latter doesn't expect two non-empty base classes 
having the same offset and crashes with assertion. Patch attempts to infer this 
attribute by detecting two consecutive base classes with the same offset to 
derived class and marking first of those classes as empty and adding 
no_unique_address attribute to all of its fields.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143347

Files:
  clang/include/clang/AST/DeclCXX.h
  clang/lib/AST/ASTImporter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/types/TestEmptyBase.py
  lldb/test/API/types/empty_base_type.cpp

Index: lldb/test/API/types/empty_base_type.cpp
===
--- /dev/null
+++ lldb/test/API/types/empty_base_type.cpp
@@ -0,0 +1,23 @@
+struct C
+{
+ long c,d;
+};
+
+struct D
+{
+};
+
+struct B
+{
+  [[no_unique_address]] D x;
+};
+
+struct A : B,C
+{
+ long a,b;
+} _a;
+
+
+int main() {
+  return _a.a; // Set breakpoint here.
+}
Index: lldb/test/API/types/TestEmptyBase.py
===
--- /dev/null
+++ lldb/test/API/types/TestEmptyBase.py
@@ -0,0 +1,42 @@
+"""
+Test that recursive types are handled correctly.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class EmptyBaseTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+# Find the line number to break for main.c.
+self.line = line_number('empty_base_type.cpp',
+'// Set breakpoint here.')
+
+self.sources = {
+'CXX_SOURCES': 'empty_base_type.cpp'}
+
+def test(self):
+"""Test that recursive structs are displayed correctly."""
+self.build(dictionary=self.sources)
+self.setTearDownCleanup(dictionary=self.sources)
+self.run_expr()
+
+def run_expr(self):
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"empty_base_type.cpp",
+self.line,
+num_expected_locations=-1,
+loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.runCmd("expression _a")
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1460,6 +1460,8 @@
   if (!result)
 return;
 
+  const clang::CXXBaseSpecifier *prev_base =
+  base_classes.empty() ? nullptr : base_classes.back().get();
   base_classes.push_back(std::move(result));
 
   if (is_virtual) {
@@ -1476,6 +1478,20 @@
 // be removed from LayoutRecordType() in the external
 // AST source in clang.
   } else {
+if (prev_base) {
+  clang::CXXRecordDecl *prev_base_decl =
+  prev_base->getType()->getAsCXXRecordDecl();
+  if (prev_base_decl && !prev_base_decl->isEmpty()) {
+auto it = layout_info.base_offsets.find(prev_base_decl);
+assert(it != layout_info.base_offsets.end());
+if (it->second.getQuantity() == member_byte_offset) {
+  prev_base_decl->markEmpty();
+  for (auto *field : prev_base_decl->fields())
+field->addAttr(clang::NoUniqueAddressAttr::Create(
+ast->getASTContext(), clang::SourceRange()));
+}
+  }
+}
 layout_info.base_offsets.insert(std::make_pair(
 ast->GetAsCXXRecordDecl(base_class_clang_type.GetOpaqueQualType()),
 clang::CharUnits::fromQuantity(member_byte_offset)));
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -548,6 +548,7 @@
 
 void ClangASTImporter::SetRecordLayout(clang::RecordDecl *decl,
 const LayoutInfo &layout) {
+
   m_record_decl_to_layout_map.insert(std::make_pair(decl, layout));
 }
 
Index: clang/lib/AST/ASTImporter.cpp
==

[Lldb-commits] [PATCH] D144390: [lldb] Send QPassSignals packet on attach, and at start for remote platforms

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: clayborg, davide, k8stone, DavidSpickett.
kpdev42 added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added a subscriber: lldb-commits.

Before this patch, lldb did not send signal filtering information (QPassSignals 
packet) to lldb-server on remote platforms until signal settings were 
explicitly changed. Patch changes last sent signals version to be initialized 
with an invalid value instead of 0, so that the signal filtering information is 
sent to the server when the version is actually 0, which happens on remote 
platforms when the signal structure is copied and the version is reset. Also 
changes Process so that the information is sent not only right after launch, 
but right after attach too to be more consistent.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144390

Files:
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Process.cpp
  
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteSignalFiltering.py


Index: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteSignalFiltering.py
===
--- /dev/null
+++ 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteSignalFiltering.py
@@ -0,0 +1,31 @@
+"""Test that GDBRemoteProcess sends default signal filtering info when 
necessary"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestGDBRemoteSignalFiltering(GDBRemoteTestBase):
+
+def test_signals_sent_on_connect(self):
+"""Test that signal filtering info is sent on connect"""
+class Responder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+;QPassSignals+"
+
+def respond(self, packet):
+if packet == "QPassSignals":
+return self.QPassSignals()
+return MockGDBServerResponder.respond(self, packet)
+
+def QPassSignals(self):
+return "OK"
+
+self.server.responder = Responder()
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+self.assertGreater(
+len([p for p in self.server.responder.packetLog if 
p.startswith("QPassSignals:")]),
+0)
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2592,7 +2592,7 @@
 if (!m_os_up)
   LoadOperatingSystemPlugin(false);
 
-// We successfully launched the process and stopped, now it the
+// We successfully launched the process and stopped, now it is the
 // right time to set up signal filters before resuming.
 UpdateAutomaticSignalFiltering();
 return Status();
@@ -3026,6 +3026,10 @@
 : "");
 }
   }
+
+  // We successfully attached to the process and stopped, now it is the
+  // right time to set up signal filters before resuming.
+  UpdateAutomaticSignalFiltering();
 }
 
 Status Process::ConnectRemote(llvm::StringRef remote_url) {
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -420,7 +420,7 @@
   // For ProcessGDBRemote only
   std::string m_partial_profile_data;
   std::map m_thread_id_to_used_usec_map;
-  uint64_t m_last_signals_version = 0;
+  uint64_t m_last_signals_version = UINT64_MAX;
 
   static bool NewThreadNotifyBreakpointHit(void *baton,
StoppointCallbackContext *context,


Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteSignalFiltering.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteSignalFiltering.py
@@ -0,0 +1,31 @@
+"""Test that GDBRemoteProcess sends default signal filtering info when necessary"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestGDBRemoteSignalFiltering(GDBRemoteTestBase):
+
+def test_signals_sent_on_connect(self):
+"""Test that signal filtering info is sent on connect"""
+class Responder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "PacketSize=3fff;QStartNoAckMode+;QPassSignals+"
+
+def respond(self, packet):
+

[Lldb-commits] [PATCH] D144392: [lldb] Skip signal handlers for ignored signals on lldb-server's side when stepping

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: DavidSpickett, clayborg, k8stone, davide.
kpdev42 added a project: LLDB.
Herald added subscribers: JDevlieghere, s.egerton, dmgreen, simoncook, emaste.
Herald added a project: All.
kpdev42 requested review of this revision.
Herald added subscribers: lldb-commits, pcwang-thead.

Before this patch, stepping off a breakpoint in lldb might end up inside a 
signal handler if a signal was received, which would result in continuing and 
hitting the same breakpoint again, while on the surface no instruction was 
executed and the user is not interested in that specific signal or its 
handling. This patch uses the machinery set up for software single-stepping to 
circumvent this behavior. This changes what a eStateStepping in lldb-server 
does to a thread on linux platforms: if a single-step is requested right after 
an ignored (pass=true, stop=false, notify=false) signal is received by the 
inferior, then a breakpoint is installed on the current pc, and the thread is 
continued (not single-stepped). When that breakpoint hits (presumably after the 
user signal handler returns or immediately after the continue, if the handler 
is not installed), it is removed, the stop is ignored and the thread is 
continued with single-stepping again. If a stop occurs inside the handler, then 
the stepping ends there, and the breakpoint is removed as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144392

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/NativeProcessContinueUntilBreakpoint.cpp
  lldb/source/Plugins/Process/Utility/NativeProcessContinueUntilBreakpoint.h
  lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
  lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.h
  lldb/test/API/functionalities/thread/signal_during_breakpoint_step/Makefile
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/SignalDuringBreakpointStepTestCase.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointFuncStepIn.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointFuncStepOut.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointFuncStepOver.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointFuncStepUntil.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointStepOut.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointStepOver.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalDuringBreakpointStepUntil.py
  
lldb/test/API/functionalities/thread/signal_during_breakpoint_step/TestSignalStepOverHandler.py
  lldb/test/API/functionalities/thread/signal_during_breakpoint_step/main.cpp

Index: lldb/test/API/functionalities/thread/signal_during_breakpoint_step/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/thread/signal_during_breakpoint_step/main.cpp
@@ -0,0 +1,166 @@
+// This test is intended to create a situation in which signals are received by
+// a thread while it is stepping off a breakpoint. The intended behavior is to
+// skip the handler and to not stop at the breakpoint we are trying to step off
+// the second time, as the corresponding instruction was not executed anyway.
+// If a breakpoint is hit inside the handler, the breakpoint on the line with
+// the original instruction should be hit when the handler is finished.
+//
+// This checks stepping off breakpoints set on single instructions and function
+// calls as well, to see a potential pc change when single-stepping.
+
+#include "pseudo_barrier.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+pseudo_barrier_t g_barrier;
+std::atomic g_send_signals;
+int g_action = 0;
+int g_signal_count = 0;
+int g_cur_iter = 0;
+
+// number of times to repeat the action
+int NUM_ITERS = 0;
+// number of times to send a signal per action
+int NUM_SIGNAL_ITERS = 0;
+// number of microseconds to wait between new action checks
+int SIGNAL_ITER_BACKOFF_US = 0;
+// number of microseconds to wait before sending another signal
+int SIGNAL_SIGNAL_ITER_BACKOFF_US = 0;
+// number of microseconds to wait inside the signal handler
+int HANDLER_SLEEP_US = 0;
+
+using action_t = void (*)();
+
+void do_action_func(action_t action) {

[Lldb-commits] [PATCH] D144392: [lldb] Skip signal handlers for ignored signals on lldb-server's side when stepping

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

1. There is a situation where the patch might go wrong. When stepping while 
inside:
  - a signal handler when `SA_NODEFER` flag is set (the same signal can be 
received while it is still being handled), and the signal is received again;
  - a handler that is set on multiple signals and they are not masked, and 
these signals are received one after another;
  - a handler that happens to call the current function

recursion happens and stopping at this specifically set up breakpoint on pc is 
not valid. With this patch this would result in unexpected stops, though this 
situation is very specific. Would it make sense to complicate this logic and 
store current `sp` or `fp` values from `NativeRegisterContext` and compare if 
they are the same?

2. The basic behavior is pretty easy to test by sending a lot of signals, but 
testing specific cases like setting the temporary breakpoint on an address 
where there is already another breakpoint is tough. One test was added for 
this, but it does not actually trigger the situation. This very much depends on 
when the signal is going to be received by the process, and it can't really be 
controlled.

3. This slightly changes 1 line of code in `NativeProcessFreeBSD`, but I don't 
have the means to test if it even compiles, though the logic should be 
equivalent.

4. When I looked at `NativeProcessFreeBSD`, it seems that software single 
stepping isn't even being set up there, just checked for when the process 
stops. As I can see from history, software single stepping with similar code 
was in the "legacy FreeBSD plugin" which was removed here: 
https://reviews.llvm.org/D96555 , and then single stepping with shared code was 
added to the new plugin here: https://reviews.llvm.org/D95802 . Because I 
couldn't find where the `SetupSoftwareSingleStepping` would be called for 
FreeBSD I didn't change that, but this probably should be addressed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144392

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

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3896
+  if (D->hasAttrs())
+ToField->setAttrs(D->getAttrs());
   ToField->setAccess(D->getAccess());

balazske wrote:
> The import of attributes is handled in function `ASTImporter::Import(Decl*)`. 
> This new line will probably copy all attributes, that may not work in all 
> cases dependent on the attribute types. This may interfere with the later 
> import of attributes, probably these will be duplicated. What was the need 
> for this line? (Simple attributes that do not have references to other nodes 
> could be copied at this place.)
Unfortunately it is too late to copy attribute in `ASTImporter::Import(Decl*)`, 
because field has already been added to record in a call to `ImportImpl 
(VisitFieldDecl/addDeclInternal)`. I've reused the current way of cloning 
attributes in `VisitFieldDecl`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D144390: [lldb] Send QPassSignals packet on attach, and at start for remote platforms

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

In D144390#4138869 , @DavidSpickett 
wrote:

> What is the practical impact of the bug you are fixing?
>
> I guess it is something like if you set signal handling info, then attach to 
> something, that info is not used. Until you set it again, then it'll be sent 
> and used.

The signal filtering settings can only be changed for an already created 
process, so the bug is that lldb does not send the default settings for remote 
platforms (for example, ignoring of real-time signals inside lldb-server is not 
applied)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144390

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

[Lldb-commits] [PATCH] D144392: [lldb] Skip signal handlers for ignored signals on lldb-server's side when stepping

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

In D144392#4139001 , @labath wrote:

> I'm afraid you're fixing this at the wrong end. This kind of complex thread 
> control does not belong inside the debug stub.
>
> IIUC, the problematic sequence of events is:
>
> 1. lldb sends a `vCont:s` packet
> 2. lldb-server resumes (PTRACE_SINGLESTEP)
> 3. process immediatelly stops again (without stepping) due to a pending signal
> 4. lldb-server decides to inject the signal and resumes (steps) again
> 5. process stops inside the signal handler
> 6. confusion ensues
>
> If that's the case, then I think the bug is at step 4, specifically in this 
> code:
>
>   // Check if debugger should stop at this signal or just ignore it and resume
>   // the inferior.
>   if (m_signals_to_ignore.contains(signo)) {
>  ResumeThread(thread, thread.GetState(), signo);
>  return;
>   }
>
> I believe we should not be attempting to inject the signal if the thread was 
> stepping. I think we should change this to report the signal to the client 
> and let it figure out what to do. I.e., change this code into something like:
>
>   if (m_signals_to_ignore.contains(signo) && thread.GetState() == 
> eStateRunning) {
>  ResumeThread(thread, eStateRunning, signo);
>  return;
>   }
>   // else report the signal as usual
>
> If that is not enough to fix the bug, then we can figure out what needs to be 
> done on the client. @jingham might have something to say about that.

The full problematic sequence is this:

- current pc is at a breakpoint, user hits continue (or step, etc.)
- `ThreadPlanStepOverBreakpoint` is pushed
- the plan wants `eStateStepping`, so lldb sens a `vCont;s` packet to the 
lldb-server
- lldb-server resumes with `PTRACE_SINGLESTEP`
- process stops due to a (pass=true, stop=false) signal

Then there are two possibilities, depending on signal filtering settings:

1. The signal is ignored (notify=false)
  - lldb-server injects the signal back to the process and resumes (steps again)
2. The signal is not ignored (notify=true)
  - lldb-server sends the stop reason with signal to the lldb client
  - lldb does not care, because it is configured to not stop, so wants to step 
again, sends the packet

After 1. and 2., the events are the same:

- process stops due to stepping into the signal handler, lldb client sees a 
successful step
- `StepOverBreakpoint` plan sees a pc != breakpoint address, thinks its job is 
done, pops off successfuly with an autocontinue
- process resumes, gets out of the handler right back to the breakpoint address
- the breakpoint hits, so the user sees a second breakpoint hit, but the 
instruction under that address was still not executed

Technically, this is correct, because the pc was at a breakpoint, the process 
did execute some instructions and went back to the breakpoint, and the program 
state could have changed. But this is very annoying when a lot of signals are 
received in the background (and the signal is not interesting to the user, as 
it, for example, comes from a low level library, the same reason real-time 
signals are `stop=false` `notify=false` by default right now: 
https://reviews.llvm.org/D12795)

So it does not depend on the signal filtering (it can be configured anyway), 
but the problem I would think is that client does not handle the situation with 
signals while stepping (at least stepping off a breakpoint) properly, and 
lldb-server does not help.

Gdb does this for example:

> GDB optimizes for stepping the mainline code. If a signal that has handle 
> nostop and handle pass set arrives while a stepping command (e.g., stepi, 
> step, next) is in progress, GDB lets the signal handler run and then resumes 
> stepping the mainline code once the signal handler returns. In other words, 
> GDB steps over the signal handler. This prevents signals that you’ve 
> specified as not interesting (with handle nostop) from changing the focus of 
> debugging unexpectedly.

(https://sourceware.org/gdb/onlinedocs/gdb/Signals.html), which seems 
reasonable.

If this logic does not belong in lldb-server, then it could be fixed right 
inside the `StepOverBreakpoint` plan, by enabling the breakpoint being stepped 
over when a signal is received, waiting for it to hit when the potential 
handler has returned, and then trying to step off again. But then doing a 
`step-into` from a line with a breakpoint will step over the signal handler, 
and from a line without a breakpoint will stop inside the handler, if a signal 
is received. Then probably creating a new `ThreadPlan` instead with the same 
logic, executing on top of the plan stack, is the way to go?

Anyway, in my attempts at fixing it on the client side, additionally, for some 
reason the following situation is often triggered:

- process steps onto a breakpoint, but right before executing the instruction 
and actually hitting (the `SIGTRAP` signal) another signal (`stop=false`) is 
delivered
- the process wan

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 507272.
kpdev42 added a comment.

Update patch after landing D145057 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/types/TestEmptyBase.py
  lldb/test/API/types/empty_base_type.cpp

Index: lldb/test/API/types/empty_base_type.cpp
===
--- /dev/null
+++ lldb/test/API/types/empty_base_type.cpp
@@ -0,0 +1,23 @@
+struct C
+{
+ long c,d;
+};
+
+struct D
+{
+};
+
+struct B
+{
+  [[no_unique_address]] D x;
+};
+
+struct A : B,C
+{
+ long a,b;
+} _a;
+
+
+int main() {
+  return _a.a; // Set breakpoint here.
+}
Index: lldb/test/API/types/TestEmptyBase.py
===
--- /dev/null
+++ lldb/test/API/types/TestEmptyBase.py
@@ -0,0 +1,42 @@
+"""
+Test that recursive types are handled correctly.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class EmptyBaseTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+# Find the line number to break for main.c.
+self.line = line_number('empty_base_type.cpp',
+'// Set breakpoint here.')
+
+self.sources = {
+'CXX_SOURCES': 'empty_base_type.cpp'}
+
+def test(self):
+"""Test that recursive structs are displayed correctly."""
+self.build(dictionary=self.sources)
+self.setTearDownCleanup(dictionary=self.sources)
+self.run_expr()
+
+def run_expr(self):
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"empty_base_type.cpp",
+self.line,
+num_expected_locations=-1,
+loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.runCmd("expression _a")
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1460,6 +1460,8 @@
   if (!result)
 return;
 
+  const clang::CXXBaseSpecifier *prev_base =
+  base_classes.empty() ? nullptr : base_classes.back().get();
   base_classes.push_back(std::move(result));
 
   if (is_virtual) {
@@ -1476,6 +1478,20 @@
 // be removed from LayoutRecordType() in the external
 // AST source in clang.
   } else {
+if (prev_base) {
+  clang::CXXRecordDecl *prev_base_decl =
+  prev_base->getType()->getAsCXXRecordDecl();
+  if (prev_base_decl && !prev_base_decl->isEmpty()) {
+auto it = layout_info.base_offsets.find(prev_base_decl);
+assert(it != layout_info.base_offsets.end());
+if (it->second.getQuantity() == member_byte_offset) {
+  prev_base_decl->markEmpty();
+  for (auto *field : prev_base_decl->fields())
+field->addAttr(clang::NoUniqueAddressAttr::Create(
+ast->getASTContext(), clang::SourceRange()));
+}
+  }
+}
 layout_info.base_offsets.insert(std::make_pair(
 ast->GetAsCXXRecordDecl(base_class_clang_type.GetOpaqueQualType()),
 clang::CharUnits::fromQuantity(member_byte_offset)));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 511362.
kpdev42 added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/types/TestEmptyBase.py
  lldb/test/API/types/empty_base_type.cpp

Index: lldb/test/API/types/empty_base_type.cpp
===
--- /dev/null
+++ lldb/test/API/types/empty_base_type.cpp
@@ -0,0 +1,29 @@
+struct C
+{
+ long c,d;
+};
+
+struct D
+{
+};
+
+struct B
+{
+  [[no_unique_address]] D x;
+};
+
+struct E
+{
+  [[no_unique_address]] D x;
+};
+
+
+struct A : B,E,C
+{
+ long a,b;
+} _a;
+
+
+int main() {
+  return _a.a; // Set breakpoint here.
+}
Index: lldb/test/API/types/TestEmptyBase.py
===
--- /dev/null
+++ lldb/test/API/types/TestEmptyBase.py
@@ -0,0 +1,42 @@
+"""
+Test that recursive types are handled correctly.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class EmptyBaseTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+# Find the line number to break for main.c.
+self.line = line_number('empty_base_type.cpp',
+'// Set breakpoint here.')
+
+self.sources = {
+'CXX_SOURCES': 'empty_base_type.cpp'}
+
+def test(self):
+"""Test that recursive structs are displayed correctly."""
+self.build(dictionary=self.sources)
+self.setTearDownCleanup(dictionary=self.sources)
+self.run_expr()
+
+def run_expr(self):
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"empty_base_type.cpp",
+self.line,
+num_expected_locations=-1,
+loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.runCmd("expression _a")
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1460,8 +1460,6 @@
   if (!result)
 return;
 
-  base_classes.push_back(std::move(result));
-
   if (is_virtual) {
 // Do not specify any offset for virtual inheritance. The DWARF
 // produced by clang doesn't give us a constant offset, but gives
@@ -1476,10 +1474,33 @@
 // be removed from LayoutRecordType() in the external
 // AST source in clang.
   } else {
+// DWARF doesn't have any representation for [[no_unique_address]]
+// attribute. Empty base classes with [[no_unique_address]] fields
+// confuse lldb and prevent construction of object memory layout.
+// To fix this we scan base classes in reverse order to determine
+// overlapping offsets. Wnen found we consider such class as empty
+// base with all fields having [[no_unique_address]] attribute.
+for (auto it = base_classes.rbegin(); it != base_classes.rend(); ++it) {
+  clang::CXXRecordDecl *prev_base_decl =
+  (*it)->getType()->getAsCXXRecordDecl();
+  // We've already marked this class, exit.
+  if (prev_base_decl->isEmpty())
+break;
+  auto it_layout = layout_info.base_offsets.find(prev_base_decl);
+  assert(it_layout != layout_info.base_offsets.end());
+  // We found a normal base class, exit.
+  if (it_layout->second.getQuantity() < member_byte_offset)
+break;
+  prev_base_decl->markEmpty();
+  for (auto *field : prev_base_decl->fields())
+field->addAttr(clang::NoUniqueAddressAttr::Create(
+ast->getASTContext(), clang::SourceRange()));
+}
 layout_info.base_offsets.insert(std::make_pair(
 ast->GetAsCXXRecordDecl(base_class_clang_type.GetOpaqueQualType()),
 clang::CharUnits::fromQuantity(member_byte_offset)));
   }
+  base_classes.push_back(std::move(result));
 }
 
 TypeSP DWARFASTParserClang::UpdateSymbolContextScopeForType(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 511683.
kpdev42 edited the summary of this revision.
kpdev42 added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/lang/cpp/no_unique_address/Makefile
  lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
  lldb/test/API/lang/cpp/no_unique_address/main.cpp

Index: lldb/test/API/lang/cpp/no_unique_address/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/main.cpp
@@ -0,0 +1,67 @@
+struct C
+{
+ long c,d;
+};
+
+struct Q
+{
+ long h;
+};
+
+struct D
+{
+};
+
+struct B
+{
+  [[no_unique_address]] D x;
+};
+
+struct E
+{
+  [[no_unique_address]] D x;
+};
+
+struct Foo1 : B,E,C
+{
+ long a = 42,b = 52;
+} _f1;
+
+struct Foo2 : B,E
+{
+ long v = 42;
+} _f2;
+
+struct Foo3 : C,B,E
+{
+ long v = 42;
+} _f3;
+
+struct Foo4 : B,C,E,Q
+{
+ long v = 42;
+} _f4;
+
+struct Foo5 : B,C,E
+{
+ [[no_unique_address]] D x1;
+ [[no_unique_address]] D x2;
+ long v1 = 42;
+ [[no_unique_address]] D y1;
+ [[no_unique_address]] D y2;
+ long v2 = 52;
+ [[no_unique_address]] D z1;
+ [[no_unique_address]] D z2;
+} _f5;
+
+struct Foo6 : B,E
+{
+ long v1 = 42;
+ [[no_unique_address]] D y1;
+ [[no_unique_address]] D y2;
+ long v2 = 52;
+} _f6;
+
+int main() {
+  return 0; // Set breakpoint here.
+}
Index: lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
@@ -0,0 +1,28 @@
+"""
+Test that we correctly handle [[no_unique_address]] attribute.
+"""
+
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestInlineNamespace(TestBase):
+
+def test(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self,
+"// Set breakpoint here.", lldb.SBFileSpec("main.cpp"))
+
+self.expect_expr("_f1.a", result_type="long", result_value="42")
+self.expect_expr("_f1.b", result_type="long", result_value="52")
+self.expect_expr("_f2.v", result_type="long", result_value="42")
+self.expect_expr("_f3.v", result_type="long", result_value="42")
+self.expect_expr("_f4.v", result_type="long", result_value="42")
+self.expect_expr("_f5.v1", result_type="long", result_value="42")
+self.expect_expr("_f5.v2", result_type="long", result_value="52")
+self.expect_expr("_f6.v1", result_type="long", result_value="42")
+self.expect_expr("_f6.v2", result_type="long", result_value="52")
Index: lldb/test/API/lang/cpp/no_unique_address/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -223,6 +223,7 @@
 uint64_t bit_size = 0;
 uint64_t bit_offset = 0;
 bool is_bitfield = false;
+clang::FieldDecl *field_decl = nullptr;
 
 FieldInfo() = default;
 
@@ -275,6 +276,10 @@
   const ParsedDWARFTypeAttributes &attrs);
   lldb::TypeSP ParsePointerToMemberType(const DWARFDIE &die,
 const ParsedDWARFTypeAttributes &attrs);
+  void FixupBaseClasses(
+  std::vector> &base_classes,
+  const lldb_private::ClangASTImporter::LayoutInfo &layout_info,
+  long byte_offset);
 
   /// Parses a DW_TAG_inheritance DIE into a base/super class.
   ///
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1366,6 +1366,28 @@
   return nullptr;
 }
 
+void DWARFASTParserClang::FixupBaseClasses(
+std::vector> &base_classes,
+const ClangASTImporter::LayoutInfo &layout_info, long byte_offset) {
+  for (auto it = base_classes.rbegin(); it != base_classes.rend(); ++it) {
+clang::CXXRecordDecl *prev_base_decl =
+(*it)->getType()->getAsCXXRecordDecl();
+// We've already marked this class, exit.
+if (prev_base_decl->isEmpty())
+  break;
+auto it_layout = layout_info.base_offsets.find(prev_base_decl);
+if (it_layout == layout_info.base_offsets.end())
+  continue;
+// We found a normal base 

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1483
+// base with all fields having [[no_unique_address]] attribute.
+for (auto it = base_classes.rbegin(); it != base_classes.rend(); ++it) {
+  clang::CXXRecordDecl *prev_base_decl =

Michael137 wrote:
> Michael137 wrote:
> > The main problem I still see with this is that if we have something like:
> > ```
> > struct A : C, B {
> > 
> > };
> > ```
> > 
> > then we mark `C`'s fields as empty and leave `B` as is. This still leads to 
> > the same crash later on.
> > 
> > Perhaps we should mark we could check the size of the struct and decide 
> > based on that which one is the "empty" one
> Interestingly there was a discussion on the DWARF mailing list about this 
> some time ago: 
> https://www.mail-archive.com/dwarf-discuss@lists.dwarfstd.org/msg00880.html
> 
> There might be room to changing the emitted DWARF to make it easier to 
> determine the empty structure. I will gauge opinions on this thread later 
> today
Unfortunately we cannot analyze record size, because it is always 1 for empty 
records, whether or not [[no_unique_address]] is used. However we still can 
analyze field offsets, I think. This what an updated patch does and it seems to 
handle more different cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1483
+// base with all fields having [[no_unique_address]] attribute.
+for (auto it = base_classes.rbegin(); it != base_classes.rend(); ++it) {
+  clang::CXXRecordDecl *prev_base_decl =

kpdev42 wrote:
> Michael137 wrote:
> > Michael137 wrote:
> > > The main problem I still see with this is that if we have something like:
> > > ```
> > > struct A : C, B {
> > > 
> > > };
> > > ```
> > > 
> > > then we mark `C`'s fields as empty and leave `B` as is. This still leads 
> > > to the same crash later on.
> > > 
> > > Perhaps we should mark we could check the size of the struct and decide 
> > > based on that which one is the "empty" one
> > Interestingly there was a discussion on the DWARF mailing list about this 
> > some time ago: 
> > https://www.mail-archive.com/dwarf-discuss@lists.dwarfstd.org/msg00880.html
> > 
> > There might be room to changing the emitted DWARF to make it easier to 
> > determine the empty structure. I will gauge opinions on this thread later 
> > today
> Unfortunately we cannot analyze record size, because it is always 1 for empty 
> records, whether or not [[no_unique_address]] is used. However we still can 
> analyze field offsets, I think. This what an updated patch does and it seems 
> to handle more different cases
Yes, this will help a lot, however many people use older versions of clang 
compiler and also gcc. This fix might be useful for them so far


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 512945.
kpdev42 added a comment.

Thanks for pointing out the bug @Michael137 . It seems that clang assigns 
arbitrary offsets for non_unique_address so analyzing them brings me nowhere. 
In this patch I tried assigning no_unique_address to every empty structure and 
this fixed issue you found, making the code changes much smaller and simpler. 
The lldb test suite pass for me as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/no_unique_address/Makefile
  lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
  lldb/test/API/lang/cpp/no_unique_address/main.cpp

Index: lldb/test/API/lang/cpp/no_unique_address/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/main.cpp
@@ -0,0 +1,77 @@
+struct C
+{
+ long c,d;
+};
+
+struct Q
+{
+ long h;
+};
+
+struct D
+{
+};
+
+struct B
+{
+  [[no_unique_address]] D x;
+};
+
+struct E
+{
+  [[no_unique_address]] D x;
+};
+
+struct Foo1 : B,E,C
+{
+ long a = 42,b = 52;
+} _f1;
+
+struct Foo2 : B,E
+{
+ long v = 42;
+} _f2;
+
+struct Foo3 : C,B,E
+{
+ long v = 42;
+} _f3;
+
+struct Foo4 : B,C,E,Q
+{
+ long v = 42;
+} _f4;
+
+struct Foo5 : B,C,E
+{
+ [[no_unique_address]] D x1;
+ [[no_unique_address]] D x2;
+ long v1 = 42;
+ [[no_unique_address]] D y1;
+ [[no_unique_address]] D y2;
+ long v2 = 52;
+ [[no_unique_address]] D z1;
+ [[no_unique_address]] D z2;
+} _f5;
+
+struct Foo6 : B,E
+{
+ long v1 = 42;
+ [[no_unique_address]] D y1;
+ [[no_unique_address]] D y2;
+ long v2 = 52;
+} _f6;
+
+namespace NS {
+template struct Wrap {};
+}
+
+struct Foo7 : NS::Wrap,B,E {
+  NS::Wrap w1;
+  B b1;
+  long v = 42;
+} _f7;
+
+int main() {
+  return 0; // Set breakpoint here.
+}
Index: lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/TestNoUniqueAddress.py
@@ -0,0 +1,33 @@
+"""
+Test that we correctly handle [[no_unique_address]] attribute.
+"""
+
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestInlineNamespace(TestBase):
+
+def test(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self,
+"// Set breakpoint here.", lldb.SBFileSpec("main.cpp"))
+
+
+self.expect_expr("_f3", result_type="Foo3")
+self.expect_expr("_f7", result_type="Foo7")
+
+self.expect_expr("_f1.a", result_type="long", result_value="42")
+self.expect_expr("_f1.b", result_type="long", result_value="52")
+self.expect_expr("_f2.v", result_type="long", result_value="42")
+self.expect_expr("_f3.v", result_type="long", result_value="42")
+self.expect_expr("_f4.v", result_type="long", result_value="42")
+self.expect_expr("_f5.v1", result_type="long", result_value="42")
+self.expect_expr("_f5.v2", result_type="long", result_value="52")
+self.expect_expr("_f6.v1", result_type="long", result_value="42")
+self.expect_expr("_f6.v2", result_type="long", result_value="52")
+self.expect_expr("_f7.v", result_type="long", result_value="42")
Index: lldb/test/API/lang/cpp/no_unique_address/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/no_unique_address/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1566,6 +1566,16 @@
   return qualified_name;
 }
 
+static bool IsEmptyStruct(const DWARFDIE &die) {
+  if (!die.HasChildren())
+return true;
+
+  // Empty templates are actually empty structures.
+  return llvm::all_of(die.children(), [](const DWARFDIE &die) {
+return die.Tag() == DW_TAG_template_type_parameter;
+  });
+}
+
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
const DWARFDIE &die,
@@ -1829,7 +1839,7 @@
 // has child classes or types that require the class to be created
 // for use as their decl contexts the class will be ready to accept
 // these child definitions.
-if (!die.HasChildren()) {
+if (IsEmptyStruct(die)) {
   // No children for this struct/union/class, lets finish it
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
 TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
@@ -1852,6 +1862,9 @@
 clang::RecordDecl *record_decl

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2212
 m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
-if (record_decl)
+if (record_decl) {
+  bool is_empty = true;

Michael137 wrote:
> Generally I'm not sure if attaching a `clang::NoUniqueAddressAttr` to every 
> empty field is the right approach. That goes slightly against our attempts to 
> construct an AST that's faithful to the source to avoid unpredictable 
> behaviour (which isn't always possible but for the most part we try). This 
> approach was considered in https://reviews.llvm.org/D101237 but concern was 
> raised about it affecting ABI, etc., leading to subtle issues down the line.
> 
> Based on the the discussion in https://reviews.llvm.org/D101237 it seemed to 
> me like the only two viable solutions are:
> 1. Add a `DW_AT_byte_size` of `0` to the empty field
> 2. Add a `DW_AT_no_unique_address`
> 
> AFAICT Jan tried to implement (1) but never seemed to be able to fully add 
> support for this in the ASTImporter/LLDB. Another issue I see with this is 
> that sometimes the byte-size of said field is not `0`, depending on the 
> context in which the structure is used.
> 
> I'm still leaning towards proposing a `DW_AT_no_unique_address`. Which is 
> pretty easy to implement and also reason about from LLDB's perspective. 
> @dblaikie @aprantl does that sound reasonable to you?
I think that lldb jitter relies on value of `DW_AT_member` location when/if 
empty structure address is taken, so assigning no_unique_address attribute 
shouldn't, in my opinion, affect anything. Also, as I understand, AST obtained 
from DWARF will not (and cannot) be identical to the source (e.g. because of 
optimizations)



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2219
+if (is_empty_field)
+  field->addAttr(clang::NoUniqueAddressAttr::Create(
+  m_ast.getASTContext(), clang::SourceRange()));

Michael137 wrote:
> Typically the call to `record_decl->fields()` below would worry me, because 
> if the decl `!hasLoadedFieldsFromExternalStorage()` then we'd start another 
> `ASTImport` process, which could lead to some unpredictable behaviour if we 
> are already in the middle of an import. But since 
> `CompleteTagDeclarationDefinition` sets 
> `setHasLoadedFieldsFromExternalStorage(true)` I *think* we'd be ok. Might 
> warrant a comment.
We can probably use keys of `DenseMap` in `layout_info.base_offsets` to stay 
safe, can't we?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2231
+if (is_empty)
+  record_decl->markEmpty();
+  }

Michael137 wrote:
> Why do we need to mark the parents empty here again? Wouldn't they have been 
> marked in `ParseStructureLikeDIE`?
`ParseStructureLikeDIE` marks only trivially empty records (with no children or 
with only children being template parameters). All non-trivially empty structs 
(with only children being other empty structs) are marked here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D144390: [lldb] Send QPassSignals packet on attach, and at start for remote platforms

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144390

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

[Lldb-commits] [PATCH] D144392: [lldb] Skip signal handlers for ignored signals on lldb-server's side when stepping

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.
Herald added a subscriber: asb.

@jingham could you please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144392

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

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

So what are next steps? Are we going for implementation of 
`DW_AT_no_unique_address` (which is going to be a non-standard extension) ? 
@dblaikie @aprantl @Michael137


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: clayborg, granata.enrico, labath.
kpdev42 added a project: LLDB.
Herald added subscribers: JDevlieghere, pengfei.
kpdev42 requested review of this revision.
Herald added a subscriber: lldb-commits.

On Linux some C++ and C include files reside in target specific directories, 
like /usr/include/x86_64-linux-gnu. 
Patch adds them to libclang, so LLDB jitter has more chances to compile 
expression.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110827

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
  lldb/unittests/Expression/CppModuleConfigurationTest.cpp

Index: lldb/unittests/Expression/CppModuleConfigurationTest.cpp
===
--- lldb/unittests/Expression/CppModuleConfigurationTest.cpp
+++ lldb/unittests/Expression/CppModuleConfigurationTest.cpp
@@ -63,16 +63,20 @@
   // Test the average Linux configuration.
 
   std::string usr = "/usr/include";
+  std::string usr_target = "/usr/include/x86_64-linux-gnu";
   std::string libcpp = "/usr/include/c++/v1";
-  std::vector files = {// C library
-usr + "/stdio.h",
-// C++ library
-libcpp + "/vector",
-libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  std::string libcpp_target = "/usr/include/x86_64-unknown-linux-gnu/c++/v1";
+  std::vector files = {
+  // C library
+  usr + "/stdio.h", usr_target + "/sys/cdefs.h",
+  // C++ library
+  libcpp + "/vector", libcpp + "/module.modulemap"};
+  CppModuleConfiguration config(makeFiles(files),
+llvm::Triple("x86_64-unknown-linux-gnu"));
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
-  testing::ElementsAre(libcpp, ResourceInc(), usr));
+  testing::ElementsAre(libcpp, ResourceInc(), usr, usr_target,
+   libcpp_target));
 }
 
 TEST_F(CppModuleConfigurationTest, Sysroot) {
@@ -85,7 +89,7 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceInc(), usr));
@@ -101,7 +105,7 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceInc(), usr));
@@ -112,6 +116,8 @@
 
   std::string usr = "/usr/include";
   std::string libcpp = "/home/user/llvm-build/include/c++/v1";
+  std::string libcpp_target =
+  "/home/user/llvm-build/include/x86_64-unknown-linux-gnu/c++/v1";
   std::vector files = {// C library
 usr + "/stdio.h",
 // unrelated library
@@ -119,10 +125,11 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files),
+llvm::Triple("x86_64-unknown-linux-gnu"));
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
-  testing::ElementsAre(libcpp, ResourceInc(), usr));
+  testing::ElementsAre(libcpp, ResourceInc(), usr, libcpp_target));
 }
 
 TEST_F(CppModuleConfigurationTest, Xcode) {
@@ -139,7 +146,7 @@
   libcpp + "/vector",
   libcpp + "/module.modulemap",
   };
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceInc(), usr));
@@ -154,7 +161,7 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleCon

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

In D110827#3034767 , @clayborg wrote:

> Should we be testing if these directories exist before trying to use them? 
> Might be nice to avoid compiler warnings if the compiler will emit warnings 
> or errors if these directories don't exist.

I think testing if these directories exists may be a little bit redundant 
because clang ignores non-existent include paths passed with -I

> LLDB also tests with compilers that were built, like when LLDB builds clang 
> and uses that clang and clang++ that it built to run the test suite. If we 
> had settings in LLDB that users could set, then the test suite would be able 
> to use the include files for the compiler that is being used instead of 
> always defaulting to the system headers.

Could you please clarify: "LLDB builds clang" - here you mean clang which was 
build with LLDB? And I would like to mention that starting from 
https://reviews.llvm.org/D89013 libcxx puts __config_site header to target 
specific folder


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110827

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

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

In D110827#3059120 , @clayborg wrote:

> In D110827#3042820 , @kpdev42 wrote:
>
>> In D110827#3034767 , @clayborg 
>> wrote:
>>
>>> LLDB also tests with compilers that were built, like when LLDB builds clang 
>>> and uses that clang and clang++ that it built to run the test suite. If we 
>>> had settings in LLDB that users could set, then the test suite would be 
>>> able to use the include files for the compiler that is being used instead 
>>> of always defaulting to the system headers.
>>
>> Could you please clarify: "LLDB builds clang" - here you mean clang which 
>> was build with LLDB? And I would like to mention that starting from 
>> https://reviews.llvm.org/D89013 libcxx puts __config_site header to target 
>> specific folder
>
> We often build the full clang compiler during LLDB builds and then use the 
> clang we built as the compiler when running our test suite. So anything we 
> can do to make sure what ever clang we use for building the test suite 
> binaries has all of the headers that it was built with along with any support 
> library headers (libcxx, etc) that would be great.

That's fine, but patch is about *libclang* seeing target specific headers, not 
clang frontend. While clang frontend obtains system header paths through 
Toolchain-derived classes (lib/Driver/Toolchain), one has to *explicitly* set 
those paths when calling libclang. That's exactly what lldb does in 
CppModuleConfiguration.cpp when evaluating expression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110827

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

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110827

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

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 388118.
kpdev42 added a comment.

Addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110827

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
  lldb/unittests/Expression/CppModuleConfigurationTest.cpp

Index: lldb/unittests/Expression/CppModuleConfigurationTest.cpp
===
--- lldb/unittests/Expression/CppModuleConfigurationTest.cpp
+++ lldb/unittests/Expression/CppModuleConfigurationTest.cpp
@@ -58,7 +58,6 @@
   return std::string(resource_dir);
 }
 
-
 TEST_F(CppModuleConfigurationTest, Linux) {
   // Test the average Linux configuration.
 
@@ -69,12 +68,32 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceInc(), usr));
 }
 
+TEST_F(CppModuleConfigurationTest, LinuxTargetSpecificInclude) {
+  // Test the average Linux configuration.
+
+  std::string usr = "/usr/include";
+  std::string usr_target = "/usr/include/x86_64-linux-gnu";
+  std::string libcpp = "/usr/include/c++/v1";
+  std::string libcpp_target = "/usr/include/x86_64-unknown-linux-gnu/c++/v1";
+  std::vector files = {
+  // C library
+  usr + "/stdio.h", usr_target + "/sys/cdefs.h",
+  // C++ library
+  libcpp + "/vector", libcpp + "/module.modulemap"};
+  CppModuleConfiguration config(makeFiles(files),
+llvm::Triple("x86_64-unknown-linux-gnu"));
+  EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
+  EXPECT_THAT(config.GetIncludeDirs(),
+  testing::ElementsAre(libcpp, ResourceInc(), usr, usr_target,
+   libcpp_target));
+}
+
 TEST_F(CppModuleConfigurationTest, Sysroot) {
   // Test that having a sysroot for the whole system works fine.
 
@@ -85,7 +104,7 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceInc(), usr));
@@ -101,7 +120,7 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceInc(), usr));
@@ -119,12 +138,33 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceInc(), usr));
 }
 
+TEST_F(CppModuleConfigurationTest, UnrelatedLibraryWithTargetSpecificInclude) {
+  // Test that having an unrelated library in /usr/include doesn't break.
+
+  std::string usr = "/usr/include";
+  std::string libcpp = "/home/user/llvm-build/include/c++/v1";
+  std::string libcpp_target =
+  "/home/user/llvm-build/include/x86_64-unknown-linux-gnu/c++/v1";
+  std::vector files = {// C library
+usr + "/stdio.h",
+// unrelated library
+usr + "/boost/vector",
+// C++ library
+libcpp + "/vector",
+libcpp + "/module.modulemap"};
+  CppModuleConfiguration config(makeFiles(files),
+llvm::Triple("x86_64-unknown-linux-gnu"));
+  EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
+  EXPECT_THAT(config.GetIncludeDirs(),
+  testing::ElementsAre(libcpp, ResourceInc(), usr, libcpp_target));

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1aab5e653d2c: [LLDB] Provide target specific directories to 
libclang (authored by kpdev42).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110827

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
  lldb/unittests/Expression/CppModuleConfigurationTest.cpp

Index: lldb/unittests/Expression/CppModuleConfigurationTest.cpp
===
--- lldb/unittests/Expression/CppModuleConfigurationTest.cpp
+++ lldb/unittests/Expression/CppModuleConfigurationTest.cpp
@@ -58,7 +58,6 @@
   return std::string(resource_dir);
 }
 
-
 TEST_F(CppModuleConfigurationTest, Linux) {
   // Test the average Linux configuration.
 
@@ -69,12 +68,32 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceInc(), usr));
 }
 
+TEST_F(CppModuleConfigurationTest, LinuxTargetSpecificInclude) {
+  // Test the average Linux configuration.
+
+  std::string usr = "/usr/include";
+  std::string usr_target = "/usr/include/x86_64-linux-gnu";
+  std::string libcpp = "/usr/include/c++/v1";
+  std::string libcpp_target = "/usr/include/x86_64-unknown-linux-gnu/c++/v1";
+  std::vector files = {
+  // C library
+  usr + "/stdio.h", usr_target + "/sys/cdefs.h",
+  // C++ library
+  libcpp + "/vector", libcpp + "/module.modulemap"};
+  CppModuleConfiguration config(makeFiles(files),
+llvm::Triple("x86_64-unknown-linux-gnu"));
+  EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
+  EXPECT_THAT(config.GetIncludeDirs(),
+  testing::ElementsAre(libcpp, ResourceInc(), usr, usr_target,
+   libcpp_target));
+}
+
 TEST_F(CppModuleConfigurationTest, Sysroot) {
   // Test that having a sysroot for the whole system works fine.
 
@@ -85,7 +104,7 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceInc(), usr));
@@ -101,7 +120,7 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceInc(), usr));
@@ -119,12 +138,33 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
   testing::ElementsAre(libcpp, ResourceInc(), usr));
 }
 
+TEST_F(CppModuleConfigurationTest, UnrelatedLibraryWithTargetSpecificInclude) {
+  // Test that having an unrelated library in /usr/include doesn't break.
+
+  std::string usr = "/usr/include";
+  std::string libcpp = "/home/user/llvm-build/include/c++/v1";
+  std::string libcpp_target =
+  "/home/user/llvm-build/include/x86_64-unknown-linux-gnu/c++/v1";
+  std::vector files = {// C library
+usr + "/stdio.h",
+// unrelated library
+usr + "/boost/vector",
+// C++ library
+libcpp + "/vector",
+libcpp + "/module.modulemap"};
+  CppModuleConfiguration config(makeFiles(files),
+llvm::Triple("x86_64-unknown-linux-gnu"));
+  EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
+  EXPECT_THAT(config.GetIncludeD

[Lldb-commits] [PATCH] D130062: [lldb][DWARF5] Enable macro evaluation

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130062

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

[Lldb-commits] [PATCH] D130062: [lldb][DWARF5] Enable macro evaluation

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa0fb69d17b4d: [lldb][DWARF5] Enable macro evaluation 
(authored by kpdev42).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130062

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugMacro.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/API/commands/expression/macros/Makefile
  lldb/test/API/commands/expression/macros/TestMacros.py
  lldb/test/API/lang/c/macro/Makefile
  lldb/test/API/lang/c/macro/TestMacro.py
  lldb/test/API/lang/c/macro/main.c

Index: lldb/test/API/lang/c/macro/main.c
===
--- /dev/null
+++ lldb/test/API/lang/c/macro/main.c
@@ -0,0 +1,7 @@
+#define DM 10
+#define DF(x) (42 + (x))
+
+int main (int argc, char const *argv[])
+{
+return 0;  Set break point at this line.
+}
Index: lldb/test/API/lang/c/macro/TestMacro.py
===
--- /dev/null
+++ lldb/test/API/lang/c/macro/TestMacro.py
@@ -0,0 +1,31 @@
+"""Tests lldb macro evaluation."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MacroTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Find the line number to break inside main().
+self.line = line_number('main.c', '// Set break point at this line.')
+
+def test(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+# Break inside the main.
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect("expr DM + DF(10)", VARIABLES_DISPLAYED_CORRECTLY,
+substrs=['int', '62'])
+
Index: lldb/test/API/lang/c/macro/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/c/macro/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -fdebug-macro
+
+include Makefile.rules
Index: lldb/test/API/commands/expression/macros/TestMacros.py
===
--- lldb/test/API/commands/expression/macros/TestMacros.py
+++ lldb/test/API/commands/expression/macros/TestMacros.py
@@ -8,12 +8,6 @@
 
 class TestMacros(TestBase):
 
-@expectedFailureAll(
-compiler="clang",
-bugnumber="clang does not emit .debug_macro[.dwo] sections.")
-@expectedFailureAll(
-debug_info="dwo",
-bugnumber="GCC produces multiple .debug_macro.dwo sections and the spec is unclear as to what it means")
 @expectedFailureAll(
 hostoslist=["windows"],
 compiler="gcc",
Index: lldb/test/API/commands/expression/macros/Makefile
===
--- lldb/test/API/commands/expression/macros/Makefile
+++ lldb/test/API/commands/expression/macros/Makefile
@@ -5,5 +5,6 @@
 # GCC produces incorrect .debug_macro section when "-include" option is used:
 # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93075.
 NO_TEST_COMMON_H := 1
+CFLAGS_EXTRAS := -fdebug-macro
 
 include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -55,6 +55,8 @@
 class SymbolFileDWARFDwo;
 class SymbolFileDWARFDwp;
 
+class DWARFStrOffsetsInfo;
+
 #define DIE_IS_BEING_PARSED ((lldb_private::Type *)1)
 
 class SymbolFileDWARF : public lldb_private::SymbolFileCommon,
@@ -246,7 +248,9 @@
 
   bool Supports_DW_AT_APPLE_objc_complete_type(DWARFUnit *cu);
 
-  lldb_private::DebugMacrosSP ParseDebugMacros(lldb::offset_t *offset);
+  lldb_private::DebugMacrosSP
+  ParseDebugMacros(lldb::offset_t *offset,
+   const DWARFStrOffsetsInfo *str_offsets_info);
 
   static DWARFDIE GetParentSymbolContextDIE(const DWARFDIE &die);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1194,7 +1194,8 @@
 }
 
 lldb_private::DebugMacrosSP
-SymbolFileDWARF::ParseDebugMacros(lldb::offset_t *offset) {
+SymbolFileDWARF::ParseDebugMacros(lldb::offset_t *offset,
+  

[Lldb-commits] [PATCH] D130062: [lldb][DWARF5] Enable macro evaluation

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

In D130062#3789671 , @aprantl wrote:

> Out of curiosity — did you get an email notification from the bot?

Hi, yes, I will add a priority to these notifications to react faster, sorry


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130062

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