[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-10-10 Thread Chuanqi Xu via Phabricator via lldb-commits
ChuanqiXu added inline comments.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:623-626
+  VisitDeclaratorDecl(D);
+  Record.AddDeclarationNameLoc(D->DNLoc, D->getDeclName());
+  Record.push_back(D->getIdentifierNamespace());
+

mizvekov wrote:
> ChuanqiXu wrote:
> > ChuanqiXu wrote:
> > > mizvekov wrote:
> > > > ChuanqiXu wrote:
> > > > > mizvekov wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > mizvekov wrote:
> > > > > > > > ChuanqiXu wrote:
> > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > mizvekov wrote:
> > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > I still don't get the reason for the move. What's the 
> > > > > > > > > > > > > benefit? Or why is it necessary?
> > > > > > > > > > > > Yeah, now the type can reference the template decl, so 
> > > > > > > > > > > > without moving this, it can happen during import of the 
> > > > > > > > > > > > type that we try to read this function template bits 
> > > > > > > > > > > > without having imported them yet.
> > > > > > > > > > > Oh, I guess I met the problem before (D129748 ) and I 
> > > > > > > > > > > made a workaround for it 
> > > > > > > > > > > (https://reviews.llvm.org/D130331). If I understood 
> > > > > > > > > > > right, the patch will solve that problem. I'll check it 
> > > > > > > > > > > out later.
> > > > > > > > > > > 
> > > > > > > > > > > (This kind of code move looks dangerous you know and I'll 
> > > > > > > > > > > take a double check)
> > > > > > > > > > After looking into the detailed change for the 
> > > > > > > > > > serialization part, I feel it is a not-so-good workaround 
> > > > > > > > > > indeed.. It looks like we need a better method to delay 
> > > > > > > > > > reading the type in the serializer. And I'm looking at it. 
> > > > > > > > > > @mizvekov would you like to rebase the series of patches to 
> > > > > > > > > > the main branch so that I can test it actually.
> > > > > > > > > Or would it be simpler to rebase and squash them into a draft 
> > > > > > > > > revision?
> > > > > > > > I had given this some thought, and it made sense to me that we 
> > > > > > > > should deal with the template bits first, since these are 
> > > > > > > > closer to the introducer for these declarations, and so that it 
> > > > > > > > would be harder to have a dependence the other way around.
> > > > > > > > 
> > > > > > > > But I would like to hear your thoughts on this after you have 
> > > > > > > > taken a better look.
> > > > > > > > I am working on a bunch of things right now, I should be able 
> > > > > > > > to rebase this on the next few days, but otherwise
> > > > > > > > I last rebased about 4 days ago, so you can also check that out 
> > > > > > > > at https://github.com/mizvekov/llvm-project/tree/resugar
> > > > > > > > That link has the whole stack, you probably should check out 
> > > > > > > > just the commit for this patch, as you are probably going to 
> > > > > > > > encounter issues with the resugarer if you try it on 
> > > > > > > > substantial code bases.
> > > > > > > > It will carry other changes with it, but I think those should 
> > > > > > > > be safe.
> > > > > > > I won't say it is bad to deal with template bits first. I just 
> > > > > > > feel it is a workaround to avoid the circular dependent problem 
> > > > > > > in deserialization. Or in another word, here the method works due 
> > > > > > > to you put some decls* in the template parameter types. And we 
> > > > > > > avoid the circular dependent problem by adjusting the order we 
> > > > > > > deserializes. The reasons why I don't feel it is good include:
> > > > > > > (1) Although we touched template function decl and template var 
> > > > > > > decl, we don't touched template var decl. I guess it'll be a 
> > > > > > > problem now or later.
> > > > > > > (2) The solution here can't solve the similar circular dependent 
> > > > > > > problem I sawed in attributes. So the method only workarounds 
> > > > > > > some resulting of the same problem.
> > > > > > > 
> > > > > > > Or in one shorter explanation, it should be greater to solve the 
> > > > > > > root problems. I have an idea and I am going to to do a 
> > > > > > > proof-of-concept implementation first since I feel like nobody 
> > > > > > > are happy about an unimplementable idea. Generally I don't like 
> > > > > > > to block patches due to such reasons since it is completely not 
> > > > > > > your fault but I guess it may be better to wait some time. Since 
> > > > > > > if we want to allow workarounds first and clear the workarounds, 
> > > > > > > things will be harder. If you want a timeline, I guess 2 months 
> > > > > > > may be reasonable choices. I mean if I can't make it in 2 months 
> > > > > > > and other reviewers feel this is good (what I am seeing), I feel 
> > > > > > > bad to block this. (But if we're more patient, it'll be better). 
> > > > > > > How do you think 

[Lldb-commits] [PATCH] D135607: Summary:

2022-10-10 Thread Henrique Bucher via Phabricator via lldb-commits
HenriqueBucher added a comment.

To be frank, I think this formatting is silly and wrong now that I am looking 
at it. 
Looking at the rendered markdown at Github 

 you can see that the browser itself word wraps for you to the allocated width. 
There is no need to break down manually. 
However if you break down inside the tables it will break the syntax of 
markdown tables. 
And the wrapping does not affect at all the output on the web. 
I'd say delete this patch and forget about this 80 column limit. 
But it's up to you, I'm here just as a helper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135607

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


[Lldb-commits] [PATCH] D135577: Summary:

2022-10-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

lgtm. Jonas?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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


[Lldb-commits] [PATCH] D135607: Summary:

2022-10-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

I am not sure how and if any of these changes will affect the output. Any 
chance we can attach a before and after screenshot of the rendered output?




Comment at: lldb/tools/lldb-vscode/README.md:18-19
 The `lldb-vscode` tool creates a command line tool that implements the [Visual
-Studio Code Debug 
API](https://code.visualstudio.com/docs/extensionAPI/api-debugging).
-It can be installed as an extension for the Visual Studio Code and Nuclide IDE.
-The protocol is easy to run remotely and also can allow other tools and IDEs to
-get a full featured debugger with a well defined protocol.
+Studio Code Debug 
API](https://code.visualstudio.com/docs/extensionAPI/api-debugging). It can be 
installed as an extension for the Visual Studio Code 
+and Nuclide IDE. The protocol is easy to run remotely and also can allow other 
tools 
+and IDEs to get a full featured debugger with a well defined protocol.

Should these two lines have been combined?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135607

___
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 (RFC)

2022-10-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, DavidSpickett, clayborg, mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

This patch copies over log files to the diagnostic directory. It's a bit of a 
straw man proposal as it has some shortcomings that might be acceptable, 
depending on whether we value completeness or performance more.

I see two ways to achieve having log files end up in the diagnostics:

1. The approach implemented here which piggybacks of the mapping kept by the 
debugger. The advantage is that it's free until you generate the diagnostics 
(at which point you pay the price of copying over the file). The obvious 
disadvantage is that this only works for log channels which are written to a 
file.
2. The alternative approach would be to have a log handler similar to our 
StreamTee that basically emits every log message to two log handlers. The first 
log handler would be the one the user chooses (this can be a file, the debugger 
output, etc) and the second log handler is a ring buffer managed by the 
diagnostics (similar to the always-on log). This ensures all the log messages 
are captured, but increases the performance hit of a log message and increases 
the debugger's memory usage.

This didn't seem like it was worth its own RFC on the mailing list so I rolled 
a dice and implemented one of the approaches so we can discuss it here.


https://reviews.llvm.org/D135631

Files:
  lldb/include/lldb/Utility/Diagnostics.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/Diagnostics.cpp
  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
+# The "ll''db" below is not a typo but a way to prevent lit from substituting 
'lldb'.
+# RUN: %lldb -o 'log enable ll''db commands -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/source/Utility/Diagnostics.cpp
===
--- lldb/source/Utility/Diagnostics.cpp
+++ lldb/source/Utility/Diagnostics.cpp
@@ -30,6 +30,8 @@
   InstanceImpl().reset();
 }
 
+bool Diagnostics::Enabled() { return InstanceImpl().operator bool(); }
+
 Optional ::InstanceImpl() {
   static Optional g_diagnostics;
   return g_diagnostics;
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -810,6 +810,22 @@
   if (!GetOutputFile().GetIsTerminalWithColors())
 SetUseColor(false);
 
+  if (Diagnostics::Enabled()) {
+Diagnostics::Instance().AddCallback(
+[&](const FileSpec ) -> llvm::Error {
+  for (auto  : 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/include/lldb/Utility/Diagnostics.h
===
--- lldb/include/lldb/Utility/Diagnostics.h
+++ lldb/include/lldb/Utility/Diagnostics.h
@@ -43,6 +43,7 @@
   static Diagnostics ();
   static void Initialize();
   static void Terminate();
+  static bool Enabled();
 
   /// Create a unique diagnostic directory.
   static llvm::Expected CreateUniqueDirectory();


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
+# The "ll''db" below is not a typo but a way to prevent lit from substituting 'lldb'.
+# RUN: %lldb -o 'log enable ll''db commands -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/source/Utility/Diagnostics.cpp
===
--- lldb/source/Utility/Diagnostics.cpp
+++ lldb/source/Utility/Diagnostics.cpp
@@ -30,6 +30,8 @@
   InstanceImpl().reset();
 }
 
+bool Diagnostics::Enabled() { return InstanceImpl().operator bool(); }
+
 Optional ::InstanceImpl() {
   static Optional g_diagnostics;
   return g_diagnostics;

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

2022-10-10 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/source/Target/Statistics.cpp:293
+
+llvm::Error DebuggerStats::Dump(const FileSpec ) {
+  for (size_t debugger_idx = 0; debugger_idx < Debugger::GetNumDebuggers();

Should this return a Expected> in case the caller 
wants to know which files were created by this call?



Comment at: lldb/source/Target/Statistics.cpp:305
+std::error_code ec;
+raw_fd_ostream stats_stream(stat_file.GetPath(), ec, sys::fs::OF_None);
+if (ec)

What happens if the file already exists here? Will this return an error? I am 
guessing we are passing in a temp directory for the logs



Comment at: lldb/source/Target/Statistics.cpp:309
+
+stats_stream << ReportStatistics(*debugger_sp, nullptr);
+  }

There are many syscalls and OS APIs that aren't supposed to be called during a 
interrupt handler, and this function might call many of them. Deadlocks are a 
serious concern here as well since any thread might have a mutex locked that 
could stop the statistics reporting in its tracks since the program was 
asynchronously interrupted. If this does happen this process will deadlock 
forever and not be able to respond. We might need a bool parameter do 
"ReportStatistics" like "bool during_interrupt" and if this is true, we need to 
try and lock any module mutexes and if they fail, don't report that module. So 
the entire ReportStatistics function will need to be checked for potential 
deadlocks, mostly the Module mutex.


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

https://reviews.llvm.org/D134991

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


[Lldb-commits] [PATCH] D134962: [LLDB] Change pointer to ref in EmulateInstruction::ReadRegister methods

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

sorry for the delay, was on vacation for the past week


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134962

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


[Lldb-commits] [PATCH] D135620: Prevent lldb-vscode tests from source lldbinit file

2022-10-10 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.

Just add a comment as suggested and this is good to go!




Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1448-1449
+
+  auto arguments = request.getObject("arguments");
+  bool source_init_file = GetBoolean(arguments, "sourceInitFile", true);
+

Please add a comment stating this is only used for LLDB unit tests and that 
this isn't part of the actual packet definition for "initialize". Also state 
something like "When testing lldb-vscode, we never want to source the init 
files as these files can change the outcome of tests if the user replaces or 
modifies commands".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135620

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


[Lldb-commits] [PATCH] D135399: [NFCI] More TypeCategoryImpl refactoring.

2022-10-10 Thread Jorge Gorbe Moya via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe5fd507f9b6f: [NFCI] More TypeCategoryImpl refactoring. 
(authored by jgorbe).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135399

Files:
  lldb/include/lldb/DataFormatters/TypeCategory.h
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/DataFormatters/FormattersHelpers.cpp
  lldb/source/DataFormatters/TypeCategory.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -284,12 +284,12 @@
 
   lldb::TypeSummaryImplSP ObjC_BOOL_summary(new CXXFunctionSummaryFormat(
   objc_flags, lldb_private::formatters::ObjCBOOLSummaryProvider, ""));
-  objc_category_sp->GetTypeSummariesContainer()->Add(ConstString("BOOL"),
- ObjC_BOOL_summary);
-  objc_category_sp->GetTypeSummariesContainer()->Add(ConstString("BOOL &"),
- ObjC_BOOL_summary);
-  objc_category_sp->GetTypeSummariesContainer()->Add(ConstString("BOOL *"),
- ObjC_BOOL_summary);
+  objc_category_sp->AddTypeSummary("BOOL", eFormatterMatchExact,
+   ObjC_BOOL_summary);
+  objc_category_sp->AddTypeSummary("BOOL &", eFormatterMatchExact,
+   ObjC_BOOL_summary);
+  objc_category_sp->AddTypeSummary("BOOL *", eFormatterMatchExact,
+   ObjC_BOOL_summary);
 
   // we need to skip pointers here since we are special casing a SEL* when
   // retrieving its value
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -764,8 +764,8 @@
   ConstString("^std::__[[:alnum:]]+::span<.+>(( )?&)?$"), stl_deref_flags,
   true);
 
-  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
-  RegularExpression("^(std::__[[:alnum:]]+::)deque<.+>(( )?&)?$"),
+  cpp_category_sp->AddTypeSynthetic(
+  "^(std::__[[:alnum:]]+::)deque<.+>(( )?&)?$", eFormatterMatchRegex,
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
   stl_synth_flags,
   "lldb.formatters.cpp.libcxx.stddeque_SynthProvider")));
@@ -958,55 +958,52 @@
   stl_summary_flags, LibStdcppWStringSummaryProvider,
   "libstdc++ c++11 std::wstring summary provider"));
 
-  cpp_category_sp->GetTypeSummariesContainer()->Add(ConstString("std::string"),
-std_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::basic_string"), std_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::basic_string,std::"
-  "allocator >"),
-  std_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::basic_string, "
-  "std::allocator >"),
-  std_string_summary_sp);
-
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::__cxx11::string"), cxx11_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::__cxx11::basic_string, "
-  "std::allocator >"),
-  cxx11_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::__cxx11::basic_string, "
-  "std::allocator >"),
-  cxx11_string_summary_sp);
+  cpp_category_sp->AddTypeSummary("std::string", eFormatterMatchExact,
+  std_string_summary_sp);
+  cpp_category_sp->AddTypeSummary("std::basic_string",
+  eFormatterMatchRegex, std_string_summary_sp);
+  cpp_category_sp->AddTypeSummary(
+  "std::basic_string,std::allocator >",
+  eFormatterMatchExact, std_string_summary_sp);
+  cpp_category_sp->AddTypeSummary(
+  "std::basic_string, std::allocator >",
+  eFormatterMatchExact, std_string_summary_sp);
+
+  cpp_category_sp->AddTypeSummary("std::__cxx11::string", eFormatterMatchExact,
+  cxx11_string_summary_sp);
+  cpp_category_sp->AddTypeSummary(
+  "std::__cxx11::basic_string, "
+  "std::allocator >",
+  eFormatterMatchExact, cxx11_string_summary_sp);
+  cpp_category_sp->AddTypeSummary("std::__cxx11::basic_string, "
+  "std::allocator >",
+  

[Lldb-commits] [lldb] e5fd507 - [NFCI] More TypeCategoryImpl refactoring.

2022-10-10 Thread Jorge Gorbe Moya via lldb-commits

Author: Jorge Gorbe Moya
Date: 2022-10-10T15:14:55-07:00
New Revision: e5fd507f9b6f774c83c7b061fa77c906c882935b

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

LOG: [NFCI] More TypeCategoryImpl refactoring.

The main aim of this patch is to delete the remaining instances of code
reaching into the internals of `TypeCategoryImpl`. I made the following
changes:

- Add some more methods to `TieredFormatterContainer` and
  `TypeCategoryImpl` to expose functionality that is implemented in
  `FormattersContainer`.

- Add new overloads of `TypeCategoryImpl::AddTypeXXX` to make it easier
  to add formatters to categories without reaching into the internal
  `FormattersContainer` objects.

- Remove the `GetTypeXXXContainer` and `GetRegexTypeXXXContainer`
  accessors from `TypeCategoryImpl` and update all call sites to use the
  new methods instead.

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

Added: 


Modified: 
lldb/include/lldb/DataFormatters/TypeCategory.h
lldb/source/Commands/CommandObjectType.cpp
lldb/source/DataFormatters/FormatManager.cpp
lldb/source/DataFormatters/FormattersHelpers.cpp
lldb/source/DataFormatters/TypeCategory.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/TypeCategory.h 
b/lldb/include/lldb/DataFormatters/TypeCategory.h
index edf3f5d6605e4..bad39aa676af3 100644
--- a/lldb/include/lldb/DataFormatters/TypeCategory.h
+++ b/lldb/include/lldb/DataFormatters/TypeCategory.h
@@ -45,14 +45,10 @@ template  class 
TieredFormatterContainer {
   sc = std::make_shared(change_listener);
   }
 
-  /// Returns the subcontainer containing formatters for exact matching.
-  std::shared_ptr GetExactMatch() const {
-return m_subcontainers[lldb::eFormatterMatchExact];
-  }
-
-  /// Returns the subcontainer containing formatters for regex matching.
-  std::shared_ptr GetRegexMatch() const {
-return m_subcontainers[lldb::eFormatterMatchRegex];
+  /// Clears all subcontainers.
+  void Clear() {
+for (auto sc : m_subcontainers)
+  sc->Clear();
   }
 
   /// Adds a formatter to the right subcontainer depending on the matching type
@@ -69,6 +65,15 @@ template  class 
TieredFormatterContainer {
 TypeMatcher(type_sp));
   }
 
+  /// Deletes all formatters registered with the string `name`, in all
+  /// subcontainers.
+  bool Delete(ConstString name) {
+bool success = false;
+for (auto sc : m_subcontainers)
+  success = sc->Delete(name) || success;
+return success;
+  }
+
   /// Returns the total count of elements across all subcontainers.
   uint32_t GetCount() {
 uint32_t result = 0;
@@ -100,6 +105,15 @@ template  class 
TieredFormatterContainer {
 return false;
   }
 
+  bool AnyMatches(ConstString type_name) {
+std::shared_ptr entry;
+for (auto sc : m_subcontainers) {
+  if (sc->Get(type_name, entry))
+return true;
+}
+return false;
+  }
+
   /// Returns a formatter that is an exact match for `type_specifier_sp`. It
   /// looks for a formatter with the same matching type that was created from
   /// the same string. This is useful so we can refer to a formatter using the
@@ -140,6 +154,11 @@ template  class 
TieredFormatterContainer {
 }
   }
 
+  void AutoComplete(CompletionRequest ) {
+for (auto sc: m_subcontainers)
+  sc->AutoComplete(request);
+  }
+
  private:
   std::array, lldb::eLastFormatterMatchType + 1>
   m_subcontainers;
@@ -188,36 +207,6 @@ class TypeCategoryImpl {
 m_synth_cont.ForEach(callback.callback);
   }
 
-  FormatContainer::SubcontainerSP GetTypeFormatsContainer() {
-return m_format_cont.GetExactMatch();
-  }
-
-  FormatContainer::SubcontainerSP GetRegexTypeFormatsContainer() {
-return m_format_cont.GetRegexMatch();
-  }
-
-  FormatContainer () { return m_format_cont; }
-
-  SummaryContainer::SubcontainerSP GetTypeSummariesContainer() {
-return m_summary_cont.GetExactMatch();
-  }
-
-  SummaryContainer::SubcontainerSP GetRegexTypeSummariesContainer() {
-return m_summary_cont.GetRegexMatch();
-  }
-
-  SummaryContainer () { return m_summary_cont; }
-
-  FilterContainer::SubcontainerSP GetTypeFiltersContainer() {
-return m_filter_cont.GetExactMatch();
-  }
-
-  FilterContainer::SubcontainerSP GetRegexTypeFiltersContainer() {
-return m_filter_cont.GetRegexMatch();
-  }
-
-  FilterContainer () { return m_filter_cont; }
-
   FormatContainer::MapValueType
   GetFormatForType(lldb::TypeNameSpecifierImplSP type_sp);
 
@@ -235,21 +224,50 @@ class TypeCategoryImpl {
 m_format_cont.Add(type_sp, format_sp);
   }
 
+  void AddTypeFormat(llvm::StringRef name, lldb::FormatterMatchType 

[Lldb-commits] [PATCH] D135622: [lldb] Add a "diagnostics dump" command

2022-10-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, DavidSpickett, clayborg, jingham.
Herald added a project: All.
JDevlieghere requested review of this revision.

Add a "diagnostics dump" command to, as the name implies, dump the diagnostics 
to disk. The goal of this command is to let the user generate the diagnostics 
in case of an issue that doesn't cause the debugger to crash. This is also 
useful for testing.


https://reviews.llvm.org/D135622

Files:
  lldb/include/lldb/Utility/Diagnostics.h
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectDiagnostics.cpp
  lldb/source/Commands/CommandObjectDiagnostics.h
  lldb/source/Commands/Options.td
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Utility/Diagnostics.cpp
  lldb/test/Shell/Diagnostics/TestAlwaysOnLog.test
  lldb/test/Shell/Diagnostics/TestDump.test
  lldb/test/Shell/Diagnostics/TestStats.test

Index: lldb/test/Shell/Diagnostics/TestStats.test
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/TestStats.test
@@ -0,0 +1,6 @@
+# RUN: rm -rf %t
+# RUN: mkdir -p %t
+# RUN: %lldb -o 'diagnostics dump -d %t'
+# RUN: cat %t/debugger-0-stats.json | FileCheck %s
+
+# CHECK: "totalModuleCount":0
Index: lldb/test/Shell/Diagnostics/TestDump.test
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/TestDump.test
@@ -0,0 +1,12 @@
+# Check that the diagnostics dump command uses the correct directory.
+
+# Dump to an existing directory.
+# RUN: rm -rf %t.existing
+# RUN: mkdir -p %t.existing
+# RUN: %lldb -o 'diagnostics dump -d %t.existing'
+# RUN: cat %t.existing/always-on.log
+
+# Dump to a non-existing directory.
+# RUN: rm -rf %t.nonexisting
+# RUN: %lldb -o 'diagnostics dump -d %t.nonexisting'
+# RUN: cat %t.nonexisting/always-on.log
Index: lldb/test/Shell/Diagnostics/TestAlwaysOnLog.test
===
--- /dev/null
+++ lldb/test/Shell/Diagnostics/TestAlwaysOnLog.test
@@ -0,0 +1,6 @@
+# RUN: rm -rf %t
+# RUN: mkdir -p %t
+# RUN: %lldb -o 'diagnostics dump -d %t'
+# RUN: cat %t/always-on.log | FileCheck %s
+
+# CHECK: Dumping {{.*}} diagnostics to '{{.*}}'
Index: lldb/source/Utility/Diagnostics.cpp
===
--- lldb/source/Utility/Diagnostics.cpp
+++ lldb/source/Utility/Diagnostics.cpp
@@ -56,19 +56,18 @@
 }
 
 bool Diagnostics::Dump(raw_ostream ) {
-  SmallString<128> diagnostics_dir;
-  std::error_code ec =
-  sys::fs::createUniqueDirectory("diagnostics", diagnostics_dir);
-  if (ec) {
+  Expected diagnostics_dir = CreateUniqueDirectory();
+  if (!diagnostics_dir) {
 stream << "unable to create diagnostic dir: "
-   << toString(errorCodeToError(ec)) << '\n';
+   << toString(diagnostics_dir.takeError()) << '\n';
 return false;
   }
 
-  stream << "LLDB diagnostics written to " << diagnostics_dir << "\n";
+  stream << "LLDB diagnostics written to " << diagnostics_dir->GetPath()
+ << "\n";
   stream << "Please include the directory content when filing a bug report\n";
 
-  Error error = Create(FileSpec(diagnostics_dir.str()));
+  Error error = Create(*diagnostics_dir);
   if (error) {
 stream << toString(std::move(error)) << '\n';
 return false;
@@ -77,6 +76,15 @@
   return true;
 }
 
+llvm::Expected Diagnostics::CreateUniqueDirectory() {
+  SmallString<128> diagnostics_dir;
+  std::error_code ec =
+  sys::fs::createUniqueDirectory("diagnostics", diagnostics_dir);
+  if (ec)
+return errorCodeToError(ec);
+  return FileSpec(diagnostics_dir.str());
+}
+
 Error Diagnostics::Create(const FileSpec ) {
   LLDB_LOG(GetLog(LLDBLog::AlwaysOn), "Dumping {0} diagnostics to '{1}'",
m_callbacks.size(), dir.GetPath());
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -15,6 +15,7 @@
 #include "Commands/CommandObjectApropos.h"
 #include "Commands/CommandObjectBreakpoint.h"
 #include "Commands/CommandObjectCommands.h"
+#include "Commands/CommandObjectDiagnostics.h"
 #include "Commands/CommandObjectDisassemble.h"
 #include "Commands/CommandObjectExpression.h"
 #include "Commands/CommandObjectFrame.h"
@@ -518,6 +519,7 @@
   REGISTER_COMMAND_OBJECT("apropos", CommandObjectApropos);
   REGISTER_COMMAND_OBJECT("breakpoint", CommandObjectMultiwordBreakpoint);
   REGISTER_COMMAND_OBJECT("command", CommandObjectMultiwordCommands);
+  REGISTER_COMMAND_OBJECT("diagnostics", CommandObjectDiagnostics);
   REGISTER_COMMAND_OBJECT("disassemble", CommandObjectDisassemble);
   REGISTER_COMMAND_OBJECT("expression", CommandObjectExpression);
   REGISTER_COMMAND_OBJECT("frame", CommandObjectMultiwordFrame);
Index: lldb/source/Commands/Options.td

[Lldb-commits] [PATCH] D135621: [lldb] Add an always-on log channel

2022-10-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, DavidSpickett, jingham, mib, clayborg.
Herald added a project: All.
JDevlieghere requested review of this revision.

Add an "always on" log channel. The channel is meant to be used sparsely and 
deliberately for logging high-value information. Unlike other log channels, 
it's not exposed to the user. Log messages are logged to a ring buffer with a 
fixed number of entries. The ring buffer is dumped to disk as part of the 
diagnostics, i.e. when the debugger crashes or when the user explicitly 
requests a dump.


https://reviews.llvm.org/D135621

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

Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -48,7 +48,8 @@
   lambda("all", "all available logging categories");
   lambda("default", "default set of logging categories");
   for (const auto  : entry.second.m_channel.categories)
-lambda(category.name, category.description);
+if (!category.internal)
+  lambda(category.name, category.description);
 }
 
 void Log::ListCategories(llvm::raw_ostream ,
Index: lldb/source/Utility/LLDBLog.cpp
===
--- lldb/source/Utility/LLDBLog.cpp
+++ lldb/source/Utility/LLDBLog.cpp
@@ -63,6 +63,10 @@
 {{"on-demand"},
  {"log symbol on-demand related activities"},
  LLDBLog::OnDemand},
+{{"always-on"},
+ {"log pertinent activities for diagnosing issues in the debugger"},
+ LLDBLog::AlwaysOn,
+ /*internal=*/true},
 };
 
 static Log::Channel g_log_channel(g_categories,
Index: lldb/source/Utility/Diagnostics.cpp
===
--- lldb/source/Utility/Diagnostics.cpp
+++ lldb/source/Utility/Diagnostics.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Utility/Diagnostics.h"
 #include "lldb/Utility/LLDBAssert.h"
+#include "lldb/Utility/LLDBLog.h"
 
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
@@ -17,6 +18,8 @@
 using namespace lldb;
 using namespace llvm;
 
+const size_t Diagnostics::g_log_messages = 100;
+
 void Diagnostics::Initialize() {
   lldbassert(!InstanceImpl() && "Already initialized.");
   InstanceImpl().emplace();
@@ -34,9 +37,18 @@
 
 Diagnostics ::Instance() { return *InstanceImpl(); }
 
-Diagnostics::Diagnostics() {}
+Diagnostics::Diagnostics()
+: m_always_on_log_handler(
+  std::make_shared(g_log_messages)) {
+  const uint32_t log_options = LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
+  Log::EnableLogChannel(m_always_on_log_handler, log_options, "lldb",
+{"always-on"}, llvm::nulls());
+  AddCallback([&](const FileSpec ) { return DumpAlwaysOnLog(dir); });
+}
 
-Diagnostics::~Diagnostics() {}
+Diagnostics::~Diagnostics() {
+  Log::DisableLogChannel("lldb", {"always-on"}, llvm::nulls());
+}
 
 void Diagnostics::AddCallback(Callback callback) {
   std::lock_guard guard(m_callbacks_mutex);
@@ -66,9 +78,22 @@
 }
 
 Error Diagnostics::Create(const FileSpec ) {
+  LLDB_LOG(GetLog(LLDBLog::AlwaysOn), "Dumping {0} diagnostics to '{1}'",
+   m_callbacks.size(), dir.GetPath());
+
   for (Callback c : m_callbacks) {
 if (Error err = c(dir))
   return err;
   }
   return Error::success();
 }
+
+llvm::Error Diagnostics::DumpAlwaysOnLog(const FileSpec ) const {
+  FileSpec log_file = dir.CopyByAppendingPathComponent("always-on.log");
+  std::error_code ec;
+  llvm::raw_fd_ostream stream(log_file.GetPath(), ec, llvm::sys::fs::OF_None);
+  if (ec)
+return errorCodeToError(ec);
+  m_always_on_log_handler->Dump(stream);
+  return Error::success();
+}
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -131,11 +131,14 @@
 llvm::StringLiteral name;
 llvm::StringLiteral description;
 MaskType flag;
+bool internal;
 
 template 
 constexpr Category(llvm::StringLiteral name,
-   llvm::StringLiteral description, Cat mask)
-: name(name), description(description), flag(MaskType(mask)) {
+   llvm::StringLiteral description, Cat mask,
+   bool internal = false)
+: name(name), description(description), flag(MaskType(mask)),
+  internal(internal) {
   static_assert(
   std::is_same>::value);
 }
Index: lldb/include/lldb/Utility/LLDBLog.h
===
--- lldb/include/lldb/Utility/LLDBLog.h
+++ lldb/include/lldb/Utility/LLDBLog.h
@@ -48,7 +48,8 @@
   Unwind = Log::ChannelFlag<29>,
   Watchpoints = Log::ChannelFlag<30>,
   

[Lldb-commits] [PATCH] D135620: Prevent lldb-vscode tests from source lldbinit file

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

lldb-vscode is hard-coded to source .lldbinit file which causes some tests to
fail on my machine. 
This patch adds a new option to control this:

1. vscode.py and lldb-vscode tests will not source .lldbinit by default
2. lldb-vscode will source .lldbinit in production if not specified otherwise


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135620

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1444,8 +1444,12 @@
   auto log_cb = [](const char *buf, void *baton) -> void {
 g_vsc.SendOutput(OutputType::Console, llvm::StringRef{buf});
   };
+
+  auto arguments = request.getObject("arguments");
+  bool source_init_file = GetBoolean(arguments, "sourceInitFile", true);
+
   g_vsc.debugger =
-  lldb::SBDebugger::Create(true /*source_init_files*/, log_cb, nullptr);
+  lldb::SBDebugger::Create(source_init_file, log_cb, nullptr);
   g_vsc.progress_event_thread = std::thread(ProgressEventThreadFunction);
 
   // Start our event thread so we can receive events from the debugger, target,
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -613,7 +613,7 @@
 }
 return self.send_recv(command_dict)
 
-def request_initialize(self):
+def request_initialize(self, sourceInitFile):
 command_dict = {
 'command': 'initialize',
 'type': 'request',
@@ -626,7 +626,8 @@
 'pathFormat': 'path',
 'supportsRunInTerminalRequest': True,
 'supportsVariablePaging': True,
-'supportsVariableType': True
+'supportsVariableType': True,
+'sourceInitFile': sourceInitFile
 }
 }
 response = self.send_recv(command_dict)
@@ -1004,7 +1005,7 @@
 
 
 def run_vscode(dbg, args, options):
-dbg.request_initialize()
+dbg.request_initialize(options.sourceInitFile)
 if attach_options_specified(options):
 response = dbg.request_attach(program=options.program,
   pid=options.pid,
@@ -1112,6 +1113,13 @@
 default=False,
 help='Pause waiting for a debugger to attach to the debug adaptor')
 
+parser.add_option(
+'--sourceInitFile',
+action='store_true',
+dest='sourceInitFile',
+default=False,
+help='Whether lldb-vscode should source .lldbinit file or not')
+
 parser.add_option(
 '--port',
 type='int',
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -253,7 +253,7 @@
initCommands=None, preRunCommands=None, stopCommands=None,
exitCommands=None, attachCommands=None, coreFile=None,
disconnectAutomatically=True, terminateCommands=None,
-   postRunCommands=None, sourceMap=None):
+   postRunCommands=None, sourceMap=None, sourceInitFile=False):
 '''Build the default Makefile target, create the VSCode debug adaptor,
and attach to the process.
 '''
@@ -267,7 +267,7 @@
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
 # Initialize and launch the program
-self.vscode.request_initialize()
+self.vscode.request_initialize(sourceInitFile)
 response = self.vscode.request_attach(
 program=program, pid=pid, waitFor=waitFor, trace=trace,
 initCommands=initCommands, preRunCommands=preRunCommands,
@@ -284,7 +284,7 @@
disableSTDIO=False, shellExpandArguments=False,
trace=False, initCommands=None, preRunCommands=None,
stopCommands=None, exitCommands=None, terminateCommands=None,
-   sourcePath=None, debuggerRoot=None, launchCommands=None,
+   sourcePath=None, debuggerRoot=None, sourceInitFile=False, launchCommands=None,
sourceMap=None, 

[Lldb-commits] [PATCH] D126260: [lldb/crashlog] Add support for Application Specific Backtraces & Information

2022-10-10 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 466636.
mib marked an inline comment as done.
mib edited the summary of this revision.
mib added a comment.

- Address @JDevlieghere comments.
- Change implementation to have the ASI in the process extended crash info and 
ASB as an extended HistoryThread of the actually crashed thread.
- Fix test.


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

https://reviews.llvm.org/D126260

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
  lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/application_specific_info/asi.ips
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/application_specific_info/asi.yaml
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/application_specific_info/main.m
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test
@@ -0,0 +1,52 @@
+# REQUIRES: python, native && target-aarch64 && system-darwin
+
+# RUN: mkdir -p %t.dir
+# RUN: yaml2obj %S/Inputs/application_specific_info/asi.yaml > %t.dir/asi
+# RUN: %lldb -o 'command script import lldb.macosx.crashlog' \
+# RUN: -o 'crashlog -a -i -t %t.dir/asi %S/Inputs/application_specific_info/asi.ips' \
+# RUN: -o "thread list" -o "bt all" 2>&1 | FileCheck %s
+
+# CHECK: "crashlog" {{.*}} commands have been installed, use the "--help" options on these commands
+
+# CHECK: (lldb) process status --verbose
+# CHECK-NEXT: Process 96535 stopped
+# CHECK-NEXT: * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_CRASH (code=0, subcode=0x0)
+# CHECK-NEXT: frame #0: 0x0001a08c7224{{.*}}[artificial]
+# CHECK: Extended Crash Information:
+# CHECK:   Application Specific Information :
+# CHECK-NEXT: CoreFoundation : *** Terminating app due to uncaught exception 'NSRangeException', reason: '*** __boundsFail: index 10 beyond bounds [0 .. 3]'
+# CHECK-NEXT: libc++abi.dylib : terminating with uncaught exception of type NSException
+# CHECK-NEXT: libsystem_c.dylib : abort() called
+
+
+# CHECK: (lldb) thread backtrace --extended true
+# CHECK-NEXT: * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_CRASH (code=0, subcode=0x0)
+# CHECK-NEXT:   * frame #0: 0x0001a08c7224{{.*}}[artificial]
+# CHECK-NEXT: frame #1: 0x0001a08fdceb{{.*}}[artificial]
+# CHECK-NEXT: frame #2: 0x0001a08372c7{{.*}}[artificial]
+# CHECK-NEXT: frame #3: 0x0001a08b7b17{{.*}}[artificial]
+# CHECK-NEXT: frame #4: 0x0001a08a7a0b{{.*}}[artificial]
+# CHECK-NEXT: frame #5: 0x0001a05ab763{{.*}}[artificial]
+# CHECK-NEXT: frame #6: 0x0001a08b6eb3{{.*}}[artificial]
+# CHECK-NEXT: frame #7: 0x0001a08b9c2b{{.*}}[artificial]
+# CHECK-NEXT: frame #8: 0x0001a08b9bd7{{.*}}[artificial]
+# CHECK-NEXT: frame #9: 0x0001a05a3007{{.*}}[artificial]
+# CHECK-NEXT: frame #10: 0x0001a0b3dcc3{{.*}}[artificial]
+# CHECK-NEXT: frame #11: 0x0001a0b46af3{{.*}}[artificial]
+# CHECK-NEXT: frame #12: 0x0001a09a12a3{{.*}}[artificial]
+# CHECK-NEXT: frame #13: 0x0001047e3ecf asi`main{{.*}}[artificial]
+# CHECK-NEXT: frame #14: 0x0001a05d3e4f{{.*}}[artificial]
+
+# CHECK:   thread #4294967295: tid = 0x0001, 0x0001a0a58418 CoreFoundation`__exceptionPreprocess{{.*}}, queue = 'Application Specific Backtrace'
+# CHECK-NEXT: frame #0: 0x0001a0a58418 CoreFoundation`__exceptionPreprocess{{.*}}
+# CHECK-NEXT: frame #1: 0x0001a05a2ea7 libobjc.A.dylib`objc_exception_throw{{.*}}
+# CHECK-NEXT: frame #2: 0x0001a0b3dcc3 CoreFoundation`_CFThrowFormattedException{{.*}}
+# CHECK-NEXT: frame #3: 0x0001a0b46af3 CoreFoundation`__boundsFail{{.*}}
+# CHECK-NEXT: frame #4: 0x0001a09a12a3 CoreFoundation`-[__NSArrayI objectAtIndex:]{{.*}}
+# 

[Lldb-commits] [PATCH] D126260: [lldb/crashlog] Add support for Application Specific Backtraces & Information

2022-10-10 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib marked 2 inline comments as done.
mib added inline comments.



Comment at: lldb/examples/python/crashlog.py:581-597
+def parse_asi_backtrace(self, thread, bt):
+for line in bt.split('\n'):
+frame_match = TextCrashLogParser.frame_regex.search(line)
+if not frame_match:
+print("error: can't parse application specific backtrace.")
+return False
+

JDevlieghere wrote:
> Can this be a top level function? It's hard to tell if this is capturing 
> anything. We have other place where we have helper functions that are called 
> from a loop.
This parses a single backtrace. I'm not sure what you mean by "We have other 
place where we have helper functions that are called from a loop."



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1000
+StructuredData::Array *arr = val->GetAsArray();
+if (!arr || !arr->GetSize())
+  return false;

JDevlieghere wrote:
> Should this check if the size == 1? 
I don't think this should only work if the dictionary has only 1 item.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:491
+
+  Status error;
+

JDevlieghere wrote:
> Is this ever populated?
It is used below in `ErrorWithMessage`.


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

https://reviews.llvm.org/D126260

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


[Lldb-commits] [PATCH] D135547: [lldb/Utility] Add GetDescription(Stream&) to StructureData::*

2022-10-10 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 466626.
mib added a comment.

Add test file


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

https://reviews.llvm.org/D135547

Files:
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/include/lldb/Utility/StructuredData.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Utility/StructuredData.cpp
  lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/Inputs/StructuredData-full.json
  lldb/unittests/Utility/StructuredDataTest.cpp

Index: lldb/unittests/Utility/StructuredDataTest.cpp
===
--- lldb/unittests/Utility/StructuredDataTest.cpp
+++ lldb/unittests/Utility/StructuredDataTest.cpp
@@ -31,6 +31,22 @@
   }
 }
 
+TEST(StructuredDataTest, GetDescription) {
+  Status status;
+  std::string input = GetInputFilePath("StructuredData-full.json");
+  auto object_sp = StructuredData::ParseJSONFromFile(FileSpec(input), status);
+  ASSERT_NE(nullptr, object_sp);
+
+  const std::string expected =
+  "  Array :\n3.14\n1.234000  \n  Dictionary :\nFalseBool "
+  ": False  \n  Integer : 1\n  Null : NULL\n  String : value\n  TrueBool : "
+  "True";
+
+  StreamString S;
+  object_sp->GetDescription(S);
+  EXPECT_EQ(expected, S.GetString());
+}
+
 TEST(StructuredDataTest, ParseJSONFromFile) {
   Status status;
   auto object_sp = StructuredData::ParseJSONFromFile(
Index: lldb/unittests/Utility/Inputs/StructuredData-full.json
===
--- /dev/null
+++ lldb/unittests/Utility/Inputs/StructuredData-full.json
@@ -0,0 +1 @@
+{ "Array": [ 3.14, 1.234 ], "Dictionary": { "FalseBool": false }, "Integer": 1, "Null": null, "String": "value", "TrueBool": true }
Index: lldb/unittests/Utility/CMakeLists.txt
===
--- lldb/unittests/Utility/CMakeLists.txt
+++ lldb/unittests/Utility/CMakeLists.txt
@@ -54,6 +54,9 @@
 Support
   )
 
-add_unittest_inputs(UtilityTests
+set(test_inputs
   StructuredData-basic.json
+  StructuredData-full.json
   )
+
+add_unittest_inputs(UtilityTests "${test_inputs}")
Index: lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
===
--- lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
+++ lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
@@ -37,7 +37,7 @@
 patterns=["Process .* launched: .*a.out"])
 
 self.expect('process status --verbose',
-patterns=["\"message\".*pointer being freed was not allocated"])
+patterns=["message: .*pointer being freed was not allocated"])
 
 
 @skipIfAsan # The test process intentionally hits a memory bug.
Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -175,3 +175,62 @@
 void StructuredData::Generic::Serialize(json::OStream ) const {
   s.value(llvm::formatv("{0:X}", m_object));
 }
+
+void StructuredData::Array::GetDescription(lldb_private::Stream ) const {
+  s.IndentMore();
+  for (const auto _sp : m_items) {
+s.Indent();
+item_sp->GetDescription(s);
+if (item_sp != *(--m_items.end()))
+  s.EOL();
+  }
+  s.IndentLess();
+  s.Indent();
+}
+
+void StructuredData::Integer::GetDescription(lldb_private::Stream ) const {
+  s.Printf("%" PRId64, static_cast(m_value));
+}
+
+void StructuredData::Float::GetDescription(lldb_private::Stream ) const {
+  s.Printf("%f", m_value);
+}
+
+void StructuredData::Boolean::GetDescription(lldb_private::Stream ) const {
+  s.Printf(m_value ? "True" : "False");
+}
+
+void StructuredData::String::GetDescription(lldb_private::Stream ) const {
+  s.Printf("%s", m_value.c_str());
+}
+
+void StructuredData::Dictionary::GetDescription(lldb_private::Stream ) const {
+  s.IndentMore();
+
+  for (const auto  : m_dict) {
+if (pair.first.IsNull() || pair.first.IsEmpty() || !pair.second)
+  continue;
+s.Indent();
+s.Printf("%s :", pair.first.AsCString());
+auto value_type = pair.second->GetType();
+if (value_type == lldb::eStructuredDataTypeArray ||
+value_type == lldb::eStructuredDataTypeDictionary)
+  s.EOL();
+else
+  s.PutChar(' ');
+pair.second->GetDescription(s);
+if (pair != *(--m_dict.end()))
+  s.EOL();
+  }
+
+  s.IndentLess();
+  s.Indent();
+}
+
+void StructuredData::Null::GetDescription(lldb_private::Stream ) const {
+  s.Printf("NULL");
+}
+
+void StructuredData::Generic::GetDescription(lldb_private::Stream ) const {
+  s.Printf("%p", m_object);
+}
Index: lldb/source/Commands/CommandObjectProcess.cpp

[Lldb-commits] [PATCH] D135547: [lldb/Utility] Add GetDescription(Stream&) to StructureData::*

2022-10-10 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 466624.
mib added a comment.

Address @JDevlieghere comment:

- Add unittest


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

https://reviews.llvm.org/D135547

Files:
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/include/lldb/Utility/StructuredData.h
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Utility/StructuredData.cpp
  lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/StructuredDataTest.cpp

Index: lldb/unittests/Utility/StructuredDataTest.cpp
===
--- lldb/unittests/Utility/StructuredDataTest.cpp
+++ lldb/unittests/Utility/StructuredDataTest.cpp
@@ -31,6 +31,22 @@
   }
 }
 
