[Lldb-commits] [lldb] 27aeb58 - [NFC][TargetParser] Remove llvm/Support/ARMTargetParser.h

2023-02-07 Thread Archibald Elliott via lldb-commits 

Author: Archibald Elliott
Date: 2023-02-07T11:05:58Z
New Revision: 27aeb58ce4d12e15b966dba86738eb65a96703f7

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

LOG: [NFC][TargetParser] Remove llvm/Support/ARMTargetParser.h

Added: 


Modified: 
clang/lib/Basic/Targets/ARM.h
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Driver/ToolChains/Arch/ARM.h
lldb/source/Utility/ArchSpec.cpp
llvm/include/llvm/MC/MCStreamer.h
llvm/lib/BinaryFormat/MachO.cpp
llvm/lib/Target/ARM/ARMSubtarget.cpp
llvm/lib/Target/ARM/ARMTargetMachine.cpp

Removed: 
llvm/include/llvm/Support/ARMTargetParser.h



diff  --git a/clang/lib/Basic/Targets/ARM.h b/clang/lib/Basic/Targets/ARM.h
index e662a609017b..fd3966b69d55 100644
--- a/clang/lib/Basic/Targets/ARM.h
+++ b/clang/lib/Basic/Targets/ARM.h
@@ -17,9 +17,9 @@
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/ADT/Triple.h"
-#include "llvm/Support/ARMTargetParser.h"
 #include "llvm/Support/ARMTargetParserCommon.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/TargetParser/ARMTargetParser.h"
 
 namespace clang {
 namespace targets {

diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp 
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index b6a9df28500a..13a9c485ac9b 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -12,9 +12,9 @@
 #include "clang/Driver/Options.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Option/ArgList.h"
-#include "llvm/Support/ARMTargetParser.h"
-#include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/TargetParser.h"
+#include "llvm/TargetParser/ARMTargetParser.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;

diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.h 
b/clang/lib/Driver/ToolChains/Arch/ARM.h
index 782bdf3d0202..a9a4ca1eb05a 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.h
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -13,8 +13,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Option/Option.h"
-#include "llvm/Support/ARMTargetParser.h"
 #include "llvm/Support/TargetParser.h"
+#include "llvm/TargetParser/ARMTargetParser.h"
 #include 
 #include 
 

diff  --git a/lldb/source/Utility/ArchSpec.cpp 
b/lldb/source/Utility/ArchSpec.cpp
index e1d7ee3ee276..c82097a9e6cd 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -16,8 +16,8 @@
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/BinaryFormat/MachO.h"
-#include "llvm/Support/ARMTargetParser.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/TargetParser/ARMTargetParser.h"
 
 using namespace lldb;
 using namespace lldb_private;

diff  --git a/llvm/include/llvm/MC/MCStreamer.h 
b/llvm/include/llvm/MC/MCStreamer.h
index 898fbc8c0fab..5331d9bd3cc4 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -22,11 +22,11 @@
 #include "llvm/MC/MCLinkerOptimizationHint.h"
 #include "llvm/MC/MCPseudoProbe.h"
 #include "llvm/MC/MCWinEH.h"
-#include "llvm/Support/ARMTargetParser.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/VersionTuple.h"
+#include "llvm/TargetParser/ARMTargetParser.h"
 #include 
 #include 
 #include 

diff  --git a/llvm/include/llvm/Support/ARMTargetParser.h 
b/llvm/include/llvm/Support/ARMTargetParser.h
deleted file mode 100644
index a0c0edd6d0f1..
--- a/llvm/include/llvm/Support/ARMTargetParser.h
+++ /dev/null
@@ -1,15 +0,0 @@
-//===-- llvm/Support/ARMTargetParser.h --*- C++ 
-*-===//
-//
-// 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
-//
-//===--===//
-///
-/// \file
-/// This header is deprecated in favour of
-/// `llvm/TargetParser/ARMTargetParser.h`.
-///
-//===--===//
-
-#include "llvm/TargetParser/ARMTargetParser.h"

diff  --git a/llvm/lib/BinaryFormat/MachO.cpp b/llvm/lib/BinaryFormat/MachO.cpp
index 02a515c94399..c4cc8d6152d1 100644
--- a/llvm/lib/BinaryFormat/MachO.cpp
+++ b/llvm/lib/BinaryFormat/MachO.cpp
@@ -8,7 +8,7 @@
 
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/ADT/Triple.h"
-#include "llvm/Support/ARMTargetParser.h"
+#include "llvm/TargetParser/ARMTargetParser.h"
 
 using namespace llvm;
 

diff  --git a/llvm/lib/Target/ARM/ARMSubtarget.cpp 
b/llvm/lib/Target/ARM/ARMSubtarget.cpp
index 79244f634ce3..

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

2023-02-07 Thread Balázs Kéri via Phabricator via lldb-commits 
balazske added inline comments.



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

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.)


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] D143398: [lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation

2023-02-07 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 updated this revision to Diff 495469.
Michael137 added a comment.

- use `find()` instead of `operator[]`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143398

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/commands/expression/namespace-alias/Makefile
  lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
  lldb/test/API/commands/expression/namespace-alias/main.cpp

Index: lldb/test/API/commands/expression/namespace-alias/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/main.cpp
@@ -0,0 +1,21 @@
+namespace A {
+inline namespace _A {
+namespace B {
+namespace C {
+int a = -1;
+
+int func() { return 0; }
+} // namespace C
+} // namespace B
+
+namespace C = B::C;
+namespace D = B::C;
+
+} // namespace _A
+} // namespace A
+
+namespace E = A;
+namespace F = E::C;
+namespace G = F;
+
+int main(int argc, char **argv) { return A::B::C::a; }
Index: lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
@@ -0,0 +1,29 @@
+"""
+Test that we correctly handle namespace
+expression evaluation through namespace
+aliases.
+"""
+
+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,
+  "return A::B::C::a", lldb.SBFileSpec("main.cpp"))
+
+self.expect_expr("A::C::a", result_type="int", result_value="-1")
+self.expect_expr("A::D::a", result_type="int", result_value="-1")
+
+self.expect_expr("A::C::func()", result_type="int", result_value="0")
+self.expect_expr("A::D::func()", result_type="int", result_value="0")
+
+self.expect_expr("E::C::a", result_type="int", result_value="-1")
+self.expect_expr("E::D::a", result_type="int", result_value="-1")
+self.expect_expr("F::a", result_type="int", result_value="-1")
+self.expect_expr("G::a", result_type="int", result_value="-1")
Index: lldb/test/API/commands/expression/namespace-alias/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/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
@@ -132,6 +132,17 @@
 
   clang::NamespaceDecl *ResolveNamespaceDIE(const DWARFDIE &die);
 
+  /// Returns the namespace decl that a DW_TAG_imported_declaration imports.
+  ///
+  /// \param[in] die The import declaration to resolve. If the DIE is not a
+  ///DW_TAG_imported_declaration the behaviour is undefined.
+  ///
+  /// \returns The decl corresponding to the namespace that the specified
+  ///  'die' imports. If the imported entity is not a namespace
+  ///  or another import declaration, returns nullptr. If an error
+  ///  occurs, returns nullptr.
+  clang::NamespaceDecl *ResolveImportedDeclarationDIE(const DWARFDIE &die);
+
   bool ParseTemplateDIE(const DWARFDIE &die,
 lldb_private::TypeSystemClang::TemplateParameterInfos
 &template_param_infos);
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3304,6 +3304,11 @@
   try_parsing_type = false;
   break;
 
+case DW_TAG_imported_declaration:
+  decl_ctx = ResolveImportedDeclarationDIE(die);
+  try_parsing_type = false;
+  break;
+
 case DW_TAG_lexical_block:
   decl_ctx = GetDeclContextForBlock(die);
   try_parsing_type = false;
@@ -3465,6 +3470,42 @@
   return nullptr;
 }
 
+clang::NamespaceDecl *
+DWARFASTParserClang::ResolveImportedDeclarationDIE(const DWARFDIE &die) {
+  assert(die && die.Tag() == DW_TAG_imported_declaration);
+
+  // See if we cached a NamespaceDecl for this imported declaration
+  // already
+  auto it = m_die_to_decl_ctx.find(die.GetDIE());
+  if (it != m_die_to_decl_ctx.end())
+return static_cast(it->getSecond());
+
+  clang::NamespaceDecl *namespace_decl = nullptr;
+
+  const DWARFDIE imported_u

[Lldb-commits] [PATCH] D143398: [lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation

2023-02-07 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 updated this revision to Diff 495470.
Michael137 added a comment.

- Reduce scope of variable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143398

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/commands/expression/namespace-alias/Makefile
  lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
  lldb/test/API/commands/expression/namespace-alias/main.cpp

Index: lldb/test/API/commands/expression/namespace-alias/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/main.cpp
@@ -0,0 +1,21 @@
+namespace A {
+inline namespace _A {
+namespace B {
+namespace C {
+int a = -1;
+
+int func() { return 0; }
+} // namespace C
+} // namespace B
+
+namespace C = B::C;
+namespace D = B::C;
+
+} // namespace _A
+} // namespace A
+
+namespace E = A;
+namespace F = E::C;
+namespace G = F;
+
+int main(int argc, char **argv) { return A::B::C::a; }
Index: lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
@@ -0,0 +1,29 @@
+"""
+Test that we correctly handle namespace
+expression evaluation through namespace
+aliases.
+"""
+
+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,
+  "return A::B::C::a", lldb.SBFileSpec("main.cpp"))
+
+self.expect_expr("A::C::a", result_type="int", result_value="-1")
+self.expect_expr("A::D::a", result_type="int", result_value="-1")
+
+self.expect_expr("A::C::func()", result_type="int", result_value="0")
+self.expect_expr("A::D::func()", result_type="int", result_value="0")
+
+self.expect_expr("E::C::a", result_type="int", result_value="-1")
+self.expect_expr("E::D::a", result_type="int", result_value="-1")
+self.expect_expr("F::a", result_type="int", result_value="-1")
+self.expect_expr("G::a", result_type="int", result_value="-1")
Index: lldb/test/API/commands/expression/namespace-alias/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/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
@@ -132,6 +132,17 @@
 
   clang::NamespaceDecl *ResolveNamespaceDIE(const DWARFDIE &die);
 
+  /// Returns the namespace decl that a DW_TAG_imported_declaration imports.
+  ///
+  /// \param[in] die The import declaration to resolve. If the DIE is not a
+  ///DW_TAG_imported_declaration the behaviour is undefined.
+  ///
+  /// \returns The decl corresponding to the namespace that the specified
+  ///  'die' imports. If the imported entity is not a namespace
+  ///  or another import declaration, returns nullptr. If an error
+  ///  occurs, returns nullptr.
+  clang::NamespaceDecl *ResolveImportedDeclarationDIE(const DWARFDIE &die);
+
   bool ParseTemplateDIE(const DWARFDIE &die,
 lldb_private::TypeSystemClang::TemplateParameterInfos
 &template_param_infos);
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3304,6 +3304,11 @@
   try_parsing_type = false;
   break;
 
+case DW_TAG_imported_declaration:
+  decl_ctx = ResolveImportedDeclarationDIE(die);
+  try_parsing_type = false;
+  break;
+
 case DW_TAG_lexical_block:
   decl_ctx = GetDeclContextForBlock(die);
   try_parsing_type = false;
@@ -3465,6 +3470,42 @@
   return nullptr;
 }
 
