[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)

[Pavel Labath via lldb-commits]

labath wrote:

This is not specific to linux. I've now figured out how to scroll the green 
dragon output, and I've found 
[at](https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/9537/) 
[least](https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/9474/)
 
[three](https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/9457/)
 failures on mac as well.

I don't think disabling the test everywhere is a solution. If we can't get this 
resolved quickly (O(days)), we should revert the patch and continue the 
investigation offline. I've captured the [gdb packet 
log](https://paste.debian.net/hidden/1c7c754e/) from one of the failing runs, 
maybe that can get you started?

https://github.com/llvm/llvm-project/pull/90930
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,459 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_CORE_DILAST_H
+#define LLDB_CORE_DILAST_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"

labath wrote:

Okay, so could one say that it's usage is similar to the code in 
[CPlusPlusNameParser](https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp),
 which uses clang to tokenize the (function) name, but then does the parsing 
itself? Or does it go deeper than that?

The thing I'm thinking about is that "having all of the tokens defined" may not 
be such an advantage if we later add some features from other languages (I 
don't know, let's say the `?.` operator from Swift). For lexing, that may not 
matter cause you could maybe lex `?` and `.` separately and then join them 
later, but if it's doing more, then that may be a problem..

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)

[Pavel Labath via lldb-commits]

labath wrote:

> That being said, if there is something we can do to this PR to make it easy 
> for you (Pavel) to implement this feature, we can make those changes. Like I 
> think it would be fair to change all `PeekMemory/DoPeekMemory` over a virtual 
> `ReadMemory/DoReadMemory` that returns a `DataExtractor` as this can easily 
> take place of the `ArrayRef` values that are returned.
> 
> This:
> 
> ```
>   virtual llvm::ArrayRef PeekMemory(lldb::addr_t low, lldb::addr_t 
> high);
> ```
> 
> Could become:
> 
> ```
>   virtual DataExtractor ReadMemory(lldb::addr_t low, lldb::addr_t high);
> ```
> 
> And we can adjust all users of `PeekMemory` and `DoPeakMemory` to use the the 
> above functions?

I'm not sure which feature are you referring to now. I'm not particularly 
worried about the `/proc/kcore` use case, as I don't know if I'll ever get to 
that, and it will require more substantial changes anyway.

The thing I am looking into right now is the ability to make `memory find` 
better in general, and not just for core files. My thinking is that if I can 
implement (and I'm offering to do that) a better generic implementation of 
`memory find` then probably the entirety of this patch becomes redundant. But 
to know that for sure, I'd like to understand more about your motivation for 
it. Like, what is the main problem that you're trying to solve with this? Is it 
to fix some specific bug (my bug was that the operation aborts as soon as it 
encounters a hole in the address space)? Or is it to make it faster? If so, 
what level of performance would you consider satisfactory?

https://github.com/llvm/llvm-project/pull/102536
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

@@ -174,6 +177,84 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(lldb_private::ConstString name,
+ lldb_private::ConstString impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_summary_count(0) {}
+
+  lldb_private::ConstString GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; }
+
+  ConstString GetImplType() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+  // Basic RAII class to increment the summary count when the call is complete.
+  // In the future this can be extended to collect information about the
+  // elapsed time for a single request.
+  class SummaryInvocation {
+  public:
+SummaryInvocation(SummaryStatistics &summary)
+: m_summary(summary), m_elapsed_time(summary.GetDurationReference()) {}
+~SummaryInvocation() { m_summary.OnInvoked(); }
+
+// Delete the copy constructor and assignment operator to prevent
+// accidental double counting.
+SummaryInvocation(const SummaryInvocation &) = delete;
+SummaryInvocation &operator=(const SummaryInvocation &) = delete;
+
+  private:
+SummaryStatistics &m_summary;
+ElapsedTime m_elapsed_time;
+  };
+
+  SummaryInvocation GetSummaryInvocation() { return SummaryInvocation(*this); }
+
+private:
+  /// Called when Summary Invocation is destructed.
+  void OnInvoked() noexcept {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+  }
+
+  lldb_private::StatsDuration m_total_time;
+  lldb_private::ConstString m_impl_type;
+  lldb_private::ConstString m_name;
+  std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+  /// Get the SummaryStatistics object for a given provider name, or insert
+  /// if statistics for that provider is not in the map.
+  lldb_private::SummaryStatistics &
+  GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) 
{
+std::lock_guard guard(m_map_mutex);
+m_summary_stats_map.try_emplace(provider.GetName(), provider.GetName(),
+provider.GetImplType());
+
+SummaryStatistics &summary_stats =
+m_summary_stats_map.at(provider.GetName());

Michael137 wrote:

`try_emplace` already returns the iterator. So no need to do another lookup here

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

@@ -174,6 +177,84 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(lldb_private::ConstString name,
+ lldb_private::ConstString impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_summary_count(0) {}
+
+  lldb_private::ConstString GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; }
+
+  ConstString GetImplType() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+  // Basic RAII class to increment the summary count when the call is complete.
+  // In the future this can be extended to collect information about the
+  // elapsed time for a single request.
+  class SummaryInvocation {
+  public:
+SummaryInvocation(SummaryStatistics &summary)
+: m_summary(summary), m_elapsed_time(summary.GetDurationReference()) {}
+~SummaryInvocation() { m_summary.OnInvoked(); }
+
+// Delete the copy constructor and assignment operator to prevent
+// accidental double counting.
+SummaryInvocation(const SummaryInvocation &) = delete;
+SummaryInvocation &operator=(const SummaryInvocation &) = delete;
+
+  private:
+SummaryStatistics &m_summary;
+ElapsedTime m_elapsed_time;
+  };
+
+  SummaryInvocation GetSummaryInvocation() { return SummaryInvocation(*this); }
+
+private:
+  /// Called when Summary Invocation is destructed.
+  void OnInvoked() noexcept {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+  }
+
+  lldb_private::StatsDuration m_total_time;
+  lldb_private::ConstString m_impl_type;
+  lldb_private::ConstString m_name;
+  std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+  /// Get the SummaryStatistics object for a given provider name, or insert
+  /// if statistics for that provider is not in the map.
+  lldb_private::SummaryStatistics &
+  GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) 
{
+std::lock_guard guard(m_map_mutex);
+m_summary_stats_map.try_emplace(provider.GetName(), provider.GetName(),
+provider.GetImplType());
+
+SummaryStatistics &summary_stats =
+m_summary_stats_map.at(provider.GetName());
+return summary_stats;
+  }
+
+  llvm::json::Value ToJSON();
+
+private:
+  std::map
+  m_summary_stats_map;

Michael137 wrote:

If we're going to be handing out references to members of the map, we need to 
make sure nothing invalidates them. Off the top of my head I'm not sure what 
iterator/reference invalidation guarantees `StringMap` has, but just from a 
brief skim of the file I saw the word "rehash", which makes me think reference 
might get stale after enough calls to `GetSummaryStatisticsForProviderName`

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

@@ -145,15 +148,34 @@ std::string CXXFunctionSummaryFormat::GetDescription() {
   return std::string(sstr.GetString());
 }
 