+TEST(StructuredDataTest, GetDescription) {
+  Status status;
+  std::string input = GetInputFilePath("StructuredData-full.json");
+  auto object_sp = StructuredData::ParseJSONFromFile(FileSpec(input), status);
+  ASSERT_NE(nullptr, object_sp);
+
+  const std::string expected =
+  "  Array :\n3.14\n1.234000  \n  Dictionary :\nFalseBool "
+  ": False  \n  Integer : 1\n  Null : NULL\n  String : value\n  TrueBool : "
+  "True";
+
+  StreamString S;
+  object_sp->GetDescription(S);
+  EXPECT_EQ(expected, S.GetString());
+}
+
 TEST(StructuredDataTest, ParseJSONFromFile) {
   Status status;
   auto object_sp = StructuredData::ParseJSONFromFile(
Index: lldb/unittests/Utility/CMakeLists.txt
===
--- lldb/unittests/Utility/CMakeLists.txt
+++ lldb/unittests/Utility/CMakeLists.txt
@@ -54,6 +54,9 @@
 Support
   )
 
-add_unittest_inputs(UtilityTests
+set(test_inputs
   StructuredData-basic.json
+  StructuredData-full.json
   )
+
+add_unittest_inputs(UtilityTests "${test_inputs}")
Index: lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
===
--- lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
+++ lldb/test/API/functionalities/process_crash_info/TestProcessCrashInfo.py
@@ -37,7 +37,7 @@
 patterns=["Process .* launched: .*a.out"])
 
 self.expect('process status --verbose',
-patterns=["\"message\".*pointer being freed was not allocated"])
+patterns=["message: .*pointer being freed was not allocated"])
 
 
 @skipIfAsan # The test process intentionally hits a memory bug.
Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -175,3 +175,62 @@
 void StructuredData::Generic::Serialize(json::OStream ) const {
   s.value(llvm::formatv("{0:X}", m_object));
 }
+
+void StructuredData::Array::GetDescription(lldb_private::Stream ) const {
+  s.IndentMore();
+  for (const auto _sp : m_items) {
+s.Indent();
+item_sp->GetDescription(s);
+if (item_sp != *(--m_items.end()))
+  s.EOL();
+  }
+  s.IndentLess();
+  s.Indent();
+}
+
+void StructuredData::Integer::GetDescription(lldb_private::Stream ) const {
+  s.Printf("%" PRId64, static_cast(m_value));
+}
+
+void StructuredData::Float::GetDescription(lldb_private::Stream ) const {
+  s.Printf("%f", m_value);
+}
+
+void StructuredData::Boolean::GetDescription(lldb_private::Stream ) const {
+  s.Printf(m_value ? "True" : "False");
+}
+
+void StructuredData::String::GetDescription(lldb_private::Stream ) const {
+  s.Printf("%s", m_value.c_str());
+}
+
+void StructuredData::Dictionary::GetDescription(lldb_private::Stream ) const {
+  s.IndentMore();
+
+  for (const auto  : m_dict) {
+if (pair.first.IsNull() || pair.first.IsEmpty() || !pair.second)
+  continue;
+s.Indent();
+s.Printf("%s :", pair.first.AsCString());
+auto value_type = pair.second->GetType();
+if (value_type == lldb::eStructuredDataTypeArray ||
+value_type == lldb::eStructuredDataTypeDictionary)
+  s.EOL();
+else
+  s.PutChar(' ');
+pair.second->GetDescription(s);
+if (pair != *(--m_dict.end()))
+  s.EOL();
+  }
+
+  s.IndentLess();
+  s.Indent();
+}
+
+void StructuredData::Null::GetDescription(lldb_private::Stream ) const {
+  s.Printf("NULL");
+}
+
+void StructuredData::Generic::GetDescription(lldb_private::Stream ) const {
+  s.Printf("%p", m_object);
+}
Index: lldb/source/Commands/CommandObjectProcess.cpp
===
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -1537,8 +1537,9 @@
   StructuredData::DictionarySP crash_info_sp = *expected_crash_info;
 
   if (crash_info_sp) {
+strm.EOL();
 strm.PutCString("Extended Crash Information:\n");
-crash_info_sp->Dump(strm);
+

[Lldb-commits] [PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

2022-10-10 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks added inline comments.



Comment at: clang/lib/AST/StmtPrinter.cpp:1298
+  case BuiltinType::WChar_U:
+break; // no suffix.
   }

DavidSpickett wrote:
> Is there any reason to have a suffix here?
> 
> I admit this function is puzzling to me anyway given that char has a prefix 
> and this won't. Perhaps this is because char here is not "character" it's an 
> integer of a certain size. Where wide char is more about, well, being an 
> actual character.
> 
> And reading https://en.cppreference.com/w/cpp/language/character_literal the 
> suffix would be "L" which we've already used.
> 
> As long as it prints in a way that's useful for the developer that's fine.
https://en.cppreference.com/w/cpp/language/character_literal is about char 
literal prefixes, which is not what this is

e.g.
```
$ lldb a.out
(lldb) im loo -t A
...
static const unsigned char uchar_max = 255Ui8;
```
this output isn't valid C++, I believe it's just more clarification for the 
user on the exact type of the integer literal. I could make this output 
something like "UW"/"SW"?

with and without this change, we already have
```
(lldb) print A::wchar_min
(const wchar_t) $0 = L'\Ufffd'
```

this change makes the following not crash
```
$ lldb a.out
(lldb) im loo -t A
...
static const wchar_t wchar_max = 2147483647;
```
any suffix we choose here is appended to the `2147483647`



Comment at: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp:38
   std::numeric_limits::max();
+  const static auto wchar_max = std::numeric_limits::max();
 

DavidSpickett wrote:
> Is there a specific `signed wchar_t` and `unsigned wchar_t` like for char?
```
/tmp/a.cc:1:7: error: 'wchar_t' cannot be signed or unsigned 
[-Wsigned-unsigned-wchar]
const unsigned wchar_t a = 0;
```
https://en.cppreference.com/w/cpp/language/types
```
wchar_t - type for wide character representation (see wide strings). It has the 
same size, signedness, and alignment as one of the integer types, but is a 
distinct type.
```
so it sounds like under the hood the signedness is platform dependent, but 
there's only one wchar_t type


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135461

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


[Lldb-commits] [PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

2022-10-10 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks updated this revision to Diff 466622.
aeubanks added a comment.

update python test, although those changes don't actually test the code path 
changed here, the code path here is tested via the existing `self.expect("image 
lookup -t A")`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135461

Files:
  clang/lib/AST/StmtPrinter.cpp
  
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
  lldb/test/API/lang/cpp/const_static_integral_member/main.cpp


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -36,6 +36,7 @@
   const static auto longlong_max = std::numeric_limits::max();
   const static auto ulonglong_max =
   std::numeric_limits::max();
+  const static auto wchar_max = std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
   const static auto schar_min = std::numeric_limits::min();
@@ -47,6 +48,7 @@
   const static auto longlong_min = std::numeric_limits::min();
   const static auto ulonglong_min =
   std::numeric_limits::min();
+  const static auto wchar_min = std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
@@ -93,6 +95,7 @@
   auto ulong_max = A::ulong_max;
   auto longlong_max = A::longlong_max;
   auto ulonglong_max = A::ulonglong_max;
+  auto wchar_max = A::wchar_max;
 
   auto char_min = A::char_min;
   auto schar_min = A::schar_min;
@@ -103,6 +106,7 @@
   auto ulong_min = A::ulong_min;
   auto longlong_min = A::longlong_min;
   auto ulonglong_min = A::ulonglong_min;
+  auto wchar_min = A::wchar_min;
 
   int member_copy = ClassWithOnlyConstStatic::member;
 
Index: 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
===
--- 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
+++ 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
@@ -42,6 +42,7 @@
 self.expect_expr("A::ulong_max == ulong_max", result_value="true")
 self.expect_expr("A::longlong_max == longlong_max", 
result_value="true")
 self.expect_expr("A::ulonglong_max == ulonglong_max", 
result_value="true")
+self.expect_expr("A::wchar_max == wchar_max", result_value="true")
 
 self.expect_expr("A::char_min == char_min", result_value="true")
 self.expect_expr("A::schar_min == schar_min", result_value="true")
@@ -52,6 +53,7 @@
 self.expect_expr("A::ulong_min == ulong_min", result_value="true")
 self.expect_expr("A::longlong_min == longlong_min", 
result_value="true")
 self.expect_expr("A::ulonglong_min == ulonglong_min", 
result_value="true")
+self.expect_expr("A::wchar_min == wchar_min", result_value="true")
 
 # Test an unscoped enum.
 self.expect_expr("A::enum_val", result_value="enum_case2")
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1293,6 +1293,9 @@
 break; // no suffix.
   case BuiltinType::UInt128:
 break; // no suffix.
+  case BuiltinType::WChar_S:
+  case BuiltinType::WChar_U:
+break; // no suffix
   }
 }
 


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -36,6 +36,7 @@
   const static auto longlong_max = std::numeric_limits::max();
   const static auto ulonglong_max =
   std::numeric_limits::max();
+  const static auto wchar_max = std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
   const static auto schar_min = std::numeric_limits::min();
@@ -47,6 +48,7 @@
   const static auto longlong_min = std::numeric_limits::min();
   const static auto ulonglong_min =
   std::numeric_limits::min();
+  const static auto wchar_min = std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
@@ -93,6 +95,7 @@
   auto ulong_max = A::ulong_max;
   auto longlong_max = A::longlong_max;
   auto ulonglong_max = A::ulonglong_max;
+  auto wchar_max = A::wchar_max;
 
   auto char_min = A::char_min;
   auto schar_min = A::schar_min;
@@ -103,6 +106,7 @@
   auto ulong_min = A::ulong_min;
   auto longlong_min = A::longlong_min;
   auto ulonglong_min = A::ulonglong_min;
+  auto wchar_min = A::wchar_min;
 
   int 

[Lldb-commits] [PATCH] D134066: [LLDB][NativePDB] Forcefully complete a record type it has incomplete type debug info.

2022-10-10 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:681
+auto  = llvm::cast(*ct.GetTypeSystem());
+ts.GetMetadata()->SetIsForcefullyCompleted();
+  }

labath wrote:
> zequanwu wrote:
> > rnk wrote:
> > > Is this what we do for DWARF? The same kind of situation seems like it 
> > > can arise, where clang requires a type to be complete, but the 
> > > information is missing, and we still try to proceed with evaluation.
> > I think it's here: 
> > https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L221-L250,
> >  relevant: https://reviews.llvm.org/D85968.
> The trick is to do the forced completion only when the type is used within 
> contexts where the c++ rules require it to be complete. something like `class 
> A; A* a;` is perfectly legal c++. `class A; class B : A {};` is not. You 
> can't do this from within the completion callback. In DWARF code, we check 
> for this when we're parsing the enclosing entity (so, when we're parsing `B`, 
> we'd check, and if needed, "forcefully complete" the class `A`.
What does dwarf plugin do in this case that we don't have debug info for class 
A: `class A; class B : A {};`? The problem I'm trying to fix is when the base 
class has no debug info to complete it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134066

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


[Lldb-commits] [PATCH] D135616: [lldb/Utility] Fix StructuredData::ParseJSONValue for null items

2022-10-10 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added a reviewer: JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch fixes the JSON parser for StructuredData to handle JSON null
entries.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135616

Files:
  lldb/source/Utility/StructuredData.cpp


Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -69,6 +69,9 @@
   if (auto d = value.getAsNumber())
 return std::make_shared(*d);
 
+  if (auto n = value.getAsNull())
+return std::make_shared();
+
   return StructuredData::ObjectSP();
 }
 


Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -69,6 +69,9 @@
   if (auto d = value.getAsNumber())
 return std::make_shared(*d);
 
+  if (auto n = value.getAsNull())
+return std::make_shared();
+
   return StructuredData::ObjectSP();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D135577: Summary:

2022-10-10 Thread Henrique Bucher via Phabricator via lldb-commits
HenriqueBucher added a comment.

Ok submitted another patch. Added you as reviewer.  Check if it is okay.
https://reviews.llvm.org/D135607


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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


[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

2022-10-10 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:459
+  uint64_t end_offset = offset + fields.back()->bit_size;
+  parent->fields.push_back(fields.back());
+  end_offset_map[end_offset].push_back(parent);

labath wrote:
> Can `parent` be a union here? Would the algorithm still be correct?
`parent` could only be union when the top level record is a union (at this line 
`Member *parent = `). That's the only case when we add an union into 
`end_offset_map`. In that case, all the fields would start at the same offset 
and we only iterate the loop `for (auto  : fields) {` once and then we are 
done. 
Otherwise, we only insert {end offset, struct/field} into `end_offset_map` 
where field must be within an union.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:471
+  } else {
+for (auto  : fields) {
+  parent->fields.push_back(field);

labath wrote:
> IIUC, this code is reached when the `parent` object is a union. However, the 
> parent is chosen such that it ends before the start of these new fields? 
> Therefore its start address will be before the start of these fields as well. 
> Is it correct to add the fields to the union under these circumstances, or am 
> I misunderstanding something?
This is a special case to handle the case when top level record is an union. 
That's the only case we would reach here. This is to avoid adding unnecessary 
nested union. For example, the unit test `TestAnonymousUnionInUnion` would 
become the following if we always add an anonymous union:
```
union {
  union {
m1;
m2;
m3;
m4;
  }
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

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


[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

2022-10-10 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 466575.
zequanwu marked 2 inline comments as done.
zequanwu added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp
  lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp
  lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
  lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp
  lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt
  lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp

Index: lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
===
--- /dev/null
+++ lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
@@ -0,0 +1,178 @@
+//===-- UdtRecordCompleterTests.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private::npdb;
+using namespace llvm;
+
+namespace {
+using Member = UdtRecordCompleter::Member;
+using Record = UdtRecordCompleter::Record;
+using MemberUP = std::unique_ptr;
+
+class UdtRecordCompleterRecordTests : public testing::Test {
+  Record record;
+
+public:
+  void SetKind(Member::Kind kind) { record.record.kind = kind; }
+  void CollectMember(StringRef name, uint64_t byte_offset, uint64_t byte_size) {
+record.CollectMember(name, byte_offset * 8, byte_size * 8,
+ clang::QualType(), lldb::eAccessPublic, 0);
+  }
+  void ConstructRecord() { record.ConstructRecord(); }
+
+  static bool VerifyMembers(const llvm::SmallVector ,
+const llvm::SmallVector ) {
+if (lhs.size() != rhs.size())
+  return false;
+for (size_t i = 0; i < lhs.size(); ++i) {
+  if (lhs[i]->kind != rhs[i]->kind || lhs[i]->name != rhs[i]->name ||
+  lhs[i]->bit_offset != rhs[i]->bit_offset ||
+  lhs[i]->bit_size != rhs[i]->bit_size ||
+  lhs[i]->base_offset != rhs[i]->base_offset ||
+  !VerifyMembers(lhs[i]->fields, rhs[i]->fields))
+return false;
+}
+return true;
+  }
+  bool VerifyRecord(Record ) {
+return record.start_offset == testRecord.start_offset &&
+   VerifyMembers(record.record.fields, testRecord.record.fields);
+  }
+};
+Member *AddField(Member *member, StringRef name, uint64_t byte_offset,
+ uint64_t byte_size, Member::Kind kind,
+ uint64_t base_offset = 0) {
+  auto field =
+  std::make_unique(name, byte_offset * 8, byte_size * 8,
+   clang::QualType(), lldb::eAccessPublic, 0);
+  field->kind = kind;
+  field->base_offset = base_offset;
+  member->fields.push_back(std::move(field));
+  return member->fields.back().get();
+}
+} // namespace
+
+TEST_F(UdtRecordCompleterRecordTests, TestAnonymousUnionInStruct) {
+  SetKind(Member::Kind::Struct);
+  CollectMember("m1", 0, 4);
+  CollectMember("m2", 0, 4);
+  CollectMember("m3", 0, 1);
+  CollectMember("m4", 0, 8);
+  ConstructRecord();
+
+  // struct {
+  //   union {
+  //   m1;
+  //   m2;
+  //   m3;
+  //   m4;
+  //   };
+  // };
+  Record record;
+  record.start_offset = 0;
+  Member *u = AddField(, "", 0, 0, Member::Union);
+  AddField(u, "m1", 0, 4, Member::Field);
+  AddField(u, "m2", 0, 4, Member::Field);
+  AddField(u, "m3", 0, 1, Member::Field);
+  AddField(u, "m4", 0, 8, Member::Field);
+  EXPECT_TRUE(VerifyRecord(record));
+}
+
+TEST_F(UdtRecordCompleterRecordTests, TestAnonymousUnionInUnion) {
+  SetKind(Member::Kind::Union);
+  CollectMember("m1", 0, 4);
+  CollectMember("m2", 0, 4);
+  CollectMember("m3", 0, 1);
+  CollectMember("m4", 0, 8);
+  ConstructRecord();
+
+  // union {
+  //   m1;
+  //   m2;
+  //   m3;
+  //   m4;
+  // };
+  Record record;
+  record.start_offset = 0;
+  AddField(, "m1", 0, 4, Member::Field);
+  AddField(, "m2", 0, 4, Member::Field);
+  AddField(, "m3", 0, 1, Member::Field);
+  AddField(, "m4", 0, 8, Member::Field);
+  EXPECT_TRUE(VerifyRecord(record));
+}
+
+TEST_F(UdtRecordCompleterRecordTests, TestNestedUnionStructInStruct) {
+  SetKind(Member::Kind::Struct);
+  CollectMember("m1", 

[Lldb-commits] [PATCH] D135607: Summary:

2022-10-10 Thread Henrique Bucher via Phabricator via lldb-commits
HenriqueBucher created this revision.
HenriqueBucher added reviewers: JDevlieghere, clayborg.
Herald added a project: All.
HenriqueBucher requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Reformatted doc text so it is more or less around 80 columns limit - in 
markdown equivalent.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135607

Files:
  lldb/tools/lldb-vscode/README.md

Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -15,16 +15,16 @@
 # Introduction
 
 The `lldb-vscode` tool creates a command line tool that implements the [Visual
-Studio Code Debug API](https://code.visualstudio.com/docs/extensionAPI/api-debugging).
-It can be installed as an extension for the Visual Studio Code and Nuclide IDE.
-The protocol is easy to run remotely and also can allow other tools and IDEs to
-get a full featured debugger with a well defined protocol.
+Studio Code Debug API](https://code.visualstudio.com/docs/extensionAPI/api-debugging). It can be installed as an extension for the Visual Studio Code 
+and Nuclide IDE. The protocol is easy to run remotely and also can allow other tools 
+and IDEs to get a full featured debugger with a well defined protocol.
 
 # Installation for Visual Studio Code
 
-Installing the plug-in involves creating a directory in the `~/.vscode/extensions` folder and copying the package.json file that is in the same directory as this
-documentation into it, and copying to symlinking a lldb-vscode binary into
-the `bin` directory inside the plug-in directory.
+Installing the plug-in involves creating a directory in the `~/.vscode/extensions` 
+folder and copying the package.json file that is in the same directory as this
+documentation into it, and copying to symlinking a lldb-vscode binary into the `bin` 
+directory inside the plug-in directory.
 
 If you want to make a stand alone plug-in that you can send to others on unix systems:
 
@@ -47,8 +47,10 @@
 $ rsync -av /path/to/a/built/LLDB.framework LLDB.framework
 ```
 
-You might need to create additional directories for the `liblldb.so` or `LLDB.framework` inside or next to the `bin` folder depending on how the [rpath](https://en.wikipedia.org/wiki/Rpath) is set in your `lldb-vscode` binary. By default the `Debug` builds of LLDB usually includes
-the current executable directory in the rpath, so these steps should work for most people.
+You might need to create additional directories for the `liblldb.so` or `LLDB.framework` 
+inside or next to the `bin` folder depending on how the [rpath](https://en.wikipedia.org/wiki/Rpath) is set in your `lldb-vscode` 
+binary. By default the `Debug` builds of LLDB usually includes the current executable 
+directory in the rpath, so these steps should work for most people.
 
 To create a plug-in that symlinks into your `lldb-vscode` in your build directory:
 
@@ -59,18 +61,20 @@
 $ ln -s /path/to/a/built/lldb-vscode
 ```
 
-This is handy if you want to debug and develope the `lldb-vscode` executable when adding features or fixing bugs.
+This is handy if you want to debug and develope the `lldb-vscode` executable when 
+adding features or fixing bugs.
 
 # Configurations
 
-Launching to attaching require you to create a [launch configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations). This file
-defines arguments that get passed to `lldb-vscode` and the configuration settings
-control how the launch or attach happens.
+Launching to attaching require you to create a [launch configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations). This file defines 
+arguments that get passed to `lldb-vscode` and the configuration settings control how 
+the launch or attach happens.
 
 ## Launch Configuration Settings
 
 When you launch a program with Visual Studio Code you will need to create a [launch.json](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations)
-file that defines how your program will be run. The JSON configuration file can contain the following `lldb-vscode` specific launch key/value pairs:
+file that defines how your program will be run. The JSON configuration file can contain 
+the following `lldb-vscode` specific launch key/value pairs:
 
 |parameter  |type|req | |
 |---||:--:|-|
@@ -78,17 +82,17 @@
 |**type**   |string|Y| Must be "lldb-vscode".
 |**request**|string|Y| Must be "launch".
 |**program**|string|Y| Path to the executable to launch.
-|**args**   |[string]|| An array of command line argument strings to be passed to the program being launched.
+|**args**   |[string]|| An array of command line argument strings to be passed to the program being launched.
 |**cwd**|string| | The program working 

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

2022-10-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 466549.
JDevlieghere added a comment.

Changed my mind (again). I made the Callback a std::function so we can use 
lambas. I used function pointers originally so we could easily compare and 
remove callbacks, but there's too many places where this overcomplicates things.


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

https://reviews.llvm.org/D134991

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Target/Statistics.h
  lldb/include/lldb/Utility/Diagnostics.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Initialization/SystemInitializerCommon.cpp
  lldb/source/Target/Statistics.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/Diagnostics.cpp
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -788,6 +788,10 @@
<< '\n';
 return 1;
   }
+
+  // Setup LLDB signal handlers once the debugger has been initialized.
+  SBDebugger::PrintDiagnosticsOnError();
+
   SBHostOS::ThreadCreated("");
 
   signal(SIGINT, sigint_handler);
Index: lldb/source/Utility/Diagnostics.cpp
===
--- /dev/null
+++ lldb/source/Utility/Diagnostics.cpp
@@ -0,0 +1,74 @@
+//===-- Diagnostics.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Utility/Diagnostics.h"
+#include "lldb/Utility/LLDBAssert.h"
+
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace lldb_private;
+using namespace lldb;
+using namespace llvm;
+
+void Diagnostics::Initialize() {
+  lldbassert(!InstanceImpl() && "Already initialized.");
+  InstanceImpl().emplace();
+}
+
+void Diagnostics::Terminate() {
+  lldbassert(InstanceImpl() && "Already terminated.");
+  InstanceImpl().reset();
+}
+
+Optional ::InstanceImpl() {
+  static Optional g_diagnostics;
+  return g_diagnostics;
+}
+
+Diagnostics ::Instance() { return *InstanceImpl(); }
+
+Diagnostics::Diagnostics() {}
+
+Diagnostics::~Diagnostics() {}
+
+void Diagnostics::AddCallback(Callback callback) {
+  std::lock_guard guard(m_callbacks_mutex);
+  m_callbacks.push_back(callback);
+}
+
+bool Diagnostics::Dump(raw_ostream ) {
+  SmallString<128> diagnostics_dir;
+  std::error_code ec =
+  sys::fs::createUniqueDirectory("diagnostics", diagnostics_dir);
+  if (ec) {
+stream << "unable to create diagnostic dir: "
+   << toString(errorCodeToError(ec)) << '\n';
+return false;
+  }
+
+  stream << "LLDB diagnostics written to " << diagnostics_dir << "\n";
+  stream << "Please include the directory content when filing a bug report\n";
+
+  Error error = Create(FileSpec(diagnostics_dir.str()));
+  if (error) {
+stream << toString(std::move(error)) << '\n';
+return false;
+  }
+
+  return true;
+}
+
+Error Diagnostics::Create(const FileSpec ) {
+  for (Callback c : m_callbacks) {
+if (Error err = c(dir))
+  return err;
+  }
+  return Error::success();
+}
Index: lldb/source/Utility/CMakeLists.txt
===
--- lldb/source/Utility/CMakeLists.txt
+++ lldb/source/Utility/CMakeLists.txt
@@ -35,6 +35,7 @@
   DataBufferLLVM.cpp
   DataEncoder.cpp
   DataExtractor.cpp
+  Diagnostics.cpp
   Environment.cpp
   Event.cpp
   FileSpec.cpp
Index: lldb/source/Target/Statistics.cpp
===
--- lldb/source/Target/Statistics.cpp
+++ lldb/source/Target/Statistics.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/UnixSignals.h"
+#include "lldb/Utility/Diagnostics.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -284,3 +285,29 @@
   };
   return std::move(global_stats);
 }
+
+void DebuggerStats::Initialize() { Diagnostics::Instance().AddCallback(Dump); }
+
+void DebuggerStats::Terminate() {}
+
+llvm::Error DebuggerStats::Dump(const FileSpec ) {
+  for (size_t debugger_idx = 0; debugger_idx < Debugger::GetNumDebuggers();
+   debugger_idx++) {
+DebuggerSP debugger_sp(Debugger::GetDebuggerAtIndex(debugger_idx));
+if (!debugger_sp)
+  continue;
+
+std::string filename =
+(Twine("debugger-") + Twine(debugger_idx) + Twine("-stats.json")).str();
+FileSpec stat_file = dir.CopyByAppendingPathComponent(filename);
+
+std::error_code ec;
+raw_fd_ostream stats_stream(stat_file.GetPath(), ec, sys::fs::OF_None);
+if (ec)
+  return 

[Lldb-commits] [PATCH] D135577: Summary:

2022-10-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/tools/lldb-vscode/README.md:39
 
+It is important to note that the directory `~/.vscode/extensions` works for 
users logged in locally to the machine. If you are remoting into the box using 
Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for 
extensions on `~/.vscode-server/extensions` only and you will not see your just 
installed lldb-vscode plug-in. If you want this plugin to be visible to 
remoting users, you will need to either repeat the process above for the 
`~/.vscode-server` folder or create a symbolic link from it to 
`~/.vscode/extensions`:
+

HenriqueBucher wrote:
> JDevlieghere wrote:
> > Please wrap this so it fits in the 80 col limit. 
> Most of this file has lines with more than 80 columns. Would you want the 
> other lines to be reformatted as well?
I wouldn't say "most": there are a few lines that exceed the 80 col limit for a 
valid reason (i.e. having a URL in them) but you're right that there's a few 
lines that should be reflowed (line 25 for example). Since that would be 
orthogonal to this patch, it should be a separate patch. For the same reason 
there's no expectation for you to do that, but it would of course be very much 
appreciated! 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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


[Lldb-commits] [lldb] b8a8c2d - Allow DynamicLoaderDarwinKernel to activate without binary

2022-10-10 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2022-10-10T10:19:09-07:00
New Revision: b8a8c2d47a38ba08012fe9cbb28169a1e0f7be2a

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

LOG: Allow DynamicLoaderDarwinKernel to activate without binary

In https://reviews.llvm.org/D133534 I made a little cleanup
to DynamicLoaderDarwinKernel::CreateInstance and unintentionally
changed the logic.  Previously it would not create an instance
if there was a binary given to lldb and it was not a kernel.
With my change, the absence of any binary would also cause it
to not create.  So connecting to a kernel without any binaries
would fail.

rdar://100985097

Added: 


Modified: 

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp 
b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
index 533585a9b8d18..b0617ce3159c7 100644
--- 
a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ 
b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -152,7 +152,8 @@ DynamicLoader 
*DynamicLoaderDarwinKernel::CreateInstance(Process *process,
   if (!force) {
 // If the user provided an executable binary and it is not a kernel, this
 // plugin should not create an instance.
-if (!is_kernel(process->GetTarget().GetExecutableModulePointer()))
+Module *exec = process->GetTarget().GetExecutableModulePointer();
+if (exec && !is_kernel(exec))
   return nullptr;
 
 // If the target's architecture does not look like an Apple environment,



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


[Lldb-commits] [PATCH] D135577: Summary:

2022-10-10 Thread Henrique Bucher via Phabricator via lldb-commits
HenriqueBucher added inline comments.



Comment at: lldb/tools/lldb-vscode/README.md:39
 
+It is important to note that the directory `~/.vscode/extensions` works for 
users logged in locally to the machine. If you are remoting into the box using 
Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for 
extensions on `~/.vscode-server/extensions` only and you will not see your just 
installed lldb-vscode plug-in. If you want this plugin to be visible to 
remoting users, you will need to either repeat the process above for the 
`~/.vscode-server` folder or create a symbolic link from it to 
`~/.vscode/extensions`:
+

JDevlieghere wrote:
> Please wrap this so it fits in the 80 col limit. 
Most of this file has lines with more than 80 columns. Would you want the other 
lines to be reformatted as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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


[Lldb-commits] [PATCH] D135577: Summary:

2022-10-10 Thread Henrique Bucher via Phabricator via lldb-commits
HenriqueBucher updated this revision to Diff 466538.
HenriqueBucher added a comment.

Removed 2 extra blank lines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

Files:
  lldb/tools/lldb-vscode/README.md


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,19 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for
+users logged in locally to the machine. If you are remoting into the box using
+Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for
+extensions on `~/.vscode-server/extensions` only and you will not see your just
+installed lldb-vscode plug-in. If you want this plugin to be visible to
+remoting users, you will need to either repeat the process above for the
+`~/.vscode-server` folder or create a symbolic link from it to
+`~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  
llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS 
systems:
 


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,19 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for
+users logged in locally to the machine. If you are remoting into the box using
+Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for
+extensions on `~/.vscode-server/extensions` only and you will not see your just
+installed lldb-vscode plug-in. If you want this plugin to be visible to
+remoting users, you will need to either repeat the process above for the
+`~/.vscode-server` folder or create a symbolic link from it to
+`~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS systems:
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D135577: Summary:

2022-10-10 Thread Henrique Bucher via Phabricator via lldb-commits
HenriqueBucher updated this revision to Diff 466537.
HenriqueBucher added a comment.

Formatted text to wrap in 80 columns


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

Files:
  lldb/tools/lldb-vscode/README.md


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,19 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for
+users logged in locally to the machine. If you are remoting into the box using
+Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for
+extensions on `~/.vscode-server/extensions` only and you will not see your just
+installed lldb-vscode plug-in. If you want this plugin to be visible to
+remoting users, you will need to either repeat the process above for the
+`~/.vscode-server` folder or create a symbolic link from it to
+`~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  
llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS 
systems:
 
@@ -61,6 +74,8 @@
 
 This is handy if you want to debug and develope the `lldb-vscode` executable 
when adding features or fixing bugs.
 
+
+
 # Configurations
 
 Launching to attaching require you to create a [launch 
configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations).
 This file


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,19 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for
+users logged in locally to the machine. If you are remoting into the box using
+Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for
+extensions on `~/.vscode-server/extensions` only and you will not see your just
+installed lldb-vscode plug-in. If you want this plugin to be visible to
+remoting users, you will need to either repeat the process above for the
+`~/.vscode-server` folder or create a symbolic link from it to
+`~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS systems:
 
@@ -61,6 +74,8 @@
 
 This is handy if you want to debug and develope the `lldb-vscode` executable when adding features or fixing bugs.
 
+
+
 # Configurations
 
 Launching to attaching require you to create a [launch configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations). This file
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D135577: Summary:

2022-10-10 Thread J. Ryan Stinnett via Phabricator via lldb-commits
jryans added a comment.

Please also give this patch a title that would make sense as the first line of 
a commit message. It currently just says "Summary:" at the moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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


[Lldb-commits] [lldb] fd91e8f - [lldb][test] Skip TestStepAvoidsRegexp.py on Windows

2022-10-10 Thread Michael Buch via lldb-commits

Author: Michael Buch
Date: 2022-10-10T17:07:52+01:00
New Revision: fd91e8f5016626328093aedc8419f12af20ff554

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

LOG: [lldb][test] Skip TestStepAvoidsRegexp.py on Windows

Added: 


Modified: 
lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py 
b/lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
index a3de53e3c4b9..781928ab5a44 100644
--- a/lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
+++ b/lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
@@ -19,6 +19,7 @@ def setUp(self):
 self.dbg.HandleCommand(
 "settings set target.process.thread.step-avoid-regexp 
^ignore::")
 
+@skipIfWindows
 def test_step_avoid_regex(self):
 """Tests stepping into a function which matches the avoid regex"""
 self.build()
@@ -36,6 +37,7 @@ def test_step_avoid_regex(self):
 self.thread.StepInto()
 self.hit_correct_function("main")
 
+@skipIfWindows
 @expectedFailureAll(bugnumber="rdar://100645742")
 def test_step_avoid_regex_abi_tagged_template(self):
 """Tests stepping into an ABI tagged function that matches the avoid 
regex"""



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


[Lldb-commits] [PATCH] D135547: [lldb/Utility] Add GetDescription(Stream&) to StructureData::*

2022-10-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

The change looks good but this needs unit tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135547

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


[Lldb-commits] [PATCH] D135577: Summary:

2022-10-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere requested changes to this revision.
JDevlieghere added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/tools/lldb-vscode/README.md:39
 
+It is important to note that the directory `~/.vscode/extensions` works for 
users logged in locally to the machine. If you are remoting into the box using 
Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for 
extensions on `~/.vscode-server/extensions` only and you will not see your just 
installed lldb-vscode plug-in. If you want this plugin to be visible to 
remoting users, you will need to either repeat the process above for the 
`~/.vscode-server` folder or create a symbolic link from it to 
`~/.vscode/extensions`:
+

Please wrap this so it fits in the 80 col limit. 



Comment at: lldb/tools/lldb-vscode/README.md:70-71
 
+
+
 # Configurations

Remove


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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


[Lldb-commits] [PATCH] D135516: [lldb] [MainLoopPosix] Fix crash upon adding lots of pending callbacks

2022-10-10 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

In D135516#3846182 , @labath wrote:

> I never expected we would have so many callbacks that we'd have to worry 
> about this, but yes, this is one way to fix the problem. Another (slightly 
> simpler, but also less performant) would be to make the write pipe 
> nonblocking, and do not treat EAGAIN as an error.

Well, I've only hit this in a pathological case (sending a lot of small packets 
without checking whether the loop in the communication thread didn't exit) but 
I've figured out we want some protection anyway.

The pipe logic uses select helper anyway, so it does return a timeout error if 
the buffer is full. However, I've figured that it'd be better to avoid 
polluting the buffer with tons of "interrupts" if only one is really meaningful.


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

https://reviews.llvm.org/D135516

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


[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

2022-10-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 466526.
JDevlieghere marked 3 inline comments as done.
JDevlieghere added a comment.

Address code review feedback


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

https://reviews.llvm.org/D134991

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Target/Statistics.h
  lldb/include/lldb/Utility/Diagnostics.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Initialization/SystemInitializerCommon.cpp
  lldb/source/Target/Statistics.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/Diagnostics.cpp
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -788,6 +788,10 @@
<< '\n';
 return 1;
   }
+
+  // Setup LLDB signal handlers once the debugger has been initialized.
+  SBDebugger::PrintDiagnosticsOnError();
+
   SBHostOS::ThreadCreated("");
 
   signal(SIGINT, sigint_handler);
Index: lldb/source/Utility/Diagnostics.cpp
===
--- /dev/null
+++ lldb/source/Utility/Diagnostics.cpp
@@ -0,0 +1,87 @@
+//===-- Diagnostics.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Utility/Diagnostics.h"
+#include "lldb/Utility/LLDBAssert.h"
+
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace lldb_private;
+using namespace lldb;
+using namespace llvm;
+
+void Diagnostics::Initialize() {
+  lldbassert(!InstanceImpl() && "Already initialized.");
+  InstanceImpl().emplace();
+}
+
+void Diagnostics::Terminate() {
+  lldbassert(InstanceImpl() && "Already terminated.");
+  InstanceImpl().reset();
+}
+
+Optional ::InstanceImpl() {
+  static Optional g_diagnostics;
+  return g_diagnostics;
+}
+
+Diagnostics ::Instance() { return *InstanceImpl(); }
+
+Diagnostics::Diagnostics() {}
+
+Diagnostics::~Diagnostics() {}
+
+void Diagnostics::AddCallback(Callback callback) {
+  std::lock_guard guard(m_callbacks_mutex);
+  assert(std::find(m_callbacks.begin(), m_callbacks.end(), callback) ==
+ m_callbacks.end() &&
+ "callback already added");
+  m_callbacks.push_back(callback);
+}
+
+void Diagnostics::RemoveCallback(Callback callback) {
+  std::lock_guard guard(m_callbacks_mutex);
+  assert(std::find(m_callbacks.begin(), m_callbacks.end(), callback) !=
+ m_callbacks.end() &&
+ "callback already removed");
+  m_callbacks.erase(
+  std::remove(m_callbacks.begin(), m_callbacks.end(), callback),
+  m_callbacks.end());
+}
+
+bool Diagnostics::Dump(raw_ostream ) {
+  SmallString<128> diagnostics_dir;
+  std::error_code ec =
+  sys::fs::createUniqueDirectory("diagnostics", diagnostics_dir);
+  if (ec) {
+stream << "unable to create diagnostic dir: "
+   << toString(errorCodeToError(ec)) << '\n';
+return false;
+  }
+
+  stream << "LLDB diagnostics written to " << diagnostics_dir << "\n";
+  stream << "Please include the directory content when filing a bug report\n";
+
+  Error error = Create(FileSpec(diagnostics_dir.str()));
+  if (error) {
+stream << toString(std::move(error)) << '\n';
+return false;
+  }
+
+  return true;
+}
+
+Error Diagnostics::Create(const FileSpec ) {
+  for (Callback c : m_callbacks) {
+if (Error err = c(dir))
+  return err;
+  }
+  return Error::success();
+}
Index: lldb/source/Utility/CMakeLists.txt
===
--- lldb/source/Utility/CMakeLists.txt
+++ lldb/source/Utility/CMakeLists.txt
@@ -35,6 +35,7 @@
   DataBufferLLVM.cpp
   DataEncoder.cpp
   DataExtractor.cpp
+  Diagnostics.cpp
   Environment.cpp
   Event.cpp
   FileSpec.cpp
Index: lldb/source/Target/Statistics.cpp
===
--- lldb/source/Target/Statistics.cpp
+++ lldb/source/Target/Statistics.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/UnixSignals.h"
+#include "lldb/Utility/Diagnostics.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -284,3 +285,31 @@
   };
   return std::move(global_stats);
 }
+
+void DebuggerStats::Initialize() { Diagnostics::Instance().AddCallback(Dump); }
+
+void DebuggerStats::Terminate() {
+  Diagnostics::Instance().RemoveCallback(Dump);
+}
+
+llvm::Error DebuggerStats::Dump(const FileSpec ) {
+  for (size_t debugger_idx = 0; debugger_idx < Debugger::GetNumDebuggers();
+   debugger_idx++) {
+DebuggerSP 

[Lldb-commits] [PATCH] D135413: [lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with auto return types

2022-10-10 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added a comment.

In D135413#3846954 , 
@stella.stamenova wrote:

> Looks like this change broke the windows lldb bot: 
> https://lab.llvm.org/buildbot/#/builders/83/builds/24631

I thought I had skipped it on Windows. Will fix now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135413

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


[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

2022-10-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere marked 4 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/Log.h:231
llvm::StringRef function, const char *format,
-   Args &&... args) {
+   Args &&...args) {
 Format(file, function,

DavidSpickett wrote:
> These seem unrelated but not doing any harm.
This was unintentional, probably my editor removing trailing whitespace and 
clang-format kicking in because the line got changed.



Comment at: lldb/source/API/SBDebugger.cpp:222
 
+static void DumpDiagnostics(void* cookie) {
+  Diagnostics::Instance().Dump(llvm::errs());

DavidSpickett wrote:
> What is `cookie` used for? (or rather, used elsewhere)
It's like the `baton` we sometimes pass around, just a way to pass additional 
data to your signal handler. 



Comment at: lldb/source/Utility/Diagnostics.cpp:54
+bool Diagnostics::Dump(raw_ostream ) {
+  SmallString<128> diagnostics_dir;
+  std::error_code ec =

labath wrote:
> I am not sure how common this is, but I have recently seen (not in lldb, but 
> another app) a bug, which essentially caused two threads to crash at once 
> (one SEGV, one ABRT). In those situations, you probably don't want to 
> crash-printing routines to race with each other. You can consider putting a 
> (global) mutex in this function, or something like that. (unless the llvm 
> function takes care of that already).
LLVM uses an atomic flag to make sure an exception handler is only run once. 


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

https://reviews.llvm.org/D134991

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


[Lldb-commits] [PATCH] D135413: [lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with auto return types

2022-10-10 Thread Stella Stamenova via Phabricator via lldb-commits
stella.stamenova added a comment.

Looks like this change broke the windows lldb bot: 
https://lab.llvm.org/buildbot/#/builders/83/builds/24631


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135413

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


[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-10-10 Thread Erich Keane via Phabricator via lldb-commits
erichkeane accepted this revision.
erichkeane added a comment.

This LGTM, but please give @davrec time (a few days?) to do another review 
before merging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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


[Lldb-commits] [PATCH] D135577: Summary:

2022-10-10 Thread Henrique Bucher via Phabricator via lldb-commits
HenriqueBucher created this revision.
HenriqueBucher added a reviewer: clayborg.
Herald added a project: All.
HenriqueBucher requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This documentation patch adds information to allow remote users to also use the 
plugin as it will be invisible to them using the current instructions. It 
solves issue #58252.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135577

Files:
  lldb/tools/lldb-vscode/README.md


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,12 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for 
users logged in locally to the machine. If you are remoting into the box using 
Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for 
extensions on `~/.vscode-server/extensions` only and you will not see your just 
installed lldb-vscode plug-in. If you want this plugin to be visible to 
remoting users, you will need to either repeat the process above for the 
`~/.vscode-server` folder or create a symbolic link from it to 
`~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  
llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS 
systems:
 
@@ -61,6 +67,8 @@
 
 This is handy if you want to debug and develope the `lldb-vscode` executable 
when adding features or fixing bugs.
 
+
+
 # Configurations
 
 Launching to attaching require you to create a [launch 
configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations).
 This file


Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -36,6 +36,12 @@
 $ cp /path/to/a/built/liblldb.so .
 ```
 
+It is important to note that the directory `~/.vscode/extensions` works for users logged in locally to the machine. If you are remoting into the box using Visual Studio Code's Remote plugins (SSH, WSL, Docker) it will look for extensions on `~/.vscode-server/extensions` only and you will not see your just installed lldb-vscode plug-in. If you want this plugin to be visible to remoting users, you will need to either repeat the process above for the `~/.vscode-server` folder or create a symbolic link from it to `~/.vscode/extensions`:
+
+```
+$ cd ~/.vscode-server/extensions
+$ ln -s ~/.vscode/extensions/llvm-org.lldb-vscode-0.1.0  llvm-org.lldb-vscode-0.1.0
+```
 
 If you want to make a stand alone plug-in that you can send to others on macOS systems:
 
@@ -61,6 +67,8 @@
 
 This is handy if you want to debug and develope the `lldb-vscode` executable when adding features or fixing bugs.
 
+
+
 # Configurations
 
 Launching to attaching require you to create a [launch configuration](https://code.visualstudio.com/Docs/editor/debugging#_launch-configurations). This file
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D135413: [lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with auto return types

2022-10-10 Thread Michael Buch via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa4561d934877: [lldb][CPlusPlusLanguage] Respect the 
step-avoid-regex for functions with auto… (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135413

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
  lldb/test/API/functionalities/step-avoids-regexp/Makefile
  lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
  lldb/test/API/functionalities/step-avoids-regexp/main.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -65,8 +65,10 @@
"XX::(anonymous namespace)::anon_class::anon_func"},
 
   // Lambda
-  {"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()() const",
-   "main::{lambda()#1}::operator()() const::{lambda()#1}", "operator()", "()", "const",
+  {"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()() "
+   "const",
+   "main::{lambda()#1}::operator()() const::{lambda()#1}", "operator()",
+   "()", "const",
"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()"},
 
   // Function pointers
@@ -110,8 +112,15 @@
"f, sizeof(B)", "()", "",
"f, sizeof(B)"},
   {"llvm::Optional::operator*() const volatile &&",
-   "llvm::Optional", "operator*", "()", "const volatile &&",
-   "llvm::Optional::operator*"}};
+   "llvm::Optional", "operator*", "()",
+   "const volatile &&", "llvm::Optional::operator*"},
+
+  // auto return type
+  {"auto std::test_return_auto() const", "std",
+   "test_return_auto", "()", "const", "std::test_return_auto"},
+  {"decltype(auto) std::test_return_auto(int) const", "std",
+   "test_return_auto", "(int)", "const",
+   "std::test_return_auto"}};
 
   for (const auto  : test_cases) {
 CPlusPlusLanguage::MethodName method(ConstString(test.input));
Index: lldb/test/API/functionalities/step-avoids-regexp/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/step-avoids-regexp/main.cpp
@@ -0,0 +1,16 @@
+namespace ignore {
+template  auto auto_ret(T x) { return 0; }
+[[gnu::abi_tag("test")]] int with_tag() { return 0; }
+template  [[gnu::abi_tag("test")]] int with_tag_template() {
+  return 0;
+}
+
+template  decltype(auto) decltype_auto_ret(T x) { return 0; }
+} // namespace ignore
+
+int main() {
+  auto v1 = ignore::auto_ret(5);
+  auto v2 = ignore::with_tag();
+  auto v3 = ignore::decltype_auto_ret(5);
+  auto v4 = ignore::with_tag_template();
+}
Index: lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
===
--- /dev/null
+++ lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
@@ -0,0 +1,47 @@
+"""
+Test thread step-in ignores frames according to the "Avoid regexp" option.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class StepAvoidsRegexTestCase(TestBase):
+def hit_correct_function(self, pattern):
+name = self.thread.frames[0].GetFunctionName()
+self.assertTrue(
+pattern in name, "Got to '%s' not the expected function '%s'." %
+(name, pattern))
+
+def setUp(self):
+TestBase.setUp(self)
+self.dbg.HandleCommand(
+"settings set target.process.thread.step-avoid-regexp ^ignore::")
+
+def test_step_avoid_regex(self):
+"""Tests stepping into a function which matches the avoid regex"""
+self.build()
+(_, _, self.thread, _) = lldbutil.run_to_source_breakpoint(self, "main", lldb.SBFileSpec('main.cpp'))
+
+# Try to step into ignore::auto_ret
+self.thread.StepInto()
+self.hit_correct_function("main")
+
+# Try to step into ignore::with_tag
+self.thread.StepInto()
+self.hit_correct_function("main")
+
+# Try to step into ignore::decltype_auto_ret
+self.thread.StepInto()
+self.hit_correct_function("main")
+
+@expectedFailureAll(bugnumber="rdar://100645742")
+def test_step_avoid_regex_abi_tagged_template(self):
+"""Tests stepping into an ABI tagged function that matches the avoid regex"""
+self.build()
+(_, _, self.thread, _) = lldbutil.run_to_source_breakpoint(self, "with_tag_template", lldb.SBFileSpec('main.cpp'))
+
+# Try to step into ignore::with_tag_template
+self.thread.StepInto()
+self.hit_correct_function("main")
Index: 

[Lldb-commits] [lldb] a4561d9 - [lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with auto return types

2022-10-10 Thread Michael Buch via lldb-commits

Author: Michael Buch
Date: 2022-10-10T12:50:15+01:00
New Revision: a4561d934877fbba5cfb3cac3195a41707ba6043

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

LOG: [lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with 
auto return types

**Summary**

The primary motivation for this patch is to make sure we handle
the step-in behaviour for functions in the `std` namespace which
have an `auto` return type. Currently the default `step-avoid-regex`
setting is `^std::` but LLDB will still step into template functions
with `auto` return types in the `std` namespace.

**Details**
When we hit a breakpoint and check whether we should stop, we call
into `ThreadPlanStepInRange::FrameMatchesAvoidCriteria`. We then ask
for the frame function name via 
`SymbolContext::GetFunctionName(Mangled::ePreferDemangledWithoutArguments)`.
This ends up trying to parse the function name using 
`CPlusPlusLanguage::MethodName::GetBasename` which
parses the raw demangled name string.

`CPlusPlusNameParser::ParseFunctionImpl` calls `ConsumeTypename` to skip
the (in our case auto) return type of the demangled name (according to the
Itanium ABI this is a valid thing to encode into the mangled name). However,
`ConsumeTypename` doesn't strip out a plain `auto` identifier
(it will strip a `decltype(auto) return type though). So we are now left with
a basename that still has the return type in it, thus failing to match the 
`^std::`
regex.

Example frame where the return type is still part of the function name:
```
Process 1234 stopped
* thread #1, stop reason = step in
frame #0: 0x12345678 repro`auto std::test_return_auto() at 
main.cpp:12:5
   9
   10   template 
   11   auto test_return_auto() {
-> 12   return 42;
   13   }
```

This is another case where the `CPlusPlusNameParser` breaks us in subtle ways
due to evolving C++ syntax. There are longer-term plans of replacing the 
hand-rolled
C++ parser with an alternative that uses the mangle tree API to do the parsing 
for us.

**Testing**

* Added API and unit-tests
* Adding support for ABI tags into the parser is a larger undertaking
  which we would rather solve properly by using libcxxabi's mangle tree
  parser

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

Added: 
lldb/test/API/functionalities/step-avoids-regexp/Makefile
lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
lldb/test/API/functionalities/step-avoids-regexp/main.cpp

Modified: 
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
index bc3b42a4ef07a..ac70226ca2a4d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -9,6 +9,7 @@
 #include "CPlusPlusNameParser.h"
 
 #include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/Threading.h"
 
@@ -106,7 +107,7 @@ CPlusPlusNameParser::ParseFunctionImpl(bool 
expect_return_type) {
   Bookmark start_position = SetBookmark();
   if (expect_return_type) {
 // Consume return type if it's expected.
-if (!ConsumeTypename())
+if (!ConsumeToken(tok::kw_auto) && !ConsumeTypename())
   return None;
   }
 

diff  --git a/lldb/test/API/functionalities/step-avoids-regexp/Makefile 
b/lldb/test/API/functionalities/step-avoids-regexp/Makefile
new file mode 100644
index 0..4f79c0a900c3a
--- /dev/null
+++ b/lldb/test/API/functionalities/step-avoids-regexp/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++20
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py 
b/lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
new file mode 100644
index 0..a3de53e3c4b91
--- /dev/null
+++ b/lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
@@ -0,0 +1,47 @@
+"""
+Test thread step-in ignores frames according to the "Avoid regexp" option.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class StepAvoidsRegexTestCase(TestBase):
+def hit_correct_function(self, pattern):
+name = self.thread.frames[0].GetFunctionName()
+self.assertTrue(
+pattern in name, "Got to '%s' not the expected function '%s'." %
+(name, pattern))
+
+def setUp(self):
+TestBase.setUp(self)
+self.dbg.HandleCommand(
+

[Lldb-commits] [PATCH] D135413: [lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with auto return types

2022-10-10 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added a comment.

In D135413#3846390 , @labath wrote:

>> There are longer-term plans of replacing the hand-rolled C++ parser with an 
>> alternative that uses the mangle tree API to do the parsing for us.
>
> You may be aware of this, but I feel I should mention that there are cases 
> when a function just does not have a mangled name, either because it is in an 
> `extern "C"` block, or because it was complied with 
> `--dwarf-linkage-names=Abstract` (default for `-gsce`). In this case, we 
> construct a fake demangled name from the DWARF debug info (the names of 
> enclosing (DW_TAG_)namespaces, and the types for (DW_TAG_)formal_parameters. 
> Of course, in this case, it makes even less sense to parse the resulting 
> string, since we're the ones who constructed it in the first place. However, 
> it may not be sufficient to assume that one can just start with a mangled 
> name, and get everything out that way.

Good points, thanks! Didn't know about the `--dwarf-linkage-names=Abstract` 
option


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135413

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


[Lldb-commits] [PATCH] D135413: [lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with auto return types

2022-10-10 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 466467.
Michael137 added a comment.

- Reword commit message
- Remove redundant header from test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135413

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
  lldb/test/API/functionalities/step-avoids-regexp/Makefile
  lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
  lldb/test/API/functionalities/step-avoids-regexp/main.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -65,8 +65,10 @@
"XX::(anonymous namespace)::anon_class::anon_func"},
 
   // Lambda
-  {"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()() const",
-   "main::{lambda()#1}::operator()() const::{lambda()#1}", "operator()", "()", "const",
+  {"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()() "
+   "const",
+   "main::{lambda()#1}::operator()() const::{lambda()#1}", "operator()",
+   "()", "const",
"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()"},
 
   // Function pointers
@@ -110,8 +112,15 @@
"f, sizeof(B)", "()", "",
"f, sizeof(B)"},
   {"llvm::Optional::operator*() const volatile &&",
-   "llvm::Optional", "operator*", "()", "const volatile &&",
-   "llvm::Optional::operator*"}};
+   "llvm::Optional", "operator*", "()",
+   "const volatile &&", "llvm::Optional::operator*"},
+
+  // auto return type
+  {"auto std::test_return_auto() const", "std",
+   "test_return_auto", "()", "const", "std::test_return_auto"},
+  {"decltype(auto) std::test_return_auto(int) const", "std",
+   "test_return_auto", "(int)", "const",
+   "std::test_return_auto"}};
 
   for (const auto  : test_cases) {
 CPlusPlusLanguage::MethodName method(ConstString(test.input));
Index: lldb/test/API/functionalities/step-avoids-regexp/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/step-avoids-regexp/main.cpp
@@ -0,0 +1,16 @@
+namespace ignore {
+template  auto auto_ret(T x) { return 0; }
+[[gnu::abi_tag("test")]] int with_tag() { return 0; }
+template  [[gnu::abi_tag("test")]] int with_tag_template() {
+  return 0;
+}
+
+template  decltype(auto) decltype_auto_ret(T x) { return 0; }
+} // namespace ignore
+
+int main() {
+  auto v1 = ignore::auto_ret(5);
+  auto v2 = ignore::with_tag();
+  auto v3 = ignore::decltype_auto_ret(5);
+  auto v4 = ignore::with_tag_template();
+}
Index: lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
===
--- /dev/null
+++ lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
@@ -0,0 +1,47 @@
+"""
+Test thread step-in ignores frames according to the "Avoid regexp" option.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class StepAvoidsRegexTestCase(TestBase):
+def hit_correct_function(self, pattern):
+name = self.thread.frames[0].GetFunctionName()
+self.assertTrue(
+pattern in name, "Got to '%s' not the expected function '%s'." %
+(name, pattern))
+
+def setUp(self):
+TestBase.setUp(self)
+self.dbg.HandleCommand(
+"settings set target.process.thread.step-avoid-regexp ^ignore::")
+
+def test_step_avoid_regex(self):
+"""Tests stepping into a function which matches the avoid regex"""
+self.build()
+(_, _, self.thread, _) = lldbutil.run_to_source_breakpoint(self, "main", lldb.SBFileSpec('main.cpp'))
+
+# Try to step into ignore::auto_ret
+self.thread.StepInto()
+self.hit_correct_function("main")
+
+# Try to step into ignore::with_tag
+self.thread.StepInto()
+self.hit_correct_function("main")
+
+# Try to step into ignore::decltype_auto_ret
+self.thread.StepInto()
+self.hit_correct_function("main")
+
+@expectedFailureAll(bugnumber="rdar://100645742")
+def test_step_avoid_regex_abi_tagged_template(self):
+"""Tests stepping into an ABI tagged function that matches the avoid regex"""
+self.build()
+(_, _, self.thread, _) = lldbutil.run_to_source_breakpoint(self, "with_tag_template", lldb.SBFileSpec('main.cpp'))
+
+# Try to step into ignore::with_tag_template
+self.thread.StepInto()
+self.hit_correct_function("main")
Index: lldb/test/API/functionalities/step-avoids-regexp/Makefile

[Lldb-commits] [lldb] b3d4d9c - [LLDB] Complete set of char tests for static integral members

2022-10-10 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2022-10-10T11:16:19Z
New Revision: b3d4d9ced17c5b3ebd6bf5b61731ddcdde3cbca5

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

LOG: [LLDB] Complete set of char tests for static integral members

Previously we had a bit of a mix of "signed char" "unsigned char" and
"char".

This adds seperate min and max checks for all three types.

Depends on D135170

Reviewed By: Michael137

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

Added: 


Modified: 

lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
lldb/test/API/lang/cpp/const_static_integral_member/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
 
b/lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
index 5252247191383..91ed14ed48ab7 100644
--- 
a/lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
+++ 
b/lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
@@ -34,6 +34,7 @@ def test(self):
 
 # Test that minimum and maximum values for each data type are right.
 self.expect_expr("A::char_max == char_max", result_value="true")
+self.expect_expr("A::schar_max == schar_max", result_value="true")
 self.expect_expr("A::uchar_max == uchar_max", result_value="true")
 self.expect_expr("A::int_max == int_max", result_value="true")
 self.expect_expr("A::uint_max == uint_max", result_value="true")
@@ -43,6 +44,7 @@ def test(self):
 self.expect_expr("A::ulonglong_max == ulonglong_max", 
result_value="true")
 
 self.expect_expr("A::char_min == char_min", result_value="true")
+self.expect_expr("A::schar_min == schar_min", result_value="true")
 self.expect_expr("A::uchar_min == uchar_min", result_value="true")
 self.expect_expr("A::int_min == int_min", result_value="true")
 self.expect_expr("A::uint_min == uint_min", result_value="true")

diff  --git a/lldb/test/API/lang/cpp/const_static_integral_member/main.cpp 
b/lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
index 977e12295760a..09ab9e6698132 100644
--- a/lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ b/lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -26,7 +26,8 @@ struct A {
   const static int int_val_with_address = 2;
   const static bool bool_val = true;
 
-  const static auto char_max = std::numeric_limits::max();
+  const static auto char_max = std::numeric_limits::max();
+  const static auto schar_max = std::numeric_limits::max();
   const static auto uchar_max = std::numeric_limits::max();
   const static auto int_max = std::numeric_limits::max();
   const static auto uint_max = std::numeric_limits::max();
@@ -37,6 +38,7 @@ struct A {
   std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
+  const static auto schar_min = std::numeric_limits::min();
   const static auto uchar_min = std::numeric_limits::min();
   const static auto int_min = std::numeric_limits::min();
   const static auto uint_min = std::numeric_limits::min();
@@ -83,6 +85,7 @@ int main() {
   A a;
 
   auto char_max = A::char_max;
+  auto schar_max = A::schar_max;
   auto uchar_max = A::uchar_max;
   auto int_max = A::int_max;
   auto uint_max = A::uint_max;
@@ -92,6 +95,7 @@ int main() {
   auto ulonglong_max = A::ulonglong_max;
 
   auto char_min = A::char_min;
+  auto schar_min = A::schar_min;
   auto uchar_min = A::uchar_min;
   auto int_min = A::int_min;
   auto uint_min = A::uint_min;



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


[Lldb-commits] [PATCH] D135352: [LLDB] Complete set of char tests for static integral members

2022-10-10 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb3d4d9ced17c: [LLDB] Complete set of char tests for static 
integral members (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135352

Files:
  
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
  lldb/test/API/lang/cpp/const_static_integral_member/main.cpp


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -26,7 +26,8 @@
   const static int int_val_with_address = 2;
   const static bool bool_val = true;
 
-  const static auto char_max = std::numeric_limits::max();
+  const static auto char_max = std::numeric_limits::max();
+  const static auto schar_max = std::numeric_limits::max();
   const static auto uchar_max = std::numeric_limits::max();
   const static auto int_max = std::numeric_limits::max();
   const static auto uint_max = std::numeric_limits::max();
@@ -37,6 +38,7 @@
   std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
+  const static auto schar_min = std::numeric_limits::min();
   const static auto uchar_min = std::numeric_limits::min();
   const static auto int_min = std::numeric_limits::min();
   const static auto uint_min = std::numeric_limits::min();
@@ -83,6 +85,7 @@
   A a;
 
   auto char_max = A::char_max;
+  auto schar_max = A::schar_max;
   auto uchar_max = A::uchar_max;
   auto int_max = A::int_max;
   auto uint_max = A::uint_max;
@@ -92,6 +95,7 @@
   auto ulonglong_max = A::ulonglong_max;
 
   auto char_min = A::char_min;
+  auto schar_min = A::schar_min;
   auto uchar_min = A::uchar_min;
   auto int_min = A::int_min;
   auto uint_min = A::uint_min;
Index: 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
===
--- 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
+++ 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
@@ -34,6 +34,7 @@
 
 # Test that minimum and maximum values for each data type are right.
 self.expect_expr("A::char_max == char_max", result_value="true")
+self.expect_expr("A::schar_max == schar_max", result_value="true")
 self.expect_expr("A::uchar_max == uchar_max", result_value="true")
 self.expect_expr("A::int_max == int_max", result_value="true")
 self.expect_expr("A::uint_max == uint_max", result_value="true")
@@ -43,6 +44,7 @@
 self.expect_expr("A::ulonglong_max == ulonglong_max", 
result_value="true")
 
 self.expect_expr("A::char_min == char_min", result_value="true")
+self.expect_expr("A::schar_min == schar_min", result_value="true")
 self.expect_expr("A::uchar_min == uchar_min", result_value="true")
 self.expect_expr("A::int_min == int_min", result_value="true")
 self.expect_expr("A::uint_min == uint_min", result_value="true")


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -26,7 +26,8 @@
   const static int int_val_with_address = 2;
   const static bool bool_val = true;
 
-  const static auto char_max = std::numeric_limits::max();
+  const static auto char_max = std::numeric_limits::max();
+  const static auto schar_max = std::numeric_limits::max();
   const static auto uchar_max = std::numeric_limits::max();
   const static auto int_max = std::numeric_limits::max();
   const static auto uint_max = std::numeric_limits::max();
@@ -37,6 +38,7 @@
   std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
+  const static auto schar_min = std::numeric_limits::min();
   const static auto uchar_min = std::numeric_limits::min();
   const static auto int_min = std::numeric_limits::min();
   const static auto uint_min = std::numeric_limits::min();
@@ -83,6 +85,7 @@
   A a;
 
   auto char_max = A::char_max;
+  auto schar_max = A::schar_max;
   auto uchar_max = A::uchar_max;
   auto int_max = A::int_max;
   auto uint_max = A::uint_max;
@@ -92,6 +95,7 @@
   auto ulonglong_max = A::ulonglong_max;
 
   auto char_min = A::char_min;
+  auto schar_min = A::schar_min;
   auto uchar_min = A::uchar_min;
   auto int_min = A::int_min;
   auto uint_min = A::uint_min;
Index: lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
===
--- 

[Lldb-commits] [PATCH] D135352: [LLDB] Complete set of char tests for static integral members

2022-10-10 Thread Michael Buch via Phabricator via lldb-commits
Michael137 accepted this revision.
Michael137 added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135352

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


[Lldb-commits] [PATCH] D135352: [LLDB] Complete set of char tests for static integral members

2022-10-10 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135352

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


[Lldb-commits] [PATCH] D135413: [lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with auto return types

2022-10-10 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

> There are longer-term plans of replacing the hand-rolled C++ parser with an 
> alternative that uses the mangle tree API to do the parsing for us.

You may be aware of this, but I feel I should mention that there are cases when 
a function just does not have a mangled name, either because it is in an 
`extern "C"` block, or because it was complied with 
`--dwarf-linkage-names=Abstract` (default for `-gsce`). In this case, we 
construct a fake demangled name from the DWARF debug info (the names of 
enclosing (DW_TAG_)namespaces, and the types for (DW_TAG_)formal_parameters. Of 
course, in this case, it makes even less sense to parse the resulting string, 
since we're the ones who constructed it in the first place. However, it may not 
be sufficient to assume that one can just start with a mangled name, and get 
everything out that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135413

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


[Lldb-commits] [PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

2022-10-10 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

You need to update the python file for the test as well.

What does the output look like, do we actually print the character itself or 
it's value? (just curious, which one we do is probably out of scope for this 
change)




Comment at: clang/lib/AST/StmtPrinter.cpp:1298
+  case BuiltinType::WChar_U:
+break; // no suffix.
   }

Is there any reason to have a suffix here?

I admit this function is puzzling to me anyway given that char has a prefix and 
this won't. Perhaps this is because char here is not "character" it's an 
integer of a certain size. Where wide char is more about, well, being an actual 
character.

And reading https://en.cppreference.com/w/cpp/language/character_literal the 
suffix would be "L" which we've already used.

As long as it prints in a way that's useful for the developer that's fine.



Comment at: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp:38
   std::numeric_limits::max();
+  const static auto wchar_max = std::numeric_limits::max();
 

Is there a specific `signed wchar_t` and `unsigned wchar_t` like for char?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135461

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


[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

2022-10-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

The test looks great, and the comments have helped. I still have a couple of 
questions about the algorithm though.




Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:431
+uint64_t offset = pair.first;
+auto  = pair.second;
+lldbassert(offset >= start_offset);

This shadowing the fields member confused me for quite some time. Could you 
choose a different name for one of them?



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:459
+  uint64_t end_offset = offset + fields.back()->bit_size;
+  parent->fields.push_back(fields.back());
+  end_offset_map[end_offset].push_back(parent);

Can `parent` be a union here? Would the algorithm still be correct?



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:471
+  } else {
+for (auto  : fields) {
+  parent->fields.push_back(field);

IIUC, this code is reached when the `parent` object is a union. However, the 
parent is chosen such that it ends before the start of these new fields? 
Therefore its start address will be before the start of these fields as well. 
Is it correct to add the fields to the union under these circumstances, or am I 
misunderstanding something?



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h:64
+uint64_t base_offset;
+llvm::SmallVector fields;
+

zequanwu wrote:
> labath wrote:
> > Can we drop the `SP` part? I think that the owning references (I guess 
> > that's this field) could be just plain Field instances (unique_ptr 
> > at worst), and the rest could just be plain pointers and references.
> The Field object is shared between fields and m_fields. And we can't have 
> Field member instance inside Field class.
You can't have a `Field` member, but you can have a Field*, unique_ptr 
and std::vector members. SmallVector is also not possible, for 
reasons that are mostly obvious, but then again, storing a pointer inside a 
SmallVector negates most of the benefits that one could hope to gain by using 
it.

My point is that that using a shared pointer makes it harder to understand the 
relationships between the objects because it obfuscates the ownership aspect. 
Sometimes that is unavoidable, like when there just isn't a single object that 
can own some other object (although llvm mostly avoids that by putting the 
ownership inside some "context" object), but it's not clear to me why that 
would be the case here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

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


[Lldb-commits] [PATCH] D135516: [lldb] [MainLoopPosix] Fix crash upon adding lots of pending callbacks

2022-10-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I never expected we would have so many callbacks that we'd have to worry about 
this, but yes, this is one way to fix the problem. Another (slightly simpler, 
but also less performant) would be to make the write pipe nonblocking, and do 
not treat EAGAIN as an error.




Comment at: lldb/source/Host/posix/MainLoopPosix.cpp:408
   assert(bytes_written == 1);
+  m_trigger_done = true;
 }

I /think/ this is not right. The other thread can wake up as soon as the Write 
call is done, and can proceed to clear the `done` flag before we are able to 
set it. If we set it afterwards, then we we suppress all subsequent writes, and 
the callbacks would never run. If we set the flag before we make the Write 
call, then it should be ok -- though the flag should probably have a different 
name then.


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

https://reviews.llvm.org/D135516

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