+clang::NamespaceDecl *
+DWARFASTParserClang::ResolveImportedDeclarationDIE(const DWARFDIE &die) {
+  assert(die && die.Tag() == DW_TAG_imported_declaration);
+
+  // See if we cached a NamespaceDecl for this imported declaration
+  // already
+  if (auto it = m_die_to_decl_ctx.find(die.GetDIE());
+  it != m_die_to_decl_ctx.end())
+return static_cast(it->getSecond());
+
+  clang::NamespaceDecl *namespace_decl = nullptr;
+
+  const DWARFDIE imported_uid =
+

[Lldb-commits] [PATCH] D143501: [WIP][clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

2023-02-07 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 created this revision.
Michael137 added reviewers: aprantl, dblaikie.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

**Summary**

This patch makes debug-info generation aware of the
`[[clang::preferred_name]]` attribute. The attribute tells clang
to print the annotated class template as some typedef (e.g., in
diagnostics).

When printing a typename for emission into `DW_AT_name` this patch now
uses the preferred_name (if available). This is behind an LLDB tuning
because by default we try to avoid diverging GCC and Clang typename
format (which is why the `PrintingPolicy::UsePreferredNames` has
previously been disabled by default in `CGDebugInfo`).

**Motivation**

This will reduce noise in type summaries when showing variable
types in LLDB. E.g.,:

  (lldb) v
  (std::vector >) vec = size=0 {}

becomes

  (lldb) v
  (std::vector) vec = size=0 {}

**Testing**

- Added Clang test
- Adjusted LLDB API tests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143501

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGen/debug-info-preferred-names.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
@@ -22,7 +22,7 @@
 
 def make_expected_basic_string_ptr(self) -> str:
 if self.expectedCompilerVersion(['>', '16.0']):
-return f'std::unique_ptr >'
+return f'std::unique_ptr'
 else:
 return 'std::unique_ptr, std::allocator >, ' \
'std::default_delete, std::allocator > > >'
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
@@ -59,13 +59,13 @@
 self.assertNotEqual(valobj.child[0].unsigned, 0)
 
 if self.expectedCompilerVersion(['>', '16.0']):
-string_type = "std::basic_string"
+string_type = "std::string"
 else:
-string_type = "std::basic_string, std::allocator >"
+string_type = "std::basic_string, std::allocator > "
 
 valobj = self.expect_var_path(
 "sp_str",
-type="std::shared_ptr<" + string_type + " >",
+type="std::shared_ptr<" + string_type + ">",
 children=[ValueCheck(name="__ptr_", summary='"hello"')],
 )
 self.assertRegex(valobj.summary, r'^"hello"( strong=1)? weak=1$')
Index: clang/test/CodeGen/debug-info-preferred-names.cpp
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-preferred-names.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - -debugger-tuning=lldb | FileCheck --check-prefixes=COMMON,LLDB %s
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - -debugger-tuning=gdb | FileCheck --check-prefixes=COMMON,GDB %s
+
+template
+class Qux {};
+
+template
+struct Foo;
+
+template
+using Bar = Foo;
+
+template
+struct [[clang::preferred_name(Bar)]] Foo {};
+
+int main() {
+/* Trivial cases */
+
+Bar b;
+// COMMON: !DIDerivedType(tag: DW_TAG_typedef, name: "Bar"
+
+Foo f1;
+// COMMON: !DICompositeType(tag: DW_TAG_structure_type, name: "Foo"
+
+/* Alias template case */
+
+Bar> f2;
+// GDB:!DIDerivedType(tag: DW_TAG_typedef, name: "Bar >"
+// LLDB:   !DIDerivedType(tag: DW_TAG_typedef, name: "Bar >"
+
+/* Nested cases */
+
+Foo> f3; 
+// GDB:!DICompositeType(tag: DW_TAG_structure_type, name: "Foo >"
+// LLDB:   !DICompositeType(tag: DW_TAG_structure_type, name: "Foo >"
+
+Qux> f4;
+// GDB:!DICompositeType(tag: DW_TAG_class_type, name: "Qux >"
+// LLDB:   !DICompositeType(tag: DW_TAG_class_type, name: "Qux >"
+
+return 0;
+}
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -789,6 +789,11 @@
   std::memcpy(Data + A.size(),

[Lldb-commits] [PATCH] D143501: [WIP][clang][DebugInfo] lldb: Use preferred name's type when emitting DW_AT_names

2023-02-07 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 added a comment.
Herald added a subscriber: JDevlieghere.

The alternative would be attaching the preferred name as a new attribute or an 
existing attribute like `DW_AT_description`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143501

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

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

2023-02-07 Thread Alexander Yermolovich via Phabricator via lldb-commits 
ayermolo updated this revision to Diff 495582.
ayermolo added a comment.

Implemented Gregs suggestion. Also consolidated three diffs back into one. 
To make it clear the scope of changes, and for ease of testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

Files:
  lldb/include/lldb/Core/dwarf.h
  lldb/include/lldb/Symbol/DWARFCallFrameInfo.h
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DIERef.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
  lldb/unittests/Expression/DWARFExpressionTest.cpp
  lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp

Index: lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
===
--- lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
+++ lldb/unittests/SymbolFile/DWARF/DWARFIndexCachingTest.cpp
@@ -45,6 +45,26 @@
   EncodeDecode(DIERef(200, DIERef::Section::DebugTypes, 0x11223344));
 }
 
+TEST(DWARFIndexCachingTest, DIERefEncodeDecodeMax) {
+  // Tests DIERef::Encode(...) and DIERef::Decode(...)
+  EncodeDecode(DIERef(std::nullopt, DIERef::Section::DebugInfo,
+  DIERef::k_die_offset_mask - 1));
+  EncodeDecode(DIERef(std::nullopt, DIERef::Section::DebugTypes,
+  DIERef::k_die_offset_mask - 1));
+  EncodeDecode(
+  DIERef(100, DIERef::Section::DebugInfo, DIERef::k_die_offset_mask - 1));
+  EncodeDecode(
+  DIERef(200, DIERef::Section::DebugTypes, DIERef::k_die_offset_mask - 1));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugInfo,
+  DIERef::k_file_index_mask));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugTypes,
+  DIERef::k_file_index_mask));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugInfo,
+  0x11223344));
+  EncodeDecode(DIERef(DIERef::k_file_index_mask, DIERef::Section::DebugTypes,
+  0x11223344));
+}
+
 static void EncodeDecode(const NameToDIE &object, ByteOrder byte_order) {
   const uint8_t addr_size = 8;
   DataEncoder encoder(byte_order, addr_size);
Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -713,7 +713,7 @@
   //   Entries:
   // - AbbrCode:0x1
   //   Values:
-  // - Value:   0x01020304
+  // - Value:   0x0120304
   // - AbbrCode:0x0
   const char *dwo_yamldata = R"(
 --- !ELF
@@ -750,7 +750,7 @@
   auto dwo_module_sp = std::make_shared(dwo_file->moduleSpec());
   SymbolFileDWARFDwo dwo_symfile(
   skeleton_symfile, dwo_module_sp->GetObjectFile()->shared_from_this(),
-  0x01020304);
+  0x0120304);
   auto *dwo_dwarf_unit = dwo_symfile.DebugInfo().GetUnitAtIndex(0);
 
   testExpressionVendorExtensions(dwo_module_sp, *dwo_dwarf_unit);
Index: lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/debug_rnglists-dwo.s
@@ -4,9 +4,9 @@
 # RUN:   -o exit | FileCheck %s
 
 # CHECK-LABEL: image lookup -v -s lookup_rnglists
-# CHECK:  Function: id = {0x4028}, name = "rnglists", range = [0x-0x0003)
-# CHECK:Blocks: id = {0x4028}, range = [0x-0x0003)
-# CHECK-NEXT:   id = {0x4037}, range = [0x0001-0x0002)
+# CHECK:  Function: id = {0x2028}, name = "rnglists", range = [0x-0x0003)
+# CHECK:Blocks: id = {0x2028}, range = [0x-0x

[Lldb-commits] [PATCH] D143520: Add a new SBDebugger::SetDestroyCallback() API

2023-02-07 Thread jeffrey tan via Phabricator via lldb-commits 
yinghuitan created this revision.
yinghuitan added reviewers: clayborg, labath, jingham, jdoerfert, JDevlieghere, 
kusmour, GeorgeHuyubo.
Herald added a project: All.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Adding a new SBDebugger::SetDestroyCallback() API.
This API can be used by any client to query for statistics/metrics before
exiting debug sessions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143520

Files:
  lldb/bindings/interface/SBDebugger.i
  lldb/bindings/python/python-typemaps.swig
  lldb/bindings/python/python-wrapper.swig
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/lldb-types.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/test/API/python_api/debugger/TestDebuggerAPI.py

Index: lldb/test/API/python_api/debugger/TestDebuggerAPI.py
===
--- lldb/test/API/python_api/debugger/TestDebuggerAPI.py
+++ lldb/test/API/python_api/debugger/TestDebuggerAPI.py
@@ -30,7 +30,7 @@
 self.dbg.SetPrompt(None)
 self.dbg.SetCurrentPlatform(None)
 self.dbg.SetCurrentPlatformSDKRoot(None)
-
+
 fresh_dbg = lldb.SBDebugger()
 self.assertEquals(len(fresh_dbg), 0)
 
@@ -146,3 +146,16 @@
 self.assertEqual(platform2.GetName(), expected_platform)
 self.assertTrue(platform2.GetWorkingDirectory().endswith("bar"),
 platform2.GetWorkingDirectory())
+
+def test_SetDestroyCallback(self):
+destroy_dbg_id = None
+def foo(dbg_id):
+# Need nonlocal to modify closure variable.
+nonlocal destroy_dbg_id
+destroy_dbg_id = dbg_id
+
+self.dbg.SetDestroyCallback(foo)
+
+original_dbg_id = self.dbg.GetID()
+self.dbg.Destroy(self.dbg)
+self.assertEqual(destroy_dbg_id,  original_dbg_id)
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -560,6 +560,12 @@
   assert(g_debugger_list_ptr &&
  "Debugger::Terminate called without a matching Debugger::Initialize!");
 
+  if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
+std::lock_guard guard(*g_debugger_list_mutex_ptr);
+for (const auto &debugger : *g_debugger_list_ptr)
+  HandleDestroyCallback(debugger);
+  }
+
   if (g_thread_pool) {
 // The destructor will wait for all the threads to complete.
 delete g_thread_pool;
@@ -679,10 +685,19 @@
   return debugger_sp;
 }
 
+void Debugger::HandleDestroyCallback(const DebuggerSP &debugger_sp) {
+  if (debugger_sp->m_destroy_callback) {
+debugger_sp->m_destroy_callback(
+*debugger_sp, debugger_sp->m_destroy_callback_baton_sp->data());
+debugger_sp->m_destroy_callback = nullptr;
+  }
+}
+
 void Debugger::Destroy(DebuggerSP &debugger_sp) {
   if (!debugger_sp)
 return;
 
+  HandleDestroyCallback(debugger_sp);
   CommandInterpreter &cmd_interpreter = debugger_sp->GetCommandInterpreter();
 
   if (cmd_interpreter.GetSaveSessionOnQuit()) {
@@ -1285,6 +1300,12 @@
   std::make_shared(log_callback, baton);
 }
 
+void Debugger::SetDestroyCallback(
+lldb::DebuggerDestroyCallback destroy_callback, lldb::BatonSP &baton_sp) {
+  m_destroy_callback = destroy_callback;
+  m_destroy_callback_baton_sp = baton_sp;
+}
+
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
   const std::string &message,
   uint64_t completed, uint64_t total,
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -1672,6 +1672,44 @@
   }
 }
 
+struct CallbackData {
+  SBDebuggerDestroyCallback callback;
+  void *callback_baton;
+};
+
+class SBDebuggerDestroyCallbackBaton
+: public lldb_private::TypedBaton {
+public:
+  SBDebuggerDestroyCallbackBaton(SBDebuggerDestroyCallback callback,
+ void *baton)
+  : TypedBaton(std::make_unique()) {
+getItem()->callback = callback;
+getItem()->callback_baton = baton;
+  }
+
+  ~SBDebuggerDestroyCallbackBaton() = default;
+
+  static void PrivateCallback(Debugger &debugger, void *baton);
+};
+
+void SBDebugger::SetDestroyCallback(
+lldb::SBDebuggerDestroyCallback destroy_callback, void *baton) {
+  LLDB_INSTRUMENT_VA(this, destroy_callback, baton);
+  if (m_opaque_sp) {
+BatonSP baton_sp(
+new SBDebuggerDestroyCallbackBaton(destroy_callback, baton));
+return m_opaque_sp->SetDestroyCallback(
+SBDebuggerDestroyCallbackBaton::PrivateCallback, baton_sp);
+  }
+}
+
+/*static*/ void
+SBDebuggerDestroyCallbackBaton::PrivateCallback(Debugger &debugger,
+

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

2023-02-07 Thread Dominik Adamski via Phabricator via lldb-commits 
domada updated this revision to Diff 495609.
domada added a reviewer: akyrtzi.
domada added a comment.

Added changes in `clang/lib/AST/CMakeLists.txt` to address build issue reported 
by @akyrtzi .

I modified CMakeLists.txt so that it requires generation of missing 
`Attributes.inc`.

@akyrtzi  Please let me know if it solves your issue


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

https://reviews.llvm.org/D141910

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/CMakeLists.txt
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3053,6 +3053,23 @@
   Builder.CreateBr(NewBlocks.front());
 }
 
+unsigned
+OpenMPIRBuilder::getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+   const StringMap &Features) {
+  if (TargetTriple.isX86()) {
+if (Features.lookup("avx512f"))
+  return 512;
+else if (Features.lookup("avx"))
+  return 256;
+return 128;
+  }
+  if (TargetTriple.isPPC())
+return 128;
+  if (TargetTriple.isWasm())
+return 128;
+  return 0;
+}
+
 void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop,
 MapVector AlignedVars,
 Value *IfCond, OrderKind Order,
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -502,6 +502,13 @@
ArrayRef Loops,
InsertPointTy ComputeIP);
 
+  /// Get the default alignment value for given target
+  ///
+  /// \param TargetTriple   Target triple
+  /// \param Features   StringMap which describes extra CPU features
+  static unsigned getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+const StringMap &Features);
+
 private:
   /// Modifies the canonical loop to be a statically-scheduled workshare loop.
   ///
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -500,8 +500,6 @@
   auto target_info = TargetInfo::CreateTargetInfo(
   m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
   if (log) {
-LLDB_LOGF(log, "Using SIMD alignment: %d",
-  target_info->getSimdDefaultAlign());
 LLDB_LOGF(log, "Target datalayout string: '%s'",
   target_info->getDataLayoutString());
 LLDB_LOGF(log, "Target ABI: '%s'", target_info->getABI().str().c_str());
Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -400,9 +400,6 @@
 return false;
   }
 
-  SimdDefaultAlign =
-  hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
-
   // FIXME: We should allow long double type on 32-bits to match with GCC.
   // This requires backend to be able to lower f80 without x87 first.
   if (!HasX87 && LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -49,7 +49,6 @@
 SuitableAlign = 128;
 LargeArrayMinWidth = 128;
 LargeArrayAlign = 128;
-SimdDefaultAlign = 128;
 SigAtomicType = SignedLong;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -87,7 +87,6 @@
   PPCTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple) {
 SuitableAlign = 128;
-SimdDefaultAlign = 128;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble();
 HasStrictFP = true;
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -119,7 +119,6 @@
   MaxAtomicPromoteWidth = MaxAtomicInlineW

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

2023-02-07 Thread Dominik Adamski via Phabricator via lldb-commits 
domada updated this revision to Diff 495627.
domada added a comment.

Patch rebased


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

https://reviews.llvm.org/D141910

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/CMakeLists.txt
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3053,6 +3053,23 @@
   Builder.CreateBr(NewBlocks.front());
 }
 
+unsigned
+OpenMPIRBuilder::getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+   const StringMap &Features) {
+  if (TargetTriple.isX86()) {
+if (Features.lookup("avx512f"))
+  return 512;
+else if (Features.lookup("avx"))
+  return 256;
+return 128;
+  }
+  if (TargetTriple.isPPC())
+return 128;
+  if (TargetTriple.isWasm())
+return 128;
+  return 0;
+}
+
 void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop,
 MapVector AlignedVars,
 Value *IfCond, OrderKind Order,
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -502,6 +502,13 @@
ArrayRef Loops,
InsertPointTy ComputeIP);
 
+  /// Get the default alignment value for given target
+  ///
+  /// \param TargetTriple   Target triple
+  /// \param Features   StringMap which describes extra CPU features
+  static unsigned getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+const StringMap &Features);
+
 private:
   /// Modifies the canonical loop to be a statically-scheduled workshare loop.
   ///
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -500,8 +500,6 @@
   auto target_info = TargetInfo::CreateTargetInfo(
   m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
   if (log) {
-LLDB_LOGF(log, "Using SIMD alignment: %d",
-  target_info->getSimdDefaultAlign());
 LLDB_LOGF(log, "Target datalayout string: '%s'",
   target_info->getDataLayoutString());
 LLDB_LOGF(log, "Target ABI: '%s'", target_info->getABI().str().c_str());
Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -400,9 +400,6 @@
 return false;
   }
 
-  SimdDefaultAlign =
-  hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
-
   // FIXME: We should allow long double type on 32-bits to match with GCC.
   // This requires backend to be able to lower f80 without x87 first.
   if (!HasX87 && LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -49,7 +49,6 @@
 SuitableAlign = 128;
 LargeArrayMinWidth = 128;
 LargeArrayAlign = 128;
-SimdDefaultAlign = 128;
 SigAtomicType = SignedLong;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -87,7 +87,6 @@
   PPCTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple) {
 SuitableAlign = 128;
-SimdDefaultAlign = 128;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble();
 HasStrictFP = true;
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -119,7 +119,6 @@
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 0;
   MaxVectorAlign = 0;
   MaxTLSAlign = 0;
-  SimdDefaultAlign = 0;
   SizeType = UnsignedLong;
   PtrDiffType = SignedLong;
   IntMaxType = SignedLongLong;
Index: clang/lib/AST/CMakeLists.txt
==

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

2023-02-07 Thread Argyrios Kyrtzidis via Phabricator via lldb-commits 
akyrtzi added a comment.

In D141910#4111048 , @domada wrote:

> Added changes in `clang/lib/AST/CMakeLists.txt` to address build issue 
> reported by @akyrtzi .
>
> I modified CMakeLists.txt so that it requires generation of missing 
> `Attributes.inc`.
>
> @akyrtzi  Please let me know if it solves your issue

This fixes the build issue 👍, thank you!


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

https://reviews.llvm.org/D141910

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

[Lldb-commits] [PATCH] D143012: Add a bit of error checking to SBProcess::ReadMemory to check if a nullptr destination buffer was provided, return error

2023-02-07 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda updated this revision to Diff 495634.
jasonmolenda added a comment.

Update patch for Jonas' suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143012

Files:
  lldb/source/API/SBProcess.cpp
  lldb/test/API/python_api/process/TestProcessAPI.py


Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -72,6 +72,20 @@
 exe=False,
 startstr=b'x')
 
+# Try to read an impossibly large amount of memory; swig
+# will try to malloc it and fail, we should get an error 
+# result.
+error = lldb.SBError()
+content = process.ReadMemory(
+val.AddressOf().GetValueAsUnsigned(), 
+0xffe8, error)
+if error.Success():
+self.assertFalse(error.Success(), "SBProcessReadMemory claims to 
have "
+  "successfully read 0xffe8 bytes")
+if self.TraceOn():
+print("Tried to read 0xffe8 bytes, got error message: 
",
+  error.GetCString())
+
 # Read (char *)my_char_ptr.
 val = frame.FindValue("my_char_ptr", lldb.eValueTypeVariableGlobal)
 self.DebugSBValue(val)
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -802,8 +802,13 @@
  SBError &sb_error) {
   LLDB_INSTRUMENT_VA(this, addr, dst, dst_len, sb_error);
 
-  size_t bytes_read = 0;
+  if (!dst) {
+sb_error.SetErrorStringWithFormat(
+"no buffer provided to read %zu bytes into", dst_len);
+return 0;
+  }
 
+  size_t bytes_read = 0;
   ProcessSP process_sp(GetSP());
 
 


Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -72,6 +72,20 @@
 exe=False,
 startstr=b'x')
 
+# Try to read an impossibly large amount of memory; swig
+# will try to malloc it and fail, we should get an error 
+# result.
+error = lldb.SBError()
+content = process.ReadMemory(
+val.AddressOf().GetValueAsUnsigned(), 
+0xffe8, error)
+if error.Success():
+self.assertFalse(error.Success(), "SBProcessReadMemory claims to have "
+  "successfully read 0xffe8 bytes")
+if self.TraceOn():
+print("Tried to read 0xffe8 bytes, got error message: ",
+  error.GetCString())
+
 # Read (char *)my_char_ptr.
 val = frame.FindValue("my_char_ptr", lldb.eValueTypeVariableGlobal)
 self.DebugSBValue(val)
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -802,8 +802,13 @@
  SBError &sb_error) {
   LLDB_INSTRUMENT_VA(this, addr, dst, dst_len, sb_error);
 
-  size_t bytes_read = 0;
+  if (!dst) {
+sb_error.SetErrorStringWithFormat(
+"no buffer provided to read %zu bytes into", dst_len);
+return 0;
+  }
 
+  size_t bytes_read = 0;
   ProcessSP process_sp(GetSP());
 
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D143520: Add a new SBDebugger::SetDestroyCallback() API

2023-02-07 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

If I am reading the code for this patch correctly, we need the BatonSP stuff 
because we have differing callback bytes for public vs private APIs. If we 
switch to using a common callback type of:

  typedef void (*DebuggerDestroyCallback)(lldb::user_id_t &debugger_id, void 
*baton);

(we can define this both internally and leave the current 
"SBDebuggerDestroyCallback" in SBDefines.h alone), then we can avoid having to 
add any of the BatonSP stuff and just store a "void * 
m_destroy_callback_baton;" in Debugger.h.




Comment at: lldb/include/lldb/Core/Debugger.h:598-599
 
+  lldb::DebuggerDestroyCallback m_destroy_callback = nullptr;
+  lldb::BatonSP m_destroy_callback_baton_sp;
+

Why do we need the fancy BatonSP stuff here? Can't we just store this as a void 
*? Or is the fancy baton stuff required for the python for some reason? I 
couldn't see any reason by reading the code quickly, please correct me if so.



Comment at: lldb/include/lldb/lldb-types.h:71-72
 typedef void (*LogOutputCallback)(const char *, void *baton);
+typedef void (*DebuggerDestroyCallback)(lldb_private::Debugger &debugger,
+void *baton);
 typedef bool (*CommandOverrideCallback)(void *baton, const char **argv);

This can't be in the public lldb-types.h header file as no on will be able to 
use it since it uses "lldb_private::Debugger". If this is to be in the public 
header files, then this needs to change to be one of:
```
typedef void (*DebuggerDestroyCallback)(lldb::user_id_t &debugger_id, void 
*baton);
typedef void (*DebuggerDestroyCallback)(lldb::SBDebugger &debugger, void 
*baton);
```

The "debugger_id" is the easier one to do since we need to make this work with 
python so that python users can install a callback from python and have it all 
work.

This can however exist in the lldb-private-types.h which is not part of the 
public API, so it can be moved there and then it won't cause any issues. I 
realize there are other typedefs in here than mention lldb_private stuff, but 
they shouldn't be here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143520

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

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

2023-02-07 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

Very clean patch now, just a few nits about asserts!




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:129
 DWARFUnit &cu, llvm::function_ref callback) {
-  lldbassert(!cu.GetSymbolFileDWARF().GetDwoNum());
+  lldbassert(!cu.GetSymbolFileDWARF().GetFileIndex());
   uint64_t cu_offset = cu.GetOffset();

Does this assert really need to exist? Why would we not require a .dwo file 
(old code) be able to index? Can we remove this assert? It seems wrong?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:402
 DWARFUnit &unit, llvm::function_ref callback) {
-  lldbassert(!unit.GetSymbolFileDWARF().GetDwoNum());
+  lldbassert(!unit.GetSymbolFileDWARF().GetFileIndex());
   Index();

Does this assert really need to exist? Why would we not require a .dwo file 
(old code) be able to index? Can we remove this assert? It seems wrong?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp:53
 DWARFUnit &s_unit, llvm::function_ref callback) const {
-  lldbassert(!s_unit.GetSymbolFileDWARF().GetDwoNum());
+  lldbassert(!s_unit.GetSymbolFileDWARF().GetFileIndex());
   const DWARFUnit &ns_unit = s_unit.GetNonSkeletonUnit();

Does this assert really need to exist? Why would we not require a .dwo file 
(old code) be able to index? Can we remove this assert? It seems wrong?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1405
+DWARFDIE
+SymbolFileDWARF::GetDIE(lldb::user_id_t uid) {
   // This method can be called without going through the symbol vendor so we

Pavel: note we now really on "SymbolFileDWARF::GetDie(user_id_t)" to be the one 
source of truth when finding a DIE. We could make 
"SymbolFileDWARF:GetDie(DIERef ref)" be the one source of truth and then have 
"SymbolFileDWARF::GetDie(user_id_t)" just create a local DIERef and then call 
"SymbolFileDWARF:GetDie(DIERef ref)" if that would be cleaner.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1682
 SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
-  if (die_ref.dwo_num()) {
-SymbolFileDWARF *dwarf = *die_ref.dwo_num() == 0x3fff
- ? m_dwp_symfile.get()
- : this->DebugInfo()
-   .GetUnitAtIndex(*die_ref.dwo_num())
-   ->GetDwoSymbolFile();
-return dwarf->DebugInfo().GetDIE(die_ref);
-  }
-
-  return DebugInfo().GetDIE(die_ref);
+  return GetDIE(die_ref.get_id());
 }

Pavel: note we now really on "SymbolFileDWARF::GetDie(user_id_t)" to be the one 
source of truth when finding a DIE. We could make 
"SymbolFileDWARF:GetDie(DIERef ref)" be the one source of truth and then have 
"SymbolFileDWARF::GetDie(user_id_t)" just create a local DIERef and then call 
"SymbolFileDWARF:GetDie(DIERef ref)" if that would be cleaner.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:570-572
+  /// If this DWARF file a .DWO file or a DWARF .o file on mac when
+  /// no dSYM file is being used, this file index will be set to a
+  /// valid value that can be used in DIERef objects.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

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

[Lldb-commits] [lldb] 62c7475 - Check if null buffer handed to SBProcess::ReadMemory

2023-02-07 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2023-02-07T14:16:04-08:00
New Revision: 62c747517cd9a0d57f198e0fd0984f71fe75240f

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

LOG: Check if null buffer handed to SBProcess::ReadMemory

Add a check for a null destination buffer in SBProcess::ReadMemory,
and return an error if that happens.  If a Python SB API script
tries to allocate a huge amount of memory, the malloc done by the
intermediate layers will fail and will hand a null pointer to
ReadMemory.  lldb will eventually crash trying to write in to that
buffer.

Also add a test that tries to allocate an impossibly large amount
of memory, and hopefully should result in a failed malloc and hitting
this error codepath.

Differential Revision: https://reviews.llvm.org/D143012
rdar://104846609

Added: 


Modified: 
lldb/source/API/SBProcess.cpp
lldb/test/API/python_api/process/TestProcessAPI.py

Removed: 




diff  --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp
index 1a7881ccb11f2..5c8f17fa97fb1 100644
--- a/lldb/source/API/SBProcess.cpp
+++ b/lldb/source/API/SBProcess.cpp
@@ -802,8 +802,13 @@ size_t SBProcess::ReadMemory(addr_t addr, void *dst, 
size_t dst_len,
  SBError &sb_error) {
   LLDB_INSTRUMENT_VA(this, addr, dst, dst_len, sb_error);
 
-  size_t bytes_read = 0;
+  if (!dst) {
+sb_error.SetErrorStringWithFormat(
+"no buffer provided to read %zu bytes into", dst_len);
+return 0;
+  }
 
+  size_t bytes_read = 0;
   ProcessSP process_sp(GetSP());
 
 

diff  --git a/lldb/test/API/python_api/process/TestProcessAPI.py 
b/lldb/test/API/python_api/process/TestProcessAPI.py
index cf05335b23840..36291fcc66b8a 100644
--- a/lldb/test/API/python_api/process/TestProcessAPI.py
+++ b/lldb/test/API/python_api/process/TestProcessAPI.py
@@ -72,6 +72,20 @@ def test_read_memory(self):
 exe=False,
 startstr=b'x')
 
+# Try to read an impossibly large amount of memory; swig
+# will try to malloc it and fail, we should get an error 
+# result.
+error = lldb.SBError()
+content = process.ReadMemory(
+val.AddressOf().GetValueAsUnsigned(), 
+0xffe8, error)
+if error.Success():
+self.assertFalse(error.Success(), "SBProcessReadMemory claims to 
have "
+  "successfully read 0xffe8 bytes")
+if self.TraceOn():
+print("Tried to read 0xffe8 bytes, got error message: 
",
+  error.GetCString())
+
 # Read (char *)my_char_ptr.
 val = frame.FindValue("my_char_ptr", lldb.eValueTypeVariableGlobal)
 self.DebugSBValue(val)



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

[Lldb-commits] [PATCH] D143012: Add a bit of error checking to SBProcess::ReadMemory to check if a nullptr destination buffer was provided, return error

2023-02-07 Thread Jason Molenda via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG62c747517cd9: Check if null buffer handed to 
SBProcess::ReadMemory (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143012

Files:
  lldb/source/API/SBProcess.cpp
  lldb/test/API/python_api/process/TestProcessAPI.py


Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -72,6 +72,20 @@
 exe=False,
 startstr=b'x')
 
+# Try to read an impossibly large amount of memory; swig
+# will try to malloc it and fail, we should get an error 
+# result.
+error = lldb.SBError()
+content = process.ReadMemory(
+val.AddressOf().GetValueAsUnsigned(), 
+0xffe8, error)
+if error.Success():
+self.assertFalse(error.Success(), "SBProcessReadMemory claims to 
have "
+  "successfully read 0xffe8 bytes")
+if self.TraceOn():
+print("Tried to read 0xffe8 bytes, got error message: 
",
+  error.GetCString())
+
 # Read (char *)my_char_ptr.
 val = frame.FindValue("my_char_ptr", lldb.eValueTypeVariableGlobal)
 self.DebugSBValue(val)
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -802,8 +802,13 @@
  SBError &sb_error) {
   LLDB_INSTRUMENT_VA(this, addr, dst, dst_len, sb_error);
 
-  size_t bytes_read = 0;
+  if (!dst) {
+sb_error.SetErrorStringWithFormat(
+"no buffer provided to read %zu bytes into", dst_len);
+return 0;
+  }
 
+  size_t bytes_read = 0;
   ProcessSP process_sp(GetSP());
 
 


Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -72,6 +72,20 @@
 exe=False,
 startstr=b'x')
 
+# Try to read an impossibly large amount of memory; swig
+# will try to malloc it and fail, we should get an error 
+# result.
+error = lldb.SBError()
+content = process.ReadMemory(
+val.AddressOf().GetValueAsUnsigned(), 
+0xffe8, error)
+if error.Success():
+self.assertFalse(error.Success(), "SBProcessReadMemory claims to have "
+  "successfully read 0xffe8 bytes")
+if self.TraceOn():
+print("Tried to read 0xffe8 bytes, got error message: ",
+  error.GetCString())
+
 # Read (char *)my_char_ptr.
 val = frame.FindValue("my_char_ptr", lldb.eValueTypeVariableGlobal)
 self.DebugSBValue(val)
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -802,8 +802,13 @@
  SBError &sb_error) {
   LLDB_INSTRUMENT_VA(this, addr, dst, dst_len, sb_error);
 
-  size_t bytes_read = 0;
+  if (!dst) {
+sb_error.SetErrorStringWithFormat(
+"no buffer provided to read %zu bytes into", dst_len);
+return 0;
+  }
 
+  size_t bytes_read = 0;
   ProcessSP process_sp(GetSP());
 
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 4a8cc28 - Fix TestProcessAPI.py to only allocate sys.maxsize buffer

2023-02-07 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2023-02-07T16:05:24-08:00
New Revision: 4a8cc285e9f3252ca31941dccfddf8fd10c9911b

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

LOG: Fix TestProcessAPI.py to only allocate sys.maxsize buffer

I hardcoded nearly a UINT64_MAX number in this test case,
and python is not able to convert it to a long on some
platforms.  Use sys.maxsize instead; this also would have
failed if the testsuite was run on a 32-bit system.

Added: 


Modified: 
lldb/test/API/python_api/process/TestProcessAPI.py

Removed: 




diff  --git a/lldb/test/API/python_api/process/TestProcessAPI.py 
b/lldb/test/API/python_api/process/TestProcessAPI.py
index 36291fcc66b8..66f438a24f5b 100644
--- a/lldb/test/API/python_api/process/TestProcessAPI.py
+++ b/lldb/test/API/python_api/process/TestProcessAPI.py
@@ -3,6 +3,7 @@
 """
 
 import lldb
+import sys
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test.lldbutil import get_stopped_thread, state_type_to_str
@@ -76,15 +77,16 @@ def test_read_memory(self):
 # will try to malloc it and fail, we should get an error 
 # result.
 error = lldb.SBError()
+bigsize = sys.maxsize - 8;
 content = process.ReadMemory(
 val.AddressOf().GetValueAsUnsigned(), 
-0xffe8, error)
+bigsize, error)
 if error.Success():
 self.assertFalse(error.Success(), "SBProcessReadMemory claims to 
have "
-  "successfully read 0xffe8 bytes")
+  "successfully read 0x%x bytes" % bigsize)
 if self.TraceOn():
-print("Tried to read 0xffe8 bytes, got error message: 
",
-  error.GetCString())
+print("Tried to read 0x%x bytes, got error message: %s" %
+  (bigsize, error.GetCString()))
 
 # Read (char *)my_char_ptr.
 val = frame.FindValue("my_char_ptr", lldb.eValueTypeVariableGlobal)



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

[Lldb-commits] [lldb] b1d8f40 - Only run the weird new try-to-read-too-much test on Darwin

2023-02-07 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2023-02-07T16:05:24-08:00
New Revision: b1d8f40484dfcb28b19c83aa33a674308b17e5dc

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

LOG: Only run the weird new try-to-read-too-much test on Darwin

I'm still getting linux CI bot failures for this test.  It's not
critical, and it depends on a failure mode that is true on Darwin
but I was always gambling that it might fail in the same way on
other systems.

Added: 


Modified: 
lldb/test/API/python_api/process/TestProcessAPI.py

Removed: 




diff  --git a/lldb/test/API/python_api/process/TestProcessAPI.py 
b/lldb/test/API/python_api/process/TestProcessAPI.py
index 66f438a24f5b..afcc9d7cdee0 100644
--- a/lldb/test/API/python_api/process/TestProcessAPI.py
+++ b/lldb/test/API/python_api/process/TestProcessAPI.py
@@ -73,20 +73,21 @@ def test_read_memory(self):
 exe=False,
 startstr=b'x')
 
-# Try to read an impossibly large amount of memory; swig
-# will try to malloc it and fail, we should get an error 
-# result.
-error = lldb.SBError()
-bigsize = sys.maxsize - 8;
-content = process.ReadMemory(
-val.AddressOf().GetValueAsUnsigned(), 
-bigsize, error)
-if error.Success():
-self.assertFalse(error.Success(), "SBProcessReadMemory claims to 
have "
-  "successfully read 0x%x bytes" % bigsize)
-if self.TraceOn():
-print("Tried to read 0x%x bytes, got error message: %s" %
-  (bigsize, error.GetCString()))
+if self.platformIsDarwin():
+  # Try to read an impossibly large amount of memory; swig
+  # will try to malloc it and fail, we should get an error 
+  # result.
+  error = lldb.SBError()
+  bigsize = sys.maxsize - 8;
+  content = process.ReadMemory(
+  val.AddressOf().GetValueAsUnsigned(), 
+  bigsize, error)
+  if error.Success():
+  self.assertFalse(error.Success(), "SBProcessReadMemory claims to 
have "
+"successfully read 0x%x bytes" % bigsize)
+  if self.TraceOn():
+  print("Tried to read 0x%x bytes, got error message: %s" %
+(bigsize, error.GetCString()))
 
 # Read (char *)my_char_ptr.
 val = frame.FindValue("my_char_ptr", lldb.eValueTypeVariableGlobal)



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

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

2023-02-07 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda updated this revision to Diff 495686.
jasonmolenda added a comment.

Went over the rewrite one more time, fixing up comments, fixing a couple of 
logic bugs I wrote, and having tested it with a debugserver I hacked to remove 
the `watchpoint_exceptions_received:before;` key in the qHostInfo reply packet, 
so lldb had to use its knowledge of the target system to determine this.

I have changed ProcessWindows and can't build that to ensure it's correct, but 
the changes are tiny and I'll keep an eye on the bots when I land it.

I think this is ready for review now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/API/SBProcess.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
  lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Process.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -790,19 +790,18 @@
 }
 
 static bool CheckIfWatchpointsSupported(Target *target, Status &error) {
-  uint32_t num_supported_hardware_watchpoints;
-  Status rc = target->GetProcessSP()->GetWatchpointSupportInfo(
-  num_supported_hardware_watchpoints);
+  std::optional num_supported_hardware_watchpoints =
+  target->GetProcessSP()->GetWatchpointSlotCount();
 
   // If unable to determine the # of watchpoints available,
   // assume they are supported.
-  if (rc.Fail())
+  if (!num_supported_hardware_watchpoints)
 return true;
 
   if (num_supported_hardware_watchpoints == 0) {
 error.SetErrorStringWithFormat(
 "Target supports (%u) hardware watchpoint slots.\n",
-num_supported_hardware_watchpoints);
+*num_supported_hardware_watchpoints);
 return false;
   }
   return true;
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -823,16 +823,8 @@
 // stop
 
 ProcessSP process_sp = exe_ctx.GetProcessSP();
-uint32_t num;
-bool wp_triggers_after;
+bool wp_triggers_after = process_sp->GetWatchpointReportedAfter();
 
-if (!process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
-.Success()) {
-  m_should_stop_is_valid = true;
-  m_should_stop = true;
-  return m_should_stop;
-}
-
 if (!wp_triggers_after) {
   // We have to step over the watchpoint before we know what to do:   
   StopInfoWatchpointSP me_as_siwp_sp 
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2355,6 +2355,24 @@
   return error;
 }
 
+bool Process::GetWatchpointReportedAfter() {
+  std::optional subclass_override = DoGetWatchpointReportedAfter();
+  if (subclass_override) {
+return *subclass_override;
+  }
+  bool reported_after = true;
+  const ArchSpec &arch = GetTarget().GetArchitecture();
+  if (!arch.IsValid()) {
+return reported_after;
+  }
+  llvm::Triple triple = arch.GetTriple();
+  if (triple.isMIPS() || triple.isPPC64())
+reported_after = false;
+  if (triple.isAArch64() || triple.isArmMClass() || triple.isARM())
+reported_after = false;
+  return reported_after;
+}
+
 ModuleSP Process::ReadModuleFromMemory(const FileSpec &file_spec,
lldb::addr_t header_addr,
size_t size_to_read) {
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
@@ -159,7 +159,7 @@
 
   Status DisableWatchpoint(Watchpoint *wp, bool notify = true) override;
 
-  Status GetWatchpointSupportInfo(uint32_t &num) override;
+  std::optional GetWatchpointSlotCount() override;
 
   llvm::Expected TraceSupported() override;
 
@@ -172,7 +172,7 @@
   llvm::Expected>
   TraceGetBinaryData(const TraceGetBinaryDataRequest &request) override;
 
-  Status GetWatchpointSupportInfo(uint32_t &num, bool &after) override;
+  std::optional DoGetWatchpointReportedAfter() override;
 
   bool StartNoticingNewThreads() override;
 
Index: lldb/source/Plugins/P

[Lldb-commits] [PATCH] D135631: [lldb] Copy log files into diagnostic directory

2023-02-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 495693.
JDevlieghere marked an inline comment as done.

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

https://reviews.llvm.org/D135631

Files:
  lldb/source/Core/Debugger.cpp
  lldb/test/Shell/Diagnostics/Inputs/TestCopyLogs.in
  lldb/test/Shell/Diagnostics/TestCopyLogs.test


Index: lldb/test/Shell/Diagnostics/TestCopyLogs.test
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/TestCopyLogs.test
@@ -0,0 +1,7 @@
+# RUN: rm -rf %t
+# RUN: mkdir -p %t
+
+# RUN: %lldb -s %S/Inputs/TestCopyLogs.in -o 'logcommands -f %t/commands.log' 
-o 'diagnostics dump -d %t/diags'
+
+# RUN: cat %t/diags/commands.log | FileCheck %s
+# CHECK: Processing command: diagnostics dump
Index: lldb/test/Shell/Diagnostics/Inputs/TestCopyLogs.in
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/Inputs/TestCopyLogs.in
@@ -0,0 +1 @@
+command alias logcommands log enable lldb commands
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -828,6 +828,22 @@
   if (!GetOutputFile().GetIsTerminalWithColors())
 SetUseColor(false);
 
+  if (Diagnostics::Enabled()) {
+Diagnostics::Instance().AddCallback(
+[&](const FileSpec &dir) -> llvm::Error {
+  for (auto &entry : m_stream_handlers) {
+llvm::StringRef log_path = entry.first();
+llvm::StringRef file_name = llvm::sys::path::filename(log_path);
+FileSpec destination = dir.CopyByAppendingPathComponent(file_name);
+std::error_code ec =
+llvm::sys::fs::copy_file(entry.first(), destination.GetPath());
+if (ec)
+  return llvm::errorCodeToError(ec);
+  }
+  return llvm::Error::success();
+});
+  }
+
 #if defined(_WIN32) && defined(ENABLE_VIRTUAL_TERMINAL_PROCESSING)
   // Enabling use of ANSI color codes because LLDB is using them to highlight
   // text.


Index: lldb/test/Shell/Diagnostics/TestCopyLogs.test
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/TestCopyLogs.test
@@ -0,0 +1,7 @@
+# RUN: rm -rf %t
+# RUN: mkdir -p %t
+
+# RUN: %lldb -s %S/Inputs/TestCopyLogs.in -o 'logcommands -f %t/commands.log' -o 'diagnostics dump -d %t/diags'
+
+# RUN: cat %t/diags/commands.log | FileCheck %s
+# CHECK: Processing command: diagnostics dump
Index: lldb/test/Shell/Diagnostics/Inputs/TestCopyLogs.in
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/Inputs/TestCopyLogs.in
@@ -0,0 +1 @@
+command alias logcommands log enable lldb commands
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -828,6 +828,22 @@
   if (!GetOutputFile().GetIsTerminalWithColors())
 SetUseColor(false);
 
+  if (Diagnostics::Enabled()) {
+Diagnostics::Instance().AddCallback(
+[&](const FileSpec &dir) -> llvm::Error {
+  for (auto &entry : m_stream_handlers) {
+llvm::StringRef log_path = entry.first();
+llvm::StringRef file_name = llvm::sys::path::filename(log_path);
+FileSpec destination = dir.CopyByAppendingPathComponent(file_name);
+std::error_code ec =
+llvm::sys::fs::copy_file(entry.first(), destination.GetPath());
+if (ec)
+  return llvm::errorCodeToError(ec);
+  }
+  return llvm::Error::success();
+});
+  }
+
 #if defined(_WIN32) && defined(ENABLE_VIRTUAL_TERMINAL_PROCESSING)
   // Enabling use of ANSI color codes because LLDB is using them to highlight
   // text.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141425: [lldb] Add --gdb-format flag to dwim-print

2023-02-07 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione updated this revision to Diff 495694.
kastiglione added a comment.

Add test with standard lldb flags


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141425

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectDWIMPrint.h
  lldb/source/Interpreter/OptionGroupFormat.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -27,22 +27,37 @@
 before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
 return re.escape(before) + r"\$\d+" + re.escape(after)
 
-def _expect_cmd(self, expr: str, base_cmd: str) -> None:
+def _expect_cmd(
+self,
+dwim_cmd: str,
+actual_cmd: str,
+) -> None:
 """Run dwim-print and verify the output against the expected command."""
-cmd = f"{base_cmd} {expr}"
-cmd_output = self._run_cmd(cmd)
+# Resolve the dwim-print command to either `expression` or `frame variable`.
+substitute_cmd = dwim_cmd.replace("dwim-print", actual_cmd, 1)
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+interp.ResolveCommand(substitute_cmd, result)
+if not result.Succeeded():
+raise RuntimeError(result.GetError())
+
+resolved_cmd = result.GetOutput()
+if actual_cmd == "frame variable":
+resolved_cmd = resolved_cmd.replace(" -- ", " ", 1)
+
+expected_output = self._run_cmd(resolved_cmd)
 
 # Verify dwim-print chose the expected command.
 self.runCmd("settings set dwim-print-verbosity full")
-substrs = [f"note: ran `{cmd}`"]
+substrs = [f"note: ran `{resolved_cmd}`"]
 patterns = []
 
-if base_cmd == "expression --" and self.PERSISTENT_VAR.search(cmd_output):
-patterns.append(self._mask_persistent_var(cmd_output))
+if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+patterns.append(self._mask_persistent_var(expected_output))
 else:
-substrs.append(cmd_output)
+substrs.append(expected_output)
 
-self.expect(f"dwim-print {expr}", substrs=substrs, patterns=patterns)
+self.expect(dwim_cmd, substrs=substrs, patterns=patterns)
 
 def test_variables(self):
 """Test dwim-print with variables."""
@@ -50,7 +65,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 vars = ("argc", "argv")
 for var in vars:
-self._expect_cmd(var, "frame variable")
+self._expect_cmd(f"dwim-print {var}", "frame variable")
 
 def test_variable_paths(self):
 """Test dwim-print with variable path expressions."""
@@ -58,7 +73,7 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("&argc", "*argv", "argv[0]")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_expressions(self):
 """Test dwim-print with expressions."""
@@ -66,8 +81,20 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("argc + 1", "(void)argc", "(int)abs(argc)")
 for expr in exprs:
-self._expect_cmd(expr, "expression --")
+self._expect_cmd(f"dwim-print {expr}", "expression")
 
 def test_dummy_target_expressions(self):
 """Test dwim-print's ability to evaluate expressions without a target."""
-self._expect_cmd("1 + 2", "expression --")
+self._expect_cmd("dwim-print 1 + 2", "expression")
+
+def test_gdb_format(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print/x argc", "frame variable")
+self._expect_cmd(f"dwim-print/x argc + 1", "expression")
+
+def test_flags(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print -fx -- argc", "frame variable")
+self._expect_cmd(f"dwim-print -fx -- argc + 1", "expression")
Index: lldb/source/Interpreter/OptionGroupFormat.cpp
===
--- lldb/source/Interpreter/OptionGroupFormat.cpp
+++ lldb/source/Interpreter/OptionGroupFormat.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Target.h"
+#include "lldb/lldb-enumerations.h"
 
 using namespace lldb;
 using namespace lldb_private;
Index: lldb/source/Commands/CommandObjectDWIMPrint.h
===
--- lldb/source/Commands/Co

[Lldb-commits] [PATCH] D142926: [lldb][RFC] Replace SB swig interfaces with API headers

2023-02-07 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord updated this revision to Diff 495695.
bulbazord added a comment.
Herald added a reviewer: jdoerfert.
Herald added subscribers: Michael137, sstefan1.

First pass at a complete implementation. Actions taken:

- Added swig flags to ignore swig warnings indicating that swig is ignoring 
certain operator overloads (e.g. operator=)
- Broke SB${Class}.i files into SB${Class}Docstrings.i (containing the 
docstrings) and SB${Class}Extensions.i (containing additional python 
functionality)
- Deleted all SB${Class}.i files.
- Moved necessary typemaps into python-typemaps.swig.
- Audited all SB class headers and add preprocessor macros to ignore any 
functions and methods that were not implemented in the SB${Class}.i files.
  - This is just to make this change more "atomic" (i.e. not adding any new 
functionality)

I can imagine that folks will have some opinions about this patch. It is pretty 
large, contains a few
"warts" (some of the swig header guards aren't very pretty), and there are 
probably better ways to do some things.
I'd therefore like to start getting feedback on this patch now that it works on 
my machine.
I've run the test suite on my macOS machine which I'm sure won't run every 
single test in every single configuration that
get run on the bots so I expect there to be some amount of churn even if this 
is approved and lands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

Files:
  lldb/bindings/CMakeLists.txt
  lldb/bindings/interface/SBAddress.i
  lldb/bindings/interface/SBAddressDocstrings.i
  lldb/bindings/interface/SBAddressExtensions.i
  lldb/bindings/interface/SBAttachInfo.i
  lldb/bindings/interface/SBAttachInfoDocstrings.i
  lldb/bindings/interface/SBBlock.i
  lldb/bindings/interface/SBBlockDocstrings.i
  lldb/bindings/interface/SBBlockExtensions.i
  lldb/bindings/interface/SBBreakpoint.i
  lldb/bindings/interface/SBBreakpointDocstrings.i
  lldb/bindings/interface/SBBreakpointExtensions.i
  lldb/bindings/interface/SBBreakpointLocation.i
  lldb/bindings/interface/SBBreakpointLocationDocstrings.i
  lldb/bindings/interface/SBBreakpointLocationExtensions.i
  lldb/bindings/interface/SBBreakpointName.i
  lldb/bindings/interface/SBBreakpointNameDocstrings.i
  lldb/bindings/interface/SBBreakpointNameExtensions.i
  lldb/bindings/interface/SBBroadcaster.i
  lldb/bindings/interface/SBBroadcasterDocstrings.i
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/bindings/interface/SBCommandInterpreterDocstrings.i
  lldb/bindings/interface/SBCommandInterpreterRunOptions.i
  lldb/bindings/interface/SBCommandInterpreterRunOptionsDocstrings.i
  lldb/bindings/interface/SBCommandReturnObject.i
  lldb/bindings/interface/SBCommandReturnObjectDocstrings.i
  lldb/bindings/interface/SBCommandReturnObjectExtensions.i
  lldb/bindings/interface/SBCommunication.i
  lldb/bindings/interface/SBCommunicationDocstrings.i
  lldb/bindings/interface/SBCompileUnit.i
  lldb/bindings/interface/SBCompileUnitDocstrings.i
  lldb/bindings/interface/SBCompileUnitExtensions.i
  lldb/bindings/interface/SBData.i
  lldb/bindings/interface/SBDataDocstrings.i
  lldb/bindings/interface/SBDataExtensions.i
  lldb/bindings/interface/SBDebugger.i
  lldb/bindings/interface/SBDebuggerDocstrings.i
  lldb/bindings/interface/SBDebuggerExtensions.i
  lldb/bindings/interface/SBDeclaration.i
  lldb/bindings/interface/SBDeclarationDocstrings.i
  lldb/bindings/interface/SBDeclarationExtensions.i
  lldb/bindings/interface/SBEnvironment.i
  lldb/bindings/interface/SBEnvironmentDocstrings.i
  lldb/bindings/interface/SBError.i
  lldb/bindings/interface/SBErrorDocstrings.i
  lldb/bindings/interface/SBErrorExtensions.i
  lldb/bindings/interface/SBEvent.i
  lldb/bindings/interface/SBEventDocstrings.i
  lldb/bindings/interface/SBExecutionContext.i
  lldb/bindings/interface/SBExecutionContextDocstrings.i
  lldb/bindings/interface/SBExecutionContextExtensions.i
  lldb/bindings/interface/SBExpressionOptions.i
  lldb/bindings/interface/SBExpressionOptionsDocstrings.i
  lldb/bindings/interface/SBFile.i
  lldb/bindings/interface/SBFileDocstrings.i
  lldb/bindings/interface/SBFileExtensions.i
  lldb/bindings/interface/SBFileSpec.i
  lldb/bindings/interface/SBFileSpecDocstrings.i
  lldb/bindings/interface/SBFileSpecExtensions.i
  lldb/bindings/interface/SBFileSpecList.i
  lldb/bindings/interface/SBFileSpecListDocstrings.i
  lldb/bindings/interface/SBFrame.i
  lldb/bindings/interface/SBFrameDocstrings.i
  lldb/bindings/interface/SBFrameExtensions.i
  lldb/bindings/interface/SBFunction.i
  lldb/bindings/interface/SBFunctionDocstrings.i
  lldb/bindings/interface/SBFunctionExtensions.i
  lldb/bindings/interface/SBHostOS.i
  lldb/bindings/interface/SBHostOSDocstrings.i
  lldb/bindings/interface/SBInstruction.i
  lldb/bindings/interface/SBInstructionDocstrings.i
  lldb/bindings/interface/SBInstructionExtensions.i
  lldb/bindings/interface/SBInstructionList.i
  lldb/bindings/

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

2023-02-07 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord added a comment.

In addition to what I wrote above, I also fixed several documentation bugs 
(putting docstrings on the wrong method).

I manually audited the generated code before and after this change. Here are my 
notes:

Not too interesting:

- Some parameter names changed
- Fixed lots of documentation bugs
- Some documentation changed because of a re-ordering of function declarations

Potentially interesting:

- SBData::GetDescription base_addr parameter has default value now
- SBInstructionList::GetInstructionsCount canSetBreakpoint has default value now
- SBMemoryRegionInfo::SBMemoryRegionInfo 3rd constructor parameter stack_memory 
has default value now

Certainly interesting:

- SBListener::StopListeningForEventClass return type conflicts (ABI break?)
- SBProcess::GetQueueAtIndex parameter types conflict (ABI break?)
- SBTarget::BreakpointCreateByNames first parameter is different: `const char 
**symbol_name` vs `const char *symbol_name[]`. I'm not sure if this is going to 
be an issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

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

[Lldb-commits] [PATCH] D142926: [lldb] Replace SB swig interfaces with API headers

2023-02-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

I can see how having parity between the signatures that were exposed in the 
interface files makes the verification easier, but do we actually want to land 
the patch like that? Does anyone care that we expose more?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142926

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

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

2023-02-07 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added a comment.

If I was going to describe this patch briefly at this point, I would say: 
"Change lldb from depending on the remote gdb stub to tell it whether 
watchpoints are received before or after the instruction, to knowing the 
architectures where watchpoint exceptions are received before the instruction 
executes, and allow the remote gdb stub to override this behavior in its 
qHostInfo response".  The rest is mechanical change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215

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

[Lldb-commits] [PATCH] D143548: [lldb] Add the ability to remove diagnostic callbacks

2023-02-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added reviewers: mib, bulbazord, labath.
Herald added a project: All.
JDevlieghere requested review of this revision.

Add the ability to remove diagnostic callbacks.


https://reviews.llvm.org/D143548

Files:
  lldb/include/lldb/Utility/Diagnostics.h
  lldb/source/Utility/Diagnostics.cpp


Index: lldb/source/Utility/Diagnostics.cpp
===
--- lldb/source/Utility/Diagnostics.cpp
+++ lldb/source/Utility/Diagnostics.cpp
@@ -43,9 +43,19 @@
 
 Diagnostics::~Diagnostics() {}
 
-void Diagnostics::AddCallback(Callback callback) {
+Diagnostics::CallbackID Diagnostics::AddCallback(Callback callback) {
   std::lock_guard guard(m_callbacks_mutex);
-  m_callbacks.push_back(callback);
+  CallbackID id = m_callback_id++;
+  m_callbacks.emplace_back(id, callback);
+  return id;
+}
+
+void Diagnostics::RemoveCallback(CallbackID id) {
+  std::lock_guard guard(m_callbacks_mutex);
+  m_callbacks.erase(
+  std::remove_if(m_callbacks.begin(), m_callbacks.end(),
+ [id](const CallbackEntry &e) { return e.id == id; }),
+  m_callbacks.end());
 }
 
 bool Diagnostics::Dump(raw_ostream &stream) {
@@ -84,8 +94,8 @@
   if (Error err = DumpDiangosticsLog(dir))
 return err;
 
-  for (Callback c : m_callbacks) {
-if (Error err = c(dir))
+  for (CallbackEntry e : m_callbacks) {
+if (Error err = e.callback(dir))
   return err;
   }
 
Index: lldb/include/lldb/Utility/Diagnostics.h
===
--- lldb/include/lldb/Utility/Diagnostics.h
+++ lldb/include/lldb/Utility/Diagnostics.h
@@ -42,8 +42,10 @@
   void Report(llvm::StringRef message);
 
   using Callback = std::function;
+  using CallbackID = uint64_t;
 
-  void AddCallback(Callback callback);
+  CallbackID AddCallback(Callback callback);
+  void RemoveCallback(CallbackID id);
 
   static Diagnostics &Instance();
 
@@ -61,7 +63,21 @@
 
   RotatingLogHandler m_log_handler;
 
-  llvm::SmallVector m_callbacks;
+  struct CallbackEntry {
+CallbackEntry(CallbackID id, Callback callback)
+: id(id), callback(std::move(callback)) {}
+CallbackID id;
+Callback callback;
+  };
+
+  /// Monotonically increasing callback identifier. Unique per Diagnostic
+  /// instance.
+  CallbackID m_callback_id;
+
+  /// List of callback entries.
+  llvm::SmallVector m_callbacks;
+
+  /// Mutex to protect callback list and callback identifier.
   std::mutex m_callbacks_mutex;
 };
 


Index: lldb/source/Utility/Diagnostics.cpp
===
--- lldb/source/Utility/Diagnostics.cpp
+++ lldb/source/Utility/Diagnostics.cpp
@@ -43,9 +43,19 @@
 
 Diagnostics::~Diagnostics() {}
 
-void Diagnostics::AddCallback(Callback callback) {
+Diagnostics::CallbackID Diagnostics::AddCallback(Callback callback) {
   std::lock_guard guard(m_callbacks_mutex);
-  m_callbacks.push_back(callback);
+  CallbackID id = m_callback_id++;
+  m_callbacks.emplace_back(id, callback);
+  return id;
+}
+
+void Diagnostics::RemoveCallback(CallbackID id) {
+  std::lock_guard guard(m_callbacks_mutex);
+  m_callbacks.erase(
+  std::remove_if(m_callbacks.begin(), m_callbacks.end(),
+ [id](const CallbackEntry &e) { return e.id == id; }),
+  m_callbacks.end());
 }
 
 bool Diagnostics::Dump(raw_ostream &stream) {
@@ -84,8 +94,8 @@
   if (Error err = DumpDiangosticsLog(dir))
 return err;
 
-  for (Callback c : m_callbacks) {
-if (Error err = c(dir))
+  for (CallbackEntry e : m_callbacks) {
+if (Error err = e.callback(dir))
   return err;
   }
 
Index: lldb/include/lldb/Utility/Diagnostics.h
===
--- lldb/include/lldb/Utility/Diagnostics.h
+++ lldb/include/lldb/Utility/Diagnostics.h
@@ -42,8 +42,10 @@
   void Report(llvm::StringRef message);
 
   using Callback = std::function;
+  using CallbackID = uint64_t;
 
-  void AddCallback(Callback callback);
+  CallbackID AddCallback(Callback callback);
+  void RemoveCallback(CallbackID id);
 
   static Diagnostics &Instance();
 
@@ -61,7 +63,21 @@
 
   RotatingLogHandler m_log_handler;
 
-  llvm::SmallVector m_callbacks;
+  struct CallbackEntry {
+CallbackEntry(CallbackID id, Callback callback)
+: id(id), callback(std::move(callback)) {}
+CallbackID id;
+Callback callback;
+  };
+
+  /// Monotonically increasing callback identifier. Unique per Diagnostic
+  /// instance.
+  CallbackID m_callback_id;
+
+  /// List of callback entries.
+  llvm::SmallVector m_callbacks;
+
+  /// Mutex to protect callback list and callback identifier.
   std::mutex m_callbacks_mutex;
 };
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D135631: [lldb] Copy log files into diagnostic directory

2023-02-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 495715.
JDevlieghere added a comment.

Remove callback in dtor


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

https://reviews.llvm.org/D135631

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/Core/Debugger.cpp
  lldb/test/Shell/Diagnostics/Inputs/TestCopyLogs.in
  lldb/test/Shell/Diagnostics/TestCopyLogs.test


Index: lldb/test/Shell/Diagnostics/TestCopyLogs.test
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/TestCopyLogs.test
@@ -0,0 +1,7 @@
+# RUN: rm -rf %t
+# RUN: mkdir -p %t
+
+# RUN: %lldb -s %S/Inputs/TestCopyLogs.in -o 'logcommands -f %t/commands.log' 
-o 'diagnostics dump -d %t/diags'
+
+# RUN: cat %t/diags/commands.log | FileCheck %s
+# CHECK: Processing command: diagnostics dump
Index: lldb/test/Shell/Diagnostics/Inputs/TestCopyLogs.in
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/Inputs/TestCopyLogs.in
@@ -0,0 +1 @@
+command alias logcommands log enable lldb commands
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -44,7 +44,6 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Utility/AnsiTerminal.h"
-#include "lldb/Utility/Diagnostics.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Listener.h"
@@ -828,6 +827,22 @@
   if (!GetOutputFile().GetIsTerminalWithColors())
 SetUseColor(false);
 
+  if (Diagnostics::Enabled()) {
+m_diagnostics_callback_id = Diagnostics::Instance().AddCallback(
+[this](const FileSpec &dir) -> llvm::Error {
+  for (auto &entry : m_stream_handlers) {
+llvm::StringRef log_path = entry.first();
+llvm::StringRef file_name = llvm::sys::path::filename(log_path);
+FileSpec destination = dir.CopyByAppendingPathComponent(file_name);
+std::error_code ec =
+llvm::sys::fs::copy_file(entry.first(), destination.GetPath());
+if (ec)
+  return llvm::errorCodeToError(ec);
+  }
+  return llvm::Error::success();
+});
+  }
+
 #if defined(_WIN32) && defined(ENABLE_VIRTUAL_TERMINAL_PROCESSING)
   // Enabling use of ANSI color codes because LLDB is using them to highlight
   // text.
@@ -866,6 +881,9 @@
 GetInputFile().Close();
 
 m_command_interpreter_up->Clear();
+
+if (Diagnostics::Enabled())
+  Diagnostics::Instance().RemoveCallback(m_diagnostics_callback_id);
   });
 }
 
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -31,6 +31,7 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/UserID.h"
+#include "lldb/Utility/Diagnostics.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
@@ -589,6 +590,7 @@
   lldb::ListenerSP m_forward_listener_sp;
   llvm::once_flag m_clear_once;
   lldb::TargetSP m_dummy_target_sp;
+  Diagnostics::CallbackID m_diagnostics_callback_id;
 
   // Events for m_sync_broadcaster
   enum {


Index: lldb/test/Shell/Diagnostics/TestCopyLogs.test
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/TestCopyLogs.test
@@ -0,0 +1,7 @@
+# RUN: rm -rf %t
+# RUN: mkdir -p %t
+
+# RUN: %lldb -s %S/Inputs/TestCopyLogs.in -o 'logcommands -f %t/commands.log' -o 'diagnostics dump -d %t/diags'
+
+# RUN: cat %t/diags/commands.log | FileCheck %s
+# CHECK: Processing command: diagnostics dump
Index: lldb/test/Shell/Diagnostics/Inputs/TestCopyLogs.in
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/Inputs/TestCopyLogs.in
@@ -0,0 +1 @@
+command alias logcommands log enable lldb commands
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -44,7 +44,6 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Utility/AnsiTerminal.h"
-#include "lldb/Utility/Diagnostics.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Listener.h"
@@ -828,6 +827,22 @@
   if (!GetOutputFile().GetIsTerminalWithColors())
 SetUseColor(false);
 
+  if (Diagnostics::Enabled()) {
+m_diagnostics_callback_id = Diagnostics::Instance().AddCallback(
+[this](const FileSpec &dir) -> llvm::Error {
+  for (auto &entry : m_stream_handlers) {
+llvm::StringRef log_path = entry.first();
+llvm::StringRef file_name = llvm::sys::path::filename(log_path);
+FileSpe

[Lldb-commits] [PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

2023-02-07 Thread Erik Desjardins via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd768b97424f9: [Support] change StringMap hash function from 
djbHash to xxHash (authored by erikdesjardins).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142862

Files:
  clang-tools-extra/test/modularize/ProblemsDisplayLists.modularize
  clang/unittests/Basic/SarifTest.cpp
  compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
  llvm/lib/Support/StringMap.cpp
  llvm/test/DebugInfo/Generic/accel-table-hash-collisions.ll
  llvm/test/DebugInfo/Generic/debug-names-hash-collisions.ll
  llvm/test/DebugInfo/X86/debug-pubtables-dwarf64.ll
  llvm/test/DebugInfo/X86/gnu-public-names-gmlt.ll
  llvm/test/DebugInfo/X86/gnu-public-names.ll
  llvm/test/tools/dsymutil/ARM/extern-alias.test
  llvm/test/tools/llvm-profdata/suppl-instr-with-sample.test
  mlir/test/mlir-lsp-server/completion.test

Index: mlir/test/mlir-lsp-server/completion.test
===
--- mlir/test/mlir-lsp-server/completion.test
+++ mlir/test/mlir-lsp-server/completion.test
@@ -84,18 +84,18 @@
 // CHECK-NEXT:"isIncomplete": false,
 // CHECK-NEXT:"items": [
 // CHECK-NEXT:  {
-// CHECK-NEXT:"detail": "builtin.unrealized_conversion_cast: !pdl.value",
-// CHECK-NEXT:"insertText": "cast",
+// CHECK-NEXT:"detail": "arg #0: i32",
+// CHECK-NEXT:"insertText": "arg",
 // CHECK-NEXT:"insertTextFormat": 1,
 // CHECK-NEXT:"kind": 6,
-// CHECK-NEXT:"label": "%cast"
+// CHECK-NEXT:"label": "%arg"
 // CHECK-NEXT:  },
 // CHECK-NEXT:  {
-// CHECK-NEXT:"detail": "arg #0: i32",
-// CHECK-NEXT:"insertText": "arg",
+// CHECK-NEXT:"detail": "builtin.unrealized_conversion_cast: !pdl.value",
+// CHECK-NEXT:"insertText": "cast",
 // CHECK-NEXT:"insertTextFormat": 1,
 // CHECK-NEXT:"kind": 6,
-// CHECK-NEXT:"label": "%arg"
+// CHECK-NEXT:"label": "%cast"
 // CHECK-NEXT:  }
 // CHECK: ]
 // CHECK-NEXT:  }
Index: llvm/test/tools/llvm-profdata/suppl-instr-with-sample.test
===
--- llvm/test/tools/llvm-profdata/suppl-instr-with-sample.test
+++ llvm/test/tools/llvm-profdata/suppl-instr-with-sample.test
@@ -6,15 +6,15 @@
 RUN: -suppl-min-size-threshold=0 %p/Inputs/mix_instr.proftext -o %t
 RUN: llvm-profdata show %t -all-functions -counts | FileCheck %s --check-prefix=MIX1
 
-MIX1: foo:
-MIX1-NEXT: Hash: 0x0007
-MIX1-NEXT: Counters: 5
-MIX1-NEXT: Block counts: [12, 13, 0, 0, 0]
 MIX1: goo:
 MIX1-NEXT: Hash: 0x0005
 MIX1-NEXT: Counters: 3
 MIX1-NOT: Block counts:
 MIX1-SAME: 
+MIX1: foo:
+MIX1-NEXT: Hash: 0x0007
+MIX1-NEXT: Counters: 5
+MIX1-NEXT: Block counts: [12, 13, 0, 0, 0]
 MIX1: moo:
 MIX1-NEXT: Hash: 0x0009
 MIX1-NEXT: Counters: 4
@@ -27,16 +27,16 @@
 RUN: -instr-prof-cold-threshold=30 %p/Inputs/mix_instr.proftext -o %t
 RUN: llvm-profdata show %t -all-functions -counts | FileCheck %s --check-prefix=MIX2
 
-MIX2: foo:
-MIX2-NEXT: Hash: 0x0007
-MIX2-NEXT: Counters: 5
-MIX2-NOT: Block counts:
-MIX2-SAME: 
 MIX2: goo:
 MIX2-NEXT: Hash: 0x0005
 MIX2-NEXT: Counters: 3
 MIX2-NOT: Block counts:
 MIX2-SAME: 
+MIX2: foo:
+MIX2-NEXT: Hash: 0x0007
+MIX2-NEXT: Counters: 5
+MIX2-NOT: Block counts:
+MIX2-SAME: 
 MIX2: moo:
 MIX2-NEXT: Hash: 0x0009
 MIX2-NEXT: Counters: 4
@@ -49,15 +49,15 @@
 RUN: -instr-prof-cold-threshold=30 %p/Inputs/mix_instr.proftext -o %t
 RUN: llvm-profdata show %t -all-functions -counts | FileCheck %s --check-prefix=MIX3
 
-MIX3: foo:
-MIX3-NEXT: Hash: 0x0007
-MIX3-NEXT: Counters: 5
-MIX3-NEXT: Block counts: [1384, 1500, 0, 0, 0]
 MIX3: goo:
 MIX3-NEXT: Hash: 0x0005
 MIX3-NEXT: Counters: 3
 MIX3-NOT: Block counts:
 MIX3-SAME: 
+MIX3: foo:
+MIX3-NEXT: Hash: 0x0007
+MIX3-NEXT: Counters: 5
+MIX3-NEXT: Block counts: [1384, 1500, 0, 0, 0]
 MIX3: moo:
 MIX3-NEXT: Hash: 0x0009
 MIX3-NEXT: Counters: 4
@@ -71,15 +71,15 @@
 RUN: -instr-prof-cold-threshold=30 %p/Inputs/mix_instr_small.proftext -o %t
 RUN: llvm-profdata show %t -all-functions -counts | FileCheck %s --check-prefix=MIX4
 
-MIX4: foo:
-MIX4-NEXT: Hash: 0x0007
-MIX4-NEXT: Counters: 1
-MIX4-NEXT: Block counts: [0]
 MIX4: goo:
 MIX4-NEXT: Hash: 0x0005
 MIX4-NEXT: Counters: 3
 MIX4-NOT: Block counts:
 MIX4-SAME: 
+MIX4: foo:
+MIX4-NEXT: Hash: 0x0007
+MIX4-NEXT: Counters: 1
+MIX4-NEXT: Block counts: [0]
 MIX4: moo:
 MIX4-NEXT: Hash: 0x0009
 MIX4-NEXT: Counters: 1
Index: llvm/test/tools/dsymutil/ARM/extern-alias.test
=