+ConstString CXXFunctionSummaryFormat::GetName() {
+  return ConstString(m_description);
+}
+
+ConstString CXXFunctionSummaryFormat::GetImplType() {
+  return ConstString("c++");
+}
+
 ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags,
  const char *function_name,
  const char *python_script)
 : TypeSummaryImpl(Kind::eScript, flags), m_function_name(),
   m_python_script(), m_script_function_sp() {
-  if (function_name)
+  std::string sstring;
+  // Take preference in the python script name over the function name.
+  if (function_name) {
+sstring.assign(function_name);
 m_function_name.assign(function_name);
-  if (python_script)
+  }
+  if (python_script) {
+sstring.assign(python_script);
 m_python_script.assign(python_script);
+  }
+
+  // Python scripts include their leading spacing, so we remove it so we don't
+  // save extra spaces in the const string forever.
+  sstring.erase(0, sstring.find_first_not_of('0'));

Michael137 wrote:

Why are we looking for the `0` character here?

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

@@ -258,6 +258,9 @@ class TypeSummaryImpl {
 
   virtual std::string GetDescription() = 0;
 
+  virtual ConstString GetName() = 0;
+  virtual ConstString GetImplType() = 0;

Michael137 wrote:

Could we use something more descriptive for the name? My expectation from a 
function called `GetImplType` is that it returns something other than a string, 
for example, an enum. So maybe something along the lines of 
`GetSummaryFormatKindName`?

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

@@ -174,6 +177,84 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(lldb_private::ConstString name,
+ lldb_private::ConstString impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_summary_count(0) {}
+
+  lldb_private::ConstString GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; }
+
+  ConstString GetImplType() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+  // Basic RAII class to increment the summary count when the call is complete.
+  // In the future this can be extended to collect information about the
+  // elapsed time for a single request.
+  class SummaryInvocation {
+  public:
+SummaryInvocation(SummaryStatistics &summary)
+: m_summary(summary), m_elapsed_time(summary.GetDurationReference()) {}
+~SummaryInvocation() { m_summary.OnInvoked(); }
+
+// Delete the copy constructor and assignment operator to prevent
+// accidental double counting.
+SummaryInvocation(const SummaryInvocation &) = delete;
+SummaryInvocation &operator=(const SummaryInvocation &) = delete;
+
+  private:
+SummaryStatistics &m_summary;
+ElapsedTime m_elapsed_time;
+  };
+
+  SummaryInvocation GetSummaryInvocation() { return SummaryInvocation(*this); }
+
+private:
+  /// Called when Summary Invocation is destructed.
+  void OnInvoked() noexcept {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+  }
+
+  lldb_private::StatsDuration m_total_time;
+  lldb_private::ConstString m_impl_type;
+  lldb_private::ConstString m_name;
+  std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+  /// Get the SummaryStatistics object for a given provider name, or insert
+  /// if statistics for that provider is not in the map.
+  lldb_private::SummaryStatistics &
+  GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) 
{
+std::lock_guard guard(m_map_mutex);
+m_summary_stats_map.try_emplace(provider.GetName(), provider.GetName(),
+provider.GetImplType());
+
+SummaryStatistics &summary_stats =
+m_summary_stats_map.at(provider.GetName());
+return summary_stats;
+  }
+
+  llvm::json::Value ToJSON();
+
+private:
+  std::map
+  m_summary_stats_map;
+  std::recursive_mutex m_map_mutex;

Michael137 wrote:

Just checking, does this need to be a recursive mutex? Can we add a comment as 
to why?

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

[Andy Hippo via lldb-commits]

@@ -0,0 +1,459 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_CORE_DILAST_H
+#define LLDB_CORE_DILAST_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"

werat wrote:

Yes, it's similar to what 
[CPlusPlusNameParser](https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp)
 does. DIL uses clang to tokenize the expression and parsing is done separately 
by DIL itself.

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Remove Phabricator usernames from Code Owners file (PR #102590)

[David Spickett via lldb-commits]

DavidSpickett wrote:

ping?

If there's no objections in a few days I'll just land this. Folks can fish the 
Phab usernames out of the git history if they really need them.

https://github.com/llvm/llvm-project/pull/102590
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)

[Pavel Labath via lldb-commits]

labath wrote:

> > Just to be clear, if I'm understanding the packet we'll be getting back, we 
> > have no indication that we hit the breakpoint, we only show that we are 
> > stopped at an address which has a breakpoint. Current lldb stepping 
> > behavior will work -- because the rule is, when we stop at a breakpoint 
> > address, we will say we hit the breakpoint. I am refining a patch #96260 
> > which changes this behavior -- we handle "instruction stepped / stopped at 
> > a breakpoint" differently than "hit a breakpoint". I worry this difference 
> > will be lost with a stub that reports `swbreak`/`hwbreak`, stepping won't 
> > work correctly, and we won't capture it in the testsuite or on any of our 
> > CI bots.
> 
> My work mainly concerns the case when lldb is connected to a gdbserver, and 
> as far as I can tell, gdbserver does not send a `reason:breakpoint` packet at 
> all. Maybe lldb-server sends it?

Yes, it does.

> Also, I do not think lldb itself is adjusting the PC -- I think lldb-server 
> does it, and that is also the reason why my PR did not need to alter the lldb 
> behavior at all. lldb always expects the remote to have adjusted the PC if 
> necessary, and it just uses whatever value that is reported

That is true, but I believe Jason is saying something slightly different. 
Currently you are able to use lldb+gdbserver because lldb treats a stop as a 
breakpoint hit every time we stop and the PC value is equal to one of the 
breakpoints. Jason's [patch](https://github.com/llvm/llvm-project/pull/96260) 
will change that and have lldb report a breakpoint hit only if it server tells 
it that was the reason for the stop. lldb-server does that 
(`reason:breakpoint`), and everything will work fine, but that will most likely 
break your use case.

To (preemtively) fix that, you should probably change lldb to recognize the gdb 
response (`swbreak`) and treat it the same way as the one from lldb-server. 
Ideally, you should also add a test to make sure this works (and stays working) 
-- for that, you could probably extend the test you added here to check that 
lldb reports the correct stop reason.

https://github.com/llvm/llvm-project/pull/102873
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Support minidumps where there are multiple exception streams (PR #97470)

[Pavel Labath via lldb-commits]

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/97470
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Support minidumps where there are multiple exception streams (PR #97470)

[Pavel Labath via lldb-commits]

@@ -142,6 +164,15 @@ MinidumpFile::create(MemoryBufferRef Source) {
   continue;
 }
 
+// We treat exceptions differently here because the LLDB minidump
+// makes some assumptions about uniqueness, all the streams other than
+// exceptions are lists. But exceptions are not a list, they are single
+// streams that point back to their thread So we will omit them here, and
+// will find them when needed in the MinidumpFile.
+if (Type == StreamType::Exception) {
+  continue;

labath wrote:

How about storing the of exception streams inside some (vector) member?

https://github.com/llvm/llvm-project/pull/97470
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Support minidumps where there are multiple exception streams (PR #97470)

[Pavel Labath via lldb-commits]

@@ -53,6 +53,27 @@ Expected MinidumpFile::getString(size_t Offset) 
const {
   return Result;
 }
 
+Expected>
+MinidumpFile::getExceptionStreams() const {

labath wrote:

This looks like a job for an (non-fallible) input iterator returning an 
`Expected`. (non-fallible, because the iteration always 
suceeds, but reading of any stream can fail, and can do so independendly of all 
other streams.)

Alternatively, maybe we could just return an array of minidump::Directory 
descriptors, and have the caller retrieve them one at a time via something like 
`getExceptionStream(Directory)`.

https://github.com/llvm/llvm-project/pull/97470
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Support minidumps where there are multiple exception streams (PR #97470)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

If other readers can handle these kinds of minidumps, then I guess this is 
fine. Like in the other patch, I think it'd be best to split out the llvm part 
into its own PR.

https://github.com/llvm/llvm-project/pull/97470
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Support minidumps where there are multiple exception streams (PR #97470)

[Pavel Labath via lldb-commits]

@@ -142,6 +164,15 @@ MinidumpFile::create(MemoryBufferRef Source) {
   continue;
 }
 
+// We treat exceptions differently here because the LLDB minidump
+// makes some assumptions about uniqueness, all the streams other than

labath wrote:

I'm not sure which assumptions are you referring to (from what I can tell it's 
this code here, which is assuming there can be only one stream of a given 
type), but it'd be nice if this could be explained in a way which does not 
involve LLDB (say what do we want to do, not who we're doing it for)

https://github.com/llvm/llvm-project/pull/97470
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

[via lldb-commits]

@@ -0,0 +1,459 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_CORE_DILAST_H
+#define LLDB_CORE_DILAST_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"

cmtice wrote:

Yes, Andy (werat) is correct. I mis-spoke in my previous comment (apologies!).  
We use the Clang tokenizer & lexer but wrote our own parser.

Since that is the case, is this ok?

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)

[via lldb-commits]

xusheng6 wrote:

> > > Just to be clear, if I'm understanding the packet we'll be getting back, 
> > > we have no indication that we hit the breakpoint, we only show that we 
> > > are stopped at an address which has a breakpoint. Current lldb stepping 
> > > behavior will work -- because the rule is, when we stop at a breakpoint 
> > > address, we will say we hit the breakpoint. I am refining a patch #96260 
> > > which changes this behavior -- we handle "instruction stepped / stopped 
> > > at a breakpoint" differently than "hit a breakpoint". I worry this 
> > > difference will be lost with a stub that reports `swbreak`/`hwbreak`, 
> > > stepping won't work correctly, and we won't capture it in the testsuite 
> > > or on any of our CI bots.
> > 
> > 
> > My work mainly concerns the case when lldb is connected to a gdbserver, and 
> > as far as I can tell, gdbserver does not send a `reason:breakpoint` packet 
> > at all. Maybe lldb-server sends it?
> 
> Yes, it does.
> 
> > Also, I do not think lldb itself is adjusting the PC -- I think lldb-server 
> > does it, and that is also the reason why my PR did not need to alter the 
> > lldb behavior at all. lldb always expects the remote to have adjusted the 
> > PC if necessary, and it just uses whatever value that is reported
> 
> That is true, but I believe Jason is saying something slightly different. 
> Currently you are able to use lldb+gdbserver because lldb treats a stop as a 
> breakpoint hit every time we stop and the PC value is equal to one of the 
> breakpoints. Jason's [patch](https://github.com/llvm/llvm-project/pull/96260) 
> will change that and have lldb report a breakpoint hit only if it server 
> tells it that was the reason for the stop. lldb-server does that 
> (`reason:breakpoint`), and everything will work fine, but that will most 
> likely break your use case.
> 
> To (preemtively) fix that, you should probably change lldb to recognize the 
> gdb response (`swbreak`) and treat it the same way as the one from 
> lldb-server. Ideally, you should also add a test to make sure this works (and 
> stays working) -- for that, you could probably extend the test you added here 
> to check that lldb reports the correct stop reason.

Oh I see your point now, and I also see the lldb is handling a `reason:` 
packet from the lldb-server. In this sense, I think it is fair to treat 
"swbreak/hwbreak" in the same way as "reason:breakpoint".

@jasonmolenda do you think you can handle this along with your patch, or you 
think I should do something for it preemptively as suggested by 
[labath](https://github.com/labath)? I personally prefer the former case 
because I am quite new to the lldb code base 

https://github.com/llvm/llvm-project/pull/102873
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

[via lldb-commits]

@@ -0,0 +1,560 @@
+//===-- DILAST.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/Core/DILAST.h"
+#include "lldb/API/SBType.h"
+#include "lldb/Core/ValueObjectRegister.h"
+#include "lldb/Core/ValueObjectVariable.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Target/RegisterContext.h"
+#include "llvm/ADT/StringRef.h"
+
+#include 
+
+namespace lldb_private {
+
+namespace DIL {
+
+lldb::ValueObjectSP
+GetDynamicOrSyntheticValue(lldb::ValueObjectSP in_valobj_sp,
+   lldb::DynamicValueType use_dynamic,
+   bool use_synthetic) {
+  Status error;
+
+  if (!in_valobj_sp) {
+error.SetErrorString("invalid value object");
+return in_valobj_sp;
+  }
+
+  lldb::ValueObjectSP value_sp = in_valobj_sp;
+
+  Target *target = value_sp->GetTargetSP().get();
+  // If this ValueObject holds an error, then it is valuable for that.
+  if (value_sp->GetError().Fail())
+return value_sp;
+
+  if (!target)
+return lldb::ValueObjectSP();
+
+  if (use_dynamic != lldb::eNoDynamicValues) {
+lldb::ValueObjectSP dynamic_sp = value_sp->GetDynamicValue(use_dynamic);
+if (dynamic_sp)
+  value_sp = dynamic_sp;
+  }
+
+  if (use_synthetic) {
+lldb::ValueObjectSP synthetic_sp = value_sp->GetSyntheticValue();
+if (synthetic_sp)
+  value_sp = synthetic_sp;
+  }
+
+  if (!value_sp)
+error.SetErrorString("invalid value object");
+
+  return value_sp;
+}
+
+CompilerType DILASTNode::GetDereferencedResultType() const {
+  auto type = result_type();
+  return type.IsReferenceType() ? type.GetNonReferenceType() : type;
+}
+
+std::optional

cmtice wrote:

This function is called from the parser when it is trying to parse what ought 
to be a field/member name (i.e. it's gotten through 
  field_name).  The 
"stuff-before-field-reference" can be a simple variable name, or something more 
complex; in this case it was an array subscript, 'c_arr[0]'. (The full command 
was 'frame variable c_arr[0].field_').  This function determines if the 
'stuff-before-field-reference' has a valid field/member with the given name, 
and the index of that child/member (or set of potential valid indices) and 
returns that in the "idx" parameter, along with the full information about the 
field, which gets returned in the MemberInfo struct (the main return value).  
This information is later used by the evaluator (once we have finished parsing 
the entire input) to actually find and return the value the user was asking for.

Since  whether or not the field/member name is valid really depends on the 
type, I do not understand why you seem to feel it is wrong to use the type (the 
second parameter) of the "stuff-before-field-reference" to check to see if it 
is valid and (if it's valid) what its position is?


https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,459 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_CORE_DILAST_H
+#define LLDB_CORE_DILAST_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"

labath wrote:

I think I'd be fine with that, but I also think it's not my call to make. 
@jimingham, @adrian-prantl, @bulbazord, what do you think?

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

[via lldb-commits]

cmtice wrote:

> Even though you've removed the smart pointer checking function (the most 
> obvious example of language-specific behavior), I'm still worried about 
> whether the overall design of this code will allow you to avoid it.
> 
I believe I have fixed all the code to avoid any need to to smart pointer 
checking -- I went back and looked very closely at what the current frame 
variable implementation does when handling smart pointers, and tried to do the 
same thing, which appears to have worked.

> Looking at the `IdentifierInfo` class, I see that it can be constructed with 
> only a type (the value is optional). I think this is going to be very hard to 
> make work, since a type does not know if its a smart pointer (only a value 
> does). Have you already figured out a way to make that work?
> 
The case where it is constructed only with a type is meant to be when handling 
LLDB convenience variables (the dynamic variables names "$..."); I'm not sure 
if those can be smart pointers?

> IndentifierInfo isn't the only suspicious place though. All of the places 
> that are doing something with bitfield offsets are very suspicious to me, as 
> that's something that the Values should already know.
> 
> I haven't seen the parser, but I wouldn't be surprised if this has something 
> to do with the vexing parse issues of C languages, where one needs to know if 
> something is a type just to parse the expression correctly. Now, if that's 
> the case, and if really need to know if something is a smart pointer (i.e. 
> whether it is dereferencable) without having access to a value of that type, 
> then we may need to have a bigger discussion about what to do about that.

I would be happy to share the parser code with you and walk/talk you through 
it, if that would help?


https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply "[lldb] Tolerate multiple compile units with the same DWO ID (#100577)" (PR #104041)

[Pavel Labath via lldb-commits]

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/104041

The only change vs. the first version of the patch is that I've made
DWARFUnit linking thread-safe/unit. This was necessary because, during the
indexing step, two skeleton units could attempt to associate themselves
with the split unit.

The original commit message was:

I ran into this when LTO completely emptied two compile units, so they
ended up with the same hash (see #100375). Although, ideally, the
compiler would try to ensure we don't end up with a hash collision even
in this case, guaranteeing their absence is practically impossible. This
patch ensures this situation does not bring down lldb.


>From ba693941ec0bc5f829bf57575bcc26a8664fdcc4 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Tue, 13 Aug 2024 07:08:17 +
Subject: [PATCH 1/2] Reapply "[lldb] Tolerate multiple compile units with the
 same DWO ID (#100577)"

The only change vs. the first version of the patch is that I've made
DWARFUnit linking thread-safe/unit. This was necessary because, during the
indexing step, two skeleton units could attempt to associate themselves
with the split unit.

The original commit message was:

I ran into this when LTO completely emptied two compile units, so they
ended up with the same hash (see #100375). Although, ideally, the
compiler would try to ensure we don't end up with a hash collision even
in this case, guaranteeing their absence is practically impossible. This
patch ensures this situation does not bring down lldb.
---
 .../Plugins/SymbolFile/DWARF/DWARFUnit.cpp|  26 ++--
 .../Plugins/SymbolFile/DWARF/DWARFUnit.h  |   2 +-
 .../SymbolFile/DWARF/x86/dwp-hash-collision.s | 142 ++
 3 files changed, 156 insertions(+), 14 deletions(-)
 create mode 100644 lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 66a762bf9b6854..0a52159d055bb4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -97,12 +97,14 @@ void DWARFUnit::ExtractUnitDIEIfNeeded() {
 *m_dwo_id, m_first_die.GetOffset()));
 return; // Can't fetch the compile unit from the dwo file.
   }
-  // If the skeleton compile unit gets its unit DIE parsed first, then this
-  // will fill in the DWO file's back pointer to this skeleton compile unit.
-  // If the DWO files get parsed on their own first the skeleton back link
-  // can be done manually in DWARFUnit::GetSkeletonCompileUnit() which will
-  // do a reverse lookup and cache the result.
-  dwo_cu->SetSkeletonUnit(this);
+
+  // Link the DWO unit to this object, if it hasn't been linked already (this
+  // can happen when we have an index, and the DWO unit is parsed first).
+  if (!dwo_cu->LinkToSkeletonUnit(*this)) {
+SetDwoError(Status::createWithFormat(
+"multiple compile units with Dwo ID {0:x16}", *m_dwo_id));
+return;
+  }
 
   DWARFBaseDIE dwo_cu_die = dwo_cu->GetUnitDIEOnly();
   if (!dwo_cu_die.IsValid()) {
@@ -718,13 +720,11 @@ DWARFCompileUnit *DWARFUnit::GetSkeletonUnit() {
   return llvm::dyn_cast_or_null(m_skeleton_unit);
 }
 
-void DWARFUnit::SetSkeletonUnit(DWARFUnit *skeleton_unit) {
-  // If someone is re-setting the skeleton compile unit backlink, make sure
-  // it is setting it to a valid value when it wasn't valid, or if the
-  // value in m_skeleton_unit was valid, it should be the same value.
-  assert(skeleton_unit);
-  assert(m_skeleton_unit == nullptr || m_skeleton_unit == skeleton_unit);
-  m_skeleton_unit = skeleton_unit;
+bool DWARFUnit::LinkToSkeletonUnit(DWARFUnit &skeleton_unit) {
+  if (m_skeleton_unit && m_skeleton_unit != &skeleton_unit)
+return false;
+  m_skeleton_unit = &skeleton_unit;
+  return true;
 }
 
 bool DWARFUnit::Supports_DW_AT_APPLE_objc_complete_type() {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index 85c37971ced8e0..209104fe3a054e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -170,7 +170,7 @@ class DWARFUnit : public UserID {
   /// both cases correctly and avoids crashes.
   DWARFCompileUnit *GetSkeletonUnit();
 
-  void SetSkeletonUnit(DWARFUnit *skeleton_unit);
+  bool LinkToSkeletonUnit(DWARFUnit &skeleton_unit);
 
   bool Supports_DW_AT_APPLE_objc_complete_type();
 
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s 
b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
new file mode 100644
index 00..d626b4602ad58f
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s
@@ -0,0 +1,142 @@
+## Test that lldb handles (mainly, that it doesn't crash) the situation where
+## two skeleton compile units have the same DWO ID (and try to claim the same
+## split unit from the DWP file. This can sometim

[Lldb-commits] [lldb] Reapply "[lldb] Tolerate multiple compile units with the same DWO ID (#100577)" (PR #104041)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)


Changes

The only change vs. the first version of the patch is that I've made
DWARFUnit linking thread-safe/unit. This was necessary because, during the
indexing step, two skeleton units could attempt to associate themselves
with the split unit.

The original commit message was:

I ran into this when LTO completely emptied two compile units, so they
ended up with the same hash (see #100375). Although, ideally, the
compiler would try to ensure we don't end up with a hash collision even
in this case, guaranteeing their absence is practically impossible. This
patch ensures this situation does not bring down lldb.


---
Full diff: https://github.com/llvm/llvm-project/pull/104041.diff


3 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp (+24-17) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h (+2-2) 
- (added) lldb/test/Shell/SymbolFile/DWARF/x86/dwp-hash-collision.s (+142) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index 66a762bf9b6854..81f937762e35a6 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -97,12 +97,14 @@ void DWARFUnit::ExtractUnitDIEIfNeeded() {
 *m_dwo_id, m_first_die.GetOffset()));
 return; // Can't fetch the compile unit from the dwo file.
   }
-  // If the skeleton compile unit gets its unit DIE parsed first, then this
-  // will fill in the DWO file's back pointer to this skeleton compile unit.
-  // If the DWO files get parsed on their own first the skeleton back link
-  // can be done manually in DWARFUnit::GetSkeletonCompileUnit() which will
-  // do a reverse lookup and cache the result.
-  dwo_cu->SetSkeletonUnit(this);
+
+  // Link the DWO unit to this object, if it hasn't been linked already (this
+  // can happen when we have an index, and the DWO unit is parsed first).
+  if (!dwo_cu->LinkToSkeletonUnit(*this)) {
+SetDwoError(Status::createWithFormat(
+"multiple compile units with Dwo ID {0:x16}", *m_dwo_id));
+return;
+  }
 
   DWARFBaseDIE dwo_cu_die = dwo_cu->GetUnitDIEOnly();
   if (!dwo_cu_die.IsValid()) {
@@ -708,23 +710,28 @@ uint8_t DWARFUnit::GetAddressByteSize(const DWARFUnit 
*cu) {
 uint8_t DWARFUnit::GetDefaultAddressSize() { return 4; }
 
 DWARFCompileUnit *DWARFUnit::GetSkeletonUnit() {
-  if (m_skeleton_unit == nullptr && IsDWOUnit()) {
+  if (m_skeleton_unit.load() == nullptr && IsDWOUnit()) {
 SymbolFileDWARFDwo *dwo =
 llvm::dyn_cast_or_null(&GetSymbolFileDWARF());
 // Do a reverse lookup if the skeleton compile unit wasn't set.
-if (dwo)
-  m_skeleton_unit = dwo->GetBaseSymbolFile().GetSkeletonUnit(this);
+DWARFUnit *candidate_skeleton_unit =
+dwo ? dwo->GetBaseSymbolFile().GetSkeletonUnit(this) : nullptr;
+if (candidate_skeleton_unit)
+  (void)LinkToSkeletonUnit(*candidate_skeleton_unit);
+// Linking may fail due to a race, so be sure to return the actual value.
   }
-  return llvm::dyn_cast_or_null(m_skeleton_unit);
+  return llvm::dyn_cast_or_null(m_skeleton_unit.load());
 }
 
-void DWARFUnit::SetSkeletonUnit(DWARFUnit *skeleton_unit) {
-  // If someone is re-setting the skeleton compile unit backlink, make sure
-  // it is setting it to a valid value when it wasn't valid, or if the
-  // value in m_skeleton_unit was valid, it should be the same value.
-  assert(skeleton_unit);
-  assert(m_skeleton_unit == nullptr || m_skeleton_unit == skeleton_unit);
-  m_skeleton_unit = skeleton_unit;
+bool DWARFUnit::LinkToSkeletonUnit(DWARFUnit &skeleton_unit) {
+  DWARFUnit *expected_unit = nullptr;
+  if (m_skeleton_unit.compare_exchange_strong(expected_unit, &skeleton_unit))
+return true;
+  if (expected_unit == &skeleton_unit) {
+// Exchange failed because it already contained the right  value.
+return true;
+  }
+  return false; // Already linked to a different unit.
 }
 
 bool DWARFUnit::Supports_DW_AT_APPLE_objc_complete_type() {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
index 85c37971ced8e0..148932d67b908c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -170,7 +170,7 @@ class DWARFUnit : public UserID {
   /// both cases correctly and avoids crashes.
   DWARFCompileUnit *GetSkeletonUnit();
 
-  void SetSkeletonUnit(DWARFUnit *skeleton_unit);
+  bool LinkToSkeletonUnit(DWARFUnit &skeleton_unit);
 
   bool Supports_DW_AT_APPLE_objc_complete_type();
 
@@ -308,7 +308,7 @@ class DWARFUnit : public UserID {
   const llvm::DWARFAbbreviationDeclarationSet *m_abbrevs = nullptr;
   lldb_private::CompileUnit *m_lldb_cu = nullptr;
   // If this is a DWO file, we have a backlink to our skeleton compile unit.
-  DWARFUnit *m_skeleton_unit = nullptr;
+  std::atomic

[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)

[via lldb-commits]

@@ -0,0 +1,158 @@
+"""
+Test lldb breakpoint with symlinks/realpath and source-map.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil, lldbplatformutil
+
+
+class BreakpointTestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True

royitaqi wrote:

Can someone help me double check that this is correct?

https://github.com/llvm/llvm-project/pull/102223
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)

[via lldb-commits]

@@ -0,0 +1,158 @@
+"""
+Test lldb breakpoint with symlinks/realpath and source-map.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil, lldbplatformutil
+
+
+class BreakpointTestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Find the line number to break inside main().
+self.line_in_main = line_number("main.c", "// Set break point at this 
line.")
+self.line_in_foo = line_number("real/foo.h", "// Set break point at 
this line.")
+self.line_in_bar = line_number("real/bar.h", "// Set break point at 
this line.")
+self.line_in_qux = line_number("real/qux.h", "// Set break point at 
this line.")
+# disable "There is a running process, kill it and restart?" prompt
+self.runCmd("settings set auto-confirm true")
+self.addTearDownHook(lambda: self.runCmd("settings clear 
auto-confirm"))
+
+def buildAndCreateTarget(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+# Create a target by the debugger.
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+@skipIf(oslist=["windows"])

royitaqi wrote:

And also this?

https://github.com/llvm/llvm-project/pull/102223
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)

[via lldb-commits]

https://github.com/royitaqi edited 
https://github.com/llvm/llvm-project/pull/102223
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)

[via lldb-commits]

royitaqi wrote:

Hi @jimingham and @labath ,

Could you do a final pass and see if everything looks good?   I'm hoping to 
merge this today/tomorrow if possible.

Thanks,
Roy



https://github.com/llvm/llvm-project/pull/102223
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)

[Walter Erquinigo via lldb-commits]

https://github.com/walter-erquinigo updated 
https://github.com/llvm/llvm-project/pull/103458

>From 656486aa0854db82ace6bee0639ca1d94f7b4fc5 Mon Sep 17 00:00:00 2001
From: walter erquinigo 
Date: Tue, 13 Aug 2024 17:09:38 -0400
Subject: [PATCH] [LLDB][OSX] Add a fallback support exe directory

LLDB on OSX is looking at a `bin` directory sibling to the `lib` one that 
contains liblldb for its supporting executables. This works well for CMake, 
however, for other build systems like bazel, it's not that easy to have that 
build structure, for which it's much easier to also use the `lib` directory as 
a fallback under the absence of `bin`.
This shouldn't break anything, but instead should make it a bit easier for LLDB 
to work with different build systems and folder structures.
---
 .../Host/macosx/objcxx/HostInfoMacOSX.mm  | 28 ++-
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm 
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index f96e2cf80c5fac..b714f7be187aca 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -124,6 +124,12 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
   return g_program_filespec;
 }
 
+/// Resolve the given candidate support dir and return true if it's valid.
+static bool ResolveAndVerifyCandidateSupportDir(FileSpec &path) {
+  FileSystem::Instance().Resolve(path);
+  return FileSystem::Instance().IsDirectory(path);
+};
+
 bool HostInfoMacOSX::ComputeSupportExeDirectory(FileSpec &file_spec) {
   FileSpec lldb_file_spec = GetShlibDir();
   if (!lldb_file_spec)
@@ -144,16 +150,24 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
 #endif
   } else {
 // Find the bin path relative to the lib path where the cmake-based
-// OS X .dylib lives.  This is not going to work if the bin and lib
-// dir are not both in the same dir.
+// OS X .dylib lives. We try looking first at a possible sibling `bin`
+// directory, and then at the `lib` directory itself. This last case is
+// useful for supporting build systems like Bazel which in many cases 
prefer
+// to place support binaries right next to dylibs.
 //
-// It is not going to work to do it by the executable path either,
+// It is not going to work to do it by the executable path,
 // as in the case of a python script, the executable is python, not
 // the lldb driver.
-raw_path.append("/../bin");
-FileSpec support_dir_spec(raw_path);
-FileSystem::Instance().Resolve(support_dir_spec);
-if (!FileSystem::Instance().IsDirectory(support_dir_spec)) {
+FileSpec support_dir_spec_lib(raw_path);
+FileSpec support_dir_spec_bin =
+support_dir_spec_lib.CopyByAppendingPathComponent("/../bin");
+FileSpec support_dir_spec;
+
+if (ResolveAndVerifyCandidateSupportDir(support_dir_spec_bin)) {
+  support_dir_spec = support_dir_spec_bin;
+} else if (ResolveAndVerifyCandidateSupportDir(support_dir_spec_lib)) {
+  support_dir_spec = support_dir_spec_lib;
+} else {
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOG(log, "failed to find support directory");
   return false;

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

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

@@ -258,6 +258,9 @@ class TypeSummaryImpl {
 
   virtual std::string GetDescription() = 0;
 
+  virtual ConstString GetName() = 0;
+  virtual ConstString GetImplType() = 0;

Jlalond wrote:

@Michael137 I also prefer `KindName`, but there is also an existing `Kind` enum 
used in TypeSummary, so I didn't want to cause confusion, any other ideas?

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

@@ -258,6 +258,9 @@ class TypeSummaryImpl {
 
   virtual std::string GetDescription() = 0;
 
+  virtual ConstString GetName() = 0;
+  virtual ConstString GetImplType() = 0;
+

Jlalond wrote:

Phew,

I 100% agree, this was getting quite messy

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

@@ -174,6 +177,84 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(lldb_private::ConstString name,
+ lldb_private::ConstString impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_summary_count(0) {}
+
+  lldb_private::ConstString GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; }
+
+  ConstString GetImplType() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+  // Basic RAII class to increment the summary count when the call is complete.
+  // In the future this can be extended to collect information about the
+  // elapsed time for a single request.
+  class SummaryInvocation {
+  public:
+SummaryInvocation(SummaryStatistics &summary)
+: m_summary(summary), m_elapsed_time(summary.GetDurationReference()) {}
+~SummaryInvocation() { m_summary.OnInvoked(); }
+
+// Delete the copy constructor and assignment operator to prevent
+// accidental double counting.
+SummaryInvocation(const SummaryInvocation &) = delete;
+SummaryInvocation &operator=(const SummaryInvocation &) = delete;
+
+  private:
+SummaryStatistics &m_summary;
+ElapsedTime m_elapsed_time;
+  };
+
+  SummaryInvocation GetSummaryInvocation() { return SummaryInvocation(*this); }
+
+private:
+  /// Called when Summary Invocation is destructed.
+  void OnInvoked() noexcept {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+  }
+
+  lldb_private::StatsDuration m_total_time;
+  lldb_private::ConstString m_impl_type;
+  lldb_private::ConstString m_name;
+  std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+  /// Get the SummaryStatistics object for a given provider name, or insert
+  /// if statistics for that provider is not in the map.
+  lldb_private::SummaryStatistics &
+  GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) 
{
+std::lock_guard guard(m_map_mutex);
+m_summary_stats_map.try_emplace(provider.GetName(), provider.GetName(),
+provider.GetImplType());
+
+SummaryStatistics &summary_stats =
+m_summary_stats_map.at(provider.GetName());
+return summary_stats;
+  }
+
+  llvm::json::Value ToJSON();
+
+private:
+  std::map
+  m_summary_stats_map;

Jlalond wrote:

@Michael137 I'm onboard with having the Cache return the RAII object, as we 
already have the data object create one on demand.

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

@@ -174,6 +177,84 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(lldb_private::ConstString name,
+ lldb_private::ConstString impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_summary_count(0) {}
+
+  lldb_private::ConstString GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; }
+
+  ConstString GetImplType() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+  // Basic RAII class to increment the summary count when the call is complete.
+  // In the future this can be extended to collect information about the
+  // elapsed time for a single request.
+  class SummaryInvocation {
+  public:
+SummaryInvocation(SummaryStatistics &summary)
+: m_summary(summary), m_elapsed_time(summary.GetDurationReference()) {}
+~SummaryInvocation() { m_summary.OnInvoked(); }
+
+// Delete the copy constructor and assignment operator to prevent
+// accidental double counting.
+SummaryInvocation(const SummaryInvocation &) = delete;
+SummaryInvocation &operator=(const SummaryInvocation &) = delete;
+
+  private:
+SummaryStatistics &m_summary;
+ElapsedTime m_elapsed_time;
+  };
+
+  SummaryInvocation GetSummaryInvocation() { return SummaryInvocation(*this); }
+
+private:
+  /// Called when Summary Invocation is destructed.
+  void OnInvoked() noexcept {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+  }
+
+  lldb_private::StatsDuration m_total_time;
+  lldb_private::ConstString m_impl_type;
+  lldb_private::ConstString m_name;
+  std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+  /// Get the SummaryStatistics object for a given provider name, or insert
+  /// if statistics for that provider is not in the map.
+  lldb_private::SummaryStatistics &
+  GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) 
{
+std::lock_guard guard(m_map_mutex);
+m_summary_stats_map.try_emplace(provider.GetName(), provider.GetName(),
+provider.GetImplType());
+
+SummaryStatistics &summary_stats =
+m_summary_stats_map.at(provider.GetName());
+return summary_stats;
+  }
+
+  llvm::json::Value ToJSON();
+
+private:
+  std::map
+  m_summary_stats_map;
+  std::recursive_mutex m_map_mutex;

Jlalond wrote:

We're going to move back to the regular mutex, I was worried about child 
calling into child but the lock is only for the transaction; so that was a lack 
of coffee moment.

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)

[Miro Bucko via lldb-commits]

mbucko wrote:

> > That being said, if there is something we can do to this PR to make it easy 
> > for you (Pavel) to implement this feature, we can make those changes. Like 
> > I think it would be fair to change all `PeekMemory/DoPeekMemory` over a 
> > virtual `ReadMemory/DoReadMemory` that returns a `DataExtractor` as this 
> > can easily take place of the `ArrayRef` values that are returned.
> > This:
> > ```
> >   virtual llvm::ArrayRef PeekMemory(lldb::addr_t low, lldb::addr_t 
> > high);
> > ```
> > 
> > 
> > 
> >   
> > 
> > 
> >   
> > 
> > 
> > 
> >   
> > Could become:
> > ```
> >   virtual DataExtractor ReadMemory(lldb::addr_t low, lldb::addr_t high);
> > ```
> > 
> > 
> > 
> >   
> > 
> > 
> >   
> > 
> > 
> > 
> >   
> > And we can adjust all users of `PeekMemory` and `DoPeakMemory` to use the 
> > the above functions?
> 
> I'm not sure which feature are you referring to now. I'm not particularly 
> worried about the `/proc/kcore` use case, as I don't know if I'll ever get to 
> that, and it will require more substantial changes anyway.
> 
> The thing I am looking into right now is the ability to make `memory find` 
> better in general, and not just for core files. My thinking is that if I can 
> implement (and I'm offering to do that) a better generic implementation of 
> `memory find` then probably the entirety of this patch becomes redundant. But 
> to know that for sure, I'd like to understand more about your motivation for 
> it. Like, what is the main problem that you're trying to solve with this? Is 
> it to fix some specific bug (my bug was that the operation aborts as soon as 
> it encounters a hole in the address space)? Or is it to make it faster? If 
> so, what level of performance would you consider satisfactory?

The purpose of this it to make the `find memory` faster for post mortem 
processes. This patch gives us nearly 100x speed up based on my tests.

https://github.com/llvm/llvm-project/pull/102536
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] cce2271 - [LLDB][OSX] Add a fallback support exe directory (#103458)

[via lldb-commits]

Author: Walter Erquinigo
Date: 2024-08-14T12:41:19-04:00
New Revision: cce2271eab8173f5c27c1e99863a056a9430c252

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

LOG: [LLDB][OSX] Add a fallback support exe directory (#103458)

LLDB on OSX is looking at a `bin` directory sibling to the `lib` one
that contains liblldb for its supporting executables. This works well
for CMake, however, for other build systems like bazel, it's not that
easy to have that build structure, for which it's much easier to also
use the `lib` directory as a fallback under the absence of `bin`.
This shouldn't break anything, but instead should make it a bit easier
for LLDB to work with different build systems and folder structures.

Added: 


Modified: 
lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

Removed: 




diff  --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm 
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index f96e2cf80c5fac..b714f7be187aca 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -124,6 +124,12 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
   return g_program_filespec;
 }
 
+/// Resolve the given candidate support dir and return true if it's valid.
+static bool ResolveAndVerifyCandidateSupportDir(FileSpec &path) {
+  FileSystem::Instance().Resolve(path);
+  return FileSystem::Instance().IsDirectory(path);
+};
+
 bool HostInfoMacOSX::ComputeSupportExeDirectory(FileSpec &file_spec) {
   FileSpec lldb_file_spec = GetShlibDir();
   if (!lldb_file_spec)
@@ -144,16 +150,24 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
 #endif
   } else {
 // Find the bin path relative to the lib path where the cmake-based
-// OS X .dylib lives.  This is not going to work if the bin and lib
-// dir are not both in the same dir.
+// OS X .dylib lives. We try looking first at a possible sibling `bin`
+// directory, and then at the `lib` directory itself. This last case is
+// useful for supporting build systems like Bazel which in many cases 
prefer
+// to place support binaries right next to dylibs.
 //
-// It is not going to work to do it by the executable path either,
+// It is not going to work to do it by the executable path,
 // as in the case of a python script, the executable is python, not
 // the lldb driver.
-raw_path.append("/../bin");
-FileSpec support_dir_spec(raw_path);
-FileSystem::Instance().Resolve(support_dir_spec);
-if (!FileSystem::Instance().IsDirectory(support_dir_spec)) {
+FileSpec support_dir_spec_lib(raw_path);
+FileSpec support_dir_spec_bin =
+support_dir_spec_lib.CopyByAppendingPathComponent("/../bin");
+FileSpec support_dir_spec;
+
+if (ResolveAndVerifyCandidateSupportDir(support_dir_spec_bin)) {
+  support_dir_spec = support_dir_spec_bin;
+} else if (ResolveAndVerifyCandidateSupportDir(support_dir_spec_lib)) {
+  support_dir_spec = support_dir_spec_lib;
+} else {
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOG(log, "failed to find support directory");
   return false;



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

[Lldb-commits] [lldb] [LLDB][OSX] Add a fallback support exe directory (PR #103458)

[Walter Erquinigo via lldb-commits]

https://github.com/walter-erquinigo closed 
https://github.com/llvm/llvm-project/pull/103458
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

@@ -145,15 +148,34 @@ std::string CXXFunctionSummaryFormat::GetDescription() {
   return std::string(sstr.GetString());
 }
 
+std::string CXXFunctionSummaryFormat::GetName() {
+  return m_description;
+}
+
+std::string CXXFunctionSummaryFormat::GetSummaryKindName() {
+  return "c++";
+}
+
 ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags,
  const char *function_name,
  const char *python_script)
 : TypeSummaryImpl(Kind::eScript, flags), m_function_name(),
   m_python_script(), m_script_function_sp() {
-  if (function_name)
+  std::string sstring;

Jlalond wrote:

No strong reason, my only concern was emplacing to `m_script_formatter_name` 
was that I needed to trim the leading zeros, but writing this out there really 
is no reason.

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Provide `declarationLocation` for variables (PR #102928)

[Walter Erquinigo via lldb-commits]

walter-erquinigo wrote:

@vogelsgesang , Mr. Bird, I'm swamped this week with work, but I'll look into 
this before saturday for sure.

https://github.com/llvm/llvm-project/pull/102928
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/102708

>From c0a7286b0107d3161b18de8f05d4d016150e96a5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Thu, 8 Aug 2024 08:58:52 -0700
Subject: [PATCH 1/7] Initial attempt at new classes Summary statistics in
 SummaryStatistics.h/cpp.

---
 lldb/include/lldb/Target/SummaryStatistics.h | 37 
 lldb/include/lldb/Target/Target.h|  5 +++
 lldb/source/Core/ValueObject.cpp |  5 +++
 lldb/source/Target/CMakeLists.txt|  1 +
 lldb/source/Target/SummaryStatistics.cpp | 26 ++
 lldb/source/Target/Target.cpp|  9 +
 6 files changed, 83 insertions(+)
 create mode 100644 lldb/include/lldb/Target/SummaryStatistics.h
 create mode 100644 lldb/source/Target/SummaryStatistics.cpp

diff --git a/lldb/include/lldb/Target/SummaryStatistics.h 
b/lldb/include/lldb/Target/SummaryStatistics.h
new file mode 100644
index 00..0198249ba0b170
--- /dev/null
+++ b/lldb/include/lldb/Target/SummaryStatistics.h
@@ -0,0 +1,37 @@
+//===-- SummaryStatistics.h -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_TARGET_SUMMARYSTATISTICS_H
+#define LLDB_TARGET_SUMMARYSTATISTICS_H
+
+
+#include "lldb/Target/Statistics.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private {
+
+class SummaryStatistics {
+public:
+  SummaryStatistics(lldb_private::ConstString name) : 
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+  lldb_private::StatsDuration &GetDurationReference();
+
+  lldb_private::ConstString GetName() const;
+
+  uint64_t GetSummaryCount() const;
+
+private:
+   lldb_private::StatsDuration m_total_time;
+   lldb_private::ConstString m_name;
+   uint64_t m_summary_count;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_SUMMARYSTATISTICS_H
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 119dff4d498199..ee6a009b0af95d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -30,6 +30,7 @@
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Target/SectionLoadHistory.h"
 #include "lldb/Target/Statistics.h"
+#include "lldb/Target/SummaryStatistics.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
@@ -258,6 +259,7 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+
 private:
   std::optional
   GetExperimentalPropertyValue(size_t prop_idx,
@@ -1221,6 +1223,8 @@ class Target : public 
std::enable_shared_from_this,
 
   void ClearAllLoadedSections();
 
+  lldb_private::StatsDuration& 
GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name);
+
   /// Set the \a Trace object containing processor trace information of this
   /// target.
   ///
@@ -1554,6 +1558,7 @@ class Target : public 
std::enable_shared_from_this,
   std::string m_label;
   ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
+  std::map 
m_summary_stats_map;
   SectionLoadHistory m_section_load_history;
   BreakpointList m_breakpoint_list;
   BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4f..bed4ab8d69cbda 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,6 +615,11 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl 
*summary_ptr,
   m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
 // the synthetic children being
 // up-to-date (e.g. ${svar%#})
+StatsDuration &summary_duration = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
+
.GetSummaryProviderDuration(GetTypeName());
+ElapsedTime elapsed(summary_duration);
 summary_ptr->FormatObject(this, destination, actual_options);
   }
   m_flags.m_is_getting_summary = false;
diff --git a/lldb/source/Target/CMakeLists.txt 
b/lldb/source/Target/CMakeLists.txt
index a42c44b761dc56..e51da37cd84db3 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -46,6 +46,7 @@ add_lldb_library(lldbTarget
   Statistics.cpp
   StopInfo.cpp
   StructuredDataPlugin.cpp
+  SummaryStatistics.cpp
   SystemRuntime.cpp
   Target.cpp
   TargetList.cpp
diff --git a/lldb/source/Target/SummaryStatistics.cpp 
b/lldb/sour

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/102708

>From c0a7286b0107d3161b18de8f05d4d016150e96a5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Thu, 8 Aug 2024 08:58:52 -0700
Subject: [PATCH 1/7] Initial attempt at new classes Summary statistics in
 SummaryStatistics.h/cpp.

---
 lldb/include/lldb/Target/SummaryStatistics.h | 37 
 lldb/include/lldb/Target/Target.h|  5 +++
 lldb/source/Core/ValueObject.cpp |  5 +++
 lldb/source/Target/CMakeLists.txt|  1 +
 lldb/source/Target/SummaryStatistics.cpp | 26 ++
 lldb/source/Target/Target.cpp|  9 +
 6 files changed, 83 insertions(+)
 create mode 100644 lldb/include/lldb/Target/SummaryStatistics.h
 create mode 100644 lldb/source/Target/SummaryStatistics.cpp

diff --git a/lldb/include/lldb/Target/SummaryStatistics.h 
b/lldb/include/lldb/Target/SummaryStatistics.h
new file mode 100644
index 00..0198249ba0b170
--- /dev/null
+++ b/lldb/include/lldb/Target/SummaryStatistics.h
@@ -0,0 +1,37 @@
+//===-- SummaryStatistics.h -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_TARGET_SUMMARYSTATISTICS_H
+#define LLDB_TARGET_SUMMARYSTATISTICS_H
+
+
+#include "lldb/Target/Statistics.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private {
+
+class SummaryStatistics {
+public:
+  SummaryStatistics(lldb_private::ConstString name) : 
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+  lldb_private::StatsDuration &GetDurationReference();
+
+  lldb_private::ConstString GetName() const;
+
+  uint64_t GetSummaryCount() const;
+
+private:
+   lldb_private::StatsDuration m_total_time;
+   lldb_private::ConstString m_name;
+   uint64_t m_summary_count;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_SUMMARYSTATISTICS_H
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 119dff4d498199..ee6a009b0af95d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -30,6 +30,7 @@
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Target/SectionLoadHistory.h"
 #include "lldb/Target/Statistics.h"
+#include "lldb/Target/SummaryStatistics.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
@@ -258,6 +259,7 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+
 private:
   std::optional
   GetExperimentalPropertyValue(size_t prop_idx,
@@ -1221,6 +1223,8 @@ class Target : public 
std::enable_shared_from_this,
 
   void ClearAllLoadedSections();
 
+  lldb_private::StatsDuration& 
GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name);
+
   /// Set the \a Trace object containing processor trace information of this
   /// target.
   ///
@@ -1554,6 +1558,7 @@ class Target : public 
std::enable_shared_from_this,
   std::string m_label;
   ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
+  std::map 
m_summary_stats_map;
   SectionLoadHistory m_section_load_history;
   BreakpointList m_breakpoint_list;
   BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4f..bed4ab8d69cbda 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,6 +615,11 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl 
*summary_ptr,
   m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
 // the synthetic children being
 // up-to-date (e.g. ${svar%#})
+StatsDuration &summary_duration = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
+
.GetSummaryProviderDuration(GetTypeName());
+ElapsedTime elapsed(summary_duration);
 summary_ptr->FormatObject(this, destination, actual_options);
   }
   m_flags.m_is_getting_summary = false;
diff --git a/lldb/source/Target/CMakeLists.txt 
b/lldb/source/Target/CMakeLists.txt
index a42c44b761dc56..e51da37cd84db3 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -46,6 +46,7 @@ add_lldb_library(lldbTarget
   Statistics.cpp
   StopInfo.cpp
   StructuredDataPlugin.cpp
+  SummaryStatistics.cpp
   SystemRuntime.cpp
   Target.cpp
   TargetList.cpp
diff --git a/lldb/source/Target/SummaryStatistics.cpp 
b/lldb/sour

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/102708

>From c0a7286b0107d3161b18de8f05d4d016150e96a5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Thu, 8 Aug 2024 08:58:52 -0700
Subject: [PATCH 1/7] Initial attempt at new classes Summary statistics in
 SummaryStatistics.h/cpp.

---
 lldb/include/lldb/Target/SummaryStatistics.h | 37 
 lldb/include/lldb/Target/Target.h|  5 +++
 lldb/source/Core/ValueObject.cpp |  5 +++
 lldb/source/Target/CMakeLists.txt|  1 +
 lldb/source/Target/SummaryStatistics.cpp | 26 ++
 lldb/source/Target/Target.cpp|  9 +
 6 files changed, 83 insertions(+)
 create mode 100644 lldb/include/lldb/Target/SummaryStatistics.h
 create mode 100644 lldb/source/Target/SummaryStatistics.cpp

diff --git a/lldb/include/lldb/Target/SummaryStatistics.h 
b/lldb/include/lldb/Target/SummaryStatistics.h
new file mode 100644
index 00..0198249ba0b170
--- /dev/null
+++ b/lldb/include/lldb/Target/SummaryStatistics.h
@@ -0,0 +1,37 @@
+//===-- SummaryStatistics.h -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_TARGET_SUMMARYSTATISTICS_H
+#define LLDB_TARGET_SUMMARYSTATISTICS_H
+
+
+#include "lldb/Target/Statistics.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private {
+
+class SummaryStatistics {
+public:
+  SummaryStatistics(lldb_private::ConstString name) : 
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+  lldb_private::StatsDuration &GetDurationReference();
+
+  lldb_private::ConstString GetName() const;
+
+  uint64_t GetSummaryCount() const;
+
+private:
+   lldb_private::StatsDuration m_total_time;
+   lldb_private::ConstString m_name;
+   uint64_t m_summary_count;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_SUMMARYSTATISTICS_H
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 119dff4d498199..ee6a009b0af95d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -30,6 +30,7 @@
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Target/SectionLoadHistory.h"
 #include "lldb/Target/Statistics.h"
+#include "lldb/Target/SummaryStatistics.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
@@ -258,6 +259,7 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+
 private:
   std::optional
   GetExperimentalPropertyValue(size_t prop_idx,
@@ -1221,6 +1223,8 @@ class Target : public 
std::enable_shared_from_this,
 
   void ClearAllLoadedSections();
 
+  lldb_private::StatsDuration& 
GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name);
+
   /// Set the \a Trace object containing processor trace information of this
   /// target.
   ///
@@ -1554,6 +1558,7 @@ class Target : public 
std::enable_shared_from_this,
   std::string m_label;
   ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
+  std::map 
m_summary_stats_map;
   SectionLoadHistory m_section_load_history;
   BreakpointList m_breakpoint_list;
   BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4f..bed4ab8d69cbda 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,6 +615,11 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl 
*summary_ptr,
   m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
 // the synthetic children being
 // up-to-date (e.g. ${svar%#})
+StatsDuration &summary_duration = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
+
.GetSummaryProviderDuration(GetTypeName());
+ElapsedTime elapsed(summary_duration);
 summary_ptr->FormatObject(this, destination, actual_options);
   }
   m_flags.m_is_getting_summary = false;
diff --git a/lldb/source/Target/CMakeLists.txt 
b/lldb/source/Target/CMakeLists.txt
index a42c44b761dc56..e51da37cd84db3 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -46,6 +46,7 @@ add_lldb_library(lldbTarget
   Statistics.cpp
   StopInfo.cpp
   StructuredDataPlugin.cpp
+  SummaryStatistics.cpp
   SystemRuntime.cpp
   Target.cpp
   TargetList.cpp
diff --git a/lldb/source/Target/SummaryStatistics.cpp 
b/lldb/sour

[Lldb-commits] [lldb] [lldb] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)

[Greg Clayton via lldb-commits]

clayborg wrote:

Yes, this is primarily for performance of C++ coroutine detection where we must 
search memory for magic values.

https://github.com/llvm/llvm-project/pull/102536
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Support minidumps where there are multiple exception streams (PR #97470)

[Jacob Lalonde via lldb-commits]

Jlalond wrote:

@labath Agree on all counts, I'll split this into a different PR. With what I 
learned from our obj2yaml adventure I would like to do it better anyway.

https://github.com/llvm/llvm-project/pull/97470
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Remove Phabricator usernames from Code Owners file (PR #102590)

[Alex Langford via lldb-commits]

https://github.com/bulbazord approved this pull request.

Seems ok to me. I don't see what we gain from having a map from Phabricator 
name to contributor information for just maintainers, many changes are were 
made by people not on this list.

https://github.com/llvm/llvm-project/pull/102590
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Remove Phabricator usernames from Code Owners file (PR #102590)

[Alex Langford via lldb-commits]

https://github.com/bulbazord edited 
https://github.com/llvm/llvm-project/pull/102590
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jonas Devlieghere via lldb-commits]

@@ -145,15 +148,34 @@ std::string CXXFunctionSummaryFormat::GetDescription() {
   return std::string(sstr.GetString());
 }
 
+std::string CXXFunctionSummaryFormat::GetName() {
+  return m_description;
+}
+
+std::string CXXFunctionSummaryFormat::GetSummaryKindName() {
+  return "c++";
+}
+
 ScriptSummaryFormat::ScriptSummaryFormat(const TypeSummaryImpl::Flags &flags,
  const char *function_name,
  const char *python_script)
 : TypeSummaryImpl(Kind::eScript, flags), m_function_name(),
   m_python_script(), m_script_function_sp() {
-  if (function_name)
+  std::string sstring;

JDevlieghere wrote:

Why not operate on `m_script_formatter_name ` directly?

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jonas Devlieghere via lldb-commits]

@@ -174,6 +177,82 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(std::string name,
+ std::string impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_summary_count(0) {}
+
+  std::string GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; }
+
+  std::string GetSummaryKindName() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+  // Basic RAII class to increment the summary count when the call is complete.
+  // In the future this can be extended to collect information about the
+  // elapsed time for a single request.
+  class SummaryInvocation {
+  public:
+SummaryInvocation(SummaryStatistics &summary)
+: m_summary(summary), m_elapsed_time(summary.GetDurationReference()) {}
+~SummaryInvocation() { m_summary.OnInvoked(); }
+
+// Delete the copy constructor and assignment operator to prevent
+// accidental double counting.
+SummaryInvocation(const SummaryInvocation &) = delete;
+SummaryInvocation &operator=(const SummaryInvocation &) = delete;

JDevlieghere wrote:

```suggestion
/// Delete the copy constructor and assignment operator to prevent
/// accidental double counting.
/// @{
SummaryInvocation(const SummaryInvocation &) = delete;
SummaryInvocation &operator=(const SummaryInvocation &) = delete;
/// @}
```

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jonas Devlieghere via lldb-commits]

@@ -174,6 +177,82 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(std::string name,
+ std::string impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_summary_count(0) {}
+
+  std::string GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; }
+
+  std::string GetSummaryKindName() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+  // Basic RAII class to increment the summary count when the call is complete.
+  // In the future this can be extended to collect information about the
+  // elapsed time for a single request.
+  class SummaryInvocation {
+  public:
+SummaryInvocation(SummaryStatistics &summary)
+: m_summary(summary), m_elapsed_time(summary.GetDurationReference()) {}
+~SummaryInvocation() { m_summary.OnInvoked(); }
+
+// Delete the copy constructor and assignment operator to prevent
+// accidental double counting.
+SummaryInvocation(const SummaryInvocation &) = delete;
+SummaryInvocation &operator=(const SummaryInvocation &) = delete;
+
+  private:
+SummaryStatistics &m_summary;
+ElapsedTime m_elapsed_time;
+  };
+
+  SummaryInvocation GetSummaryInvocation() { return SummaryInvocation(*this); }
+
+private:
+  /// Called when Summary Invocation is destructed.
+  void OnInvoked() noexcept {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+  }
+
+  lldb_private::StatsDuration m_total_time;
+  std::string m_impl_type;
+  std::string m_name;
+  std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+  /// Get the SummaryStatistics object for a given provider name, or insert
+  /// if statistics for that provider is not in the map.
+  SummaryStatistics::SummaryInvocation
+  GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) 
{
+std::lock_guard guard(m_map_mutex);
+auto pair = m_summary_stats_map.try_emplace(provider.GetName(), 
provider.GetName(),
+provider.GetSummaryKindName());
+
+return pair.first->second.GetSummaryInvocation();
+  }
+
+  llvm::json::Value ToJSON();
+
+private:
+  std::map
+  m_summary_stats_map;

JDevlieghere wrote:

```suggestion
  llvm::StringMap
  m_summary_stats_map;
```

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jonas Devlieghere via lldb-commits]

@@ -174,6 +177,82 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(std::string name,
+ std::string impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_summary_count(0) {}
+
+  std::string GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; }
+
+  std::string GetSummaryKindName() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+  // Basic RAII class to increment the summary count when the call is complete.
+  // In the future this can be extended to collect information about the
+  // elapsed time for a single request.

JDevlieghere wrote:

```suggestion
  /// Basic RAII class to increment the summary count when the call is complete.
  /// In the future this can be extended to collect information about the
  /// elapsed time for a single request.
```

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

@@ -615,7 +615,15 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl 
*summary_ptr,
   m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
 // the synthetic children being
 // up-to-date (e.g. ${svar%#})
-summary_ptr->FormatObject(this, destination, actual_options);
+
+TargetSP target = GetExecutionContextRef().GetTargetSP();
+if (target) {
+  /// Construct RAII types to time and collect data on summary creation.
+  SummaryStatistics::SummaryInvocation summary_inv =
+  target->GetSummaryStatisticsForProvider(*summary_ptr);

Michael137 wrote:

I think this wouldn't work anymore because you're returning a copy of the 
`SummaryInvocation` right? My previous suggestion was to have 
`SummaryInvocartion` update the `SummaryStatisticsCache` when it's done.

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Remove Phabricator usernames from Code Owners file (PR #102590)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere approved this pull request.


https://github.com/llvm/llvm-project/pull/102590
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

@@ -615,7 +615,15 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl 
*summary_ptr,
   m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
 // the synthetic children being
 // up-to-date (e.g. ${svar%#})
-summary_ptr->FormatObject(this, destination, actual_options);
+
+TargetSP target = GetExecutionContextRef().GetTargetSP();
+if (target) {
+  /// Construct RAII types to time and collect data on summary creation.
+  SummaryStatistics::SummaryInvocation summary_inv =
+  target->GetSummaryStatisticsForProvider(*summary_ptr);

Jlalond wrote:

This actually does (humorously in my opinion) work

I didn't realize the intent was for the RAII object to update the cache, I 
actually agree that is better and frees us from needing std::map for any 
reference invalidation. I will modify.

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Remove Phabricator usernames from Code Owners file (PR #102590)

[Med Ismail Bennani via lldb-commits]

https://github.com/medismailben approved this pull request.


https://github.com/llvm/llvm-project/pull/102590
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

@@ -258,6 +258,9 @@ class TypeSummaryImpl {
 
   virtual std::string GetDescription() = 0;
 
+  virtual ConstString GetName() = 0;
+  virtual ConstString GetImplType() = 0;

Jlalond wrote:

I ended up just going with kind name

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reland "[lldb] Reland 2402b3213c2f with `/H` to debug the windows build issue (PR #101672)

[Dmitry Vasilyev via lldb-commits]

slydiman wrote:

After this patch it is very hard to build lldb on Windows. We got the errors 
like the following
```
D:\llvm-project\lldb\source\Plugins\ScriptInterpreter\Python\Interfaces\ScriptedThreadPlanPythonInterface\ScriptedThreadPlanPythonInterface.cpp:
 
fatal error C1041: cannot open program database 
'D:\build-lldb\tools\lldb\source\Plugins\ScriptInterpreter\Python\Interfaces\ScriptedThreadPlanPythonInterface\CMakeFiles\lldbPluginScriptInterpreterPythonScriptedThreadPlanPythonInterface.dir\lldbPluginScriptInterpreterPythonScriptedThreadPlanPythonInterface.pdb';
 
if multiple CL.EXE write to the same .PDB file, please use /FS
```
Note MAX_PATH is 260 on Windows. But the length of the path to .pdb is 262.
It is necessary to use 7 chars or less folder in the root of a disk to build 
lldb on Windows using Microsoft VC++. 
You must reduce the length of the paths.

https://github.com/llvm/llvm-project/pull/101672
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Fred Grim via lldb-commits]

https://github.com/feg208 created 
https://github.com/llvm/llvm-project/pull/104109

To create elf core files there are multiple notes in the core file that contain 
these structs as the note. These populate methods take a Process and produce 
fully specified structures that can be used to fill these note sections. The pr 
also adds tests to ensure these structs are correctly populated.

>From 8f2f84294ea3875c88ce36a4980fd3a3afce01de Mon Sep 17 00:00:00 2001
From: Fred Grim 
Date: Tue, 18 Jun 2024 10:38:09 -0700
Subject: [PATCH] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and
 ELFLinuxPrStatus

To create elf core files there are multiple notes in the core file that
contain these structs as the note. These populate methods take a Process
and produce fully specified structures that can be used to fill these
note sections. The pr also adds tests to ensure these structs are
correctly populated.
---
 .../Process/elf-core/ThreadElfCore.cpp| 134 +++
 .../Plugins/Process/elf-core/ThreadElfCore.h  |   7 +
 lldb/unittests/Process/CMakeLists.txt |   1 +
 .../unittests/Process/elf-core/CMakeLists.txt |  15 ++
 .../Process/elf-core/ThreadElfCoreTest.cpp| 162 ++
 5 files changed, 319 insertions(+)
 create mode 100644 lldb/unittests/Process/elf-core/CMakeLists.txt
 create mode 100644 lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp

diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index 2a83163884e168..fdb4a5837cd462 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -318,6 +318,32 @@ Status ELFLinuxPrStatus::Parse(const DataExtractor &data,
   return error;
 }
 
+static struct compat_timeval
+copy_timespecs(const ProcessInstanceInfo::timespec &oth) {
+  using sec_t = decltype(compat_timeval::tv_sec);
+  using usec_t = decltype(compat_timeval::tv_usec);
+  return {static_cast(oth.tv_sec), static_cast(oth.tv_usec)};
+}
+
+std::optional
+ELFLinuxPrStatus::Populate(const lldb::ThreadSP &thread_sp) {
+  ELFLinuxPrStatus prstatus{};
+  prstatus.pr_pid = thread_sp->GetID();
+  lldb::ProcessSP process_sp = thread_sp->GetProcess();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prstatus.pr_ppid = info.GetParentProcessID();
+  prstatus.pr_pgrp = info.GetProcessGroupID();
+  prstatus.pr_sid = info.GetProcessSessionID();
+  prstatus.pr_utime = copy_timespecs(info.GetUserTime());
+  prstatus.pr_stime = copy_timespecs(info.GetSystemTime());
+  prstatus.pr_cutime = copy_timespecs(info.GetCumulativeUserTime());
+  prstatus.pr_cstime = copy_timespecs(info.GetCumulativeSystemTime());
+  return prstatus;
+}
+
 // Parse PRPSINFO from NOTE entry
 ELFLinuxPrPsInfo::ELFLinuxPrPsInfo() {
   memset(this, 0, sizeof(ELFLinuxPrPsInfo));
@@ -394,6 +420,114 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
   return error;
 }
 
+std::optional
+ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
+  ELFLinuxPrPsInfo prpsinfo{};
+  prpsinfo.pr_pid = process_sp->GetID();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prpsinfo.pr_nice = info.GetPriorityValue().value_or(0);
+  prpsinfo.pr_zomb = 0;
+  if (auto zombie_opt = info.IsZombie(); zombie_opt.value_or(false)) {
+prpsinfo.pr_zomb = 1;
+  }
+  /**
+   * In the linux kernel this comes from:
+   * state = READ_ONCE(p->__state);
+   * i = state ? ffz(~state) + 1 : 0;
+   * psinfo->pr_sname = (i > 5) ? '.' : "RSDTZW"[i];
+   *
+   * So we replicate that here. From proc_pid_stats(5)
+   * R = Running
+   * S = Sleeping on uninterrutible wait
+   * D = Waiting on uninterruptable disk sleep
+   * T = Tracing stop
+   * Z = Zombie
+   * W = Paging
+   */
+  lldb::StateType process_state = process_sp->GetState();
+  switch (process_state) {
+  case lldb::StateType::eStateSuspended:
+prpsinfo.pr_sname = 'S';
+prpsinfo.pr_state = 1;
+break;
+  case lldb::StateType::eStateStopped:
+[[fallthrough]];
+  case lldb::StateType::eStateStepping:
+prpsinfo.pr_sname = 'T';
+prpsinfo.pr_state = 3;
+break;
+  case lldb::StateType::eStateUnloaded:
+[[fallthrough]];
+  case lldb::StateType::eStateRunning:
+prpsinfo.pr_sname = 'R';
+prpsinfo.pr_state = 0;
+break;
+  default:
+break;
+  }
+
+  /**
+   * pr_flags is left as 0. The values (in linux) are specific
+   * to the kernel. We recover them from the proc filesystem
+   * but don't put them in ProcessInfo because it would really
+   * become very linux specific and the utility here seems pretty
+   * dubious
+   */
+
+  if (info.EffectiveUserIDIsValid()) {
+prpsinfo.pr_uid = info.GetUserID();
+  }
+  if (info.EffectiveGroupIDIsValid()) {
+prpsinfo.pr_gid = info.GetGroupID();
+  }
+
+  if (info.ParentProcessIDIsValid()) {
+prpsinfo.pr_ppid = info.GetParent

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Fred Grim (feg208)


Changes

To create elf core files there are multiple notes in the core file that contain 
these structs as the note. These populate methods take a Process and produce 
fully specified structures that can be used to fill these note sections. The pr 
also adds tests to ensure these structs are correctly populated.

---
Full diff: https://github.com/llvm/llvm-project/pull/104109.diff


5 Files Affected:

- (modified) lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp (+134) 
- (modified) lldb/source/Plugins/Process/elf-core/ThreadElfCore.h (+7) 
- (modified) lldb/unittests/Process/CMakeLists.txt (+1) 
- (added) lldb/unittests/Process/elf-core/CMakeLists.txt (+15) 
- (added) lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp (+162) 


``diff
diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index 2a83163884e168..fdb4a5837cd462 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -318,6 +318,32 @@ Status ELFLinuxPrStatus::Parse(const DataExtractor &data,
   return error;
 }
 
+static struct compat_timeval
+copy_timespecs(const ProcessInstanceInfo::timespec &oth) {
+  using sec_t = decltype(compat_timeval::tv_sec);
+  using usec_t = decltype(compat_timeval::tv_usec);
+  return {static_cast(oth.tv_sec), static_cast(oth.tv_usec)};
+}
+
+std::optional
+ELFLinuxPrStatus::Populate(const lldb::ThreadSP &thread_sp) {
+  ELFLinuxPrStatus prstatus{};
+  prstatus.pr_pid = thread_sp->GetID();
+  lldb::ProcessSP process_sp = thread_sp->GetProcess();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prstatus.pr_ppid = info.GetParentProcessID();
+  prstatus.pr_pgrp = info.GetProcessGroupID();
+  prstatus.pr_sid = info.GetProcessSessionID();
+  prstatus.pr_utime = copy_timespecs(info.GetUserTime());
+  prstatus.pr_stime = copy_timespecs(info.GetSystemTime());
+  prstatus.pr_cutime = copy_timespecs(info.GetCumulativeUserTime());
+  prstatus.pr_cstime = copy_timespecs(info.GetCumulativeSystemTime());
+  return prstatus;
+}
+
 // Parse PRPSINFO from NOTE entry
 ELFLinuxPrPsInfo::ELFLinuxPrPsInfo() {
   memset(this, 0, sizeof(ELFLinuxPrPsInfo));
@@ -394,6 +420,114 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
   return error;
 }
 
+std::optional
+ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
+  ELFLinuxPrPsInfo prpsinfo{};
+  prpsinfo.pr_pid = process_sp->GetID();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prpsinfo.pr_nice = info.GetPriorityValue().value_or(0);
+  prpsinfo.pr_zomb = 0;
+  if (auto zombie_opt = info.IsZombie(); zombie_opt.value_or(false)) {
+prpsinfo.pr_zomb = 1;
+  }
+  /**
+   * In the linux kernel this comes from:
+   * state = READ_ONCE(p->__state);
+   * i = state ? ffz(~state) + 1 : 0;
+   * psinfo->pr_sname = (i > 5) ? '.' : "RSDTZW"[i];
+   *
+   * So we replicate that here. From proc_pid_stats(5)
+   * R = Running
+   * S = Sleeping on uninterrutible wait
+   * D = Waiting on uninterruptable disk sleep
+   * T = Tracing stop
+   * Z = Zombie
+   * W = Paging
+   */
+  lldb::StateType process_state = process_sp->GetState();
+  switch (process_state) {
+  case lldb::StateType::eStateSuspended:
+prpsinfo.pr_sname = 'S';
+prpsinfo.pr_state = 1;
+break;
+  case lldb::StateType::eStateStopped:
+[[fallthrough]];
+  case lldb::StateType::eStateStepping:
+prpsinfo.pr_sname = 'T';
+prpsinfo.pr_state = 3;
+break;
+  case lldb::StateType::eStateUnloaded:
+[[fallthrough]];
+  case lldb::StateType::eStateRunning:
+prpsinfo.pr_sname = 'R';
+prpsinfo.pr_state = 0;
+break;
+  default:
+break;
+  }
+
+  /**
+   * pr_flags is left as 0. The values (in linux) are specific
+   * to the kernel. We recover them from the proc filesystem
+   * but don't put them in ProcessInfo because it would really
+   * become very linux specific and the utility here seems pretty
+   * dubious
+   */
+
+  if (info.EffectiveUserIDIsValid()) {
+prpsinfo.pr_uid = info.GetUserID();
+  }
+  if (info.EffectiveGroupIDIsValid()) {
+prpsinfo.pr_gid = info.GetGroupID();
+  }
+
+  if (info.ParentProcessIDIsValid()) {
+prpsinfo.pr_ppid = info.GetParentProcessID();
+  }
+
+  if (info.ProcessGroupIDIsValid()) {
+prpsinfo.pr_pgrp = info.GetProcessGroupID();
+  }
+
+  if (info.ProcessSessionIDIsValid()) {
+prpsinfo.pr_sid = info.GetProcessSessionID();
+  }
+  constexpr size_t fname_len = std::extent_v;
+  static_assert(fname_len > 0, "This should always be non zero");
+  const llvm::StringRef fname = info.GetNameAsStringRef();
+  auto fname_begin = fname.begin();
+  std::copy(fname_begin,
+std::next(fname_begin, std::min(fname_len, fname.size())),
+prpsinfo.pr_fname);
+  pr

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

[Alex Langford via lldb-commits]

@@ -0,0 +1,459 @@
+//===-- DILAST.h *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_CORE_DILAST_H
+#define LLDB_CORE_DILAST_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/LanguageRuntime.h"
+#include "lldb/Utility/ConstString.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"

bulbazord wrote:

I haven't been following the evolution of this PR closely, but from this 
comment chain it looks like the question is "Can we have clang dependencies in 
lldbCore?". I won't gatekeep this PR for this reason, but here are my positions:
1. We should avoid having clang in lldbCore if possible.
2. I think you should write a lexer for the DIL.

As Pavel already pointed out, plenty of folks downstream want to use LLDB to 
debug their programs with languages other than C/C++/ObjC/ObjC++. Swift and 
Rust are great examples and on occasion I see people try to add minor changes 
to support their languages too. Adding clang as a core dependency makes it more 
difficult to decouple LLDB from C/C++ specifically. LLDB has the potential to 
be a more language-agnostic debugger where the core of the engine is flexible 
and focuses on operating on common abstractions while the Plugins dictate how 
to interact with the specific language/OS/platform. I think LLDB is still 
pretty far away from being that, but myself and other contributors have done a 
lot of work over the last few years to bring us closer to that reality.

As for the DIL, I think we should consider the goal. If the language is meant 
to use C/C++ syntax to inspect data, I suppose it makes sense to use clang's 
lexer to tokenize input. CPlusPlusNameParser uses clang because it's trying to 
parse C++ names. However, using clang now makes it harder to extend to support 
other languages in the future. What if clang doesn't chop up my input into 
tokens correctly or the way I want? Pavel mentioned a case where we can work 
around this, but the parser is just going to be workaround after workaround 
trying to fit a square peg into a round hole. You've already written the 
parser, what's a lexer too? :)

https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP] memory find speedup+bugfix (PR #104193)

[Pavel Labath via lldb-commits]

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/104193

None

>From d9517b19db12de3530fea967ae6d577317d1c5a2 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 14 Aug 2024 19:58:27 +0200
Subject: [PATCH] [WIP] memory find speedup+bugfix

---
 lldb/source/Target/Process.cpp | 58 ++
 1 file changed, 24 insertions(+), 34 deletions(-)

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index e3c4f2ee398cc4..4e3d7651a066ec 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -114,33 +114,6 @@ class ProcessOptionValueProperties
   }
 };
 
-class ProcessMemoryIterator {
-public:
-  ProcessMemoryIterator(Process &process, lldb::addr_t base)
-  : m_process(process), m_base_addr(base) {}
-
-  bool IsValid() { return m_is_valid; }
-
-  uint8_t operator[](lldb::addr_t offset) {
-if (!IsValid())
-  return 0;
-
-uint8_t retval = 0;
-Status error;
-if (0 == m_process.ReadMemory(m_base_addr + offset, &retval, 1, error)) {
-  m_is_valid = false;
-  return 0;
-}
-
-return retval;
-  }
-
-private:
-  Process &m_process;
-  const lldb::addr_t m_base_addr;
-  bool m_is_valid = true;
-};
-
 static constexpr OptionEnumValueElement g_follow_fork_mode_values[] = {
 {
 eFollowParent,
@@ -3368,20 +3341,37 @@ lldb::addr_t Process::FindInMemory(lldb::addr_t low, 
lldb::addr_t high,
 return LLDB_INVALID_ADDRESS;
 
   std::vector bad_char_heuristic(256, size);
-  ProcessMemoryIterator iterator(*this, low);
-
   for (size_t idx = 0; idx < size - 1; idx++) {
 decltype(bad_char_heuristic)::size_type bcu_idx = buf[idx];
 bad_char_heuristic[bcu_idx] = size - idx - 1;
   }
-  for (size_t s = 0; s <= (region_size - size);) {
+
+  llvm::SmallVector mem;
+  addr_t mem_pos = low;
+  const size_t read_size = std::max(size, 0x1);
+
+  for (addr_t s = low; s <= (high - size);) {
+if (s + size >= mem.size() + mem_pos) {
+  mem.resize_for_overwrite(read_size);
+  Status error;
+  mem.resize(
+  ReadMemory(s, mem.data(), std::min(mem.size(), high - s), error));
+  mem_pos = s;
+  if (error.Fail()) {
+MemoryRegionInfo info;
+error = GetMemoryRegionInfo(s, info);
+if (error.Fail())
+  return LLDB_INVALID_ADDRESS;
+s = info.GetRange().GetRangeEnd();
+continue;
+  }
+}
 int64_t j = size - 1;
-while (j >= 0 && buf[j] == iterator[s + j])
+while (j >= 0 && buf[j] == mem[s + j - mem_pos])
   j--;
 if (j < 0)
-  return low + s;
-else
-  s += bad_char_heuristic[iterator[s + size - 1]];
+  return s;
+s += bad_char_heuristic[mem[s + size - 1 - mem_pos]];
   }
 
   return LLDB_INVALID_ADDRESS;

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

[Lldb-commits] [lldb] Fix single thread stepping timeout race condition (PR #104195)

[via lldb-commits]

https://github.com/jeffreytan81 created 
https://github.com/llvm/llvm-project/pull/104195

This PR fixes a potential race condition in 
https://github.com/llvm/llvm-project/pull/90930. 

This race can happen because the original code set `m_info->m_isAlive = true` 
**after** the timer thread is created. So if there is a context switch happens 
and timer thread checks `m_info->m_isAlive` before main thread got a chance to 
run `m_info->m_isAlive = true`, the timer thread may treat 
`ThreadPlanSingleThreadTimeout` as not alive and simply exit.



>From 5d413e737dd522603ab22105946671d95dc2d038 Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Wed, 14 Aug 2024 12:17:22 -0700
Subject: [PATCH] Fix single thread stepping timeout race condition

---
 lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp 
b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
index 0a939d55f4ce49..806ba95c508b7c 100644
--- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
+++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
@@ -29,10 +29,10 @@ 
ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(
 : ThreadPlan(ThreadPlan::eKindSingleThreadTimeout, "Single thread timeout",
  thread, eVoteNo, eVoteNoOpinion),
   m_info(info), m_state(State::WaitTimeout) {
-  // TODO: reuse m_timer_thread without recreation.
-  m_timer_thread = std::thread(TimeoutThreadFunc, this);
   m_info->m_isAlive = true;
   m_state = m_info->m_last_state;
+  // TODO: reuse m_timer_thread without recreation.
+  m_timer_thread = std::thread(TimeoutThreadFunc, this);
 }
 
 ThreadPlanSingleThreadTimeout::~ThreadPlanSingleThreadTimeout() {

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

[Lldb-commits] [lldb] Fix single thread stepping timeout race condition (PR #104195)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jeffreytan81)


Changes

This PR fixes a potential race condition in 
https://github.com/llvm/llvm-project/pull/90930. 

This race can happen because the original code set `m_info->m_isAlive = 
true` **after** the timer thread is created. So if there is a context switch 
happens and timer thread checks `m_info->m_isAlive` before main thread got a 
chance to run `m_info->m_isAlive = true`, the timer thread may treat 
`ThreadPlanSingleThreadTimeout` as not alive and simply exit.



---
Full diff: https://github.com/llvm/llvm-project/pull/104195.diff


1 Files Affected:

- (modified) lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp (+2-2) 


``diff
diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp 
b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
index 0a939d55f4ce49..806ba95c508b7c 100644
--- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
+++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
@@ -29,10 +29,10 @@ 
ThreadPlanSingleThreadTimeout::ThreadPlanSingleThreadTimeout(
 : ThreadPlan(ThreadPlan::eKindSingleThreadTimeout, "Single thread timeout",
  thread, eVoteNo, eVoteNoOpinion),
   m_info(info), m_state(State::WaitTimeout) {
-  // TODO: reuse m_timer_thread without recreation.
-  m_timer_thread = std::thread(TimeoutThreadFunc, this);
   m_info->m_isAlive = true;
   m_state = m_info->m_last_state;
+  // TODO: reuse m_timer_thread without recreation.
+  m_timer_thread = std::thread(TimeoutThreadFunc, this);
 }
 
 ThreadPlanSingleThreadTimeout::~ThreadPlanSingleThreadTimeout() {

``




https://github.com/llvm/llvm-project/pull/104195
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix single thread stepping timeout race condition (PR #104195)

[via lldb-commits]

https://github.com/jeffreytan81 edited 
https://github.com/llvm/llvm-project/pull/104195
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] New ThreadPlanSingleThreadTimeout to resolve potential deadlock in single thread stepping (PR #90930)

[via lldb-commits]

jeffreytan81 wrote:

The GDB packet log is very useful (especially with a good run log at the top to 
compare). 

I read with @clayborg . The log indicates that 
"self.thread.StepOver(lldb.eOnlyThisThread)" sent a "$vCont;c:p1a24.1a24" to 
continue single thread at the end but lacking a "\x03" async interrupt packet 
(which can be found in the good log at the top). The async interrupt should be 
sent by `ThreadPlanSingleThreadTimeout::TimeoutThreadFunc()` timer thread after 
timeout. Further reading the code, we found that there is a potential race that 
timer thread checks the m_isAlive flag before it was initialized. 

Created https://github.com/llvm/llvm-project/pull/104195 to fix the race.

https://github.com/llvm/llvm-project/pull/90930
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add 'FindInMemory()' overload for PostMortemProcess. (PR #102536)

[Pavel Labath via lldb-commits]

labath wrote:

> The purpose of this it to make the `find memory` faster for post mortem 
> processes. This patch gives us nearly 100x speed up based on my tests.

I see. And do you have an idea of how load-bearing that number is? For example, 
would you settle for a 18x speedup? That's what I got from #104193, with the 
advantage that it also works for live processes (with a ~6x speedup) and 
requires no new APIs.

With the addition of a copy-free ReadMemory API it could be made even faster 
(probably up to your 100x).

https://github.com/llvm/llvm-project/pull/102536
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Pavel Labath via lldb-commits]

https://github.com/labath edited 
https://github.com/llvm/llvm-project/pull/104109
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Pavel Labath via lldb-commits]

@@ -394,6 +420,114 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
   return error;
 }
 
+std::optional
+ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
+  ELFLinuxPrPsInfo prpsinfo{};
+  prpsinfo.pr_pid = process_sp->GetID();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prpsinfo.pr_nice = info.GetPriorityValue().value_or(0);
+  prpsinfo.pr_zomb = 0;
+  if (auto zombie_opt = info.IsZombie(); zombie_opt.value_or(false)) {
+prpsinfo.pr_zomb = 1;
+  }
+  /**
+   * In the linux kernel this comes from:
+   * state = READ_ONCE(p->__state);
+   * i = state ? ffz(~state) + 1 : 0;
+   * psinfo->pr_sname = (i > 5) ? '.' : "RSDTZW"[i];
+   *
+   * So we replicate that here. From proc_pid_stats(5)
+   * R = Running
+   * S = Sleeping on uninterrutible wait
+   * D = Waiting on uninterruptable disk sleep
+   * T = Tracing stop
+   * Z = Zombie
+   * W = Paging
+   */
+  lldb::StateType process_state = process_sp->GetState();
+  switch (process_state) {
+  case lldb::StateType::eStateSuspended:
+prpsinfo.pr_sname = 'S';
+prpsinfo.pr_state = 1;
+break;
+  case lldb::StateType::eStateStopped:
+[[fallthrough]];
+  case lldb::StateType::eStateStepping:
+prpsinfo.pr_sname = 'T';
+prpsinfo.pr_state = 3;
+break;
+  case lldb::StateType::eStateUnloaded:
+[[fallthrough]];
+  case lldb::StateType::eStateRunning:
+prpsinfo.pr_sname = 'R';
+prpsinfo.pr_state = 0;
+break;
+  default:
+break;
+  }
+
+  /**
+   * pr_flags is left as 0. The values (in linux) are specific
+   * to the kernel. We recover them from the proc filesystem
+   * but don't put them in ProcessInfo because it would really
+   * become very linux specific and the utility here seems pretty
+   * dubious
+   */
+
+  if (info.EffectiveUserIDIsValid()) {
+prpsinfo.pr_uid = info.GetUserID();
+  }
+  if (info.EffectiveGroupIDIsValid()) {
+prpsinfo.pr_gid = info.GetGroupID();
+  }
+
+  if (info.ParentProcessIDIsValid()) {
+prpsinfo.pr_ppid = info.GetParentProcessID();
+  }
+
+  if (info.ProcessGroupIDIsValid()) {
+prpsinfo.pr_pgrp = info.GetProcessGroupID();
+  }
+
+  if (info.ProcessSessionIDIsValid()) {
+prpsinfo.pr_sid = info.GetProcessSessionID();
+  }
+  constexpr size_t fname_len = std::extent_v;
+  static_assert(fname_len > 0, "This should always be non zero");
+  const llvm::StringRef fname = info.GetNameAsStringRef();
+  auto fname_begin = fname.begin();
+  std::copy(fname_begin,
+std::next(fname_begin, std::min(fname_len, fname.size())),
+prpsinfo.pr_fname);

labath wrote:

```suggestion
  std::copy_n(fname_begin,
std::min(fname_len, fname.size()),
prpsinfo.pr_fname);
```

https://github.com/llvm/llvm-project/pull/104109
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

I like how you've made a separate patch out of these functions. Just a couple 
of random improvements I noticed.

https://github.com/llvm/llvm-project/pull/104109
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Pavel Labath via lldb-commits]

@@ -394,6 +420,114 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
   return error;
 }
 
+std::optional
+ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
+  ELFLinuxPrPsInfo prpsinfo{};
+  prpsinfo.pr_pid = process_sp->GetID();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prpsinfo.pr_nice = info.GetPriorityValue().value_or(0);
+  prpsinfo.pr_zomb = 0;
+  if (auto zombie_opt = info.IsZombie(); zombie_opt.value_or(false)) {
+prpsinfo.pr_zomb = 1;
+  }
+  /**
+   * In the linux kernel this comes from:
+   * state = READ_ONCE(p->__state);
+   * i = state ? ffz(~state) + 1 : 0;
+   * psinfo->pr_sname = (i > 5) ? '.' : "RSDTZW"[i];
+   *
+   * So we replicate that here. From proc_pid_stats(5)
+   * R = Running
+   * S = Sleeping on uninterrutible wait
+   * D = Waiting on uninterruptable disk sleep
+   * T = Tracing stop
+   * Z = Zombie
+   * W = Paging
+   */
+  lldb::StateType process_state = process_sp->GetState();
+  switch (process_state) {
+  case lldb::StateType::eStateSuspended:
+prpsinfo.pr_sname = 'S';
+prpsinfo.pr_state = 1;
+break;
+  case lldb::StateType::eStateStopped:
+[[fallthrough]];
+  case lldb::StateType::eStateStepping:
+prpsinfo.pr_sname = 'T';
+prpsinfo.pr_state = 3;
+break;
+  case lldb::StateType::eStateUnloaded:
+[[fallthrough]];
+  case lldb::StateType::eStateRunning:
+prpsinfo.pr_sname = 'R';
+prpsinfo.pr_state = 0;
+break;
+  default:
+break;
+  }
+
+  /**
+   * pr_flags is left as 0. The values (in linux) are specific
+   * to the kernel. We recover them from the proc filesystem
+   * but don't put them in ProcessInfo because it would really
+   * become very linux specific and the utility here seems pretty
+   * dubious
+   */
+
+  if (info.EffectiveUserIDIsValid()) {
+prpsinfo.pr_uid = info.GetUserID();
+  }
+  if (info.EffectiveGroupIDIsValid()) {
+prpsinfo.pr_gid = info.GetGroupID();
+  }
+
+  if (info.ParentProcessIDIsValid()) {
+prpsinfo.pr_ppid = info.GetParentProcessID();
+  }
+
+  if (info.ProcessGroupIDIsValid()) {
+prpsinfo.pr_pgrp = info.GetProcessGroupID();
+  }
+
+  if (info.ProcessSessionIDIsValid()) {
+prpsinfo.pr_sid = info.GetProcessSessionID();
+  }
+  constexpr size_t fname_len = std::extent_v;
+  static_assert(fname_len > 0, "This should always be non zero");
+  const llvm::StringRef fname = info.GetNameAsStringRef();
+  auto fname_begin = fname.begin();
+  std::copy(fname_begin,
+std::next(fname_begin, std::min(fname_len, fname.size())),
+prpsinfo.pr_fname);
+  prpsinfo.pr_fname[fname_len - 1] = '\0';
+  auto args = info.GetArguments();
+  if (!args.empty()) {
+constexpr size_t psargs_len = std::extent_v;
+static_assert(psargs_len > 0, "This should always be non zero");
+size_t i = psargs_len;
+auto argentry_iterator = std::begin(args);
+char *psargs = prpsinfo.pr_psargs;
+while (i > 0 && argentry_iterator != args.end()) {
+  llvm::StringRef argentry = argentry_iterator->ref();
+  size_t len = std::min(i, argentry.size());
+  auto arg_iterator = std::begin(argentry);
+  std::copy(arg_iterator, std::next(arg_iterator, len), psargs);
+  i -= len;
+  psargs += len;
+  if (i > 0) {
+*(psargs++) = ' ';
+--i;
+  }
+  ++argentry_iterator;
+}

labath wrote:

```suggestion
auto argentry_iterator = std::begin(args);
char *psargs = prpsinfo.pr_psargs;
char *psargs_end = std::end(prpsinfo.pr_psargs);
while (psargs < psargs_end && argentry_iterator != args.end()) {
  llvm::StringRef argentry = argentry_iterator->ref();
  size_t len = std::min(std::distance(psargs, psargs_end), argentry.size());
  auto arg_iterator = std::begin(argentry);
  psargs = std::copy_n(arg_iterator, len, psargs);
  if (psargs != psargs_end)
*(psargs++) = ' ';
  ++argentry_iterator;
}
```

I noticed you like iterators, so I've put in more of them. :)

https://github.com/llvm/llvm-project/pull/104109
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,162 @@
+//===-- ThreadElfCoreTest.cpp *- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "Plugins/Process/elf-core/ThreadElfCore.h"
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Utility/Listener.h"
+#include "llvm/TargetParser/Triple.h"
+#include "gtest/gtest.h"
+
+#include 
+#include 
+#include 
+#include 
+
+using namespace lldb_private;
+
+namespace {
+
+struct ElfCoreTest : public testing::Test {
+  static void SetUpTestCase() {
+FileSystem::Initialize();
+HostInfo::Initialize();
+platform_linux::PlatformLinux::Initialize();
+std::call_once(TestUtilities::g_debugger_initialize_flag,
+   []() { Debugger::Initialize(nullptr); });
+  }
+  static void TearDownTestCase() {
+platform_linux::PlatformLinux::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+  }
+};
+
+struct DummyProcess : public Process {
+  DummyProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp)
+  : Process(target_sp, listener_sp) {
+SetID(getpid());
+  }
+
+  bool CanDebug(lldb::TargetSP target, bool plugin_specified_by_name) override 
{
+return true;
+  }
+  Status DoDestroy() override { return {}; }
+  void RefreshStateAfterStop() override {}
+  size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
+  Status &error) override {
+return 0;
+  }
+  bool DoUpdateThreadList(ThreadList &old_thread_list,
+  ThreadList &new_thread_list) override {
+return false;
+  }
+  llvm::StringRef GetPluginName() override { return "Dummy"; }
+};
+
+struct DummyThread : public Thread {
+  using Thread::Thread;
+
+  ~DummyThread() override { DestroyThread(); }
+
+  void RefreshStateAfterStop() override {}
+
+  lldb::RegisterContextSP GetRegisterContext() override { return nullptr; }
+
+  lldb::RegisterContextSP
+  CreateRegisterContextForFrame(StackFrame *frame) override {
+return nullptr;
+  }
+
+  bool CalculateStopInfo() override { return false; }
+};
+
+lldb::TargetSP CreateTarget(lldb::DebuggerSP &debugger_sp, ArchSpec &arch) {
+  lldb::PlatformSP platform_sp;
+  lldb::TargetSP target_sp;
+  debugger_sp->GetTargetList().CreateTarget(
+  *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp);
+  return target_sp;
+}
+
+lldb::ThreadSP CreateThread(lldb::ProcessSP &process_sp) {
+  lldb::ThreadSP thread_sp =
+  std::make_shared(*process_sp.get(), gettid());
+  if (thread_sp == nullptr) {
+return nullptr;
+  }
+  process_sp->GetThreadList().AddThread(thread_sp);
+
+  return thread_sp;
+}
+
+} // namespace
+
+TEST_F(ElfCoreTest, PopulatePrpsInfoTest) {
+  ArchSpec arch{HostInfo::GetTargetTriple()};
+  lldb::DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  lldb::TargetSP target_sp = CreateTarget(debugger_sp, arch);
+  ASSERT_TRUE(target_sp);
+
+  lldb::ListenerSP listener_sp(Listener::MakeListener("dummy"));
+  lldb::ProcessSP process_sp =
+  std::make_shared(target_sp, listener_sp);
+  ASSERT_TRUE(process_sp);
+  auto prpsinfo_opt = ELFLinuxPrPsInfo::Populate(process_sp);

labath wrote:

This would be easier to test if the API was like 
`ELFLinuxPrPsInfo::Populate(ProcessInfo, State)`, as then you could call the 
function with values fully under your control. That way you could e.g. test 
boundary conditions on the arguments string (the most complicated part of the 
function).

If you want you can still have an overload which takes a process (and calls 
this function), but given that it will likely only ever have a single caller, 
it could just be inlined into it.

https://github.com/llvm/llvm-project/pull/104109
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Pavel Labath via lldb-commits]

@@ -394,6 +420,114 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
   return error;
 }
 
+std::optional
+ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
+  ELFLinuxPrPsInfo prpsinfo{};
+  prpsinfo.pr_pid = process_sp->GetID();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prpsinfo.pr_nice = info.GetPriorityValue().value_or(0);
+  prpsinfo.pr_zomb = 0;
+  if (auto zombie_opt = info.IsZombie(); zombie_opt.value_or(false)) {
+prpsinfo.pr_zomb = 1;
+  }
+  /**
+   * In the linux kernel this comes from:
+   * state = READ_ONCE(p->__state);
+   * i = state ? ffz(~state) + 1 : 0;
+   * psinfo->pr_sname = (i > 5) ? '.' : "RSDTZW"[i];
+   *
+   * So we replicate that here. From proc_pid_stats(5)
+   * R = Running
+   * S = Sleeping on uninterrutible wait
+   * D = Waiting on uninterruptable disk sleep
+   * T = Tracing stop
+   * Z = Zombie
+   * W = Paging
+   */
+  lldb::StateType process_state = process_sp->GetState();
+  switch (process_state) {
+  case lldb::StateType::eStateSuspended:
+prpsinfo.pr_sname = 'S';
+prpsinfo.pr_state = 1;
+break;
+  case lldb::StateType::eStateStopped:
+[[fallthrough]];
+  case lldb::StateType::eStateStepping:
+prpsinfo.pr_sname = 'T';
+prpsinfo.pr_state = 3;
+break;
+  case lldb::StateType::eStateUnloaded:
+[[fallthrough]];
+  case lldb::StateType::eStateRunning:
+prpsinfo.pr_sname = 'R';
+prpsinfo.pr_state = 0;
+break;
+  default:
+break;
+  }
+
+  /**
+   * pr_flags is left as 0. The values (in linux) are specific
+   * to the kernel. We recover them from the proc filesystem
+   * but don't put them in ProcessInfo because it would really
+   * become very linux specific and the utility here seems pretty
+   * dubious
+   */
+
+  if (info.EffectiveUserIDIsValid()) {
+prpsinfo.pr_uid = info.GetUserID();
+  }
+  if (info.EffectiveGroupIDIsValid()) {
+prpsinfo.pr_gid = info.GetGroupID();
+  }
+
+  if (info.ParentProcessIDIsValid()) {
+prpsinfo.pr_ppid = info.GetParentProcessID();
+  }
+
+  if (info.ProcessGroupIDIsValid()) {
+prpsinfo.pr_pgrp = info.GetProcessGroupID();
+  }
+
+  if (info.ProcessSessionIDIsValid()) {
+prpsinfo.pr_sid = info.GetProcessSessionID();
+  }
+  constexpr size_t fname_len = std::extent_v;
+  static_assert(fname_len > 0, "This should always be non zero");
+  const llvm::StringRef fname = info.GetNameAsStringRef();
+  auto fname_begin = fname.begin();
+  std::copy(fname_begin,
+std::next(fname_begin, std::min(fname_len, fname.size())),
+prpsinfo.pr_fname);
+  prpsinfo.pr_fname[fname_len - 1] = '\0';
+  auto args = info.GetArguments();
+  if (!args.empty()) {

labath wrote:

The code does the right thing even without this check.

https://github.com/llvm/llvm-project/pull/104109
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Provide `declarationLocation` for variables (PR #102928)

[Adrian Vogelsgesang via lldb-commits]

vogelsgesang wrote:

No worries. I will have to rework this commit anyway.

In the meantime, a proposal was merged upstream and also already implemented in 
VS-Code. However, there were still larger changes which will require a complete 
rewrite of this commit

https://github.com/llvm/llvm-project/pull/102928
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

https://github.com/slydiman created 
https://github.com/llvm/llvm-project/pull/104238

Listen to gdbserver-port, accept the connection and run `lldb-server gdbserver 
--fd` on all platforms. Added acceptor_gdb and gdb_thread to lldb-platform.cpp 
SharedSocket has been moved to ConnectionFileDescriptorPosix.

Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now.

This is the part 2 of #101283.

Fixes #97537.

>From 46808aacc7f79cd5220651ff5976780e3aec3d3e Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Wed, 14 Aug 2024 22:36:52 +0400
Subject: [PATCH] [lldb] Removed gdbserver ports map from lldb-server

Listen to gdbserver-port, accept the connection and run `lldb-server gdbserver 
--fd` on all platforms.
Added acceptor_gdb and gdb_thread to lldb-platform.cpp
SharedSocket has been moved to ConnectionFileDescriptorPosix.

Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now.

This is the part 2 of #101283.

Fixes #97537.
---
 lldb/docs/man/lldb-server.rst |  11 +-
 lldb/docs/resources/qemu-testing.rst  |  19 +-
 .../posix/ConnectionFileDescriptorPosix.h |  26 ++
 .../posix/ConnectionFileDescriptorPosix.cpp   | 112 +-
 .../gdb-remote/GDBRemoteCommunication.cpp |  41 ++-
 .../gdb-remote/GDBRemoteCommunication.h   |   8 +-
 .../GDBRemoteCommunicationServerPlatform.cpp  | 290 ++--
 .../GDBRemoteCommunicationServerPlatform.h|  83 +
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |   2 +-
 .../Shell/lldb-server/TestGdbserverPort.test  |   4 -
 lldb/tools/lldb-server/Acceptor.cpp   |   6 +-
 lldb/tools/lldb-server/Acceptor.h |   3 +
 lldb/tools/lldb-server/lldb-gdbserver.cpp |  23 +-
 lldb/tools/lldb-server/lldb-platform.cpp  | 326 --
 .../Process/gdb-remote/CMakeLists.txt |   1 -
 .../Process/gdb-remote/PortMapTest.cpp| 115 --
 .../tools/lldb-server/tests/TestClient.h  |   4 +
 17 files changed, 462 insertions(+), 612 deletions(-)
 delete mode 100644 lldb/test/Shell/lldb-server/TestGdbserverPort.test
 delete mode 100644 lldb/unittests/Process/gdb-remote/PortMapTest.cpp

diff --git a/lldb/docs/man/lldb-server.rst b/lldb/docs/man/lldb-server.rst
index a67c00b305f6d2..31f5360df5e23e 100644
--- a/lldb/docs/man/lldb-server.rst
+++ b/lldb/docs/man/lldb-server.rst
@@ -147,15 +147,8 @@ GDB-SERVER CONNECTIONS
 
 .. option:: --gdbserver-port 
 
- Define a port to be used for gdb-server connections. Can be specified multiple
- times to allow multiple ports. Has no effect if --min-gdbserver-port
- and --max-gdbserver-port are specified.
-
-.. option:: --min-gdbserver-port 
-.. option:: --max-gdbserver-port 
-
- Specify the range of ports that can be used for gdb-server connections. Both
- options need to be specified simultaneously. Overrides --gdbserver-port.
+ Define a port to be used for gdb-server connections. This port is used for
+ multiple connections.
 
 .. option:: --port-offset 
 
diff --git a/lldb/docs/resources/qemu-testing.rst 
b/lldb/docs/resources/qemu-testing.rst
index 51a30b11717a87..e102f84a1d31f4 100644
--- a/lldb/docs/resources/qemu-testing.rst
+++ b/lldb/docs/resources/qemu-testing.rst
@@ -149,7 +149,6 @@ to the host (refer to QEMU's manuals for the specific 
options).
 * At least one to connect to the intial ``lldb-server``.
 * One more if you want to use ``lldb-server`` in ``platform mode``, and have it
   start a ``gdbserver`` instance for you.
-* A bunch more if you want to run tests against the ``lldb-server`` platform.
 
 If you are doing either of the latter 2 you should also restrict what ports
 ``lldb-server tries`` to use, otherwise it will randomly pick one that is 
almost
@@ -157,22 +156,14 @@ certainly not forwarded. An example of this is shown 
below.
 
 ::
 
-  $ lldb-server plaform --server --listen 0.0.0.0:54321 \
---min-gdbserver-port 49140 --max-gdbserver-port 49150
+  $ lldb-server plaform --server --listen 0.0.0.0:54321 --gdbserver-port 49140
 
 The result of this is that:
 
 * ``lldb-server`` platform mode listens externally on port ``54321``.
 
-* When it is asked to start a new gdbserver mode instance, it will use a port
-  in the range ``49140`` to ``49150``.
+* When it is asked to start a new gdbserver mode instance, it will use the port
+  ``49140``.
 
-Your VM configuration should have ports ``54321``, and ``49140`` to ``49150``
-forwarded for this to work.
-
-.. note::
-  These options are used to create a "port map" within ``lldb-server``.
-  Unfortunately this map is not cleaned up on Windows on connection close,
-  and across a few uses you may run out of valid ports. To work around this,
-  restart the platform every so often, especially after running a set of tests.
-  This is tracked here: https://github.com/llvm/llvm-project/issues/90923
+Your VM configuration should have ports ``54321`` and ``49140`` forwarded for
+this to work.
diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescr

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Dmitry Vasilyev (slydiman)


Changes

Listen to gdbserver-port, accept the connection and run `lldb-server gdbserver 
--fd` on all platforms. Added acceptor_gdb and gdb_thread to lldb-platform.cpp 
SharedSocket has been moved to ConnectionFileDescriptorPosix.

Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now.

This is the part 2 of #101283.

Fixes #97537.

---

Patch is 61.67 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/104238.diff


17 Files Affected:

- (modified) lldb/docs/man/lldb-server.rst (+2-9) 
- (modified) lldb/docs/resources/qemu-testing.rst (+5-14) 
- (modified) lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h (+26) 
- (modified) lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp (+109-3) 
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp 
(+24-17) 
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h 
(+6-2) 
- (modified) 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp 
(+97-193) 
- (modified) 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h 
(+13-70) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+1-1) 
- (removed) lldb/test/Shell/lldb-server/TestGdbserverPort.test (-4) 
- (modified) lldb/tools/lldb-server/Acceptor.cpp (+4-2) 
- (modified) lldb/tools/lldb-server/Acceptor.h (+3) 
- (modified) lldb/tools/lldb-server/lldb-gdbserver.cpp (+16-7) 
- (modified) lldb/tools/lldb-server/lldb-platform.cpp (+152-174) 
- (modified) lldb/unittests/Process/gdb-remote/CMakeLists.txt (-1) 
- (removed) lldb/unittests/Process/gdb-remote/PortMapTest.cpp (-115) 
- (modified) lldb/unittests/tools/lldb-server/tests/TestClient.h (+4) 


``diff
diff --git a/lldb/docs/man/lldb-server.rst b/lldb/docs/man/lldb-server.rst
index a67c00b305f6d2..31f5360df5e23e 100644
--- a/lldb/docs/man/lldb-server.rst
+++ b/lldb/docs/man/lldb-server.rst
@@ -147,15 +147,8 @@ GDB-SERVER CONNECTIONS
 
 .. option:: --gdbserver-port 
 
- Define a port to be used for gdb-server connections. Can be specified multiple
- times to allow multiple ports. Has no effect if --min-gdbserver-port
- and --max-gdbserver-port are specified.
-
-.. option:: --min-gdbserver-port 
-.. option:: --max-gdbserver-port 
-
- Specify the range of ports that can be used for gdb-server connections. Both
- options need to be specified simultaneously. Overrides --gdbserver-port.
+ Define a port to be used for gdb-server connections. This port is used for
+ multiple connections.
 
 .. option:: --port-offset 
 
diff --git a/lldb/docs/resources/qemu-testing.rst 
b/lldb/docs/resources/qemu-testing.rst
index 51a30b11717a87..e102f84a1d31f4 100644
--- a/lldb/docs/resources/qemu-testing.rst
+++ b/lldb/docs/resources/qemu-testing.rst
@@ -149,7 +149,6 @@ to the host (refer to QEMU's manuals for the specific 
options).
 * At least one to connect to the intial ``lldb-server``.
 * One more if you want to use ``lldb-server`` in ``platform mode``, and have it
   start a ``gdbserver`` instance for you.
-* A bunch more if you want to run tests against the ``lldb-server`` platform.
 
 If you are doing either of the latter 2 you should also restrict what ports
 ``lldb-server tries`` to use, otherwise it will randomly pick one that is 
almost
@@ -157,22 +156,14 @@ certainly not forwarded. An example of this is shown 
below.
 
 ::
 
-  $ lldb-server plaform --server --listen 0.0.0.0:54321 \
---min-gdbserver-port 49140 --max-gdbserver-port 49150
+  $ lldb-server plaform --server --listen 0.0.0.0:54321 --gdbserver-port 49140
 
 The result of this is that:
 
 * ``lldb-server`` platform mode listens externally on port ``54321``.
 
-* When it is asked to start a new gdbserver mode instance, it will use a port
-  in the range ``49140`` to ``49150``.
+* When it is asked to start a new gdbserver mode instance, it will use the port
+  ``49140``.
 
-Your VM configuration should have ports ``54321``, and ``49140`` to ``49150``
-forwarded for this to work.
-
-.. note::
-  These options are used to create a "port map" within ``lldb-server``.
-  Unfortunately this map is not cleaned up on Windows on connection close,
-  and across a few uses you may run out of valid ports. To work around this,
-  restart the platform every so often, especially after running a set of tests.
-  This is tracked here: https://github.com/llvm/llvm-project/issues/90923
+Your VM configuration should have ports ``54321`` and ``49140`` forwarded for
+this to work.
diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h 
b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index 35773d5907e913..08f486b92e0f07 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -26,6 +26,32 @@ class Status;
 class Socket;
 class SocketA

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[via lldb-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff d550ada5ab6cd6e49de71ac4c9aa27ced4c11de0 
46808aacc7f79cd5220651ff5976780e3aec3d3e --extensions cpp,h -- 
lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h 
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
lldb/tools/lldb-server/Acceptor.cpp lldb/tools/lldb-server/Acceptor.h 
lldb/tools/lldb-server/lldb-gdbserver.cpp 
lldb/tools/lldb-server/lldb-platform.cpp 
lldb/unittests/tools/lldb-server/tests/TestClient.h
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp 
b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
index fa7d2982d7..f90b01898a 100644
--- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -237,7 +237,7 @@ ConnectionFileDescriptor::Connect(llvm::StringRef path,
 
   if (!path.empty()) {
 auto method =
-llvm::StringSwitch(scheme)
 .Case("listen", &ConnectionFileDescriptor::AcceptTCP)
 .Cases("accept", "unix-accept",

``




https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

https://github.com/slydiman edited 
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

https://github.com/slydiman updated 
https://github.com/llvm/llvm-project/pull/104238

>From 5732bda3558a0d55f2f7b9d1a3722d11458a2360 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Wed, 14 Aug 2024 22:36:52 +0400
Subject: [PATCH] [lldb] Removed gdbserver ports map from lldb-server

Listen to gdbserver-port, accept the connection and run `lldb-server gdbserver 
--fd` on all platforms.
Added acceptor_gdb and gdb_thread to lldb-platform.cpp
SharedSocket has been moved to ConnectionFileDescriptorPosix.

Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now.

This is the part 2 of #101283.

Fixes #97537, fixes #101475.
---
 lldb/docs/man/lldb-server.rst |  11 +-
 lldb/docs/resources/qemu-testing.rst  |  19 +-
 .../posix/ConnectionFileDescriptorPosix.h |  26 ++
 .../posix/ConnectionFileDescriptorPosix.cpp   | 110 +-
 .../gdb-remote/GDBRemoteCommunication.cpp |  41 ++-
 .../gdb-remote/GDBRemoteCommunication.h   |   8 +-
 .../GDBRemoteCommunicationServerPlatform.cpp  | 290 ++--
 .../GDBRemoteCommunicationServerPlatform.h|  83 +
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |   2 +-
 .../Shell/lldb-server/TestGdbserverPort.test  |   4 -
 lldb/tools/lldb-server/Acceptor.cpp   |   6 +-
 lldb/tools/lldb-server/Acceptor.h |   3 +
 lldb/tools/lldb-server/lldb-gdbserver.cpp |  23 +-
 lldb/tools/lldb-server/lldb-platform.cpp  | 326 --
 .../Process/gdb-remote/CMakeLists.txt |   1 -
 .../Process/gdb-remote/PortMapTest.cpp| 115 --
 .../tools/lldb-server/tests/TestClient.h  |   4 +
 17 files changed, 461 insertions(+), 611 deletions(-)
 delete mode 100644 lldb/test/Shell/lldb-server/TestGdbserverPort.test
 delete mode 100644 lldb/unittests/Process/gdb-remote/PortMapTest.cpp

diff --git a/lldb/docs/man/lldb-server.rst b/lldb/docs/man/lldb-server.rst
index a67c00b305f6d2..31f5360df5e23e 100644
--- a/lldb/docs/man/lldb-server.rst
+++ b/lldb/docs/man/lldb-server.rst
@@ -147,15 +147,8 @@ GDB-SERVER CONNECTIONS
 
 .. option:: --gdbserver-port 
 
- Define a port to be used for gdb-server connections. Can be specified multiple
- times to allow multiple ports. Has no effect if --min-gdbserver-port
- and --max-gdbserver-port are specified.
-
-.. option:: --min-gdbserver-port 
-.. option:: --max-gdbserver-port 
-
- Specify the range of ports that can be used for gdb-server connections. Both
- options need to be specified simultaneously. Overrides --gdbserver-port.
+ Define a port to be used for gdb-server connections. This port is used for
+ multiple connections.
 
 .. option:: --port-offset 
 
diff --git a/lldb/docs/resources/qemu-testing.rst 
b/lldb/docs/resources/qemu-testing.rst
index 51a30b11717a87..e102f84a1d31f4 100644
--- a/lldb/docs/resources/qemu-testing.rst
+++ b/lldb/docs/resources/qemu-testing.rst
@@ -149,7 +149,6 @@ to the host (refer to QEMU's manuals for the specific 
options).
 * At least one to connect to the intial ``lldb-server``.
 * One more if you want to use ``lldb-server`` in ``platform mode``, and have it
   start a ``gdbserver`` instance for you.
-* A bunch more if you want to run tests against the ``lldb-server`` platform.
 
 If you are doing either of the latter 2 you should also restrict what ports
 ``lldb-server tries`` to use, otherwise it will randomly pick one that is 
almost
@@ -157,22 +156,14 @@ certainly not forwarded. An example of this is shown 
below.
 
 ::
 
-  $ lldb-server plaform --server --listen 0.0.0.0:54321 \
---min-gdbserver-port 49140 --max-gdbserver-port 49150
+  $ lldb-server plaform --server --listen 0.0.0.0:54321 --gdbserver-port 49140
 
 The result of this is that:
 
 * ``lldb-server`` platform mode listens externally on port ``54321``.
 
-* When it is asked to start a new gdbserver mode instance, it will use a port
-  in the range ``49140`` to ``49150``.
+* When it is asked to start a new gdbserver mode instance, it will use the port
+  ``49140``.
 
-Your VM configuration should have ports ``54321``, and ``49140`` to ``49150``
-forwarded for this to work.
-
-.. note::
-  These options are used to create a "port map" within ``lldb-server``.
-  Unfortunately this map is not cleaned up on Windows on connection close,
-  and across a few uses you may run out of valid ports. To work around this,
-  restart the platform every so often, especially after running a set of tests.
-  This is tracked here: https://github.com/llvm/llvm-project/issues/90923
+Your VM configuration should have ports ``54321`` and ``49140`` forwarded for
+this to work.
diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h 
b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
index 35773d5907e913..08f486b92e0f07 100644
--- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -26,6 +26,32 @@ class Status;
 class Socket;
 class SocketAddress;
 
+#ifde

[Lldb-commits] [lldb] [lldb-dap] Expose log path in extension settings (PR #103482)

[Walter Erquinigo via lldb-commits]

https://github.com/walter-erquinigo approved this pull request.

nice!

https://github.com/llvm/llvm-project/pull/103482
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Fred Grim via lldb-commits]

@@ -394,6 +420,114 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
   return error;
 }
 
+std::optional
+ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
+  ELFLinuxPrPsInfo prpsinfo{};
+  prpsinfo.pr_pid = process_sp->GetID();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prpsinfo.pr_nice = info.GetPriorityValue().value_or(0);
+  prpsinfo.pr_zomb = 0;
+  if (auto zombie_opt = info.IsZombie(); zombie_opt.value_or(false)) {
+prpsinfo.pr_zomb = 1;
+  }
+  /**
+   * In the linux kernel this comes from:
+   * state = READ_ONCE(p->__state);
+   * i = state ? ffz(~state) + 1 : 0;
+   * psinfo->pr_sname = (i > 5) ? '.' : "RSDTZW"[i];
+   *
+   * So we replicate that here. From proc_pid_stats(5)
+   * R = Running
+   * S = Sleeping on uninterrutible wait
+   * D = Waiting on uninterruptable disk sleep
+   * T = Tracing stop
+   * Z = Zombie
+   * W = Paging
+   */
+  lldb::StateType process_state = process_sp->GetState();
+  switch (process_state) {
+  case lldb::StateType::eStateSuspended:
+prpsinfo.pr_sname = 'S';
+prpsinfo.pr_state = 1;
+break;
+  case lldb::StateType::eStateStopped:
+[[fallthrough]];
+  case lldb::StateType::eStateStepping:
+prpsinfo.pr_sname = 'T';
+prpsinfo.pr_state = 3;
+break;
+  case lldb::StateType::eStateUnloaded:
+[[fallthrough]];
+  case lldb::StateType::eStateRunning:
+prpsinfo.pr_sname = 'R';
+prpsinfo.pr_state = 0;
+break;
+  default:
+break;
+  }
+
+  /**
+   * pr_flags is left as 0. The values (in linux) are specific
+   * to the kernel. We recover them from the proc filesystem
+   * but don't put them in ProcessInfo because it would really
+   * become very linux specific and the utility here seems pretty
+   * dubious
+   */
+
+  if (info.EffectiveUserIDIsValid()) {
+prpsinfo.pr_uid = info.GetUserID();
+  }
+  if (info.EffectiveGroupIDIsValid()) {
+prpsinfo.pr_gid = info.GetGroupID();
+  }
+
+  if (info.ParentProcessIDIsValid()) {
+prpsinfo.pr_ppid = info.GetParentProcessID();
+  }
+
+  if (info.ProcessGroupIDIsValid()) {
+prpsinfo.pr_pgrp = info.GetProcessGroupID();
+  }
+
+  if (info.ProcessSessionIDIsValid()) {
+prpsinfo.pr_sid = info.GetProcessSessionID();
+  }
+  constexpr size_t fname_len = std::extent_v;
+  static_assert(fname_len > 0, "This should always be non zero");
+  const llvm::StringRef fname = info.GetNameAsStringRef();
+  auto fname_begin = fname.begin();
+  std::copy(fname_begin,
+std::next(fname_begin, std::min(fname_len, fname.size())),
+prpsinfo.pr_fname);

feg208 wrote:

Yeah that's nicer

https://github.com/llvm/llvm-project/pull/104109
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Fred Grim via lldb-commits]

https://github.com/feg208 updated 
https://github.com/llvm/llvm-project/pull/104109

>From 8f2f84294ea3875c88ce36a4980fd3a3afce01de Mon Sep 17 00:00:00 2001
From: Fred Grim 
Date: Tue, 18 Jun 2024 10:38:09 -0700
Subject: [PATCH 1/2] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and
 ELFLinuxPrStatus

To create elf core files there are multiple notes in the core file that
contain these structs as the note. These populate methods take a Process
and produce fully specified structures that can be used to fill these
note sections. The pr also adds tests to ensure these structs are
correctly populated.
---
 .../Process/elf-core/ThreadElfCore.cpp| 134 +++
 .../Plugins/Process/elf-core/ThreadElfCore.h  |   7 +
 lldb/unittests/Process/CMakeLists.txt |   1 +
 .../unittests/Process/elf-core/CMakeLists.txt |  15 ++
 .../Process/elf-core/ThreadElfCoreTest.cpp| 162 ++
 5 files changed, 319 insertions(+)
 create mode 100644 lldb/unittests/Process/elf-core/CMakeLists.txt
 create mode 100644 lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp

diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index 2a83163884e168..fdb4a5837cd462 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -318,6 +318,32 @@ Status ELFLinuxPrStatus::Parse(const DataExtractor &data,
   return error;
 }
 
+static struct compat_timeval
+copy_timespecs(const ProcessInstanceInfo::timespec &oth) {
+  using sec_t = decltype(compat_timeval::tv_sec);
+  using usec_t = decltype(compat_timeval::tv_usec);
+  return {static_cast(oth.tv_sec), static_cast(oth.tv_usec)};
+}
+
+std::optional
+ELFLinuxPrStatus::Populate(const lldb::ThreadSP &thread_sp) {
+  ELFLinuxPrStatus prstatus{};
+  prstatus.pr_pid = thread_sp->GetID();
+  lldb::ProcessSP process_sp = thread_sp->GetProcess();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prstatus.pr_ppid = info.GetParentProcessID();
+  prstatus.pr_pgrp = info.GetProcessGroupID();
+  prstatus.pr_sid = info.GetProcessSessionID();
+  prstatus.pr_utime = copy_timespecs(info.GetUserTime());
+  prstatus.pr_stime = copy_timespecs(info.GetSystemTime());
+  prstatus.pr_cutime = copy_timespecs(info.GetCumulativeUserTime());
+  prstatus.pr_cstime = copy_timespecs(info.GetCumulativeSystemTime());
+  return prstatus;
+}
+
 // Parse PRPSINFO from NOTE entry
 ELFLinuxPrPsInfo::ELFLinuxPrPsInfo() {
   memset(this, 0, sizeof(ELFLinuxPrPsInfo));
@@ -394,6 +420,114 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
   return error;
 }
 
+std::optional
+ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
+  ELFLinuxPrPsInfo prpsinfo{};
+  prpsinfo.pr_pid = process_sp->GetID();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prpsinfo.pr_nice = info.GetPriorityValue().value_or(0);
+  prpsinfo.pr_zomb = 0;
+  if (auto zombie_opt = info.IsZombie(); zombie_opt.value_or(false)) {
+prpsinfo.pr_zomb = 1;
+  }
+  /**
+   * In the linux kernel this comes from:
+   * state = READ_ONCE(p->__state);
+   * i = state ? ffz(~state) + 1 : 0;
+   * psinfo->pr_sname = (i > 5) ? '.' : "RSDTZW"[i];
+   *
+   * So we replicate that here. From proc_pid_stats(5)
+   * R = Running
+   * S = Sleeping on uninterrutible wait
+   * D = Waiting on uninterruptable disk sleep
+   * T = Tracing stop
+   * Z = Zombie
+   * W = Paging
+   */
+  lldb::StateType process_state = process_sp->GetState();
+  switch (process_state) {
+  case lldb::StateType::eStateSuspended:
+prpsinfo.pr_sname = 'S';
+prpsinfo.pr_state = 1;
+break;
+  case lldb::StateType::eStateStopped:
+[[fallthrough]];
+  case lldb::StateType::eStateStepping:
+prpsinfo.pr_sname = 'T';
+prpsinfo.pr_state = 3;
+break;
+  case lldb::StateType::eStateUnloaded:
+[[fallthrough]];
+  case lldb::StateType::eStateRunning:
+prpsinfo.pr_sname = 'R';
+prpsinfo.pr_state = 0;
+break;
+  default:
+break;
+  }
+
+  /**
+   * pr_flags is left as 0. The values (in linux) are specific
+   * to the kernel. We recover them from the proc filesystem
+   * but don't put them in ProcessInfo because it would really
+   * become very linux specific and the utility here seems pretty
+   * dubious
+   */
+
+  if (info.EffectiveUserIDIsValid()) {
+prpsinfo.pr_uid = info.GetUserID();
+  }
+  if (info.EffectiveGroupIDIsValid()) {
+prpsinfo.pr_gid = info.GetGroupID();
+  }
+
+  if (info.ParentProcessIDIsValid()) {
+prpsinfo.pr_ppid = info.GetParentProcessID();
+  }
+
+  if (info.ProcessGroupIDIsValid()) {
+prpsinfo.pr_pgrp = info.GetProcessGroupID();
+  }
+
+  if (info.ProcessSessionIDIsValid()) {
+prpsinfo.pr_sid = info.GetProcessSessionID();
+  }
+  constexpr size_t fname_len = std::extent_v;
+  static_assert(fname_len > 0, "This shou

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Fred Grim via lldb-commits]

@@ -394,6 +420,114 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
   return error;
 }
 
+std::optional
+ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
+  ELFLinuxPrPsInfo prpsinfo{};
+  prpsinfo.pr_pid = process_sp->GetID();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prpsinfo.pr_nice = info.GetPriorityValue().value_or(0);
+  prpsinfo.pr_zomb = 0;
+  if (auto zombie_opt = info.IsZombie(); zombie_opt.value_or(false)) {
+prpsinfo.pr_zomb = 1;
+  }
+  /**
+   * In the linux kernel this comes from:
+   * state = READ_ONCE(p->__state);
+   * i = state ? ffz(~state) + 1 : 0;
+   * psinfo->pr_sname = (i > 5) ? '.' : "RSDTZW"[i];
+   *
+   * So we replicate that here. From proc_pid_stats(5)
+   * R = Running
+   * S = Sleeping on uninterrutible wait
+   * D = Waiting on uninterruptable disk sleep
+   * T = Tracing stop
+   * Z = Zombie
+   * W = Paging
+   */
+  lldb::StateType process_state = process_sp->GetState();
+  switch (process_state) {
+  case lldb::StateType::eStateSuspended:
+prpsinfo.pr_sname = 'S';
+prpsinfo.pr_state = 1;
+break;
+  case lldb::StateType::eStateStopped:
+[[fallthrough]];
+  case lldb::StateType::eStateStepping:
+prpsinfo.pr_sname = 'T';
+prpsinfo.pr_state = 3;
+break;
+  case lldb::StateType::eStateUnloaded:
+[[fallthrough]];
+  case lldb::StateType::eStateRunning:
+prpsinfo.pr_sname = 'R';
+prpsinfo.pr_state = 0;
+break;
+  default:
+break;
+  }
+
+  /**
+   * pr_flags is left as 0. The values (in linux) are specific
+   * to the kernel. We recover them from the proc filesystem
+   * but don't put them in ProcessInfo because it would really
+   * become very linux specific and the utility here seems pretty
+   * dubious
+   */
+
+  if (info.EffectiveUserIDIsValid()) {
+prpsinfo.pr_uid = info.GetUserID();
+  }
+  if (info.EffectiveGroupIDIsValid()) {
+prpsinfo.pr_gid = info.GetGroupID();
+  }
+
+  if (info.ParentProcessIDIsValid()) {
+prpsinfo.pr_ppid = info.GetParentProcessID();
+  }
+
+  if (info.ProcessGroupIDIsValid()) {
+prpsinfo.pr_pgrp = info.GetProcessGroupID();
+  }
+
+  if (info.ProcessSessionIDIsValid()) {
+prpsinfo.pr_sid = info.GetProcessSessionID();
+  }
+  constexpr size_t fname_len = std::extent_v;
+  static_assert(fname_len > 0, "This should always be non zero");
+  const llvm::StringRef fname = info.GetNameAsStringRef();
+  auto fname_begin = fname.begin();
+  std::copy(fname_begin,
+std::next(fname_begin, std::min(fname_len, fname.size())),
+prpsinfo.pr_fname);
+  prpsinfo.pr_fname[fname_len - 1] = '\0';
+  auto args = info.GetArguments();
+  if (!args.empty()) {
+constexpr size_t psargs_len = std::extent_v;
+static_assert(psargs_len > 0, "This should always be non zero");
+size_t i = psargs_len;
+auto argentry_iterator = std::begin(args);
+char *psargs = prpsinfo.pr_psargs;
+while (i > 0 && argentry_iterator != args.end()) {
+  llvm::StringRef argentry = argentry_iterator->ref();
+  size_t len = std::min(i, argentry.size());
+  auto arg_iterator = std::begin(argentry);
+  std::copy(arg_iterator, std::next(arg_iterator, len), psargs);
+  i -= len;
+  psargs += len;
+  if (i > 0) {
+*(psargs++) = ' ';
+--i;
+  }
+  ++argentry_iterator;
+}

feg208 wrote:

I do indeed like iterators. Adding to the universal stock of iterators seems 
like a positive step in any case. Done.

https://github.com/llvm/llvm-project/pull/104109
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Fred Grim via lldb-commits]

@@ -394,6 +420,114 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
   return error;
 }
 
+std::optional
+ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
+  ELFLinuxPrPsInfo prpsinfo{};
+  prpsinfo.pr_pid = process_sp->GetID();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+return std::nullopt;
+  }
+  prpsinfo.pr_nice = info.GetPriorityValue().value_or(0);
+  prpsinfo.pr_zomb = 0;
+  if (auto zombie_opt = info.IsZombie(); zombie_opt.value_or(false)) {
+prpsinfo.pr_zomb = 1;
+  }
+  /**
+   * In the linux kernel this comes from:
+   * state = READ_ONCE(p->__state);
+   * i = state ? ffz(~state) + 1 : 0;
+   * psinfo->pr_sname = (i > 5) ? '.' : "RSDTZW"[i];
+   *
+   * So we replicate that here. From proc_pid_stats(5)
+   * R = Running
+   * S = Sleeping on uninterrutible wait
+   * D = Waiting on uninterruptable disk sleep
+   * T = Tracing stop
+   * Z = Zombie
+   * W = Paging
+   */
+  lldb::StateType process_state = process_sp->GetState();
+  switch (process_state) {
+  case lldb::StateType::eStateSuspended:
+prpsinfo.pr_sname = 'S';
+prpsinfo.pr_state = 1;
+break;
+  case lldb::StateType::eStateStopped:
+[[fallthrough]];
+  case lldb::StateType::eStateStepping:
+prpsinfo.pr_sname = 'T';
+prpsinfo.pr_state = 3;
+break;
+  case lldb::StateType::eStateUnloaded:
+[[fallthrough]];
+  case lldb::StateType::eStateRunning:
+prpsinfo.pr_sname = 'R';
+prpsinfo.pr_state = 0;
+break;
+  default:
+break;
+  }
+
+  /**
+   * pr_flags is left as 0. The values (in linux) are specific
+   * to the kernel. We recover them from the proc filesystem
+   * but don't put them in ProcessInfo because it would really
+   * become very linux specific and the utility here seems pretty
+   * dubious
+   */
+
+  if (info.EffectiveUserIDIsValid()) {
+prpsinfo.pr_uid = info.GetUserID();
+  }
+  if (info.EffectiveGroupIDIsValid()) {
+prpsinfo.pr_gid = info.GetGroupID();
+  }
+
+  if (info.ParentProcessIDIsValid()) {
+prpsinfo.pr_ppid = info.GetParentProcessID();
+  }
+
+  if (info.ProcessGroupIDIsValid()) {
+prpsinfo.pr_pgrp = info.GetProcessGroupID();
+  }
+
+  if (info.ProcessSessionIDIsValid()) {
+prpsinfo.pr_sid = info.GetProcessSessionID();
+  }
+  constexpr size_t fname_len = std::extent_v;
+  static_assert(fname_len > 0, "This should always be non zero");
+  const llvm::StringRef fname = info.GetNameAsStringRef();
+  auto fname_begin = fname.begin();
+  std::copy(fname_begin,
+std::next(fname_begin, std::min(fname_len, fname.size())),
+prpsinfo.pr_fname);
+  prpsinfo.pr_fname[fname_len - 1] = '\0';
+  auto args = info.GetArguments();
+  if (!args.empty()) {

feg208 wrote:

removed

https://github.com/llvm/llvm-project/pull/104109
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/102708

>From c0a7286b0107d3161b18de8f05d4d016150e96a5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Thu, 8 Aug 2024 08:58:52 -0700
Subject: [PATCH 1/8] Initial attempt at new classes Summary statistics in
 SummaryStatistics.h/cpp.

---
 lldb/include/lldb/Target/SummaryStatistics.h | 37 
 lldb/include/lldb/Target/Target.h|  5 +++
 lldb/source/Core/ValueObject.cpp |  5 +++
 lldb/source/Target/CMakeLists.txt|  1 +
 lldb/source/Target/SummaryStatistics.cpp | 26 ++
 lldb/source/Target/Target.cpp|  9 +
 6 files changed, 83 insertions(+)
 create mode 100644 lldb/include/lldb/Target/SummaryStatistics.h
 create mode 100644 lldb/source/Target/SummaryStatistics.cpp

diff --git a/lldb/include/lldb/Target/SummaryStatistics.h 
b/lldb/include/lldb/Target/SummaryStatistics.h
new file mode 100644
index 00..0198249ba0b170
--- /dev/null
+++ b/lldb/include/lldb/Target/SummaryStatistics.h
@@ -0,0 +1,37 @@
+//===-- SummaryStatistics.h -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_TARGET_SUMMARYSTATISTICS_H
+#define LLDB_TARGET_SUMMARYSTATISTICS_H
+
+
+#include "lldb/Target/Statistics.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private {
+
+class SummaryStatistics {
+public:
+  SummaryStatistics(lldb_private::ConstString name) : 
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+  lldb_private::StatsDuration &GetDurationReference();
+
+  lldb_private::ConstString GetName() const;
+
+  uint64_t GetSummaryCount() const;
+
+private:
+   lldb_private::StatsDuration m_total_time;
+   lldb_private::ConstString m_name;
+   uint64_t m_summary_count;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_SUMMARYSTATISTICS_H
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 119dff4d498199..ee6a009b0af95d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -30,6 +30,7 @@
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Target/SectionLoadHistory.h"
 #include "lldb/Target/Statistics.h"
+#include "lldb/Target/SummaryStatistics.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
@@ -258,6 +259,7 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+
 private:
   std::optional
   GetExperimentalPropertyValue(size_t prop_idx,
@@ -1221,6 +1223,8 @@ class Target : public 
std::enable_shared_from_this,
 
   void ClearAllLoadedSections();
 
+  lldb_private::StatsDuration& 
GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name);
+
   /// Set the \a Trace object containing processor trace information of this
   /// target.
   ///
@@ -1554,6 +1558,7 @@ class Target : public 
std::enable_shared_from_this,
   std::string m_label;
   ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
+  std::map 
m_summary_stats_map;
   SectionLoadHistory m_section_load_history;
   BreakpointList m_breakpoint_list;
   BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4f..bed4ab8d69cbda 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,6 +615,11 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl 
*summary_ptr,
   m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
 // the synthetic children being
 // up-to-date (e.g. ${svar%#})
+StatsDuration &summary_duration = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
+
.GetSummaryProviderDuration(GetTypeName());
+ElapsedTime elapsed(summary_duration);
 summary_ptr->FormatObject(this, destination, actual_options);
   }
   m_flags.m_is_getting_summary = false;
diff --git a/lldb/source/Target/CMakeLists.txt 
b/lldb/source/Target/CMakeLists.txt
index a42c44b761dc56..e51da37cd84db3 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -46,6 +46,7 @@ add_lldb_library(lldbTarget
   Statistics.cpp
   StopInfo.cpp
   StructuredDataPlugin.cpp
+  SummaryStatistics.cpp
   SystemRuntime.cpp
   Target.cpp
   TargetList.cpp
diff --git a/lldb/source/Target/SummaryStatistics.cpp 
b/lldb/sour

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

@@ -25,6 +26,7 @@ namespace lldb_private {
 
 using StatsClock = std::chrono::high_resolution_clock;
 using StatsTimepoint = std::chrono::time_point;
+using Duration = std::chrono::duration;

Jlalond wrote:

Originally I had this passed down to the invocation as well, with the intent of 
the duration being checked to see if a warn should be emitted to the user. 

Decided against that as this has already gotten somewhat complicated. I intend 
to use Duration in subsequent PR's so I left it moved out of StatsDuration, but 
if we want to move it back into it's original scope I'm fine with that.

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Jacob Lalonde via lldb-commits]

https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/102708

>From c0a7286b0107d3161b18de8f05d4d016150e96a5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Thu, 8 Aug 2024 08:58:52 -0700
Subject: [PATCH 1/9] Initial attempt at new classes Summary statistics in
 SummaryStatistics.h/cpp.

---
 lldb/include/lldb/Target/SummaryStatistics.h | 37 
 lldb/include/lldb/Target/Target.h|  5 +++
 lldb/source/Core/ValueObject.cpp |  5 +++
 lldb/source/Target/CMakeLists.txt|  1 +
 lldb/source/Target/SummaryStatistics.cpp | 26 ++
 lldb/source/Target/Target.cpp|  9 +
 6 files changed, 83 insertions(+)
 create mode 100644 lldb/include/lldb/Target/SummaryStatistics.h
 create mode 100644 lldb/source/Target/SummaryStatistics.cpp

diff --git a/lldb/include/lldb/Target/SummaryStatistics.h 
b/lldb/include/lldb/Target/SummaryStatistics.h
new file mode 100644
index 00..0198249ba0b170
--- /dev/null
+++ b/lldb/include/lldb/Target/SummaryStatistics.h
@@ -0,0 +1,37 @@
+//===-- SummaryStatistics.h -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_TARGET_SUMMARYSTATISTICS_H
+#define LLDB_TARGET_SUMMARYSTATISTICS_H
+
+
+#include "lldb/Target/Statistics.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private {
+
+class SummaryStatistics {
+public:
+  SummaryStatistics(lldb_private::ConstString name) : 
+m_total_time(), m_name(name), m_summary_count(0) {}
+
+  lldb_private::StatsDuration &GetDurationReference();
+
+  lldb_private::ConstString GetName() const;
+
+  uint64_t GetSummaryCount() const;
+
+private:
+   lldb_private::StatsDuration m_total_time;
+   lldb_private::ConstString m_name;
+   uint64_t m_summary_count;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_SUMMARYSTATISTICS_H
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 119dff4d498199..ee6a009b0af95d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -30,6 +30,7 @@
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Target/SectionLoadHistory.h"
 #include "lldb/Target/Statistics.h"
+#include "lldb/Target/SummaryStatistics.h"
 #include "lldb/Target/ThreadSpec.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
@@ -258,6 +259,7 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+
 private:
   std::optional
   GetExperimentalPropertyValue(size_t prop_idx,
@@ -1221,6 +1223,8 @@ class Target : public 
std::enable_shared_from_this,
 
   void ClearAllLoadedSections();
 
+  lldb_private::StatsDuration& 
GetSummaryProviderDuration(lldb_private::ConstString summary_provider_name);
+
   /// Set the \a Trace object containing processor trace information of this
   /// target.
   ///
@@ -1554,6 +1558,7 @@ class Target : public 
std::enable_shared_from_this,
   std::string m_label;
   ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
+  std::map 
m_summary_stats_map;
   SectionLoadHistory m_section_load_history;
   BreakpointList m_breakpoint_list;
   BreakpointList m_internal_breakpoint_list;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 8f72efc2299b4f..bed4ab8d69cbda 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -615,6 +615,11 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl 
*summary_ptr,
   m_synthetic_value->UpdateValueIfNeeded(); // the summary might depend on
 // the synthetic children being
 // up-to-date (e.g. ${svar%#})
+StatsDuration &summary_duration = GetExecutionContextRef()
+.GetProcessSP()
+->GetTarget()
+
.GetSummaryProviderDuration(GetTypeName());
+ElapsedTime elapsed(summary_duration);
 summary_ptr->FormatObject(this, destination, actual_options);
   }
   m_flags.m_is_getting_summary = false;
diff --git a/lldb/source/Target/CMakeLists.txt 
b/lldb/source/Target/CMakeLists.txt
index a42c44b761dc56..e51da37cd84db3 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -46,6 +46,7 @@ add_lldb_library(lldbTarget
   Statistics.cpp
   StopInfo.cpp
   StructuredDataPlugin.cpp
+  SummaryStatistics.cpp
   SystemRuntime.cpp
   Target.cpp
   TargetList.cpp
diff --git a/lldb/source/Target/SummaryStatistics.cpp 
b/lldb/sour

[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

[Fred Grim via lldb-commits]

@@ -0,0 +1,162 @@
+//===-- ThreadElfCoreTest.cpp *- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "Plugins/Process/elf-core/ThreadElfCore.h"
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Utility/Listener.h"
+#include "llvm/TargetParser/Triple.h"
+#include "gtest/gtest.h"
+
+#include 
+#include 
+#include 
+#include 
+
+using namespace lldb_private;
+
+namespace {
+
+struct ElfCoreTest : public testing::Test {
+  static void SetUpTestCase() {
+FileSystem::Initialize();
+HostInfo::Initialize();
+platform_linux::PlatformLinux::Initialize();
+std::call_once(TestUtilities::g_debugger_initialize_flag,
+   []() { Debugger::Initialize(nullptr); });
+  }
+  static void TearDownTestCase() {
+platform_linux::PlatformLinux::Terminate();
+HostInfo::Terminate();
+FileSystem::Terminate();
+  }
+};
+
+struct DummyProcess : public Process {
+  DummyProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp)
+  : Process(target_sp, listener_sp) {
+SetID(getpid());
+  }
+
+  bool CanDebug(lldb::TargetSP target, bool plugin_specified_by_name) override 
{
+return true;
+  }
+  Status DoDestroy() override { return {}; }
+  void RefreshStateAfterStop() override {}
+  size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
+  Status &error) override {
+return 0;
+  }
+  bool DoUpdateThreadList(ThreadList &old_thread_list,
+  ThreadList &new_thread_list) override {
+return false;
+  }
+  llvm::StringRef GetPluginName() override { return "Dummy"; }
+};
+
+struct DummyThread : public Thread {
+  using Thread::Thread;
+
+  ~DummyThread() override { DestroyThread(); }
+
+  void RefreshStateAfterStop() override {}
+
+  lldb::RegisterContextSP GetRegisterContext() override { return nullptr; }
+
+  lldb::RegisterContextSP
+  CreateRegisterContextForFrame(StackFrame *frame) override {
+return nullptr;
+  }
+
+  bool CalculateStopInfo() override { return false; }
+};
+
+lldb::TargetSP CreateTarget(lldb::DebuggerSP &debugger_sp, ArchSpec &arch) {
+  lldb::PlatformSP platform_sp;
+  lldb::TargetSP target_sp;
+  debugger_sp->GetTargetList().CreateTarget(
+  *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp);
+  return target_sp;
+}
+
+lldb::ThreadSP CreateThread(lldb::ProcessSP &process_sp) {
+  lldb::ThreadSP thread_sp =
+  std::make_shared(*process_sp.get(), gettid());
+  if (thread_sp == nullptr) {
+return nullptr;
+  }
+  process_sp->GetThreadList().AddThread(thread_sp);
+
+  return thread_sp;
+}
+
+} // namespace
+
+TEST_F(ElfCoreTest, PopulatePrpsInfoTest) {
+  ArchSpec arch{HostInfo::GetTargetTriple()};
+  lldb::DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  lldb::TargetSP target_sp = CreateTarget(debugger_sp, arch);
+  ASSERT_TRUE(target_sp);
+
+  lldb::ListenerSP listener_sp(Listener::MakeListener("dummy"));
+  lldb::ProcessSP process_sp =
+  std::make_shared(target_sp, listener_sp);
+  ASSERT_TRUE(process_sp);
+  auto prpsinfo_opt = ELFLinuxPrPsInfo::Populate(process_sp);

feg208 wrote:

Yeah this is a good idea. I'll alter the interface and add more testing

https://github.com/llvm/llvm-project/pull/104109
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

@@ -25,6 +26,7 @@ namespace lldb_private {
 
 using StatsClock = std::chrono::high_resolution_clock;
 using StatsTimepoint = std::chrono::time_point;
+using Duration = std::chrono::duration;

Michael137 wrote:

If it's unused in this PR, I'd rather remove it here. And add it when you have 
the need for it

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

@@ -174,6 +177,86 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(std::string name, std::string impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_summary_count(0) {}
+
+  std::string GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; };
+
+  std::string GetSummaryKindName() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+  void IncrementSummaryCount() {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+  }
+
+private:
+  lldb_private::StatsDuration m_total_time;
+  std::string m_impl_type;
+  std::string m_name;
+  std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+  /// Basic RAII class to increment the summary count when the call is 
complete.
+  /// In the future this can be extended to collect information about the
+  /// elapsed time for a single request.

Michael137 wrote:

>   /// In the future this can be extended to collect information about the
   /// elapsed time for a single request

Isn't that what the current PR is already trying to achieve?

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

@@ -174,6 +177,86 @@ struct StatisticsOptions {
   std::optional m_include_transcript;
 };
 
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+  explicit SummaryStatistics(std::string name, std::string impl_type)
+  : m_total_time(), m_impl_type(impl_type), m_name(name),
+m_summary_count(0) {}
+
+  std::string GetName() const { return m_name; };
+  double GetTotalTime() const { return m_total_time.get().count(); }
+
+  uint64_t GetSummaryCount() const {
+return m_summary_count.load(std::memory_order_relaxed);
+  }
+
+  StatsDuration &GetDurationReference() { return m_total_time; };
+
+  std::string GetSummaryKindName() const { return m_impl_type; }
+
+  llvm::json::Value ToJSON() const;
+
+  void IncrementSummaryCount() {
+m_summary_count.fetch_add(1, std::memory_order_relaxed);
+  }
+
+private:
+  lldb_private::StatsDuration m_total_time;
+  std::string m_impl_type;
+  std::string m_name;
+  std::atomic m_summary_count;
+};
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+  /// Basic RAII class to increment the summary count when the call is 
complete.
+  /// In the future this can be extended to collect information about the
+  /// elapsed time for a single request.
+  class SummaryInvocation {
+  public:
+SummaryInvocation(SummaryStatistics &summary, SummaryStatisticsCache 
&cache)
+: m_provider_key(summary.GetName()), m_cache(cache),
+  m_elapsed_time(summary.GetDurationReference()) {}
+~SummaryInvocation() { m_cache.OnInvoked(m_provider_key); }
+
+/// Delete the copy constructor and assignment operator to prevent
+/// accidental double counting.
+/// @{
+SummaryInvocation(const SummaryInvocation &) = delete;
+SummaryInvocation &operator=(const SummaryInvocation &) = delete;
+/// @}
+
+  private:
+std::string m_provider_key;
+SummaryStatisticsCache &m_cache;
+ElapsedTime m_elapsed_time;
+  };
+
+  /// Get the SummaryStatistics object for a given provider name, or insert
+  /// if statistics for that provider is not in the map.
+  SummaryStatisticsCache::SummaryInvocation
+  GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) 
{
+std::lock_guard guard(m_map_mutex);
+auto pair = m_summary_stats_map.try_emplace(
+provider.GetName(), provider.GetName(), provider.GetSummaryKindName());
+
+return SummaryInvocation(pair.first->second, *this);
+  }
+
+  llvm::json::Value ToJSON();
+
+private:
+  /// Called when Summary Invocation is destructed.
+  void OnInvoked(std::string provider_name) noexcept {
+// .at give us a const reference, and [provider_name] = will give us a copy
+m_summary_stats_map.find(provider_name)->second.IncrementSummaryCount();

Michael137 wrote:

This needs to happen under a lock since `GetSummaryStatisticsForProviderName` 
mutates the map, while this reads from it. Also, I see you're not updating the 
elapsed time. Is that intentional?

Also, can we assert that `find` isn't `m_summary_stats_map.end()`?

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)

[Michael Buch via lldb-commits]

@@ -0,0 +1,35 @@
+// Test that the lldb command `statistics` works.

Michael137 wrote:

All this lock-talk makes me think we should have a test that exercises 
accessing the summary stats map from multiple threads. That way TSAN will help 
us in the test-suite.

https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)

[Jason Molenda via lldb-commits]

jasonmolenda wrote:


> > To (preemtively) fix that, you should probably change lldb to recognize the 
> > gdb response (`swbreak`) and treat it the same way as the one from 
> > lldb-server. Ideally, you should also add a test to make sure this works 
> > (and stays working) -- for that, you could probably extend the test you 
> > added here to check that lldb reports the correct stop reason.
> 
> Oh I see your point now, and I also see the lldb is handling a `reason:` 
> packet from the lldb-server. In this sense, I think it is fair to treat 
> "swbreak/hwbreak" in the same way as "reason:breakpoint".
> 
> @jasonmolenda do you think you can handle this along with your patch, or you 
> think I should do something for it preemptively as suggested by 
> [labath](https://github.com/labath)? I personally prefer the former case 
> because I am quite new to the lldb code base

No problem, I'll make that change in ProcessGDBRemote when I re-land my patch 
(I have a few outstanding issues the CI bots found when I first landed it).  My 
change will be untested, but it is a simple change.  We already fake up a 
reason:watchpoint stop reason when we get `watch`/`rwatch`/`awatch` in a stop 
reply packet today, it will be the same kind of thing.

https://github.com/llvm/llvm-project/pull/102873
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)

[Adrian Vogelsgesang via lldb-commits]

https://github.com/vogelsgesang created 
https://github.com/llvm/llvm-project/pull/104317

Adds support for the `readMemory` request which allows VS-Code to inspect 
memory. Also, add `memoryReference` to variablesa and `evaluate` responses, 
such that the binary view can be opened from the variables view and from the 
"watch" pane.

>From e01ea18961bbae0fb747b312670946bd768c5d73 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang 
Date: Wed, 14 Aug 2024 11:52:40 +
Subject: [PATCH] [lldb-dap] Support inspecting memory

Adds support for the `readMemory` request which allows VS-Code to
inspect memory. Also, add `memoryReference` to variablesa and `evaluate`
responses, such that the binary view can be opened from the variables
view and from the "watch" pane.
---
 .../test/tools/lldb-dap/dap_server.py |  15 ++
 lldb/test/API/tools/lldb-dap/memory/Makefile  |   3 +
 .../tools/lldb-dap/memory/TestDAP_memory.py   | 121 
 lldb/test/API/tools/lldb-dap/memory/main.cpp  |  10 +
 lldb/tools/lldb-dap/JSONUtils.cpp |  30 ++-
 lldb/tools/lldb-dap/JSONUtils.h   |   6 +
 lldb/tools/lldb-dap/lldb-dap.cpp  | 172 +-
 7 files changed, 354 insertions(+), 3 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/memory/Makefile
 create mode 100644 lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
 create mode 100644 lldb/test/API/tools/lldb-dap/memory/main.cpp

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index a324af57b61df3..35d792351e6bfc 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -691,6 +691,21 @@ def request_disassemble(
 for inst in instructions:
 self.disassembled_instructions[inst["address"]] = inst
 
+def request_read_memory(
+self, memoryReference, offset, count
+):
+args_dict = {
+"memoryReference": memoryReference,
+"offset": offset,
+"count": count,
+}
+command_dict = {
+"command": "readMemory",
+"type": "request",
+"arguments": args_dict,
+}
+return self.send_recv(command_dict)
+
 def request_evaluate(self, expression, frameIndex=0, threadId=None, 
context=None):
 stackFrame = self.get_stackFrame(frameIndex=frameIndex, 
threadId=threadId)
 if stackFrame is None:
diff --git a/lldb/test/API/tools/lldb-dap/memory/Makefile 
b/lldb/test/API/tools/lldb-dap/memory/Makefile
new file mode 100644
index 00..8b20bcb050
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/memory/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py 
b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
new file mode 100644
index 00..9ff4dbd3138428
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
@@ -0,0 +1,121 @@
+"""
+Test lldb-dap memory support
+"""
+
+
+from base64 import b64decode
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbdap_testcase
+import os
+
+
+class TestDAP_memory(lldbdap_testcase.DAPTestCaseBase):
+def test_read_memory(self):
+"""
+Tests the 'read_memory' request
+"""
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = "main.cpp"
+self.source_path = os.path.join(os.getcwd(), source)
+self.set_source_breakpoints(
+source,
+[
+line_number(source, "// Breakpoint"),
+],
+)
+self.continue_to_next_stop()
+
+locals = {l['name']: l for l in self.dap_server.get_local_variables()}
+rawptr_ref = locals['rawptr']['memoryReference']
+
+# We can read the complete string
+mem = self.dap_server.request_read_memory(rawptr_ref, 0, 5)["body"]
+self.assertEqual(mem["unreadableBytes"], 0);
+self.assertEqual(b64decode(mem["data"]), b"dead\0")
+
+# Use an offset
+mem = self.dap_server.request_read_memory(rawptr_ref, 2, 3)["body"]
+self.assertEqual(b64decode(mem["data"]), b"ad\0")
+
+# Use a negative offset
+mem = self.dap_server.request_read_memory(rawptr_ref, -1, 6)["body"]
+self.assertEqual(b64decode(mem["data"])[1:], b"dead\0")
+
+# Reads of size 0 are successful
+# VS-Code sends those in order to check if a `memoryReference` can 
actually be dereferenced.
+mem = self.dap_server.request_read_memory(rawptr_ref, 0, 0)
+self.assertEqual(mem["success"], True)
+self.assertEqual(mem["body"]["data"], "")
+
+# Reads at offset 0x0 fail
+mem = self.da

[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Adrian Vogelsgesang (vogelsgesang)


Changes

Adds support for the `readMemory` request which allows VS-Code to inspect 
memory. Also, add `memoryReference` to variablesa and `evaluate` responses, 
such that the binary view can be opened from the variables view and from the 
"watch" pane.

---
Full diff: https://github.com/llvm/llvm-project/pull/104317.diff


7 Files Affected:

- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
(+15) 
- (added) lldb/test/API/tools/lldb-dap/memory/Makefile (+3) 
- (added) lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py (+121) 
- (added) lldb/test/API/tools/lldb-dap/memory/main.cpp (+10) 
- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+28-2) 
- (modified) lldb/tools/lldb-dap/JSONUtils.h (+6) 
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+171-1) 


``diff
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index a324af57b61df3..35d792351e6bfc 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -691,6 +691,21 @@ def request_disassemble(
 for inst in instructions:
 self.disassembled_instructions[inst["address"]] = inst
 
+def request_read_memory(
+self, memoryReference, offset, count
+):
+args_dict = {
+"memoryReference": memoryReference,
+"offset": offset,
+"count": count,
+}
+command_dict = {
+"command": "readMemory",
+"type": "request",
+"arguments": args_dict,
+}
+return self.send_recv(command_dict)
+
 def request_evaluate(self, expression, frameIndex=0, threadId=None, 
context=None):
 stackFrame = self.get_stackFrame(frameIndex=frameIndex, 
threadId=threadId)
 if stackFrame is None:
diff --git a/lldb/test/API/tools/lldb-dap/memory/Makefile 
b/lldb/test/API/tools/lldb-dap/memory/Makefile
new file mode 100644
index 00..8b20bcb050
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/memory/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py 
b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
new file mode 100644
index 00..9ff4dbd3138428
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
@@ -0,0 +1,121 @@
+"""
+Test lldb-dap memory support
+"""
+
+
+from base64 import b64decode
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbdap_testcase
+import os
+
+
+class TestDAP_memory(lldbdap_testcase.DAPTestCaseBase):
+def test_read_memory(self):
+"""
+Tests the 'read_memory' request
+"""
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = "main.cpp"
+self.source_path = os.path.join(os.getcwd(), source)
+self.set_source_breakpoints(
+source,
+[
+line_number(source, "// Breakpoint"),
+],
+)
+self.continue_to_next_stop()
+
+locals = {l['name']: l for l in self.dap_server.get_local_variables()}
+rawptr_ref = locals['rawptr']['memoryReference']
+
+# We can read the complete string
+mem = self.dap_server.request_read_memory(rawptr_ref, 0, 5)["body"]
+self.assertEqual(mem["unreadableBytes"], 0);
+self.assertEqual(b64decode(mem["data"]), b"dead\0")
+
+# Use an offset
+mem = self.dap_server.request_read_memory(rawptr_ref, 2, 3)["body"]
+self.assertEqual(b64decode(mem["data"]), b"ad\0")
+
+# Use a negative offset
+mem = self.dap_server.request_read_memory(rawptr_ref, -1, 6)["body"]
+self.assertEqual(b64decode(mem["data"])[1:], b"dead\0")
+
+# Reads of size 0 are successful
+# VS-Code sends those in order to check if a `memoryReference` can 
actually be dereferenced.
+mem = self.dap_server.request_read_memory(rawptr_ref, 0, 0)
+self.assertEqual(mem["success"], True)
+self.assertEqual(mem["body"]["data"], "")
+
+# Reads at offset 0x0 fail
+mem = self.dap_server.request_read_memory("0x0", 0, 6)
+self.assertEqual(mem["success"], False)
+self.assertTrue(mem["message"].startswith("Unable to read memory: "))
+
+
+def test_memory_refs_variables(self):
+"""
+Tests memory references for evaluate
+"""
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = "main.cpp"
+self.source_path = os.path.join(os.getcwd(), source)
+self.set_source_breakpoints(
+source, [line_number(source, 

[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)

[via lldb-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 82ee31f75ac1316006fa9e21dddfddec37cf7072 
e01ea18961bbae0fb747b312670946bd768c5d73 --extensions cpp,h -- 
lldb/test/API/tools/lldb-dap/memory/main.cpp lldb/tools/lldb-dap/JSONUtils.cpp 
lldb/tools/lldb-dap/JSONUtils.h lldb/tools/lldb-dap/lldb-dap.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/test/API/tools/lldb-dap/memory/main.cpp 
b/lldb/test/API/tools/lldb-dap/memory/main.cpp
index 95ecde82ea..807b010d24 100644
--- a/lldb/test/API/tools/lldb-dap/memory/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/memory/main.cpp
@@ -1,10 +1,10 @@
-#include 
 #include 
+#include 
 
 int main() {
-int not_a_ptr = 666;
-const char* rawptr = "dead";
-std::unique_ptr smartptr(new int(42));
-// Breakpoint
-return 0;
+  int not_a_ptr = 666;
+  const char *rawptr = "dead";
+  std::unique_ptr smartptr(new int(42));
+  // Breakpoint
+  return 0;
 }
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp 
b/lldb/tools/lldb-dap/JSONUtils.cpp
index dc8932d5f3..b340c4e4a2 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1274,7 +1274,6 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t 
variablesReference,
   else
 object.try_emplace("variablesReference", (int64_t)0);
 
-  
   if (lldb::addr_t addr = GetMemoryReference(v); addr != LLDB_INVALID_ADDRESS) 
{
 object.try_emplace("memoryReference", "0x" + llvm::utohexstr(addr));
   }

``




https://github.com/llvm/llvm-project/pull/104317
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)

[via lldb-commits]

github-actions[bot] wrote:




:warning: Python code formatter, darker found issues in your code. :warning:



You can test this locally with the following command:


``bash
darker --check --diff -r 
82ee31f75ac1316006fa9e21dddfddec37cf7072...e01ea18961bbae0fb747b312670946bd768c5d73
 lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py 
lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
``





View the diff from darker here.


``diff
--- packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 2024-08-14 
22:31:50.00 +
+++ packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 2024-08-14 
22:37:06.169257 +
@@ -689,13 +689,11 @@
 }
 instructions = self.send_recv(command_dict)["body"]["instructions"]
 for inst in instructions:
 self.disassembled_instructions[inst["address"]] = inst
 
-def request_read_memory(
-self, memoryReference, offset, count
-):
+def request_read_memory(self, memoryReference, offset, count):
 args_dict = {
 "memoryReference": memoryReference,
 "offset": offset,
 "count": count,
 }
--- test/API/tools/lldb-dap/memory/TestDAP_memory.py2024-08-14 
22:31:50.00 +
+++ test/API/tools/lldb-dap/memory/TestDAP_memory.py2024-08-14 
22:37:06.268921 +
@@ -27,16 +27,16 @@
 line_number(source, "// Breakpoint"),
 ],
 )
 self.continue_to_next_stop()
 
-locals = {l['name']: l for l in self.dap_server.get_local_variables()}
-rawptr_ref = locals['rawptr']['memoryReference']
+locals = {l["name"]: l for l in self.dap_server.get_local_variables()}
+rawptr_ref = locals["rawptr"]["memoryReference"]
 
 # We can read the complete string
 mem = self.dap_server.request_read_memory(rawptr_ref, 0, 5)["body"]
-self.assertEqual(mem["unreadableBytes"], 0);
+self.assertEqual(mem["unreadableBytes"], 0)
 self.assertEqual(b64decode(mem["data"]), b"dead\0")
 
 # Use an offset
 mem = self.dap_server.request_read_memory(rawptr_ref, 2, 3)["body"]
 self.assertEqual(b64decode(mem["data"]), b"ad\0")
@@ -54,68 +54,84 @@
 # Reads at offset 0x0 fail
 mem = self.dap_server.request_read_memory("0x0", 0, 6)
 self.assertEqual(mem["success"], False)
 self.assertTrue(mem["message"].startswith("Unable to read memory: "))
 
-
 def test_memory_refs_variables(self):
 """
 Tests memory references for evaluate
 """
 program = self.getBuildArtifact("a.out")
 self.build_and_launch(program)
 source = "main.cpp"
 self.source_path = os.path.join(os.getcwd(), source)
 self.set_source_breakpoints(
-source, [line_number(source, "// Breakpoint")],
+source,
+[line_number(source, "// Breakpoint")],
 )
 self.continue_to_next_stop()
 
-locals = {l['name']: l for l in self.dap_server.get_local_variables()}
+locals = {l["name"]: l for l in self.dap_server.get_local_variables()}
 
 # Pointers should have memory-references
-self.assertIn("memoryReference", locals['rawptr'].keys())
+self.assertIn("memoryReference", locals["rawptr"].keys())
 # Smart-pointers also have memory-references
-self.assertIn("memoryReference", 
self.dap_server.get_local_variable_child("smartptr", "pointer").keys())
+self.assertIn(
+"memoryReference",
+self.dap_server.get_local_variable_child("smartptr", 
"pointer").keys(),
+)
 # Non-pointers should not have memory-references
-self.assertNotIn("memoryReference", locals['not_a_ptr'].keys())
-
+self.assertNotIn("memoryReference", locals["not_a_ptr"].keys())
 
 def test_memory_refs_evaluate(self):
 """
 Tests memory references for evaluate
 """
 program = self.getBuildArtifact("a.out")
 self.build_and_launch(program)
 source = "main.cpp"
 self.source_path = os.path.join(os.getcwd(), source)
 self.set_source_breakpoints(
-source, [line_number(source, "// Breakpoint")],
+source,
+[line_number(source, "// Breakpoint")],
 )
 self.continue_to_next_stop()
 
 # Pointers contain memory references
-self.assertIn("memoryReference", 
self.dap_server.request_evaluate("rawptr + 1")["body"].keys())
+self.assertIn(
+"memoryReference",
+self.dap_server.request_evaluate("rawptr + 1")["body"].keys(),
+)
 
 # Non-pointer expressions don't include a memory reference
-self.assertNotIn("memoryReference", 
self.dap_server.request_evaluate("1 + 3")["body"].keys())
-
+self.assertNotIn(
+"memoryReference", self.dap_server.request_evaluate("1 + 
3")["body"].keys()
+   

[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)

[Adrian Vogelsgesang via lldb-commits]

https://github.com/vogelsgesang updated 
https://github.com/llvm/llvm-project/pull/104317

>From a5b4f6e7e105d36b82f9de588d2705ad3d622953 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang 
Date: Wed, 14 Aug 2024 11:52:40 +
Subject: [PATCH] [lldb-dap] Support inspecting memory

Adds support for the `readMemory` request which allows VS-Code to
inspect memory. Also, add `memoryReference` to variablesa and `evaluate`
responses, such that the binary view can be opened from the variables
view and from the "watch" pane.
---
 .../test/tools/lldb-dap/dap_server.py |  13 ++
 lldb/test/API/tools/lldb-dap/memory/Makefile  |   3 +
 .../tools/lldb-dap/memory/TestDAP_memory.py   | 135 ++
 lldb/test/API/tools/lldb-dap/memory/main.cpp  |  10 +
 lldb/tools/lldb-dap/JSONUtils.cpp |  29 ++-
 lldb/tools/lldb-dap/JSONUtils.h   |   6 +
 lldb/tools/lldb-dap/lldb-dap.cpp  | 172 +-
 7 files changed, 365 insertions(+), 3 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/memory/Makefile
 create mode 100644 lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
 create mode 100644 lldb/test/API/tools/lldb-dap/memory/main.cpp

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index a324af57b61df3..da10e8ab5c57c2 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -691,6 +691,19 @@ def request_disassemble(
 for inst in instructions:
 self.disassembled_instructions[inst["address"]] = inst
 
+def request_read_memory(self, memoryReference, offset, count):
+args_dict = {
+"memoryReference": memoryReference,
+"offset": offset,
+"count": count,
+}
+command_dict = {
+"command": "readMemory",
+"type": "request",
+"arguments": args_dict,
+}
+return self.send_recv(command_dict)
+
 def request_evaluate(self, expression, frameIndex=0, threadId=None, 
context=None):
 stackFrame = self.get_stackFrame(frameIndex=frameIndex, 
threadId=threadId)
 if stackFrame is None:
diff --git a/lldb/test/API/tools/lldb-dap/memory/Makefile 
b/lldb/test/API/tools/lldb-dap/memory/Makefile
new file mode 100644
index 00..8b20bcb050
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/memory/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py 
b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
new file mode 100644
index 00..f950d5eecda671
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
@@ -0,0 +1,135 @@
+"""
+Test lldb-dap memory support
+"""
+
+from base64 import b64decode
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbdap_testcase
+import os
+
+
+class TestDAP_memory(lldbdap_testcase.DAPTestCaseBase):
+def test_read_memory(self):
+"""
+Tests the 'read_memory' request
+"""
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = "main.cpp"
+self.source_path = os.path.join(os.getcwd(), source)
+self.set_source_breakpoints(
+source,
+[line_number(source, "// Breakpoint")],
+)
+self.continue_to_next_stop()
+
+locals = {l["name"]: l for l in self.dap_server.get_local_variables()}
+rawptr_ref = locals["rawptr"]["memoryReference"]
+
+# We can read the complete string
+mem = self.dap_server.request_read_memory(rawptr_ref, 0, 5)["body"]
+self.assertEqual(mem["unreadableBytes"], 0)
+self.assertEqual(b64decode(mem["data"]), b"dead\0")
+
+# Use an offset
+mem = self.dap_server.request_read_memory(rawptr_ref, 2, 3)["body"]
+self.assertEqual(b64decode(mem["data"]), b"ad\0")
+
+# Use a negative offset
+mem = self.dap_server.request_read_memory(rawptr_ref, -1, 6)["body"]
+self.assertEqual(b64decode(mem["data"])[1:], b"dead\0")
+
+# Reads of size 0 are successful
+# VS-Code sends those in order to check if a `memoryReference` can 
actually be dereferenced.
+mem = self.dap_server.request_read_memory(rawptr_ref, 0, 0)
+self.assertEqual(mem["success"], True)
+self.assertEqual(mem["body"]["data"], "")
+
+# Reads at offset 0x0 fail
+mem = self.dap_server.request_read_memory("0x0", 0, 6)
+self.assertEqual(mem["success"], False)
+self.assertTrue(mem["message"].startswith("Unable to read memory: "))
+
+def test_memory_refs_variables(self):
+"""
+Tests memory references for evaluate
+"""
+   

[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)

[Walter Erquinigo via lldb-commits]

walter-erquinigo wrote:

@clayborg you'll be happy to review this

https://github.com/llvm/llvm-project/pull/104317
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)

[Walter Erquinigo via lldb-commits]

walter-erquinigo wrote:

@vogelsgesang can you share a screenshot?

https://github.com/llvm/llvm-project/pull/104317
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Support inspecting memory (PR #104317)

[Adrian Vogelsgesang via lldb-commits]

vogelsgesang wrote:


https://github.com/user-attachments/assets/f1ec37fe-414e-41ee-ad10-a213570d3e5f



https://github.com/llvm/llvm-project/pull/104317
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits