[Lldb-commits] [PATCH] D94063: lldb: Add support for DW_AT_ranges on DW_TAG_subprograms

[David Blaikie via Phabricator via lldb-commits]

dblaikie created this revision.
dblaikie added a reviewer: labath.
dblaikie requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.
Herald added a project: LLDB.

gcc already produces debug info with this form
-freorder-block-and-partition
clang produces this sort of thing with -fbasic-block-sections and with a
coming-soon tweak to use ranges in DWARFv5 where they can allow greater
reuse of debug_addr than the low/high_pc forms.

This fixes the case of breaking on a function name, but leaves broken
printing a variable - a follow-up commit will add that and improve the
test case to match.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94063

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/test/Shell/SymbolFile/DWARF/Inputs/subprogram_ranges.s
  lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test

Index: lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
@@ -0,0 +1,29 @@
+# UNSUPPORTED: system-windows
+# RUN: %clang_host -g -O0 %S/Inputs/subprogram_ranges.s -o %t.out
+# RUN: not %lldb -b -s %s %t.out 2>&1 | FileCheck %s
+
+# Test breaking on symbols and printing variables when a DW_TAG_subprogram uses
+# DW_AT_ranges instead of DW_AT_low_pc/DW_AT_high_pc.  While the assembly here
+# is a bit unrealistic - it's a single-entry range using DWARFv4 which isn't
+# useful for anything (a single-entry range with DWARFv5 can reduce address
+# relocations, and multi-entry ranges can be used for function sections), but
+# it's the simplest thing to test. If anyone's updating this test at some
+# point, feel free to replace it with another equivalent test if it's
+# especially useful, but don't dismiss it as pointless just because it's a bit
+# weird.
+
+b main
+# CHECK: (lldb) b main
+# CHECK-NEXT: Breakpoint 1: where = subprogram_ranges.test.tmp.out`main + 6 at main.c:2:7
+
+run
+# Stopping inside of the stop hook range
+# CHECK: (lldb) run
+
+print var
+# Check that local variable names can be looked up
+# FIXME: This should be: (int) $0 = {{.*}}
+# CHECK: (lldb) print var
+# CHECK-NEXT: error: :1:1: use of undeclared identifier 'var'
+
+q
Index: lldb/test/Shell/SymbolFile/DWARF/Inputs/subprogram_ranges.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/Inputs/subprogram_ranges.s
@@ -0,0 +1,159 @@
+	.text
+	.file	"main.c"
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.file	1 "/usr/local/google/home/blaikie/dev/scratch" "main.c"
+	.loc	1 1 0   # main.c:1:0
+	.cfi_startproc
+# %bb.0:# %entry
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	xorl	%eax, %eax
+.Ltmp0:
+	.loc	1 2 7 prologue_end  # main.c:2:7
+	movl	$3, -4(%rbp)
+	.loc	1 3 1   # main.c:3:1
+	popq	%rbp
+	.cfi_def_cfa %rsp, 8
+	retq
+.Ltmp1:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+# -- End function
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	17  # DW_TAG_compile_unit
+	.byte	1   # DW_CHILDREN_yes
+	.byte	37  # DW_AT_producer
+	.byte	14  # DW_FORM_strp
+	.byte	19  # DW_AT_language
+	.byte	5   # DW_FORM_data2
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	16  # DW_AT_stmt_list
+	.byte	23  # DW_FORM_sec_offset
+	.byte	27  # DW_AT_comp_dir
+	.byte	14  # DW_FORM_strp
+	.byte	17  # DW_AT_low_pc
+	.byte	1   # DW_FORM_addr
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	2   # Abbreviation Code
+	.byte	46  # DW_TAG_subprogram
+	.byte	1   # DW_CHILDREN_yes
+	.byte	85  # DW_AT_ranges
+	.byte	23  # DW_FORM_sec_offset
+	.byte	64  # DW_AT_frame_base
+	.byte	24  # DW_FORM_exprloc
+	.byte	3   # DW_AT_name

[Lldb-commits] [PATCH] D94064: lldb: Add support for printing variables with DW_AT_ranges on DW_TAG_subprograms

[David Blaikie via Phabricator via lldb-commits]

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

Finishing out the support (to the best of my knowledge/based on current
testing running the whole check-lldb with a clang forcibly using
DW_AT_ranges on all DW_TAG_subprograms) for this feature.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94064

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test


Index: lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
===
--- lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
+++ lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
@@ -1,6 +1,6 @@
 # UNSUPPORTED: system-windows
 # RUN: %clang_host -g -O0 %S/Inputs/subprogram_ranges.s -o %t.out
-# RUN: not %lldb -b -s %s %t.out 2>&1 | FileCheck %s
+# RUN: %lldb -b -s %s %t.out 2>&1 | FileCheck %s
 
 # Test breaking on symbols and printing variables when a DW_TAG_subprogram uses
 # DW_AT_ranges instead of DW_AT_low_pc/DW_AT_high_pc.  While the assembly here
@@ -22,8 +22,7 @@
 
 print var
 # Check that local variable names can be looked up
-# FIXME: This should be: (int) $0 = {{.*}}
 # CHECK: (lldb) print var
-# CHECK-NEXT: error: :1:1: use of undeclared identifier 
'var'
+# CHECK-NEXT: (int) $0 = {{.*}}
 
 q
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3040,8 +3040,13 @@
 if (sc.function) {
   DWARFDIE function_die = GetDIE(sc.function->GetID());
 
-  const dw_addr_t func_lo_pc = function_die.GetAttributeValueAsAddress(
+  dw_addr_t func_lo_pc = function_die.GetAttributeValueAsAddress(
   DW_AT_low_pc, LLDB_INVALID_ADDRESS);
+  DWARFFormValue form_value;
+  if (func_lo_pc == LLDB_INVALID_ADDRESS &&
+  function_die.GetDIE()->GetAttributeValue(function_die.GetCU(),
+   DW_AT_ranges, form_value))
+func_lo_pc = 0;
   if (func_lo_pc != LLDB_INVALID_ADDRESS) {
 const size_t num_variables = ParseVariables(
 sc, function_die.GetFirstChild(), func_lo_pc, true, true);


Index: lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
===
--- lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
+++ lldb/test/Shell/SymbolFile/DWARF/subprogram_ranges.test
@@ -1,6 +1,6 @@
 # UNSUPPORTED: system-windows
 # RUN: %clang_host -g -O0 %S/Inputs/subprogram_ranges.s -o %t.out
-# RUN: not %lldb -b -s %s %t.out 2>&1 | FileCheck %s
+# RUN: %lldb -b -s %s %t.out 2>&1 | FileCheck %s
 
 # Test breaking on symbols and printing variables when a DW_TAG_subprogram uses
 # DW_AT_ranges instead of DW_AT_low_pc/DW_AT_high_pc.  While the assembly here
@@ -22,8 +22,7 @@
 
 print var
 # Check that local variable names can be looked up
-# FIXME: This should be: (int) $0 = {{.*}}
 # CHECK: (lldb) print var
-# CHECK-NEXT: error: :1:1: use of undeclared identifier 'var'
+# CHECK-NEXT: (int) $0 = {{.*}}
 
 q
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3040,8 +3040,13 @@
 if (sc.function) {
   DWARFDIE function_die = GetDIE(sc.function->GetID());
 
-  const dw_addr_t func_lo_pc = function_die.GetAttributeValueAsAddress(
+  dw_addr_t func_lo_pc = function_die.GetAttributeValueAsAddress(
   DW_AT_low_pc, LLDB_INVALID_ADDRESS);
+  DWARFFormValue form_value;
+  if (func_lo_pc == LLDB_INVALID_ADDRESS &&
+  function_die.GetDIE()->GetAttributeValue(function_die.GetCU(),
+   DW_AT_ranges, form_value))
+func_lo_pc = 0;
   if (func_lo_pc != LLDB_INVALID_ADDRESS) {
 const size_t num_variables = ParseVariables(
 sc, function_die.GetFirstChild(), func_lo_pc, true, true);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D94077: Support unscoped enumeration members in the expression evaluator.

[Andy Yankovsky via Phabricator via lldb-commits]

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

Add unscoped enumeration members to the "globals" manual dwarf index. This
effectively makes them discoverable as global variables (which they
essentially are).

Before expression evaluator failed to lookup enumerators unless the
enumeration type has been already completed.

Consider the example:

  enum MyEnum { eFoo = 1 };
  MyEnum my_enum = eFoo;

(lldb) p eFoo
error: :1:1: use of undeclared identifier 'eFoo'
eFirst
^
(lldb) p my_enum + eFoo
(int) $0 = 2

With this patch all unscoped enumerators can be looked up same as the global
variables and the expression evaluation works as expected.
`SBTarget::FindGlobalVariables()` now returns unscoped enumerators as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94077

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
  lldb/test/API/lang/cpp/enum_types/main.cpp
  lldb/test/API/python_api/target/globals/Makefile
  lldb/test/API/python_api/target/globals/TestTargetGlobals.py
  lldb/test/API/python_api/target/globals/main.cpp

Index: lldb/test/API/python_api/target/globals/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/target/globals/main.cpp
@@ -0,0 +1,14 @@
+enum MyEnum {
+  eFirst,
+};
+MyEnum my_enum;
+
+enum class MyScopedEnum {
+  eFoo = 1,
+  eBar,
+};
+MyScopedEnum my_scoped_enum;
+
+int eFoo = 2;
+
+int main() {}
Index: lldb/test/API/python_api/target/globals/TestTargetGlobals.py
===
--- /dev/null
+++ lldb/test/API/python_api/target/globals/TestTargetGlobals.py
@@ -0,0 +1,43 @@
+"""
+Test SBTarget::FindGlobalVariables API.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TargetAPITestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(['pyapi'])
+def test_find_global_variables(self):
+"""Exercise SBTarget.FindGlobalVariables() API."""
+self.build()
+
+# Don't need to launch a process, since we're only interested in
+# looking up global variables.
+target = self.dbg.CreateTarget(self.getBuildArtifact())
+
+def test_global_var(query, name, type_name, value):
+value_list = target.FindGlobalVariables(query, 1)
+self.assertEqual(value_list.GetSize(), 1)
+var = value_list.GetValueAtIndex(0)
+self.DebugSBValue(var)
+self.assertTrue(var)
+self.assertEqual(var.GetName(), name)
+self.assertEqual(var.GetTypeName(), type_name)
+self.assertEqual(var.GetValue(), value)
+
+test_global_var(
+"eFirst",
+"eFirst", "MyEnum", "eFirst")
+
+# Global variable eFoo is looked up fine, since scoped enumeration
+# members are not available as constants in the surrounding scope.
+test_global_var(
+"eFoo",
+"::eFoo", "int", "2")
+
+# eBar is not available since it's a member of a scoped enumeration.
+value_list = target.FindGlobalVariables("eBar", 1)
+self.assertEqual(value_list.GetSize(), 0)
Index: lldb/test/API/python_api/target/globals/Makefile
===
--- /dev/null
+++ lldb/test/API/python_api/target/globals/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/lang/cpp/enum_types/main.cpp
===
--- lldb/test/API/lang/cpp/enum_types/main.cpp
+++ lldb/test/API/lang/cpp/enum_types/main.cpp
@@ -25,4 +25,14 @@
 DEFINE_UNSIGNED_ENUM(ull, unsigned long long)
 DEFINE_SIGNED_ENUM(ll, signed long long)
 
+enum MyEnum {
+  eFoo = 1,
+};
+MyEnum my_enum = eFoo;
+
+enum class MyScopedEnum {
+  eBar = 1,
+};
+MyScopedEnum my_scoped_enum = MyScopedEnum::eBar;
+
 int main(int argc, char const *argv[]) { return 0; }
Index: lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
===
--- lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
+++ lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
@@ -1,9 +1,7 @@
 """Look up enum type information and check for correct display."""
 
-import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
-import lldbsuite.test.lldbutil as lldbutil
 
 
 class CPP11EnumTypesTestCase(TestBase):
@@ -51,3 +49,10 @@
 self.check_enum("l")
 self.check_enum("ull")
 self.check_enum("ll")
+
+self.expect_expr("

[Lldb-commits] [PATCH] D94077: Support unscoped enumeration members in the expression evaluator.

[Andy Yankovsky via Phabricator via lldb-commits]

werat updated this revision to Diff 314558.
werat added a comment.
Herald added a subscriber: JDevlieghere.

Fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94077

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
  lldb/test/API/lang/cpp/enum_types/main.cpp
  lldb/test/API/python_api/target/globals/Makefile
  lldb/test/API/python_api/target/globals/TestTargetGlobals.py
  lldb/test/API/python_api/target/globals/main.cpp

Index: lldb/test/API/python_api/target/globals/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/target/globals/main.cpp
@@ -0,0 +1,14 @@
+enum MyEnum {
+  eFirst,
+};
+MyEnum my_enum;
+
+enum class MyScopedEnum {
+  eFoo = 1,
+  eBar,
+};
+MyScopedEnum my_scoped_enum;
+
+int eFoo = 2;
+
+int main() {}
Index: lldb/test/API/python_api/target/globals/TestTargetGlobals.py
===
--- /dev/null
+++ lldb/test/API/python_api/target/globals/TestTargetGlobals.py
@@ -0,0 +1,39 @@
+"""
+Test SBTarget::FindGlobalVariables API.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TargetAPITestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(['pyapi'])
+def test_find_global_variables(self):
+"""Exercise SBTarget.FindGlobalVariables() API."""
+self.build()
+
+# Don't need to launch a process, since we're only interested in
+# looking up global variables.
+target = self.dbg.CreateTarget(self.getBuildArtifact())
+
+def test_global_var(query, name, type_name, value):
+value_list = target.FindGlobalVariables(query, 1)
+self.assertEqual(value_list.GetSize(), 1)
+var = value_list.GetValueAtIndex(0)
+self.DebugSBValue(var)
+self.assertTrue(var)
+self.assertEqual(var.GetName(), name)
+self.assertEqual(var.GetTypeName(), type_name)
+self.assertEqual(var.GetValue(), value)
+
+test_global_var("eFirst", "eFirst", "MyEnum", "eFirst")
+
+# Global variable eFoo is looked up fine, since scoped enumeration
+# members are not available as constants in the surrounding scope.
+test_global_var("eFoo", "::eFoo", "int", "2")
+
+# eBar is not available since it's a member of a scoped enumeration.
+value_list = target.FindGlobalVariables("eBar", 1)
+self.assertEqual(value_list.GetSize(), 0)
Index: lldb/test/API/python_api/target/globals/Makefile
===
--- /dev/null
+++ lldb/test/API/python_api/target/globals/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/lang/cpp/enum_types/main.cpp
===
--- lldb/test/API/lang/cpp/enum_types/main.cpp
+++ lldb/test/API/lang/cpp/enum_types/main.cpp
@@ -25,4 +25,14 @@
 DEFINE_UNSIGNED_ENUM(ull, unsigned long long)
 DEFINE_SIGNED_ENUM(ll, signed long long)
 
+enum MyEnum {
+  eFoo = 1,
+};
+MyEnum my_enum = eFoo;
+
+enum class MyScopedEnum {
+  eBar = 1,
+};
+MyScopedEnum my_scoped_enum = MyScopedEnum::eBar;
+
 int main(int argc, char const *argv[]) { return 0; }
Index: lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
===
--- lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
+++ lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
@@ -1,9 +1,7 @@
 """Look up enum type information and check for correct display."""
 
-import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
-import lldbsuite.test.lldbutil as lldbutil
 
 
 class CPP11EnumTypesTestCase(TestBase):
@@ -51,3 +49,10 @@
 self.check_enum("l")
 self.check_enum("ull")
 self.check_enum("ll")
+
+self.expect_expr("eFoo", result_type="MyEnum", result_value="eFoo")
+self.expect_expr("MyEnum::eFoo", result_type="MyEnum", result_value="eFoo")
+self.expect_expr("my_enum + eFoo + MyEnum::eFoo", result_value="3")
+
+self.expect("p eBar", error=True,
+substrs=["use of undeclared identifier 'eBar'"])
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2115,7 +2115,7 @@
   sc.module_sp = m_objfile_sp->GetModule();
 assert(sc.module_sp);
 
-if (die.Tag() != DW_TAG_variable

[Lldb-commits] [PATCH] D94084: [lldb][ARM/AArch64] Update disasm flags to latest v8.7a ISA

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added subscribers: danielkiss, kristof.beyls.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Add optional memory tagging extension on AArch64.

Use isAArch64() instead of listing the AArch64 triples,
which fixes us not recognising aarch64_be.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94084

Files:
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp


Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -1056,7 +1056,7 @@
   thumb_arch_name.erase(0, 3);
   thumb_arch_name.insert(0, "thumb");
 } else {
-  thumb_arch_name = "thumbv8.2a";
+  thumb_arch_name = "thumbv8.7a";
 }
 thumb_arch.GetTriple().setArchName(llvm::StringRef(thumb_arch_name));
   }
@@ -1068,7 +1068,7 @@
   // specified)
   if (triple.getArch() == llvm::Triple::arm &&
   triple.getSubArch() == llvm::Triple::NoSubArch)
-triple.setArchName("armv8.2a");
+triple.setArchName("armv8.7a");
 
   std::string features_str = "";
   const char *triple_str = triple.getTriple().c_str();
@@ -1137,16 +1137,13 @@
   features_str += "+dspr2,";
   }
 
-  // If any AArch64 variant, enable the ARMv8.5 ISA with SVE extensions so we
-  // can disassemble newer instructions.
-  if (triple.getArch() == llvm::Triple::aarch64 || 
-  triple.getArch() == llvm::Triple::aarch64_32)
-features_str += "+v8.5a,+sve2";
+  // If any AArch64 variant, enable latest ISA with any optional
+  // extensions like SVE.
+  if (triple.isAArch64()) {
+features_str += "+v8.7a,+sve2,+mte";
 
-  if ((triple.getArch() == llvm::Triple::aarch64 ||
-   triple.getArch() == llvm::Triple::aarch64_32)
-  && triple.getVendor() == llvm::Triple::Apple) {
-cpu = "apple-latest";
+if (triple.getVendor() == llvm::Triple::Apple)
+  cpu = "apple-latest";
   }
 
   // We use m_disasm_up.get() to tell whether we are valid or not, so if this


Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -1056,7 +1056,7 @@
   thumb_arch_name.erase(0, 3);
   thumb_arch_name.insert(0, "thumb");
 } else {
-  thumb_arch_name = "thumbv8.2a";
+  thumb_arch_name = "thumbv8.7a";
 }
 thumb_arch.GetTriple().setArchName(llvm::StringRef(thumb_arch_name));
   }
@@ -1068,7 +1068,7 @@
   // specified)
   if (triple.getArch() == llvm::Triple::arm &&
   triple.getSubArch() == llvm::Triple::NoSubArch)
-triple.setArchName("armv8.2a");
+triple.setArchName("armv8.7a");
 
   std::string features_str = "";
   const char *triple_str = triple.getTriple().c_str();
@@ -1137,16 +1137,13 @@
   features_str += "+dspr2,";
   }
 
-  // If any AArch64 variant, enable the ARMv8.5 ISA with SVE extensions so we
-  // can disassemble newer instructions.
-  if (triple.getArch() == llvm::Triple::aarch64 || 
-  triple.getArch() == llvm::Triple::aarch64_32)
-features_str += "+v8.5a,+sve2";
+  // If any AArch64 variant, enable latest ISA with any optional
+  // extensions like SVE.
+  if (triple.isAArch64()) {
+features_str += "+v8.7a,+sve2,+mte";
 
-  if ((triple.getArch() == llvm::Triple::aarch64 ||
-   triple.getArch() == llvm::Triple::aarch64_32)
-  && triple.getVendor() == llvm::Triple::Apple) {
-cpu = "apple-latest";
+if (triple.getVendor() == llvm::Triple::Apple)
+  cpu = "apple-latest";
   }
 
   // We use m_disasm_up.get() to tell whether we are valid or not, so if this
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92643: [lldb] Lookup static const members in FindGlobalVariables

[Andy Yankovsky via Phabricator via lldb-commits]

werat added a comment.

@labath @jankratochvil ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92643

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

[Lldb-commits] [PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

[Balázs Kéri via Phabricator via lldb-commits]

balazske added a comment.

The problem is reproduced and fixed in D94067 
. It is caused by import of default template 
arguments indirectly, because that import causes lot of other things to be 
imported. And the import of default arguments happens in a incomplete state, 
when the "templated" CXXRecordDecl does exist but has no described template set 
yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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

[Lldb-commits] [PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

[Balázs Kéri via Phabricator via lldb-commits]

balazske added a comment.

There are still problems related to import type argument default values: If 
there are forward declarations of the same template, the "inheritedness" of 
these arguments (in AST) is not set correctly and the default arguments can 
appear at more places instead of at only one (that the others reference to).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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

[Lldb-commits] [PATCH] D93926: [lldb] Don't remove the lldb.debugger var after exiting the interpreter

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Your argument is correct because the interactive script interpreter always 
belongs to a single debugger. That being said, I don't like this for a few 
reasons:

- LLDB supports multiple debuggers and the relationship between the script 
interpreter and the debugger are an implementation detail. This shouldn't be 
something the user has to think or know about.
- The convenience variables were introduced with a very specific goad and 
empirically only a small subset of the users actually understands their purpose 
and limitations. It's already relatively complex and adding an exception for 
the debugger only makes things harder to explain and understand.
- There's always a way to get the debugger in a non-interactive context (e.g. 
`__lldb_init_module` or from the current `frame`).

FWIW this has always been LLDB's behavior. You're probably seeing it now 
because the convenience variables are initialized to `None` (instead of default 
constructed) which results in a Python exception when you try to call a method 
on them. We got some reports of this internally and in most cases the 
convenience variables were misused and potential bug got fixed. There were a 
few innocent uses, such as in the `crashlog.py` script which I've since fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93926

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 updated this revision to Diff 314641.
augusto2112 added a comment.
Herald added a subscriber: dang.

Adds support to 'vAttachOrWait', as well as the 'waitfor-interval' and 
'waitfor-duration' flags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/StringExtractor.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/Options.td
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Utility/StringExtractor.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
@@ -0,0 +1,69 @@
+
+import os
+from time import sleep
+
+import gdbremote_testcase
+import lldbgdbserverutils
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestGdbRemoteAttachWait(gdbremote_testcase.GdbRemoteTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_attach_with_vAttachWait(self):
+exe = '%s_%d' % (self.testMethodName, os.getpid())
+self.build(dictionary={'EXE': exe})
+self.set_inferior_startup_attach_manually()
+
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+
+self.add_no_ack_remote_stream()
+self.test_sequence.add_log_lines([
+# Do the attach.
+"read packet: $vAttachWait;{}#00".format(lldbgdbserverutils.gdbremote_hex_encode_string(exe)),
+], True)
+# Run the stream until attachWait.
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+
+# Sleep so we're sure that the inferior is launched after we ask for the attach.
+sleep(1)
+
+# Launch the inferior.
+inferior = self.launch_process_for_attach(
+inferior_args=["sleep:60"],
+exe_path=self.getBuildArtifact(exe))
+self.assertIsNotNone(inferior)
+self.assertTrue(inferior.pid > 0)
+self.assertTrue(
+lldbgdbserverutils.process_is_running(
+inferior.pid, True))
+
+# Make sure the attach succeeded.
+self.test_sequence.add_log_lines([
+{"direction": "send",
+ "regex": r"^\$T([0-9a-fA-F]{2})[^#]*#[0-9a-fA-F]{2}$",
+ "capture": {1: "stop_signal_hex"}},
+], True)
+self.add_process_info_collection_packets()
+
+
+# Run the stream sending the response..
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+
+# Gather process info response.
+process_info = self.parse_process_info_response(context)
+self.assertIsNotNone(process_info)
+
+# Ensure the process id matches what we expected.
+pid_text = process_info.get('pid', None)
+self.assertIsNotNone(pid_text)
+reported_pid = int(pid_text, base=16)
+self.assertEqual(reported_pid, inferior.pid)
+
Index: lldb/source/Utility/StringExtractor.cpp
===
--- lldb/source/Utility/StringExtractor.cpp
+++ lldb/source/Utility/StringExtractor.cpp
@@ -297,11 +297,11 @@
   return bytes_extracted;
 }
 
-size_t StringExtractor::GetHexByteString(std::string &str) {
+size_t StringExtractor::GetHexByteString(std::string &str, bool set_eof_on_fail) {
   str.clear();
   str.reserve(GetBytesLeft() / 2);
   char ch;
-  while ((ch = GetHexU8()) != '\0')
+  while ((ch = GetHexU8(0, set_eof_on_fail)) != '\0')
 str.append(1, ch);
   return str.size();
 }
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1184,11 +1184,24 @@
 }
   } else
 packet.PutCString("vAttachName");
+
   packet.PutChar(';');
   packet.PutBytesAsRawHex8(process_name, strlen(process_name),
endian::InlHostByteOrder(),
endian::InlHostByteOrder());
 
+  if (attach_info.HasWaitForLaunchInterval()) {
+packet.PutChar(';');
+packet.PutChar('I');
+packet.PutHex32(attach_info.GetWaitForLaunchInterval());
+  }
+
+  if (attach_info.HasWaitForLaunchDuration()) {
+packet.PutChar(';');
+packet.PutChar('d

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 marked an inline comment as done.
augusto2112 added a comment.

@clayborg I've added support for the 'waitfor-interval' and 'waitfor-duration' 
flags. Yesterday I thought they existed in macOS, but now I'm not so sure, as I 
couldn't find them on "Options.td". They were added in 2009, so maybe they did 
exist but were dropped later. Or I just didn't look at the right place.

A couple of questions:

- About the default polling interval, I've set it to 1ms for now, but I found 
that (in my system) this keeps a core at 100%. 10ms keeps still keeps it at 
around 80%, and 100ms keeps it at around 35%. Maybe 100ms is a good balance?

- On "Options.td", besides the "process attach" command, there's also a 
"platform process attach". I haven't touched it since I'm not familiar with the 
platform command. Should I add these flags to the platform counterpart as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

Question: why does lldb queries if vAttachOrWait is supported, but not 
vAttachWait? Does it make sense to keep this query? Or should I remove it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [lldb] c82beab - [lldb] Add timers to Reproducer::Keep and Reproducer::Discard

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-01-05T09:54:31-08:00
New Revision: c82beaba319657d93a62523a65f8969aad9ecab1

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

LOG: [lldb] Add timers to Reproducer::Keep and Reproducer::Discard

Added: 


Modified: 
lldb/source/Utility/Reproducer.cpp

Removed: 




diff  --git a/lldb/source/Utility/Reproducer.cpp 
b/lldb/source/Utility/Reproducer.cpp
index f238a2fabc39..f302cce4436f 100644
--- a/lldb/source/Utility/Reproducer.cpp
+++ b/lldb/source/Utility/Reproducer.cpp
@@ -9,6 +9,7 @@
 #include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/ReproducerProvider.h"
+#include "lldb/Utility/Timer.h"
 
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Threading.h"
@@ -194,6 +195,7 @@ ProviderBase 
*Generator::Register(std::unique_ptr provider) {
 }
 
 void Generator::Keep() {
+  LLDB_SCOPED_TIMER();
   assert(!m_done);
   m_done = true;
 
@@ -204,6 +206,7 @@ void Generator::Keep() {
 }
 
 void Generator::Discard() {
+  LLDB_SCOPED_TIMER();
   assert(!m_done);
   m_done = true;
 



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

[Lldb-commits] [PATCH] D93649: [lldb/Lua] add support for Lua function breakpoint

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/bindings/lua/lua-wrapper.swig:25-29
+   auto extra_args = [&]() -> llvm::Optional {
+  if (extra_args_impl == nullptr)
+ return {};
+  return lldb::SBStructuredData(extra_args_impl);
+   } ();

I don't think you need the lambda. I used `unique_ptr` but `llvm:Optional` 
would work as well. 



Comment at: 
lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_function_callback.test:15
+r
+# CHECK: 
+breakpoint command add -s lua -o "abc(frame, bp_loc, ...)"

Can we unpack the SBStructuredData and check for `foo` or `123` in the output?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93649

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

[Lldb-commits] [PATCH] D94077: Support unscoped enumeration members in the expression evaluator.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

We can have unscoped enums in namespace and class scope and the enumerators 
won't leak out from those scopes. Thus we can have shadowing going on e.g.:

  #include 
  
  enum GEnum {eOne=2,};
  
  namespace A {
 enum AEnum {eOne=0,};
  
void g() {std::cout << eOne;}
  };
  
  struct B {
enum CEnum {eOne=1,};
  
void f() {std::cout << eOne;}
  };
  
  int main() {
 std::cout << eOne ;
 A::g() ;
  
 B b;
 b.f() ;
  }

godbolt: https://godbolt.org/z/h3r76n

How does this change deal with those cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94077

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D93895#2479845 , @augusto2112 wrote:

> Question: why does lldb queries if vAttachOrWait is supported, but not 
> vAttachWait? Does it make sense to keep this query? Or should I remove it?

"vAttachOrWait" allows you to attach to a process by name even if it already 
exists, where "vAttachWait" will ignore any current existing processes with a 
matching name. LLDB will fall back to using vAttachWait if the users requested 
to NOT ignore existing processes since some support for waiting for a process 
by  name is better than nothing. So we need to check for vAttachOrWait so we 
can use it if needed to allow attaching to an existing process if the user 
requested it. So since this is very similar code to what you have it just 
doesn't make an ignore list like you did in this patch. So it might be nice to 
add support for vAttachOrWait, along with the query packet, in this patch if 
you have the time? Let me know if you have any questions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

> So it might be nice to add support for vAttachOrWait, along with the query 
> packet, in this patch if you have the time?

Hi @clayborg, I saw that that implementing vAttachOrWait was already 90% done, 
so I did just that :) The current patch as-is supports vAttachOrWait. My 
question was more on the lines of if keeping the query for vAttachOrWait made 
sense not that it's implemented on Linux. Are there be other targets where it 
isn't implemented?

I also left a comment with a couple of questions earlier, but I think it got 
hidden by my last comment. I'll repost it here:

> @clayborg I've added support for the 'waitfor-interval' and 
> 'waitfor-duration' flags. Yesterday I thought they existed in macOS, but now 
> I'm not so sure, as I couldn't find them on "Options.td". They were added in 
> 2009, so maybe they did exist but were dropped later. Or I just didn't look 
> at the right place.
>
> A couple of questions:
>
> - About the default polling interval, I've set it to 1ms for now, but I found 
> that (in my system) this keeps a core at 100%. 10ms keeps still keeps it at 
> around 80%, and 100ms keeps it at around 35%. Maybe 100ms is a good balance?
> - On "Options.td", besides the "process attach" command, there's also a 
> "platform process attach". I haven't touched it since I'm not familiar with 
> the platform command. Should I add these flags to the platform counterpart as 
> well?

Could you confirm for me that 'waitfor-interval' and 'waitfor-duration' don't 
exist on macOS? If I'm sending a different message format than what's done on 
macOS that'd be an issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D93895#2480324 , @augusto2112 wrote:

>> So it might be nice to add support for vAttachOrWait, along with the query 
>> packet, in this patch if you have the time?
>
> Hi @clayborg, I saw that implementing vAttachOrWait was already 90% done, so 
> I did just that :) The current patch as-is supports vAttachOrWait. My 
> question was more on the lines of if keeping the query for vAttachOrWait made 
> sense now that it's implemented on Linux. Are there be other targets where it 
> isn't implemented?

Best to not change anything if we don't know. Yes there are other GDB servers 
out there that may not implement vAttachOrWait, so I would vote to not change 
anything.

> I also left a comment with a couple of questions earlier, but I think it got 
> hidden by my last comment. I'll repost it here:
>
>> @clayborg I've added support for the 'waitfor-interval' and 
>> 'waitfor-duration' flags. Yesterday I thought they existed in macOS, but now 
>> I'm not so sure, as I couldn't find them on "Options.td". They were added in 
>> 2009, so maybe they did exist but were dropped later. Or I just didn't look 
>> at the right place.
>>
>> A couple of questions:
>>
>> - About the default polling interval, I've set it to 1ms for now, but I 
>> found that (in my system) this keeps a core at 100%. 10ms keeps still keeps 
>> it at around 80%, and 100ms keeps it at around 35%. Maybe 100ms is a good 
>> balance?

debugserver in the llvm sources uses 1ms as its default. I would vote to be as 
quick as possible so we can catch the process as soon as possible. The option 
--waitfor-interval specifies the time in microseconds, so we will want to 
mirror that. So even though it does peg one core at 100%, I think it might be 
worth it to catch the process sooner than later.

>> - On "Options.td", besides the "process attach" command, there's also a 
>> "platform process attach". I haven't touched it since I'm not familiar with 
>> the platform command. Should I add these flags to the platform counterpart 
>> as well?

Just "process attach" is fine to start with. The platform layer doesn't always 
have to use a GDB server, so adding those options there could end up being hard 
for some platforms to implement. In fact I would mind if we don't even add 
these options to "process attach" and just always use the defaults, because as 
you say, if someone selects or replaces the lldb-server with an older one, then 
we don't want the launching of the lldb-server to fail because it doesn't 
recognize an option that is being passed to it. The better way to fix this 
would be to make a new attach wait packet that takes these as arguments to the 
packet, but we don't need to do that.

> Could you confirm for me that 'waitfor-interval' and 'waitfor-duration' don't 
> exist on macOS? If I'm sending a different message format than what's done on 
> macOS that'd be an issue.

Here lies the problem that I mentioned above. I would like to avoid having to 
launch lldb-server with any arguments so that we continue to work with older 
lldb-servers.

So maybe we just rely on defaults for now and avoid having to add any new 
arguments to "process attach" and to the launching of the lldb-server? Let me 
know your thoughts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Another option would be to have lldb-server check for environment variables for 
default values for --waitfor-interval and --waitfor-duration. If they are set, 
they become the new default values. Of course a user can launch the lldb-server 
manually with the options set a different way and then attach with "process 
connect ..." if required. But this would provide an alternate way for users to 
control the polling and timeout. I would just hate for us to add these new 
flags and have some people not be able to debug because their GDB server gets 
launched (because they replaced the lldb-server) and doesn't support the flags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

> Here lies the problem that I mentioned above. I would like to avoid having to 
> launch lldb-server with any arguments so that we continue to work with older 
> lldb-servers.



> So maybe we just rely on defaults for now and avoid having to add any new 
> arguments to "process attach" and to the launching of the lldb-server? Let me 
> know your thoughts.

Sorry, I don't think I fully understand this point. Do you mean we shouldn't 
add  'waitfor-interval' and 'waitfor-duration' since older lldb-server versions 
might not recognize the packet format?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D93895#2480554 , @augusto2112 wrote:

>> Here lies the problem that I mentioned above. I would like to avoid having 
>> to launch lldb-server with any arguments so that we continue to work with 
>> older lldb-servers.
>
>
>
>> So maybe we just rely on defaults for now and avoid having to add any new 
>> arguments to "process attach" and to the launching of the lldb-server? Let 
>> me know your thoughts.
>
> Sorry, I don't think I fully understand this point. Do you mean we shouldn't 
> add  'waitfor-interval' and 'waitfor-duration' since older lldb-server 
> versions might not recognize the packet format?

I would vote to:

- modify lldb-server
  - add '--waitfor-interval' and '--waitfor-duration' options to lldb-server 
for people that want to launch lldb-server manually
  - add vAttachWait, vAttachOrWait and the query packet to check if 
vAttachOrWait is supported
- no modifications to lldb itself

If I understood what you were asking before when you said:

> On "Options.td", besides the "process attach" command, there's also a 
> "platform process attach". I haven't touched it since I'm not familiar with 
> the platform command. Should I add these flags to the platform counterpart as 
> well?

It sounded like you were going to modify "process attach" to take options that 
would allow people to specify the '--waitfor-interval' and '--waitfor-duration' 
and then when we spawned lldb-server, pass these same options down to 
lldb-server or debugserver on Darwin. I was thinking it would be nice to avoid 
this part of the change as it then requires any lldb-server that LLDB uses to 
support these options. Lets say someone has a lldb-server already installed on 
their system, only updates the lldb binary and then tries to debug something 
with "process attach --waitfor". It would launch the lldb-server possibly with 
the '--waitfor-interval' options and it would exit with a non zero status 
saying "I don't know that option". Do we currently add or remove options when 
launching lldb-server based on any "process attach" arguments? If we do, then 
my suggestion doesn't make sense. If we don't, then it does make sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

I think I get your point. If we pass the extra options in the packet, the 
validation on older lldb-server versions will reject the message.

> Another option would be to have lldb-server check for environment variables 
> for default values for --waitfor-interval and --waitfor-duration. If they are 
> set, they become the new default values. Of course a user can launch the 
> lldb-server manually with the options set a different way and then attach 
> with "process connect ..." if required. But this would provide an alternate 
> way for users to control the polling and timeout.

I'll defer to you guys, since you're a lot more knowledgeable on lldb than I am 
:)

I do have another possible solution we could consider: can we create a new 
message, similar to `qVAttachOrWaitSupported`, that queries if these extra 
options are supported or not. Something like 
`qVAttachWaitExtraOptionsSupported`. We might even consider returning something 
akin to a "version number", so if we support more options in the future we can 
bump up the number.

Let me know what you think (again, I'm fine doing it either way).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D93895#2480578 , @augusto2112 wrote:

> I think I get your point. If we pass the extra options in the packet, the 
> validation on older lldb-server versions will reject the message.
>
>> Another option would be to have lldb-server check for environment variables 
>> for default values for --waitfor-interval and --waitfor-duration. If they 
>> are set, they become the new default values. Of course a user can launch the 
>> lldb-server manually with the options set a different way and then attach 
>> with "process connect ..." if required. But this would provide an alternate 
>> way for users to control the polling and timeout.
>
> I'll defer to you guys, since you're a lot more knowledgeable on lldb than I 
> am :)
>
> I do have another possible solution we could consider: can we create a new 
> message, similar to `qVAttachOrWaitSupported`, that queries if these extra 
> options are supported or not. Something like 
> `qVAttachWaitExtraOptionsSupported`. We might even consider returning 
> something akin to a "version number", so if we support more options in the 
> future we can bump up the number.
>
> Let me know what you think (again, I'm fine doing it either way).

Yeah, I was thinking it might be nice to have a better attach + wait packet.

We have added new JSON packets in the past that are more flexible and allows us 
to specify arguments using a JSON dictionary. Currently the vAttachWait or 
vAttachOrWait packets have a hard coded single process name argument that is 
appended as hex ASCII so for "a.out":

  vAttachWait:612E6F7574
  vAttachOrWait:612E6F7574

But we could make a new "jAttachWait" that could be something like:

  jAttachWait:{"executable":"a.out", "waitfor-interval-usec": 1000, 
"waitfor-duration-sec": 20}

Then the question becomes where the values for "waitfor-interval-usec" and 
"waitfor-duration-sec" come from. Since "process attach" has to work for any 
process plug-in (there are many in "lldb/source/Plugins/Process"), if we add 
any options to "process attach", each of the plug-ins in 
"lldb/source/Plugins/Process" would need to handle those options. It might be 
better to add new settings for the GDB remote plug-in (AKA: ProcessGDBRemote). 
There are global settings already for this plug-in:

  (lldb) settings show
  ...
  plugin.process.gdb-remote.packet-timeout (unsigned) = 5
  plugin.process.gdb-remote.target-definition-file (file) = 
  plugin.process.gdb-remote.use-g-packet-for-reading (boolean) = false
  plugin.process.gdb-remote.use-libraries-svr4 (boolean) = true

We would easily add new settings to 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td:

  plugin.process.gdb-remote.waitfor-interval-usec (unsigned) = 1000
  plugin.process.gdb-remote.waitfor-duration-sec (unsigned) = 20

And then use these in the new JSON packet as arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895

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

[Lldb-commits] [PATCH] D93444: Make DWARFUnit use the dwo_id from the DWARF5 CU header.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe added a comment.

Thanks for accepting the patch! (and sorry for the late reply, I was on 
vacation). I'll commit this now :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93444

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

[Lldb-commits] [PATCH] D94136: [lldb] Make ModuleList iterable (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added a subscriber: mgorny.
JDevlieghere requested review of this revision.

Add begin and end iterators to the ModuleList so we can use range-based for 
loops.


https://reviews.llvm.org/D94136

Files:
  lldb/include/lldb/Core/ModuleList.h
  lldb/source/Core/ModuleList.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/ModuleListTest.cpp

Index: lldb/unittests/Core/ModuleListTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/ModuleListTest.cpp
@@ -0,0 +1,77 @@
+//===-- ModuleListTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Utility/DataBuffer.h"
+
+#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
+#include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+class ModuleListTest : public testing::Test {
+public:
+  static void SetUpTestCase() { FileSystem::Initialize(); }
+  static void TearDownTestCase() { FileSystem::Terminate(); }
+};
+} // namespace
+
+TEST_F(ModuleListTest, Iterators) {
+  auto AS = ArchSpec();
+  ModuleSP module_foo = std::make_shared(FileSpec("foo"), AS);
+  ModuleSP module_bar = std::make_shared(FileSpec("bar"), AS);
+  ModuleSP module_baz = std::make_shared(FileSpec("baz"), AS);
+
+  ModuleList module_list;
+  module_list.Append(module_foo);
+  module_list.Append(module_bar);
+  module_list.Append(module_baz);
+
+  ASSERT_EQ(module_list.GetModuleAtIndex(0), module_foo);
+  ASSERT_EQ(module_list.GetModuleAtIndex(1), module_bar);
+  ASSERT_EQ(module_list.GetModuleAtIndex(2), module_baz);
+
+  auto begin = module_list.begin();
+  auto end = module_list.end();
+  auto it = begin;
+
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_foo);
+
+  ++it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_bar);
+
+  ++it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_baz);
+
+  ++it;
+  ASSERT_EQ(it, end);
+
+  --it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_baz);
+
+  --it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_bar);
+
+  --it;
+  ASSERT_EQ(*it, module_foo);
+  ASSERT_EQ(it, begin);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@
   CommunicationTest.cpp
   MangledTest.cpp
   ModuleSpecTest.cpp
+  ModuleListTest.cpp
   RichManglingContextTest.cpp
   SourceManagerTest.cpp
   StreamCallbackTest.cpp
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1656,17 +1656,15 @@
 bool Target::ModuleIsExcludedForUnconstrainedSearches(
 const FileSpec &module_file_spec) {
   if (GetBreakpointsConsultPlatformAvoidList()) {
-ModuleList matchingModules;
+ModuleList matching_modules;
 ModuleSpec module_spec(module_file_spec);
-GetImages().FindModules(module_spec, matchingModules);
-size_t num_modules = matchingModules.GetSize();
+GetImages().FindModules(module_spec, matching_modules);
 
 // If there is more than one module for this file spec, only
 // return true if ALL the modules are on the black list.
-if (num_modules > 0) {
-  for (size_t i = 0; i < num_modules; i++) {
-if (!ModuleIsExcludedForUnconstrainedSearches(
-matchingModules.GetModuleAtIndex(i)))
+if (matching_modules.GetSize() > 0) {
+  for (ModuleSP m : matching_modules) {
+if (!ModuleIsExcludedForUnconstrainedSearches(m))
   return false;
   }
   return true;
@@ -2471,9 +2469,7 @@
   }
 
   const ModuleList &modules = GetImages();
-  const size_t num_images = modules.GetSize();
-  for (size_t idx = 0; idx < num_images; ++idx) {
-ModuleSP module_sp(modules.GetModuleAtIndex(idx));
+  for (ModuleSP module_sp : modules) {
 if (!module_sp || !module_sp->GetObjectFile())
   continue;
 
@@ -2846,11 +2842,8 @@
 
 size_t Target::UnloadModuleSections(const ModuleList &module_list) {
   size_t section_unload_count = 0;
-  size_t num_modules = module_list.GetSize();
-  for (size_t i = 0; i < num_modules; ++i) {
-section_unload_count +=
-UnloadModuleSections(module_list.GetModuleAtIndex(i));
-  }
+  for (ModuleSP module_sp : module_list)

[Lldb-commits] [PATCH] D94136: [lldb] Make ModuleList iterable (NFC)

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: lldb/include/lldb/Core/ModuleList.h:70
+class ModuleIterator
+: public std::iterator {
+public:

FWIW, std::iterator is deprecated since C++17 - probably best not to add new 
uses of it. (I think the idea is that the typedefs that std::iterator provides 
are no longer needed because of auto/decltype/etc, but I might be wrong there 
(would have to check the iterator concepts specifications))

If you need some utility help with implementing an iterator, llvm's 
iterator_facade_base might help by allowing a fairly minimal implementation to 
autogenerate various symmetric members, etc.


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

https://reviews.llvm.org/D94136

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

[Lldb-commits] [lldb] a39b198 - Make DWARFUnit use the dwo_id from the DWARF5 CU header.

[Jorge Gorbe Moya via lldb-commits]

Author: Jorge Gorbe Moya
Date: 2021-01-05T16:40:37-08:00
New Revision: a39b19821b6b8c6b4ae853f6b6a88128275ea2c7

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

LOG: Make DWARFUnit use the dwo_id from the DWARF5 CU header.

In split DWARF v5 files, the DWO id is no longer in the DW_AT_GNU_dwo_id
attribute. It's in the CU header instead. This change makes lldb look in
both places.

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

Added: 
lldb/test/Shell/SymbolFile/DWARF/dwarf5-split.s

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index d0cfb5fca82f..a761dd3daac4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -34,7 +34,8 @@ DWARFUnit::DWARFUnit(SymbolFileDWARF &dwarf, lldb::user_id_t 
uid,
  const DWARFAbbreviationDeclarationSet &abbrevs,
  DIERef::Section section, bool is_dwo)
 : UserID(uid), m_dwarf(dwarf), m_header(header), m_abbrevs(&abbrevs),
-  m_cancel_scopes(false), m_section(section), m_is_dwo(is_dwo) {}
+  m_cancel_scopes(false), m_section(section), m_is_dwo(is_dwo),
+  m_dwo_id(header.GetDWOId()) {}
 
 DWARFUnit::~DWARFUnit() = default;
 
@@ -284,6 +285,11 @@ void DWARFUnit::SetDwoStrOffsetsBase() {
   SetStrOffsetsBase(baseOffset);
 }
 
+uint64_t DWARFUnit::GetDWOId() {
+  ExtractUnitDIEIfNeeded();
+  return m_dwo_id;
+}
+
 // m_die_array_mutex must be already held as read/write.
 void DWARFUnit::AddUnitDIE(const DWARFDebugInfoEntry &cu_die) {
   llvm::Optional addr_base, gnu_addr_base, ranges_base,
@@ -337,6 +343,9 @@ void DWARFUnit::AddUnitDIE(const DWARFDebugInfoEntry 
&cu_die) {
 case DW_AT_GNU_ranges_base:
   gnu_ranges_base = form_value.Unsigned();
   break;
+case DW_AT_GNU_dwo_id:
+  m_dwo_id = form_value.Unsigned();
+  break;
 }
   }
 
@@ -350,9 +359,8 @@ void DWARFUnit::AddUnitDIE(const DWARFDebugInfoEntry 
&cu_die) {
   if (!dwo_symbol_file)
 return;
 
-  uint64_t main_dwo_id =
-  cu_die.GetAttributeValueAsUnsigned(this, DW_AT_GNU_dwo_id, 0);
-  DWARFUnit *dwo_cu = dwo_symbol_file->GetDWOCompileUnitForHash(main_dwo_id);
+  DWARFUnit *dwo_cu = dwo_symbol_file->GetDWOCompileUnitForHash(m_dwo_id);
+
   if (!dwo_cu)
 return; // Can't fetch the compile unit from the dwo file.
   dwo_cu->SetUserData(this);
@@ -788,7 +796,8 @@ DWARFUnitHeader::extract(const DWARFDataExtractor &data,
 header.m_unit_type = data.GetU8(offset_ptr);
 header.m_addr_size = data.GetU8(offset_ptr);
 header.m_abbr_offset = data.GetDWARFOffset(offset_ptr);
-if (header.m_unit_type == llvm::dwarf::DW_UT_skeleton)
+if (header.m_unit_type == llvm::dwarf::DW_UT_skeleton ||
+header.m_unit_type == llvm::dwarf::DW_UT_split_compile)
   header.m_dwo_id = data.GetU64(offset_ptr);
   } else {
 header.m_abbr_offset = data.GetDWARFOffset(offset_ptr);

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index d6009518da12..5739c36bbacb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -65,6 +65,7 @@ class DWARFUnitHeader {
   }
   uint64_t GetTypeHash() const { return m_type_hash; }
   dw_offset_t GetTypeOffset() const { return m_type_offset; }
+  uint64_t GetDWOId() const { return m_dwo_id; }
   bool IsTypeUnit() const {
 return m_unit_type == DW_UT_type || m_unit_type == DW_UT_split_type;
   }
@@ -88,6 +89,7 @@ class DWARFUnit : public lldb_private::UserID {
   virtual ~DWARFUnit();
 
   bool IsDWOUnit() { return m_is_dwo; }
+  uint64_t GetDWOId();
 
   void ExtractUnitDIEIfNeeded();
   void ExtractDIEsIfNeeded();
@@ -333,6 +335,8 @@ class DWARFUnit : public lldb_private::UserID {
 
   const DIERef::Section m_section;
   bool m_is_dwo;
+  /// Value of DW_AT_GNU_dwo_id (v4) or dwo_id from CU header (v5).
+  uint64_t m_dwo_id;
 
 private:
   void ParseProducerInfo();

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
index 3aaa7d330b84..fcb51dfa0e7a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -49,8 +49,7 @@ DWARFCompileUnit 
*SymbolFileDWARFDwo::GetDWOCompileUnitForHash(uint64_t hash) {
   DWARFCompileUnit *cu = FindSingleCompileUnit();
   if (!cu)
 return nullptr;
-  if (hash !=
-  cu->GetUnitDIEOnly().GetA

[Lldb-commits] [PATCH] D93444: Make DWARFUnit use the dwo_id from the DWARF5 CU header.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa39b19821b6b: Make DWARFUnit use the dwo_id from the DWARF5 
CU header. (authored by jgorbe).

Changed prior to commit:
  https://reviews.llvm.org/D93444?vs=312377&id=314753#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93444

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-split.s

Index: lldb/test/Shell/SymbolFile/DWARF/dwarf5-split.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/dwarf5-split.s
@@ -0,0 +1,235 @@
+## This test checks that lldb reads debug info from a dwp file when the dwo_id
+## is in a DWARF5 CU header instead of a DW_AT_GNU_dwo_id attribute.
+
+# RUN: llvm-mc --filetype=obj --triple x86_64 %s -o %t --defsym MAIN=1
+# RUN: llvm-mc --filetype=obj --triple x86_64 %s -o %t.dwp
+# RUN: %lldb %t -o "target variable i" -b | FileCheck %s
+# CHECK: (int) i = 42
+
+.ifdef MAIN
+## Main file
+	.text
+	.globl	main# -- Begin function main
+main:   # @main
+.Lfunc_begin0:
+	pushq	%rbp
+	movq	%rsp, %rbp
+	xorl	%eax, %eax
+	popq	%rbp
+	retq
+.Lfunc_end0:
+# -- End function
+	.data
+i:
+	.long	42  # 0x2a
+
+
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	74  # DW_TAG_skeleton_unit
+	.byte	0   # DW_CHILDREN_no
+	.byte	118 # DW_AT_dwo_name
+	.byte	8   # DW_FORM_string
+	.byte	17  # DW_AT_low_pc
+	.byte	27  # DW_FORM_addrx
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	115 # DW_AT_addr_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	0   # EOM(3)
+
+
+	.section	.debug_info,"",@progbits
+.Lcu_begin0:
+	.long	.Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+	.short	5 # DWARF version number
+	.byte	4   # DWARF Unit Type
+	.byte	8   # Address Size (in bytes)
+	.long	.debug_abbrev   # Offset Into Abbrev. Section
+	.quad	1026699901672188186 # dwo_id
+	.byte	1   # Abbrev [1] 0x14:0x14 DW_TAG_skeleton_unit
+	.asciz "hello.dwo"# DW_AT_dwo_name
+	.byte	1   # DW_AT_low_pc
+	.long	.Lfunc_end0-.Lfunc_begin0   # DW_AT_high_pc
+	.long	.Laddr_table_base0  # DW_AT_addr_base
+.Ldebug_info_end0:
+
+	.section	.debug_addr,"",@progbits
+	.long	.Ldebug_addr_end0-.Ldebug_addr_start0 # Length of contribution
+.Ldebug_addr_start0:
+	.short	5   # DWARF version number
+	.byte	8   # Address size
+	.byte	0   # Segment selector size
+.Laddr_table_base0:
+	.quad	i
+	.quad	.Lfunc_begin0
+.Ldebug_addr_end0:
+
+.else
+## DWP file
+	.section	.debug_str_offsets.dwo,"e",@progbits
+	.long	28
+	.short	5
+	.short	0
+
+
+	.section	.debug_str.dwo,"eMS",@progbits,1
+.Linfo_string0:
+	.asciz	"i"
+.Linfo_string1:
+	.asciz	"int"
+.Linfo_string2:
+	.asciz	"main"
+.Linfo_string3:
+	.asciz	"hand-tuned clang output"
+.Linfo_string4:
+	.asciz	"hello.c"
+.Linfo_string5:
+	.asciz	"hello.dwo"
+
+
+	.section	.debug_str_offsets.dwo,"e",@progbits
+	.long	.Linfo_string0-.debug_str.dwo
+	.long	.Linfo_string1-.debug_str.dwo
+	.long	.Linfo_string2-.debug_str.dwo
+	.long	.Linfo_string3-.debug_str.dwo
+	.long	.Linfo_string4-.debug_str.dwo
+	.long	.Linfo_string5-.debug_str.dwo
+.Lstr_offsets_end:
+
+
+	.section	.debug_info.dwo,"e",@progbits
+	.long	.Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
+.Ldebug_info_dwo_start0:
+	.short	5   # DWARF version number
+	.byte	5   # DWARF Unit Type
+	.byte	8   # Address Size (in bytes)
+	.long	0   # Offset Into Abbrev. Section
+	.quad	1026699901672188186 # dwo_id
+	.byte	1   # Abbrev [1] 0x14:0x25 DW_TAG_compile_unit
+	.byte	3   # DW_AT_producer
+	.short	12  # DW_AT_language
+	.byte	4   # DW_AT_name
+	.byte	5  

[Lldb-commits] [PATCH] D94136: [lldb] Make ModuleList iterable (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 314755.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

Use `iterator_facade_base` as suggested by @dblaikie.


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

https://reviews.llvm.org/D94136

Files:
  lldb/include/lldb/Core/ModuleList.h
  lldb/source/Core/ModuleList.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/ModuleListTest.cpp

Index: lldb/unittests/Core/ModuleListTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/ModuleListTest.cpp
@@ -0,0 +1,77 @@
+//===-- ModuleListTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Utility/DataBuffer.h"
+
+#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
+#include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+class ModuleListTest : public testing::Test {
+public:
+  static void SetUpTestCase() { FileSystem::Initialize(); }
+  static void TearDownTestCase() { FileSystem::Terminate(); }
+};
+} // namespace
+
+TEST_F(ModuleListTest, Iterators) {
+  auto AS = ArchSpec();
+  ModuleSP module_foo = std::make_shared(FileSpec("foo"), AS);
+  ModuleSP module_bar = std::make_shared(FileSpec("bar"), AS);
+  ModuleSP module_baz = std::make_shared(FileSpec("baz"), AS);
+
+  ModuleList module_list;
+  module_list.Append(module_foo);
+  module_list.Append(module_bar);
+  module_list.Append(module_baz);
+
+  ASSERT_EQ(module_list.GetModuleAtIndex(0), module_foo);
+  ASSERT_EQ(module_list.GetModuleAtIndex(1), module_bar);
+  ASSERT_EQ(module_list.GetModuleAtIndex(2), module_baz);
+
+  auto begin = module_list.begin();
+  auto end = module_list.end();
+  auto it = begin;
+
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_foo);
+
+  ++it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_bar);
+
+  ++it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_baz);
+
+  ++it;
+  ASSERT_EQ(it, end);
+
+  --it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_baz);
+
+  --it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_bar);
+
+  --it;
+  ASSERT_EQ(*it, module_foo);
+  ASSERT_EQ(it, begin);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@
   CommunicationTest.cpp
   MangledTest.cpp
   ModuleSpecTest.cpp
+  ModuleListTest.cpp
   RichManglingContextTest.cpp
   SourceManagerTest.cpp
   StreamCallbackTest.cpp
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1656,17 +1656,15 @@
 bool Target::ModuleIsExcludedForUnconstrainedSearches(
 const FileSpec &module_file_spec) {
   if (GetBreakpointsConsultPlatformAvoidList()) {
-ModuleList matchingModules;
+ModuleList matching_modules;
 ModuleSpec module_spec(module_file_spec);
-GetImages().FindModules(module_spec, matchingModules);
-size_t num_modules = matchingModules.GetSize();
+GetImages().FindModules(module_spec, matching_modules);
 
 // If there is more than one module for this file spec, only
 // return true if ALL the modules are on the black list.
-if (num_modules > 0) {
-  for (size_t i = 0; i < num_modules; i++) {
-if (!ModuleIsExcludedForUnconstrainedSearches(
-matchingModules.GetModuleAtIndex(i)))
+if (matching_modules.GetSize() > 0) {
+  for (ModuleSP m : matching_modules) {
+if (!ModuleIsExcludedForUnconstrainedSearches(m))
   return false;
   }
   return true;
@@ -2471,9 +2469,7 @@
   }
 
   const ModuleList &modules = GetImages();
-  const size_t num_images = modules.GetSize();
-  for (size_t idx = 0; idx < num_images; ++idx) {
-ModuleSP module_sp(modules.GetModuleAtIndex(idx));
+  for (ModuleSP module_sp : modules) {
 if (!module_sp || !module_sp->GetObjectFile())
   continue;
 
@@ -2846,11 +2842,8 @@
 
 size_t Target::UnloadModuleSections(const ModuleList &module_list) {
   size_t section_unload_count = 0;
-  size_t num_modules = module_list.GetSize();
-  for (size_t i = 0; i < num_modules; ++i) {
-section_unload_count +=
-UnloadModuleSections(module_list.GetModuleAtIndex(i));
-  }
+  for (ModuleSP module_sp : modu

[Lldb-commits] [PATCH] D94136: [lldb] Make ModuleList iterable (NFC)

[David Blaikie via Phabricator via lldb-commits]

dblaikie added inline comments.



Comment at: lldb/include/lldb/Core/ModuleList.h:71
+: public llvm::iterator_facade_base<
+  ModuleIterator, std::bidirectional_iterator_tag, lldb::ModuleSP> {
+public:

On the fence, but this could be a random access iterator, since it's only based 
on an integer index - would be fairly easy to implement, but I can appreciate 
not wanting to add more iterator surface area than you need for range-based-for 
loops.



Comment at: lldb/include/lldb/Core/ModuleList.h:76
+
+  const lldb::ModuleSP operator*() const;
+

Returning const value types is particularly weird/problematic in some ways - it 
prevents any moving from the return value. So should probably drop the "const" 
here.



Comment at: lldb/include/lldb/Core/ModuleList.h:93-96
+  friend bool operator!=(const ModuleIterator &lhs, const ModuleIterator &rhs) 
{
+return !(lhs == rhs);
+  }
+

Seems you don't need to implement `!=` - iterator_facade_base will provide that 
for you based on op==, I think?



Comment at: lldb/include/lldb/Core/ModuleList.h:98
+private:
+  const ModuleList &m_module_list;
+  size_t m_index;

This being a reference (rather than, say, a pointer) would prevent the iterator 
from being assignable, I think? Which isn't ideal for many iterator algorithms, 
etc.


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

https://reviews.llvm.org/D94136

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

[Lldb-commits] [PATCH] D94136: [lldb] Make ModuleList iterable (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 314767.
JDevlieghere marked 4 inline comments as done.
JDevlieghere added a comment.

Address Dave's feedback.


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

https://reviews.llvm.org/D94136

Files:
  lldb/include/lldb/Core/ModuleList.h
  lldb/source/Core/ModuleList.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/ModuleListTest.cpp

Index: lldb/unittests/Core/ModuleListTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/ModuleListTest.cpp
@@ -0,0 +1,77 @@
+//===-- ModuleListTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Utility/DataBuffer.h"
+
+#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
+#include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+class ModuleListTest : public testing::Test {
+public:
+  static void SetUpTestCase() { FileSystem::Initialize(); }
+  static void TearDownTestCase() { FileSystem::Terminate(); }
+};
+} // namespace
+
+TEST_F(ModuleListTest, Iterators) {
+  auto AS = ArchSpec();
+  ModuleSP module_foo = std::make_shared(FileSpec("foo"), AS);
+  ModuleSP module_bar = std::make_shared(FileSpec("bar"), AS);
+  ModuleSP module_baz = std::make_shared(FileSpec("baz"), AS);
+
+  ModuleList module_list;
+  module_list.Append(module_foo);
+  module_list.Append(module_bar);
+  module_list.Append(module_baz);
+
+  ASSERT_EQ(module_list.GetModuleAtIndex(0), module_foo);
+  ASSERT_EQ(module_list.GetModuleAtIndex(1), module_bar);
+  ASSERT_EQ(module_list.GetModuleAtIndex(2), module_baz);
+
+  ModuleList::iterator begin = module_list.begin();
+  ModuleList::iterator end = module_list.end();
+  ModuleIterator it = begin;
+
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_foo);
+
+  ++it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_bar);
+
+  ++it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_baz);
+
+  ++it;
+  ASSERT_EQ(it, end);
+
+  --it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_baz);
+
+  --it;
+  ASSERT_NE(it, end);
+  ASSERT_EQ(*it, module_bar);
+
+  --it;
+  ASSERT_EQ(*it, module_foo);
+  ASSERT_EQ(it, begin);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@
   CommunicationTest.cpp
   MangledTest.cpp
   ModuleSpecTest.cpp
+  ModuleListTest.cpp
   RichManglingContextTest.cpp
   SourceManagerTest.cpp
   StreamCallbackTest.cpp
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1656,17 +1656,15 @@
 bool Target::ModuleIsExcludedForUnconstrainedSearches(
 const FileSpec &module_file_spec) {
   if (GetBreakpointsConsultPlatformAvoidList()) {
-ModuleList matchingModules;
+ModuleList matching_modules;
 ModuleSpec module_spec(module_file_spec);
-GetImages().FindModules(module_spec, matchingModules);
-size_t num_modules = matchingModules.GetSize();
+GetImages().FindModules(module_spec, matching_modules);
 
 // If there is more than one module for this file spec, only
 // return true if ALL the modules are on the black list.
-if (num_modules > 0) {
-  for (size_t i = 0; i < num_modules; i++) {
-if (!ModuleIsExcludedForUnconstrainedSearches(
-matchingModules.GetModuleAtIndex(i)))
+if (matching_modules.GetSize() > 0) {
+  for (ModuleSP m : matching_modules) {
+if (!ModuleIsExcludedForUnconstrainedSearches(m))
   return false;
   }
   return true;
@@ -2471,9 +2469,7 @@
   }
 
   const ModuleList &modules = GetImages();
-  const size_t num_images = modules.GetSize();
-  for (size_t idx = 0; idx < num_images; ++idx) {
-ModuleSP module_sp(modules.GetModuleAtIndex(idx));
+  for (ModuleSP module_sp : modules) {
 if (!module_sp || !module_sp->GetObjectFile())
   continue;
 
@@ -2846,11 +2842,8 @@
 
 size_t Target::UnloadModuleSections(const ModuleList &module_list) {
   size_t section_unload_count = 0;
-  size_t num_modules = module_list.GetSize();
-  for (size_t i = 0; i < num_modules; ++i) {
-section_unload_count +=
-UnloadModuleSections(module_list.GetModuleAtIndex(i));
-  }
+  for (ModuleSP mod

[Lldb-commits] [PATCH] D94136: [lldb] Make ModuleList iterable (NFC)

[David Blaikie via Phabricator via lldb-commits]

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

Looks good to me with a few minor cleanups.




Comment at: lldb/include/lldb/Core/ModuleList.h:73-74
+public:
+  explicit ModuleIterator(const ModuleList *module_list, size_t idx);
+  ModuleIterator(const ModuleList *module_list);
+

Interesting choice to make the single-arg ctor implicit but the multi-arg 
explicit (usually people end up the other way around - because until C++ 17 or 
something, explicit was only relevant for single-arg constructors). In any case 
the ctors are only used once each & probably might as well have them both 
explicit.

Or maybe make only one ctor that takes the index explicitly and use it from 
begin/end - not sure there's much value added by having two ctors here, and 
honestly I'd have expected the no-arg version to start at zero, and the 
arg-taking version to start wherever you want, and I see they're the opposite, 
so that'd probably reinforce the idea that just having the one that takes an 
explicit index might be less confusing.



Comment at: lldb/include/lldb/Core/ModuleList.h:78-87
+  ModuleIterator &operator++() {
+++m_index;
+return *this;
+  }
+
+  ModuleIterator &operator--() {
+--m_index;

I think you can drop these two now - they'll be provided by the iterator facade.


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

https://reviews.llvm.org/D94136

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

[Lldb-commits] [PATCH] D94136: [lldb] Make ModuleList iterable (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Core/ModuleList.h:73-74
+public:
+  explicit ModuleIterator(const ModuleList *module_list, size_t idx);
+  ModuleIterator(const ModuleList *module_list);
+

dblaikie wrote:
> Interesting choice to make the single-arg ctor implicit but the multi-arg 
> explicit (usually people end up the other way around - because until C++ 17 
> or something, explicit was only relevant for single-arg constructors). In any 
> case the ctors are only used once each & probably might as well have them 
> both explicit.
> 
> Or maybe make only one ctor that takes the index explicitly and use it from 
> begin/end - not sure there's much value added by having two ctors here, and 
> honestly I'd have expected the no-arg version to start at zero, and the 
> arg-taking version to start wherever you want, and I see they're the 
> opposite, so that'd probably reinforce the idea that just having the one that 
> takes an explicit index might be less confusing.
I copied all of this from somewhere else in LLDB without giving it much thought 
because honestly I'm not sure this iterator is that much better than then 
existing `LockingAdaptedIterable`. Sorry for making you review this half-baked 
code... 


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

https://reviews.llvm.org/D94136

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

[Lldb-commits] [PATCH] D78978: [LLDB] Add support for WebAssembly debugging

[Zhongmin Wu via Phabricator via lldb-commits]

vwzm228 added a comment.
Herald added a subscriber: JDevlieghere.

Is there any progress about such patch and D78801 
？

I have implemented the debugging feature in our Wasm VM based on  
https://reviews.llvm.org/D78801, and it already work to attach, set breakpoint, 
step, show variable value, backtrace...

I am not sure if I need to change LLDB part to this one, or keep using D78801 
.

But if both patches wont be merged, I have to maintain a private LLDB 
version


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78978

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