[Lldb-commits] [PATCH] D127605: [lldb] Support non-pointer implicit this/self in GetValueForVariableExpressionPath

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D127605#3580123 , @kastiglione 
wrote:

>> This is good, but it also illustrates how the strings "->" and ".'" should 
>> actually come from the typesystem and not be hardcoded. We're just lucky 
>> that all languages have a "." operator.
>>
>> More elegant would be to just add an API to TypeSystem to get the operator 
>> to access ivars.
>
> Initially I was thinking along these lines. But I became less sure. The 
> "variable expression path" syntax is C-like but independent of the source 
> language. For example, in Swift, there is no `->`. So my rhetorical question 
> is, should the `Language` or `TypeSystem`, etc, know about "variable 
> expression path" syntax, as you suggest? Or should we rely on there being a 
> shared subset between source language and variable expression path syntax, 
> and allow the two subsystems to think they're speaking the same language even 
> though we know they're each actually speaking a different but partially 
> compatible language (source vs variable expression path).
>
> I decided to keep the `->`/`.` choice inside 
> `GetValueForVariableExpressionPath` because I feel that kind of logic belongs 
> closer to other variable expression path code.

The original intention of the variable expression paths was that they were 
really about navigating the ValueObject hierarchy, and the syntax we use should 
be governed by how to do that most conveniently, without being bound by the 
variety of languages that might underly that ValueObject hierarchy.  That's 
particularly convenient when you get to synthetic children, which aren't 
necessarily bound to the language in which the types are expressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127605

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

[Lldb-commits] [PATCH] D125943: [trace][intelpt] Support system-wide tracing [11] - Read warnings and perf conversion in the client

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

In D125943#3587778 , @wallace wrote:

> @stella.stamenova, I've just tried that compile command with several versions 
> of gcc, including g++ (GCC) 8.5.0 on CentOS and got no error Sadly I couldn't 
> install 7.3.1. Could you try again in top of tree?

Yes, it is still failing with 7.3.1 with the same error as before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125943

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

[Lldb-commits] [PATCH] D125943: [trace][intelpt] Support system-wide tracing [11] - Read warnings and perf conversion in the client

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

Hi @stella.stamenova, could you help fixing that one liner? I really have
no way to install gcc7 on my linux machine (I don't have permission to
install older gcc toolchains). I'm happy to chat with you on discord or
anywhere else to get this resolved.

Il giorno gio 16 giu 2022 alle ore 08:28 Stella Stamenova via Phabricator <
revi...@reviews.llvm.org> ha scritto:

> stella.stamenova added a comment.
>
> In D125943#3587778  
> https://reviews.llvm.org/D125943#3587778, @wallace
> wrote:
>
>> @stella.stamenova, I've just tried that compile command with several
>
> versions of gcc, including g++ (GCC) 8.5.0 on CentOS and got no error Sadly
> I couldn't install 7.3.1. Could you try again in top of tree?
>
> Yes, it is still failing with 7.3.1 with the same error as before.
>
> Repository:
>
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>
>   https://reviews.llvm.org/D125943/new/
>
> https://reviews.llvm.org/D125943


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125943

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

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, clayborg, jingham, kastiglione.
Herald added a project: All.
JDevlieghere requested review of this revision.

This patch adds a buffer logging mode to lldb. It can be enabled with `log 
buffered enable`, which enables logging for the default category for every log 
channel to an in-memory circular buffer. The buffered log messages can be 
written to disk with the `log buffered dump` command. The latter command takes 
a directory as it input argument and creates a log file per log channel.

As of right now these two new commands are pretty bare bones. For example, we 
do not support specifying categories or logging options. This is intentional, 
because I don't see them as the primary way to interact with this feature. 
Instead, I imagine a world where we have a small but meaningful subset of 
logging always enabled in buffered mode. When something goes wrong, the user 
types a command (similar to `sysdiagnose` on macOS) and we'll dump the logs as 
part of a package that's meant to help us investigate the issue.


https://reviews.llvm.org/D127986

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/Log.cpp

Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -403,6 +403,6 @@
   stream.flush();
 }
 
-std::shared_ptr RotatingLogHandlerCreate(size_t size) {
+std::shared_ptr RotatingLogHandler::Create(size_t size) {
   return std::make_shared(size);
 }
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1453,6 +1453,57 @@
error_stream);
 }
 
+bool Debugger::EnableRotatingLog(unsigned size, uint32_t log_options,
+ llvm::raw_ostream &error_stream) {
+  if (!m_rotating_handlers.empty())
+return false;
+
+  // Create a RotatingLogHandler for each channel's default category.
+  for (llvm::StringRef channel : Log::ListChannels()) {
+auto log_handler_sp = RotatingLogHandler::Create(size);
+m_rotating_handlers[channel] = log_handler_sp;
+Log::EnableLogChannel(log_handler_sp, log_options, channel, {"default"},
+  error_stream);
+  }
+  return true;
+}
+
+bool Debugger::DumpRotatingLog(const FileSpec &dir,
+   llvm::raw_ostream &error_stream) {
+  if (m_rotating_handlers.empty())
+return false;
+
+  assert(FileSystem::Instance().IsDirectory(dir) &&
+ "Caller should have made sure the directory is valid");
+
+  const File::OpenOptions flags = File::eOpenOptionWriteOnly |
+  File::eOpenOptionCanCreate |
+  File::eOpenOptionTruncate;
+
+  for (const auto &entry : m_rotating_handlers) {
+llvm::StringRef category = entry.first();
+std::shared_ptr handler = entry.second.lock();
+if (!handler)
+  continue;
+
+std::string filename = (llvm::Twine(category) + ".log").str();
+FileSpec log_file = dir.CopyByAppendingPathComponent(filename);
+
+llvm::Expected file = FileSystem::Instance().Open(
+log_file, flags, lldb::eFilePermissionsFileDefault, false);
+if (!file) {
+  error_stream << "Unable to open log file '" << log_file.GetPath()
+   << "': " << llvm::toString(file.takeError()) << "\n";
+  continue;
+}
+
+llvm::raw_fd_ostream stream((*file)->GetDescriptor(), /*shouldClose=*/true);
+handler->Dump(stream);
+  }
+
+  return true;
+}
+
 ScriptInterpreter *
 Debugger::GetScriptInterpreter(bool can_create,
llvm::Optional language) {
Index: lldb/source/Commands/CommandObjectLog.cpp
===
--- lldb/source/Commands/CommandObjectLog.cpp
+++ lldb/source/Commands/CommandObjectLog.cpp
@@ -493,6 +493,120 @@
   ~CommandObjectLogTimer() override = default;
 };
 
+class CommandObjectLogBufferedEnable : public CommandObjectParsed {
+public:
+  CommandObjectLogBufferedEnable(CommandInterpreter &interpreter)
+  : CommandObjectParsed(
+interpreter, "log buffered enable ",
+"enable logging to an in-memory buffer for the default category of "
+"every log channel. These log files can later be written to disk "
+"with the log buffer dump command.",
+nullptr) {
+
+CommandArgumentEntry arg;
+CommandArgumentData size_arg;
+
+// Define the first (and only) variant of this arg.
+size_arg.arg_type = eArgTypeUnsignedInteger;
+size_arg.arg_repetition = eArgRepeatPlain;
+
+arg.push_back(size_arg);
+m_arguments.push_back(arg);
+  }
+
+  ~CommandObjectLogBufferedEnable() override = default;
+
+protected:
+

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Adding a test is trivial, but I'm holding off on spending time on that until 
there's consensus about the approach.


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

https://reviews.llvm.org/D127986

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

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Utility/Log.cpp:323-324
   if (options.Test(LLDB_LOG_OPTION_THREADSAFE)) {
 static std::recursive_mutex g_LogThreadedMutex;
 std::lock_guard guard(g_LogThreadedMutex);
+handler_sp->Emit(message);

Since we are improving stuff here, should we add a mutex to each handler and 
use the handler specific mutex if needed?



Comment at: lldb/source/Utility/Log.cpp:325
 std::lock_guard guard(g_LogThreadedMutex);
-*stream_sp << message;
-stream_sp->flush();
+handler_sp->Emit(message);
+handler_sp->Flush();

Will this copy the string? The Emit() calls take a "std::string", maybe we 
should switch it to "const std::string &" to avoid any copies?



Comment at: lldb/source/Utility/Log.cpp:328
   } else {
-*stream_sp << message;
-stream_sp->flush();
+handler_sp->Emit(message);
+handler_sp->Flush();

Ditto


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

https://reviews.llvm.org/D127922

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

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 3 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Utility/Log.cpp:323-324
   if (options.Test(LLDB_LOG_OPTION_THREADSAFE)) {
 static std::recursive_mutex g_LogThreadedMutex;
 std::lock_guard guard(g_LogThreadedMutex);
+handler_sp->Emit(message);

clayborg wrote:
> Since we are improving stuff here, should we add a mutex to each handler and 
> use the handler specific mutex if needed?
Yup, I think that makes sense



Comment at: lldb/source/Utility/Log.cpp:325
 std::lock_guard guard(g_LogThreadedMutex);
-*stream_sp << message;
-stream_sp->flush();
+handler_sp->Emit(message);
+handler_sp->Flush();

clayborg wrote:
> Will this copy the string? The Emit() calls take a "std::string", maybe we 
> should switch it to "const std::string &" to avoid any copies?
Yeah,  was planning on `std::move`-ing everything but I already found a few 
cases where that doesn't work. This should indeed take a reference. 


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

https://reviews.llvm.org/D127922

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

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I would vote to add new options to "log enable" and enable any such features on 
a per "log enable" invocation if possible? Otherwise each time you want to 
enable this you need to do two commands, and if someone already enables 
buffered mode, then you wouldn't be able to log anything else? Lets say Xcode 
always enables logging with something like:

  (lldb) log buffered enable 100
  (lldb) log enable lldb process state thread step

Then the user doesn't know this and they try to enable logging with:

  (lldb) log enable lldb expr

Then they currently wouldn't get any logging to the screen? It would be 
buffered until we dump it with "log buffered dump"?


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

https://reviews.llvm.org/D127986

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

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Something like:

  (lldb) log enable --buffered 10 lldb process state thread step

If we do this, then we need to be able to specify which buffers to dump 
somehow. In case someone did:

  (lldb) log enable --buffered 10 lldb ...
  (lldb) log enable --buffered 10 dwarf ...

I the user types "log buffer dump" then all streams would be dumped, maybe with 
a prefix like:

  "lldb" log messages:
  ...
  "dwarf" log messages:
  ...

Or we might be able to specify one or more channels to get just the ones we 
want:

  (lldb) log buffered dump lldb
  (lldb) log buffered dump dwarf
  (lldb) log buffered dump lldb dwarf


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

https://reviews.llvm.org/D127986

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

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/source/Utility/Log.cpp:325
 std::lock_guard guard(g_LogThreadedMutex);
-*stream_sp << message;
-stream_sp->flush();
+handler_sp->Emit(message);
+handler_sp->Flush();

JDevlieghere wrote:
> clayborg wrote:
> > Will this copy the string? The Emit() calls take a "std::string", maybe we 
> > should switch it to "const std::string &" to avoid any copies?
> Yeah,  was planning on `std::move`-ing everything but I already found a few 
> cases where that doesn't work. This should indeed take a reference. 
+1


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

https://reviews.llvm.org/D127922

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

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Utility/Log.cpp:325
 std::lock_guard guard(g_LogThreadedMutex);
-*stream_sp << message;
-stream_sp->flush();
+handler_sp->Emit(message);
+handler_sp->Flush();

mib wrote:
> JDevlieghere wrote:
> > clayborg wrote:
> > > Will this copy the string? The Emit() calls take a "std::string", maybe 
> > > we should switch it to "const std::string &" to avoid any copies?
> > Yeah,  was planning on `std::move`-ing everything but I already found a few 
> > cases where that doesn't work. This should indeed take a reference. 
> +1
The other option is to have the Emit functions use llvm::StringRef?:
```
virtual void Emit(llvm::StringRef s) = 0;
```



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

https://reviews.llvm.org/D127922

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

[Lldb-commits] [PATCH] D127937: [lldb] Add RotatingLogHandler

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/include/lldb/Utility/Log.h:98
+
+  std::vector m_messages;
+  size_t m_next_index = 0;

this could be an array, since it's not being dynamically resized.


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

https://reviews.llvm.org/D127937

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

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 437618.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.

- Make Emit take a StringRef
- Move mutex in the log handler


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

https://reviews.llvm.org/D127922

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/Log.h
  lldb/include/lldb/Utility/StreamCallback.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/Log.cpp
  lldb/source/Utility/StreamCallback.cpp
  lldb/tools/lldb-server/LLDBServerUtilities.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/StreamCallbackTest.cpp
  lldb/unittests/Utility/LogTest.cpp

Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -39,13 +39,13 @@
 } // namespace lldb_private
 
 // Wrap enable, disable and list functions to make them easier to test.
-static bool EnableChannel(std::shared_ptr stream_sp,
+static bool EnableChannel(std::shared_ptr log_handler_sp,
   uint32_t log_options, llvm::StringRef channel,
   llvm::ArrayRef categories,
   std::string &error) {
   error.clear();
   llvm::raw_string_ostream error_stream(error);
-  return Log::EnableLogChannel(stream_sp, log_options, channel, categories,
+  return Log::EnableLogChannel(log_handler_sp, log_options, channel, categories,
error_stream);
 }
 
@@ -78,18 +78,28 @@
   }
 };
 
+class TestLogHandler : public LogHandler {
+public:
+  TestLogHandler() : m_messages(), m_stream(m_messages) {}
+
+  void Emit(llvm::StringRef message) override { m_stream << message; }
+  void Flush() override {}
+
+  llvm::SmallString<0> m_messages;
+  llvm::raw_svector_ostream m_stream;
+};
+
 // A test fixture which provides tests with a pre-registered and pre-enabled
 // channel. Additionally, the messages written to that channel are captured and
 // made available via getMessage().
 class LogChannelEnabledTest : public LogChannelTest {
-  llvm::SmallString<0> m_messages;
-  std::shared_ptr m_stream_sp =
-  std::make_shared(m_messages);
+  std::shared_ptr m_log_handler_sp =
+  std::make_shared();
   Log *m_log;
   size_t m_consumed_bytes = 0;
 
 protected:
-  std::shared_ptr getStream() { return m_stream_sp; }
+  std::shared_ptr getLogHandler() { return m_log_handler_sp; }
   Log *getLog() { return m_log; }
   llvm::StringRef takeOutput();
   llvm::StringRef logAndTakeOutput(llvm::StringRef Message);
@@ -103,14 +113,15 @@
   LogChannelTest::SetUp();
 
   std::string error;
-  ASSERT_TRUE(EnableChannel(m_stream_sp, 0, "chan", {}, error));
+  ASSERT_TRUE(EnableChannel(m_log_handler_sp, 0, "chan", {}, error));
 
   m_log = GetLog(TestChannel::FOO);
   ASSERT_NE(nullptr, m_log);
 }
 
 llvm::StringRef LogChannelEnabledTest::takeOutput() {
-  llvm::StringRef result = m_stream_sp->str().drop_front(m_consumed_bytes);
+  llvm::StringRef result =
+  m_log_handler_sp->m_stream.str().drop_front(m_consumed_bytes);
   m_consumed_bytes+= result.size();
   return result;
 }
@@ -138,9 +149,9 @@
   Log::Register("chan", test_channel);
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
-  std::shared_ptr stream_sp(
-  new llvm::raw_string_ostream(message));
-  EXPECT_TRUE(Log::EnableLogChannel(stream_sp, 0, "chan", {"foo"}, llvm::nulls()));
+  auto log_handler_sp = std::make_shared();
+  EXPECT_TRUE(
+  Log::EnableLogChannel(log_handler_sp, 0, "chan", {"foo"}, llvm::nulls()));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   Log::Unregister("chan");
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
@@ -149,22 +160,21 @@
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
-  std::shared_ptr stream_sp(
-  new llvm::raw_string_ostream(message));
+  auto log_handler_sp = std::make_shared();
   std::string error;
-  ASSERT_FALSE(EnableChannel(stream_sp, 0, "chanchan", {}, error));
+  ASSERT_FALSE(EnableChannel(log_handler_sp, 0, "chanchan", {}, error));
   EXPECT_EQ("Invalid log channel 'chanchan'.\n", error);
 
-  EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {}, error));
+  EXPECT_TRUE(EnableChannel(log_handler_sp, 0, "chan", {}, error));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   EXPECT_EQ(nullptr, GetLog(TestChannel::BAR));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO | TestChannel::BAR));
 
-  EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {"bar"}, error));
+  EXPECT_TRUE(EnableChannel(log_handler_sp, 0, "chan", {"bar"}, error));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   EXPECT_NE(nullptr, GetLog(TestChannel::BAR));
 
-  EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {"baz"}, error));
+  EXPECT_TRUE(EnableChannel(log_handler_sp, 0, "chan", {"baz"}, error));
   EXPECT_NE(std::string::npos, er

[Lldb-commits] [lldb] 1a3f996 - [trace][intelpt] Support system-wide tracing [13] - Add context switch decoding

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-16T11:23:01-07:00
New Revision: 1a3f996972b175a6f7e4db1eabb8310e2e93f634

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

LOG: [trace][intelpt] Support system-wide tracing [13] - Add context switch 
decoding

- Add the logic that parses all cpu context switch traces and produces blocks 
of continuous executions, which will be later used to assign intel pt subtraces 
to threads and to identify gaps. This logic can also identify if the context 
switch trace is malformed.
- The continuous executions blocks are able to indicate when there were some 
contention issues when producing the context switch trace. See the inline 
comments for more information.
- Update the 'dump info' command to show information and stats related to the 
multicore decoding flow, including timing about context switch decoding.
- Add the logic to conver nanoseconds to TSCs.
- Fix a bug when returning the context switches. Now they data returned makes 
sense and even empty traces can be returned from lldb-server.
- Finish the necessary bits for loading and saving a multi-core trace bundle 
from disk.
- Change some size_t to uint64_t for compatibility with 32 bit systems.

Tested by saving a trace session of a program that sleeps 100 times, it was 
able to produce the following 'dump info' text:

```
(lldb) trace load /tmp/trace3/trace.json
   (lldb) thread trace dump info
  Trace technology: intel-pt

thread #1: tid = 4192415
  Total number of instructions: 1

  Memory usage:
Total approximate memory usage (excluding raw trace): 2.51 KiB
Average memory usage per instruction (excluding raw trace): 2573.00 bytes

  Timing for this thread:

  Timing for global tasks:
Context switch trace decoding: 0.00s

  Events:
Number of instructions with events: 0
Number of individual events: 0

  Multi-core decoding:
Total number of continuous executions found: 2499
Number of continuous executions for this thread: 102

  Errors:
Number of TSC decoding errors: 0
```

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

Added: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h

Modified: 
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Utility/TraceGDBRemotePackets.h
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Process/Linux/Perf.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TaskTimer.cpp
lldb/source/Plugins/Trace/intel-pt/TaskTimer.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
lldb/source/Target/Trace.cpp
lldb/source/Utility/TraceGDBRemotePackets.cpp
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
lldb/test/API/commands/trace/TestTraceDumpInfo.py
lldb/test/API/commands/trace/TestTraceLoad.py

lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py

Removed: 




diff  --git a/lldb/include/lldb/Target/Trace.h 
b/lldb/include/lldb/Target/Trace.h
index 534c87a2d5b6c..233efcbf5ddd6 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -433,15 +433,15 @@ class Trace : public PluginInterface,
   GetLiveProcessBinaryData(llvm::StringRef kind);
 
   /// Get the size of the data returned by \a GetLiveThreadBinaryData
-  llvm::Optional GetLiveThreadBinaryDataSize(lldb::tid_t tid,
- llvm::StringRef kind);
+  llvm::Optional GetLiveThreadBinaryDataSize(lldb::tid_t tid,
+   llvm::StringRef kind);
 
   /// Get the size of the data returned by \a GetLiveCoreBinaryData
-  llvm::Optional GetLiveCoreBinaryDataSize(lldb::core_id_t core_id,
-   llvm::StringRef kind);
+  llvm::Optional GetLiveCoreBinaryDataSize(lldb::core_id_t core_id,
+ llvm::StringRef kind);
 
   /// Get the size of the data returned by \a GetLiveProcessBinaryData
-  llvm::Optional GetLiveProcessBinaryDataSize(llvm::StringRef kind);
+  llvm::Optional GetLiveProcessBinaryDataSi

[Lldb-commits] [lldb] a19fcc2 - [trace][intelpt] Support system-wide tracing [14] - Decode per cpu

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-16T11:23:01-07:00
New Revision: a19fcc2bec81989f90700cfc63b89a0dfd330197

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

LOG: [trace][intelpt] Support system-wide tracing [14] - Decode per cpu

This is the final functional patch to support intel pt decoding per cpu.
It works by doing the following:

- First, all context switches are split by tid and sorted in order. This 
produces a list of continuous executes per thread per core.
- Then, all intel pt subtraces are split by PSB boundaries and assigned to 
individual thread continuous executions on the same core by doing simple 
TSC-based comparisons.
- With this, we have, per thread, a sorted list of continuous executions each 
one with a list of intel pt subtraces. Up to this point, this is really fast 
because no instructions were actually decoded.
- Then, each thread can be decoded by traversing their continuous executions 
and intel pt subtraces. An advantage of having these continuous executions is 
that we can identify if a continuous exexecution doesn't have intel pt data, 
and thus has a gap in it. We can later to more sofisticated comparisons to 
identify if within a continuous execution there are gaps.

I'm adding a test as well.

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

Added: 
lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.h
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/45.intelpt_trace

lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/45.perf_context_switch_trace
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/51.intelpt_trace

lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/51.perf_context_switch_trace
lldb/test/API/commands/trace/intelpt-multi-core-trace/modules/m.out
lldb/test/API/commands/trace/intelpt-multi-core-trace/multi_thread.cpp
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json

Modified: 
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
lldb/source/Target/Trace.cpp
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
lldb/test/API/commands/trace/TestTraceLoad.py

lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
lldb/unittests/Process/Linux/PerfTests.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Trace.h 
b/lldb/include/lldb/Target/Trace.h
index 233efcbf5ddd6..89e129079393b 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -240,6 +240,9 @@ class Trace : public PluginInterface,
 
   using OnBinaryDataReadCallback =
   std::function data)>;
+  using OnCoresBinaryDataReadCallback = std::function>
+  &core_to_data)>;
 
   /// Fetch binary data associated with a thread, either live or postmortem, 
and
   /// pass it to the given callback. The reason of having a callback is to free
@@ -292,6 +295,12 @@ class Trace : public PluginInterface,
llvm::StringRef kind,
OnBinaryDataReadCallback callback);
 
+  /// Similar to \a OnCoreBinaryDataRead but this is able to fetch the same 
data
+  /// from multiple cores at once.
+  llvm::Error OnCoresBinaryDataRead(const std::set core_ids,
+llvm::StringRef kind,
+OnCoresBinaryDataReadCallback callback);
+
   /// \return
   /// All the currently traced processes.
   std::vector GetAllProcesses();
@@ -518,7 +527,12 @@ class Trace : public PluginInterface,
 
   /// core id -> data kind -> size
   llvm::DenseMap>
+  m_live_core_data_sizes;
+  /// core id -> data kind -> bytes
+  llvm::DenseMap>>
   m_live_core_data;
+
   /// data kind -> size
   std::unordered_map m_live_process_data;
   /// \}

diff  --git a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h 
b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
index 23030f6e78e84..b6acd741747aa 100644
--- a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
@@ -80,9 +80,9 @@ struct LinuxPerfZeroTscConversion {
   ///
   /// \return
   ///   Nanosecond wall time.
-  std::chrono::nanoseconds

[Lldb-commits] [lldb] ef99707 - [trace][intelpt] Support system-wide tracing [15] - Make triple optional

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-16T11:23:01-07:00
New Revision: ef9970759b5b63fed40061cd960e6d9e8edabb48

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

LOG: [trace][intelpt] Support system-wide tracing [15] - Make triple optional

The process triple should only be needed when LLDB can't identify the correct
triple on its own. Examples could be universal mach-o binaries. But in any case,
at least for most of ELF files, LLDB should be able to do the job without having
the user specify the triple manually.

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

Added: 


Modified: 
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/test/API/commands/trace/TestTraceLoad.py
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json

Removed: 




diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h 
b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
index af4c365d1c49e..9d0fd789cab58 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
@@ -36,7 +36,7 @@ struct JSONThread {
 
 struct JSONProcess {
   int64_t pid;
-  std::string triple;
+  llvm::Optional triple;
   std::vector threads;
   std::vector modules;
 };

diff  --git 
a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp 
b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
index 1977a674b61a8..85e04ff6ec6a8 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
@@ -88,7 +88,7 @@ Expected
 TraceIntelPTSessionFileParser::ParseProcess(const JSONProcess &process) {
   TargetSP target_sp;
   Status error = m_debugger.GetTargetList().CreateTarget(
-  m_debugger, /*user_exe_path*/ StringRef(), process.triple,
+  m_debugger, /*user_exe_path*/ StringRef(), process.triple.getValueOr(""),
   eLoadDependentsNo,
   /*platform_options*/ nullptr, target_sp);
 
@@ -161,8 +161,8 @@ StringRef TraceIntelPTSessionFileParser::GetSchema() {
   "processes": [
 {
   "pid": integer,
-  "triple": string,
-  // clang/llvm target triple.
+  "triple"?: string,
+  // Optional clang/llvm target triple.
   "threads": [
 {
   "tid": integer,

diff  --git a/lldb/test/API/commands/trace/TestTraceLoad.py 
b/lldb/test/API/commands/trace/TestTraceLoad.py
index 6f6994bfaa8b1..be91a4d15a909 100644
--- a/lldb/test/API/commands/trace/TestTraceLoad.py
+++ b/lldb/test/API/commands/trace/TestTraceLoad.py
@@ -94,9 +94,9 @@ def testLoadInvalidTraces(self):
 "stepping": integer
   },'''])
 
-# Now we test a missing field in the global session file
+# Now we test a wrong cpu family field in the global session file
 self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace", 
"trace_bad2.json"), error=True,
-substrs=['error: missing value at 
traceSession.processes[1].triple', "Context", "Schema"])
+substrs=['error: expected uint64_t at 
traceSession.cpuInfo.family', "Context", "Schema"])
 
 # Now we test a missing field in the intel-pt settings
 self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace", 
"trace_bad4.json"), error=True,

diff  --git a/lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json 
b/lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
index 54397ee79b3cc..4bbb427156785 100644
--- a/lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
+++ b/lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
@@ -38,8 +38,7 @@
 {
   "tid": 3497497
 }
-  ],
-  "triple": "x86_64-unknown-linux-gnu"
+  ]
 }
   ],
   "tscPerfZeroConversion": {

diff  --git a/lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json 
b/lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
index 46fa01abf45b9..98d322fb39b58 100644
--- a/lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
+++ b/lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
@@ -2,14 +2,13 @@
   "type": "intel-pt",
   "cpuInfo": {
 "vendor": "GenuineIntel",
-"family": 6,
+"family": "123",
 "model": 79,
 "stepping": 1
   },
   "processes": [
 {
   "pid": 1234,
-  "triple": "x86_64-*-linux",
   "threads": [
 {
   "tid": 5678,



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

[Lldb-commits] [PATCH] D126267: [trace][intelpt] Support system-wide tracing [13] - Add context switch decoding

[Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a3f996972b1: [trace][intelpt] Support system-wide tracing 
[13] - Add context switch decoding (authored by Walter Erquinigo 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126267

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/TraceGDBRemotePackets.h
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TaskTimer.cpp
  lldb/source/Plugins/Trace/intel-pt/TaskTimer.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Target/Trace.cpp
  lldb/source/Utility/TraceGDBRemotePackets.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  lldb/test/API/commands/trace/TestTraceDumpInfo.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py

Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -233,5 +233,6 @@
 # We must have captured the context switch of when the target resumed
 self.assertTrue(found_non_empty_context_switch)
 
+self.expect("thread trace dump instructions", substrs=['unimplemented'])
 
 self.traceStopProcess()
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -43,8 +43,8 @@
 Total approximate memory usage (excluding raw trace): 1.27 KiB
 Average memory usage per instruction (excluding raw trace): 61.76 bytes
 
-  Timing:
-Decoding instructions: ''', '''s
+  Timing for this thread:
+Decoding instructions: ''', '''
 
   Events:
 Number of instructions with events: 1
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -45,8 +45,8 @@
 Total approximate memory usage (excluding raw trace): 1.27 KiB
 Average memory usage per instruction (excluding raw trace): 61.76 bytes
 
-  Timing:
-Decoding instructions: ''', '''s
+  Timing for this thread:
+Decoding instructions: ''', '''
 
   Events:
 Number of instructions with events: 1
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -50,7 +50,8 @@
   return base;
 }
 
-std::chrono::nanoseconds LinuxPerfZeroTscConversion::ToNanos(uint64_t tsc) {
+std::chrono::nanoseconds
+LinuxPerfZeroTscConversion::ToNanos(uint64_t tsc) const {
   uint64_t quot = tsc >> time_shift;
   uint64_t rem_flag = (((uint64_t)1 << time_shift) - 1);
   uint64_t rem = tsc & rem_flag;
@@ -58,6 +59,14 @@
   ((rem * time_mult) >> time_shift)};
 }
 
+uint64_t
+LinuxPerfZeroTscConversion::ToTSC(std::chrono::nanoseconds nanos) const {
+  uint64_t time = nanos.count() - time_zero;
+  uint64_t quot = time / time_mult;
+  uint64_t rem = time % time_mult;
+  return (quot << time_shift) + (rem << time_shift) / time_mult;
+}
+
 json::Value toJSON(const LinuxPerfZeroTscConversion &packet) {
   return json::Value(json::Object{
   {"timeMult", packet.time_mult},
Index: lldb/source/Utility/TraceGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceGDBRemotePackets.cpp
@@ -120,7 +120,7 @@
 bool fromJSON(const json::Value &value, TraceCoreState &packet,
   json::Path path) {
   ObjectMapper o(value, path);
-  int64_t core_id;
+  uint64_t core_id;
   if (!o || !o.map("coreId", core_id) ||
   !o.map("binaryData", packet.b

[Lldb-commits] [lldb] ff15efc - [trace][intelpt] Support system-wide tracing [16] - Create threads automatically from context switch data in the post-mortem case

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-16T11:23:02-07:00
New Revision: ff15efc1a701a8e76242ca2a105b5adf7d68e980

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

LOG: [trace][intelpt] Support system-wide tracing [16] - Create threads 
automatically from context switch data in the post-mortem case

For some context, The context switch data contains information of which threads 
were
executed by each traced process, therefore it's not necessary to specify
them in the trace file.

So this diffs adds support for that automatic feature. Eventually we
could include it to live processes as well.

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

Added: 

lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json

Modified: 
lldb/include/lldb/Target/Trace.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
lldb/source/Target/Trace.cpp
lldb/test/API/commands/trace/TestTraceLoad.py

Removed: 




diff  --git a/lldb/include/lldb/Target/Trace.h 
b/lldb/include/lldb/Target/Trace.h
index 89e129079393..67585c22511f 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -496,6 +496,10 @@ class Trace : public PluginInterface,
   DoRefreshLiveProcessState(TraceGetStateResponse state,
 llvm::StringRef json_response) = 0;
 
+  /// Return the list of processes traced by this instance. None of the 
returned
+  /// pointers are invalid.
+  std::vector GetTracedProcesses() const;
+
   /// Method to be invoked by the plug-in to refresh the live process state. It
   /// will invoked DoRefreshLiveProcessState at some point, which should be
   /// implemented by the plug-in for custom state handling.

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
index 59454a0320be..af95c20117cb 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -75,6 +75,7 @@ Expected 
TraceIntelPT::CreateInstanceForLiveProcess(Process &process) {
 }
 
 TraceIntelPT::TraceIntelPT(JSONTraceSession &session,
+   const FileSpec &session_file_dir,
ArrayRef traced_processes,
ArrayRef traced_threads)
 : Trace(traced_processes, session.GetCoreIds()),
@@ -92,11 +93,19 @@ TraceIntelPT::TraceIntelPT(JSONTraceSession &session,
 std::vector cores;
 
 for (const JSONCore &core : *session.cores) {
+  FileSpec trace_buffer(core.trace_buffer);
+  if (trace_buffer.IsRelative())
+trace_buffer.PrependPathComponent(session_file_dir);
+
   SetPostMortemCoreDataFile(core.core_id, IntelPTDataKinds::kTraceBuffer,
-FileSpec(core.trace_buffer));
+trace_buffer);
+
+  FileSpec context_switch(core.context_switch_trace);
+  if (context_switch.IsRelative())
+context_switch.PrependPathComponent(session_file_dir);
   SetPostMortemCoreDataFile(core.core_id,
 IntelPTDataKinds::kPerfContextSwitchTrace,
-FileSpec(core.context_switch_trace));
+context_switch);
   cores.push_back(core.core_id);
 }
 
@@ -473,3 +482,45 @@ Error TraceIntelPT::OnThreadBufferRead(lldb::tid_t tid,
 }
 
 TaskTimer &TraceIntelPT::GetTimer() { return m_task_timer; }
+
+Error TraceIntelPT::CreateThreadsFromContextSwitches() {
+  DenseMap> pids_to_tids;
+
+  for (core_id_t core_id : GetTracedCores()) {
+Error err = OnCoreBinaryDataRead(
+core_id, IntelPTDataKinds::kPerfContextSwitchTrace,
+[&](ArrayRef data) -> Error {
+  Expected> executions =
+  DecodePerfContextSwitchTrace(data, core_id, *m_tsc_conversion);
+  if (!executions)
+return executions.takeError();
+  for (const ThreadContinuousExecution &execution : *executions)
+pids_to_tids[execution.pid].insert(execution.tid);
+  return Error::success();
+});
+if (err)
+  return err;
+  }
+
+  DenseMap processes;
+  for (Process *proc : GetTracedProcesses())
+processes.try_emplace(proc->GetID(), proc);
+
+  for (const auto &pid_to_tids : pids_to_tids) {
+lldb::pid_t pid = pid_to_tids.first;
+auto it = processes.find(pid);
+if (it == processes.end())
+  continue;
+
+Process &process = *it->second;
+ThreadList &thread_list = process.GetThreadList();
+
+for (lldb::tid_t tid : pid_to_ti

[Lldb-commits] [PATCH] D126394: [trace][intelpt] Support system-wide tracing [14] - Decode per cpu

[Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa19fcc2bec81: [trace][intelpt] Support system-wide tracing 
[14] - Decode per cpu (authored by Walter Erquinigo ).

Changed prior to commit:
  https://reviews.llvm.org/D126394?vs=434093&id=437621#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126394

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
  lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
  lldb/source/Target/Trace.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/45.intelpt_trace
  
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/45.perf_context_switch_trace
  lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/51.intelpt_trace
  
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/51.perf_context_switch_trace
  lldb/test/API/commands/trace/intelpt-multi-core-trace/modules/m.out
  lldb/test/API/commands/trace/intelpt-multi-core-trace/multi_thread.cpp
  lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
  
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
  lldb/unittests/Process/Linux/PerfTests.cpp

Index: lldb/unittests/Process/Linux/PerfTests.cpp
===
--- lldb/unittests/Process/Linux/PerfTests.cpp
+++ lldb/unittests/Process/Linux/PerfTests.cpp
@@ -76,14 +76,14 @@
   if (!tsc_after_sleep)
 GTEST_SKIP() << toString(tsc_after_sleep.takeError());
 
-  std::chrono::nanoseconds converted_tsc_diff =
+  uint64_t converted_tsc_diff =
   params->ToNanos(*tsc_after_sleep) - params->ToNanos(*tsc_before_sleep);
 
   std::chrono::microseconds acceptable_overhead(500);
 
-  ASSERT_GE(converted_tsc_diff.count(), SLEEP_NANOS.count());
-  ASSERT_LT(converted_tsc_diff.count(),
-(SLEEP_NANOS + acceptable_overhead).count());
+  ASSERT_GE(converted_tsc_diff, static_cast(SLEEP_NANOS.count()));
+  ASSERT_LT(converted_tsc_diff,
+static_cast((SLEEP_NANOS + acceptable_overhead).count()));
 }
 
 #endif // __x86_64__
Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -233,6 +233,6 @@
 # We must have captured the context switch of when the target resumed
 self.assertTrue(found_non_empty_context_switch)
 
-self.expect("thread trace dump instructions", substrs=['unimplemented'])
+self.expect("thread trace dump instructions")
 
 self.traceStopProcess()
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
@@ -0,0 +1,51 @@
+{
+  "cores": [
+{
+  "contextSwitchTrace": "/tmp/trace8/cores/45.perf_context_switch_trace",
+  "coreId": 45,
+  "traceBuffer": "/tmp/trace8/cores/45.intelpt_trace"
+},
+{
+  "contextSwitchTrace": "/tmp/trace8/cores/51.perf_context_switch_trace",
+  "coreId": 51,
+  "traceBuffer": "/tmp/trace8/cores/51.intelpt_trace"
+}
+  ],
+  "cpuInfo": {
+"family": 6,
+"model": 85,
+"stepping": 4,
+"vendor": "GenuineIntel"
+  },
+  "processes": [
+{
+  "modules": [
+{
+  "file": "modules/m.out",
+  "systemPath": "/tmp/m.out",
+  "loadAddress": 4194304,
+  "uuid": "AEFB0D59-233F-80FF-6D3C-4DED498534CF-11017B3B"
+}
+  ],
+  "pid": 3497234,
+  "threads": [
+{
+  "tid": 3497234
+},
+{
+  "tid": 3497496
+},
+{
+  "tid": 3497497
+}
+  ],
+  "triple": "x86_64-unknown-linux-gnu"
+}
+  ],
+  "tscPerfZeroConversion": {
+"timeMult": 1076264588,
+"timeShift": 31,
+"timeZero": 18433473881008870804
+  },
+  "type": "intel-pt"
+}
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/multi_thread.cpp

[Lldb-commits] [lldb] 03cc58f - [trace][intelpt] Support system-wide tracing [17] - Some improvements

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-16T11:23:02-07:00
New Revision: 03cc58ff2a7a88b4eb8c1409169b662b46ec85d9

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

LOG: [trace][intelpt] Support system-wide tracing [17] - Some improvements

This improves several things and addresses comments up to the diff [11] in this 
stack.

- Simplify many functions to receive less parameters that they can identify 
easily
- Create Storage classes for Trace and TraceIntelPT that can make it easier to 
reason about what can change with live process refreshes and what cannot.
- Don't cache the perf zero conversion numbers in lldb-server to make sure we 
get the most up-to-date numbers.
- Move the thread identifaction from context switches to the bundle parser, to 
leave TraceIntelPT simpler. This also makes sure that the constructor of 
TraceIntelPT is invoked when the entire data has been checked to be correct.
- Normalize all bundle paths before the Processes, Threads and Modules are 
created, so that they can assume that all paths are correct and absolute
- Fix some issues in the tests. Now they all pass.
- return the specific instance when constructing PerThread and MultiCore 
processor tracers.
- Properly implement IntelPTMultiCoreTrace::TraceStart.
- Improve some comments.
- Use the typedef ContextSwitchTrace more often for clarity.
- Move CreateContextSwitchTracePerfEvent to Perf.h as a utility function.
- Synchronize better the state of the context switch and the intel pt
perf events.
- Use a booblean instead of an enum for the PerfEvent state.

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

Added: 


Modified: 
lldb/include/lldb/Target/Trace.h
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
lldb/source/Plugins/Process/Linux/IntelPTCollector.h
lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp
lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.h
lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
lldb/source/Plugins/Process/Linux/Perf.cpp
lldb/source/Plugins/Process/Linux/Perf.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
lldb/source/Target/Trace.cpp
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json

lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json

Removed: 




diff  --git a/lldb/include/lldb/Target/Trace.h 
b/lldb/include/lldb/Target/Trace.h
index 67585c22511f..2cd4d80ba206 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -296,10 +296,9 @@ class Trace : public PluginInterface,
OnBinaryDataReadCallback callback);
 
   /// Similar to \a OnCoreBinaryDataRead but this is able to fetch the same 
data
-  /// from multiple cores at once.
-  llvm::Error OnCoresBinaryDataRead(const std::set core_ids,
-llvm::StringRef kind,
-OnCoresBinaryDataReadCallback callback);
+  /// from all cores at once.
+  llvm::Error OnAllCoresBinaryDataRead(llvm::StringRef kind,
+   OnCoresBinaryDataReadCallback callback);
 
   /// \return
   /// All the currently traced processes.
@@ -310,6 +309,11 @@ class Trace : public PluginInterface,
   /// plugin.
   llvm::ArrayRef GetTracedCores();
 
+  /// Helper method for reading a data file and passing its data to the given
+  /// callback.
+  static llvm::Error OnDataFileRead(FileSpec file,
+OnBinaryDataReadCallback callback);
+
 protected:
   /// Get the currently traced live process.
   ///
@@ -342,10 +346,6 @@ class Trace : public PluginInterface,
  llvm::StringRef kind,
  OnBinaryDataReadCallback 
callback);
 
-  /// Helper method for reading a data file and passing its data to the given
-  /// callback.
-  llvm::Error OnDataFileRead(FileSpec file, OnBinaryDataReadCallback callback);
-
   /// Get the file path containing data of a postmortem thread given a data

[Lldb-commits] [PATCH] D126990: [trace][intelpt] Support system-wide tracing [15] - Make triple optional

[Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGef9970759b5b: [trace][intelpt] Support system-wide tracing 
[15] - Make triple optional (authored by Walter Erquinigo 
).
Herald added a subscriber: Michael137.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126990

Files:
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
  lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json


Index: lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
===
--- lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
@@ -2,14 +2,13 @@
   "type": "intel-pt",
   "cpuInfo": {
 "vendor": "GenuineIntel",
-"family": 6,
+"family": "123",
 "model": 79,
 "stepping": 1
   },
   "processes": [
 {
   "pid": 1234,
-  "triple": "x86_64-*-linux",
   "threads": [
 {
   "tid": 5678,
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
===
--- lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
@@ -38,8 +38,7 @@
 {
   "tid": 3497497
 }
-  ],
-  "triple": "x86_64-unknown-linux-gnu"
+  ]
 }
   ],
   "tscPerfZeroConversion": {
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -94,9 +94,9 @@
 "stepping": integer
   },'''])
 
-# Now we test a missing field in the global session file
+# Now we test a wrong cpu family field in the global session file
 self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace", 
"trace_bad2.json"), error=True,
-substrs=['error: missing value at 
traceSession.processes[1].triple', "Context", "Schema"])
+substrs=['error: expected uint64_t at 
traceSession.cpuInfo.family', "Context", "Schema"])
 
 # Now we test a missing field in the intel-pt settings
 self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace", 
"trace_bad4.json"), error=True,
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
@@ -88,7 +88,7 @@
 TraceIntelPTSessionFileParser::ParseProcess(const JSONProcess &process) {
   TargetSP target_sp;
   Status error = m_debugger.GetTargetList().CreateTarget(
-  m_debugger, /*user_exe_path*/ StringRef(), process.triple,
+  m_debugger, /*user_exe_path*/ StringRef(), process.triple.getValueOr(""),
   eLoadDependentsNo,
   /*platform_options*/ nullptr, target_sp);
 
@@ -161,8 +161,8 @@
   "processes": [
 {
   "pid": integer,
-  "triple": string,
-  // clang/llvm target triple.
+  "triple"?: string,
+  // Optional clang/llvm target triple.
   "threads": [
 {
   "tid": integer,
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
@@ -36,7 +36,7 @@
 
 struct JSONProcess {
   int64_t pid;
-  std::string triple;
+  llvm::Optional triple;
   std::vector threads;
   std::vector modules;
 };


Index: lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
===
--- lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
@@ -2,14 +2,13 @@
   "type": "intel-pt",
   "cpuInfo": {
 "vendor": "GenuineIntel",
-"family": 6,
+"family": "123",
 "model": 79,
 "stepping": 1
   },
   "processes": [
 {
   "pid": 1234,
-  "triple": "x86_64-*-linux",
   "threads": [
 {
   "tid": 5678,
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
===
--- lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
@@ -38,8 +38,7 @@
 {
   "tid": 3497497

[Lldb-commits] [PATCH] D127001: [trace][intelpt] Support system-wide tracing [16] - Create threads automatically from context switch data in the post-mortem case

[Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGff15efc1a701: [trace][intelpt] Support system-wide tracing 
[16] - Create threads… (authored by Walter Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127001

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
  lldb/source/Target/Trace.cpp
  lldb/test/API/commands/trace/TestTraceLoad.py
  
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json

Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json
@@ -0,0 +1,44 @@
+{
+  "cores": [
+{
+  "contextSwitchTrace": "/tmp/trace8/cores/45.perf_context_switch_trace",
+  "coreId": 45,
+  "traceBuffer": "/tmp/trace8/cores/45.intelpt_trace"
+},
+{
+  "contextSwitchTrace": "/tmp/trace8/cores/51.perf_context_switch_trace",
+  "coreId": 51,
+  "traceBuffer": "/tmp/trace8/cores/51.intelpt_trace"
+}
+  ],
+  "cpuInfo": {
+"family": 6,
+"model": 85,
+"stepping": 4,
+"vendor": "GenuineIntel"
+  },
+  "processes": [
+{
+  "modules": [
+{
+  "file": "modules/m.out",
+  "systemPath": "/tmp/m.out",
+  "loadAddress": 4194304,
+  "uuid": "AEFB0D59-233F-80FF-6D3C-4DED498534CF-11017B3B"
+}
+  ],
+  "pid": 3497234,
+  "threads": [
+{
+  "tid": 3497234
+}
+  ]
+}
+  ],
+  "tscPerfZeroConversion": {
+"timeMult": 1076264588,
+"timeShift": 31,
+"timeZero": 18433473881008870804
+  },
+  "type": "intel-pt"
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -21,6 +21,18 @@
   substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
+def testLoadMultiCoreTraceWithMissingThreads(self):
+src_dir = self.getSourceDir()
+trace_definition_file = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_missing_threads.json")
+self.expect("trace load -v " + trace_definition_file, substrs=["intel-pt"])
+self.expect("thread trace dump instructions 3 -t",
+  substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+   "m.out`foo() + 65 at multi_thread.cpp:12:21",
+   "19520: [tsc=0x008fb5211bfbc69e] 0x00400ba7jg 0x400bb3"])
+self.expect("thread trace dump instructions 2 -t",
+  substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+   "m.out`bar() + 26 at multi_thread.cpp:20:6"])
+
 def testLoadTrace(self):
 src_dir = self.getSourceDir()
 trace_definition_file = os.path.join(src_dir, "intelpt-trace", "trace.json")
Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -449,3 +449,14 @@
 return *m_cores;
   return {};
 }
+
+std::vector Trace::GetTracedProcesses() const {
+  std::vector processes;
+
+  for (Process *proc : m_postmortem_processes)
+processes.push_back(proc);
+
+  if (m_live_process)
+processes.push_back(m_live_process);
+  return processes;
+}
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
@@ -52,7 +52,7 @@
   ///   errors, return a null pointer.
   llvm::Expected Parse();
 
-  lldb::TraceSP
+  llvm::Expected
   CreateTraceIntelPTInstance(JSONTraceSession &session,
  std::vector &parsed_processes);
 
@@ -64,6 +64,11 @@
   lldb::ThreadPostMortemTraceSP ParseThread(lldb::ProcessSP &process_sp,
 const JSONThread &thread);
 
+  /// Create the corresponding Threads and Process objects given the JSON
+  /// process definition.
+  ///
+  /// \param[in] process
+  ///   The JSON process definition
   llvm::Expected ParseProcess(const JSONProcess &process);
 
   llvm::Error ParseModule(lldb::TargetSP &target_

[Lldb-commits] [PATCH] D127456: [trace][intelpt] Support system-wide tracing [17] - Some improvements

[Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG03cc58ff2a7a: [trace][intelpt] Support system-wide tracing 
[17] - Some improvements (authored by Walter Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127456

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Perf.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Target/Trace.cpp
  lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
  
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json

Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json
===
--- lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json
@@ -1,14 +1,14 @@
 {
   "cores": [
 {
-  "contextSwitchTrace": "/tmp/trace8/cores/45.perf_context_switch_trace",
+  "contextSwitchTrace": "cores/45.perf_context_switch_trace",
   "coreId": 45,
-  "traceBuffer": "/tmp/trace8/cores/45.intelpt_trace"
+  "traceBuffer": "cores/45.intelpt_trace"
 },
 {
-  "contextSwitchTrace": "/tmp/trace8/cores/51.perf_context_switch_trace",
+  "contextSwitchTrace": "cores/51.perf_context_switch_trace",
   "coreId": 51,
-  "traceBuffer": "/tmp/trace8/cores/51.intelpt_trace"
+  "traceBuffer": "cores/51.intelpt_trace"
 }
   ],
   "cpuInfo": {
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
===
--- lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
@@ -1,14 +1,14 @@
 {
   "cores": [
 {
-  "contextSwitchTrace": "/tmp/trace8/cores/45.perf_context_switch_trace",
+  "contextSwitchTrace": "cores/45.perf_context_switch_trace",
   "coreId": 45,
-  "traceBuffer": "/tmp/trace8/cores/45.intelpt_trace"
+  "traceBuffer": "cores/45.intelpt_trace"
 },
 {
-  "contextSwitchTrace": "/tmp/trace8/cores/51.perf_context_switch_trace",
+  "contextSwitchTrace": "cores/51.perf_context_switch_trace",
   "coreId": 51,
-  "traceBuffer": "/tmp/trace8/cores/51.intelpt_trace"
+  "traceBuffer": "cores/51.intelpt_trace"
 }
   ],
   "cpuInfo": {
Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -116,8 +116,9 @@
 
 Optional Trace::GetLiveThreadBinaryDataSize(lldb::tid_t tid,
   llvm::StringRef kind) {
-  auto it = m_live_thread_data.find(tid);
-  if (it == m_live_thread_data.end())
+  Storage &storage = GetUpdatedStorage();
+  auto it = storage.live_thread_data.find(tid);
+  if (it == storage.live_thread_data.end())
 return None;
   std::unordered_map &single_thread_data = it->second;
   auto single_thread_data_it = single_thread_data.find(kind.str());
@@ -128,8 +129,9 @@
 
 Optional Trace::GetLiveCoreBinaryDataSize(lldb::core_id_t core_id,
 llvm::StringRef kind) {
-  auto it = m_live_core_data_sizes.find(core_id);
-  if (it == m_live_core_data_sizes.end())
+  Storage &storage = GetUpdatedStorage();
+  auto it = storage.live_core_data_sizes.find(core_id);
+  if (it == storage.live_core_data_sizes.end())
 return None;
   std::unordered_map &single_core_data = it->second;
   auto single_thread_data_it = single_core_data.find(kind.str());
@@ -139,8 +141,9 @@
 }
 
 Optional Trace::GetLiveProcessBinaryDataSize(llvm::StringRef kind) {
-  auto data_it = m_live_process_data.find(kind.str

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/Utility/Log.h:54
+  virtual void Flush() = 0;
+  std::mutex &GetMutex() { return m_mutex; }
+

Maybe we don't expose this mutex at all and we have a EmitThreadSafe(...) 
method?:
```
void EmitThreadSafe(llvm::StringRef message) {
  std::lock_guard guard(m_mutex);
  Emit(message);
}
```
  



Comment at: lldb/source/Utility/Log.cpp:323-324
   if (options.Test(LLDB_LOG_OPTION_THREADSAFE)) {
-static std::recursive_mutex g_LogThreadedMutex;
-std::lock_guard guard(g_LogThreadedMutex);
-*stream_sp << message;
-stream_sp->flush();
+std::lock_guard guard(handler_sp->GetMutex());
+handler_sp->Emit(message);
+handler_sp->Flush();

If we like the idea of EmitThreadSafe then the code can look like this



Comment at: lldb/source/Utility/Log.cpp:325
+handler_sp->Emit(message);
+handler_sp->Flush();
   } else {

Do we ever not call Flush() after a Emit() call? If so do we even need the 
flush call, or should we just leave it up to the specific implementation to do 
what it wants in Emit(...)?



Comment at: lldb/source/Utility/Log.cpp:328
+handler_sp->Emit(message);
+handler_sp->Flush();
   }

Do we ever not call Flush() after a Emit() call? If so do we even need the 
flush call, or should we just leave it up to the specific implementation to do 
what it wants in Emit(...)?


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

https://reviews.llvm.org/D127922

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

[Lldb-commits] [PATCH] D127997: Remember whether all owners of the site we hit were internal in StopInfoBreakpoint

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: JDevlieghere, jasonmolenda.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

We noticed several test suite failures (e.g. TestStopAtEntry.py) with the new 
MacOS dyld support that was recently added when run on the macOS Ventura beta.  
The reason was that he was an unexpected breakpoint stop event coming from the 
new three-step dance needed to follow dyld's loading into the process.  At the 
first two stops, we delete the instrumentation breakpoint we just hit in the 
callback, and add another.  Since these were internal breakpoints the 
ThreadPlanBase::ShouldNotify should have returned eVoteNo.  But that logic 
requires knowing that all the owners of the breakpoint site were internal, and 
by the time we got to asking the thread plan the breakpoint site had been 
removed, so we no longer knew that.

The only time you know no one could have deleted the site you hit is when you 
make the StopInfoBreakpoint for the stop.  So this change adds an ivar to 
record whether all the breakpoints at this site were internal at that point, 
and uses that in the "ShouldNotify" calculation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127997

Files:
  lldb/source/Target/StopInfo.cpp


Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -88,7 +88,7 @@
   : StopInfo(thread, break_id), m_should_stop(false),
 m_should_stop_is_valid(false), m_should_perform_action(true),
 m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
-m_was_one_shot(false) {
+m_was_all_internal(false), m_was_one_shot(false) {
 StoreBPInfo();
   }
 
@@ -96,7 +96,7 @@
   : StopInfo(thread, break_id), m_should_stop(should_stop),
 m_should_stop_is_valid(true), m_should_perform_action(true),
 m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
-m_was_one_shot(false) {
+m_was_all_internal(false), m_was_one_shot(false) {
 StoreBPInfo();
   }
 
@@ -108,11 +108,22 @@
   BreakpointSiteSP bp_site_sp(
   thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
   if (bp_site_sp) {
-if (bp_site_sp->GetNumberOfOwners() == 1) {
+uint32_t num_owners = bp_site_sp->GetNumberOfOwners();
+if (num_owners == 1) {
   BreakpointLocationSP bp_loc_sp = bp_site_sp->GetOwnerAtIndex(0);
   if (bp_loc_sp) {
-m_break_id = bp_loc_sp->GetBreakpoint().GetID();
-m_was_one_shot = bp_loc_sp->GetBreakpoint().IsOneShot();
+Breakpoint & bkpt = bp_loc_sp->GetBreakpoint();
+m_break_id = bkpt.GetID();
+m_was_one_shot = bkpt.IsOneShot();
+m_was_all_internal = bkpt.IsInternal();
+  }
+} else {
+  m_was_all_internal = true;
+  for (uint32_t i = 0; i < num_owners; i++) {
+if (!bp_site_sp->GetOwnerAtIndex(i)->GetBreakpoint().IsInternal()) 
{
+  m_was_all_internal = false;
+  break;
+}
   }
 }
 m_address = bp_site_sp->GetLoadAddress();
@@ -163,23 +174,7 @@
   }
 
   bool DoShouldNotify(Event *event_ptr) override {
-ThreadSP thread_sp(m_thread_wp.lock());
-if (thread_sp) {
-  BreakpointSiteSP bp_site_sp(
-  thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
-  if (bp_site_sp) {
-bool all_internal = true;
-
-for (uint32_t i = 0; i < bp_site_sp->GetNumberOfOwners(); i++) {
-  if (!bp_site_sp->GetOwnerAtIndex(i)->GetBreakpoint().IsInternal()) {
-all_internal = false;
-break;
-  }
-}
-return !all_internal;
-  }
-}
-return true;
+return !m_was_all_internal;
   }
 
   const char *GetDescription() override {
@@ -603,6 +598,7 @@
   // in case somebody deletes it between the time the StopInfo is made and the
   // description is asked for.
   lldb::break_id_t m_break_id;
+  bool m_was_all_internal;
   bool m_was_one_shot;
 };
 


Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -88,7 +88,7 @@
   : StopInfo(thread, break_id), m_should_stop(false),
 m_should_stop_is_valid(false), m_should_perform_action(true),
 m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
-m_was_one_shot(false) {
+m_was_all_internal(false), m_was_one_shot(false) {
 StoreBPInfo();
   }
 
@@ -96,7 +96,7 @@
   : StopInfo(thread, break_id), m_should_stop(should_stop),
 m_should_stop_is_valid(true), m_should_perform_action(true),
 m_address(LLDB_INVALID_ADDRESS), 

[Lldb-commits] [PATCH] D127997: Remember whether all owners of the site we hit were internal in StopInfoBreakpoint

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Neat!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127997

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

[Lldb-commits] [lldb] 561a61f - [trace][intelpt] Support system-wide tracing [18] - some more improvements

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-16T11:42:21-07:00
New Revision: 561a61fb261bd3642f3c6187cf9c334502cac17f

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

LOG: [trace][intelpt] Support system-wide tracing [18] - some more improvements

This applies the changes requested for diff 12.

- use DenseMap instead of std::unordered_map, 
which is more idiomatic and possibly performant.
- deduplicate some code in Trace.cpp by using helper functions for fetching in 
maps
- stop using size and offset when fetching binary data, because we in fact read 
the entire buffers all the time. If we ever need streaming, we can implement it 
then. Now, the size is used only to check that we are getting the correct 
amount of data. This is useful because in some cases determining the size 
doesn't involve fetching the actual data.
- added back the x86_64 macro to the perf tests
- added more documentation
- simplified some file handling
- fixed some comments

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

Added: 


Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Utility/TraceGDBRemotePackets.h
lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp
lldb/source/Plugins/Process/Linux/Perf.cpp
lldb/source/Plugins/Process/Linux/Perf.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
lldb/source/Target/Trace.cpp
lldb/source/Utility/TraceGDBRemotePackets.cpp

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index e62707bd7db4d..4a06964c30cab 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -562,10 +562,6 @@ read packet: {...object}/E;A
 //   Core id in decimal if the data belongs to a CPU core.
 //   "tid"?: ,
 //   Tid in decimal if the data belongs to a thread.
-//   "offset": ,
-//   Offset of the data in bytes.
-//   "size": ,
-//  Number of bytes in to read starting from the offset.
 //  }
 //
 // INTEL PT

diff  --git a/lldb/include/lldb/Target/Trace.h 
b/lldb/include/lldb/Target/Trace.h
index 2cd4d80ba206e..63d8443715931 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -327,6 +327,12 @@ class Trace : public PluginInterface,
   /// If it's not a live process session, return an empty list.
   llvm::ArrayRef GetPostMortemProcesses();
 
+  /// Dispatcher for live trace data requests with some additional error
+  /// checking.
+  llvm::Expected>
+  GetLiveTraceBinaryData(const TraceGetBinaryDataRequest &request,
+ uint64_t expected_size);
+
   /// Implementation of \a OnThreadBinaryDataRead() for live threads.
   llvm::Error OnLiveThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
  OnBinaryDataReadCallback callback);
@@ -532,19 +538,19 @@ class Trace : public PluginInterface,
 /// \{
 
 /// tid -> data kind -> size
-llvm::DenseMap>
+llvm::DenseMap>
 live_thread_data;
 
 /// core id -> data kind -> size
-llvm::DenseMap>
+llvm::DenseMap>
 live_core_data_sizes;
 /// core id -> data kind -> bytes
 llvm::DenseMap>>
+   llvm::DenseMap>>
 live_core_data;
 
 /// data kind -> size
-std::unordered_map live_process_data;
+llvm::DenseMap live_process_data;
 /// \}
 
 /// The list of cores being traced. Might be \b None depending on the
@@ -556,11 +562,11 @@ class Trace : public PluginInterface,
 /// \{
 
 /// tid -> data kind -> file
-llvm::DenseMap>
+llvm::DenseMap>
 postmortem_thread_data;
 
 /// core id -> data kind -> file
-llvm::DenseMap>
+llvm::DenseMap>
 postmortem_core_data;
 
 /// \}

diff  --git a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h 
b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
index b45f1568a48dc..44f566190e3a6 100644
--- a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
@@ -155,10 +155,6 @@ struct TraceGetBinaryDataRequest {
   llvm::Optional tid;
   /// Optional core id if the data is related to a cpu core.
   llvm::Optional core_id;
-  /// Offset in 

[Lldb-commits] [PATCH] D127752: [trace][intelpt] Support system-wide tracing [18] - some more improvements

[Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG561a61fb261b: [trace][intelpt] Support system-wide tracing 
[18] - some more improvements (authored by Walter Erquinigo 
).

Changed prior to commit:
  https://reviews.llvm.org/D127752?vs=436794&id=437639#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127752

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/TraceGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Perf.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Target/Trace.cpp
  lldb/source/Utility/TraceGDBRemotePackets.cpp

Index: lldb/source/Utility/TraceGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceGDBRemotePackets.cpp
@@ -121,8 +121,8 @@
   json::Path path) {
   ObjectMapper o(value, path);
   uint64_t core_id;
-  if (!o || !o.map("coreId", core_id) ||
-  !o.map("binaryData", packet.binary_data))
+  if (!(o && o.map("coreId", core_id) &&
+o.map("binaryData", packet.binary_data)))
 return false;
   packet.core_id = static_cast(core_id);
   return true;
@@ -139,19 +139,16 @@
 json::Value toJSON(const TraceGetBinaryDataRequest &packet) {
   return json::Value(Object{{"type", packet.type},
 {"kind", packet.kind},
-{"offset", packet.offset},
 {"tid", packet.tid},
-{"coreId", packet.core_id},
-{"size", packet.size}});
+{"coreId", packet.core_id}});
 }
 
 bool fromJSON(const json::Value &value, TraceGetBinaryDataRequest &packet,
   Path path) {
   ObjectMapper o(value, path);
   Optional core_id;
-  if (!o || !o.map("type", packet.type) || !o.map("kind", packet.kind) ||
-  !o.map("tid", packet.tid) || !o.map("offset", packet.offset) ||
-  !o.map("size", packet.size) || !o.map("coreId", core_id))
+  if (!(o && o.map("type", packet.type) && o.map("kind", packet.kind) &&
+o.map("tid", packet.tid) && o.map("coreId", core_id)))
 return false;
 
   if (core_id)
Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -42,6 +42,44 @@
 } // namespace json
 } // namespace llvm
 
+/// Helper functions for fetching data in maps and returning Optionals or
+/// pointers instead of iterators for simplicity. It's worth mentioning that the
+/// Optionals version can't return the inner data by reference because of
+/// limitations in move constructors.
+/// \{
+template 
+static Optional Lookup(DenseMap &map, K k) {
+  auto it = map.find(k);
+  if (it == map.end())
+return None;
+  return it->second;
+}
+
+template 
+static V *LookupAsPtr(DenseMap &map, K k) {
+  auto it = map.find(k);
+  if (it == map.end())
+return nullptr;
+  return &it->second;
+}
+
+template 
+static Optional Lookup2(DenseMap> &map, K1 k1, K2 k2) {
+  auto it = map.find(k1);
+  if (it == map.end())
+return None;
+  return Lookup(it->second, k2);
+}
+
+template 
+static V *Lookup2AsPtr(DenseMap> &map, K1 k1, K2 k2) {
+  auto it = map.find(k1);
+  if (it == map.end())
+return nullptr;
+  return LookupAsPtr(it->second, k2);
+}
+/// \}
+
 static Error createInvalidPlugInError(StringRef plugin_name) {
   return createStringError(
   std::errc::invalid_argument,
@@ -88,71 +126,82 @@
 
 Error Trace::Start(const llvm::json::Value &request) {
   if (!m_live_process)
-return createStringError(inconvertibleErrorCode(),
- "Tracing requires a live process.");
+return createStringError(
+inconvertibleErrorCode(),
+"Attempted to start tracing without a live process.");
   return m_live_process->TraceStart(request);
 }
 
 Error Trace::Stop() {
   if (!m_live_process)
-return createStringError(inconvertibleErrorCode(),
- "Tracing requires a live process.");
+return createStringError(
+inconvertibleErrorC

[Lldb-commits] [lldb] 67c2405 - [trace][intelpt] Support system-wide tracing [19] - Some other minor improvements

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-16T11:42:21-07:00
New Revision: 67c24051450116c2c633e8a2740c08cf904e1605

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

LOG: [trace][intelpt] Support system-wide tracing [19] - Some other minor 
improvements

This addresses the issues in diffs [13], [14] and [16]

- Add better documentation
- Fix some castings by making them safer
- Simplify CorrelateContextSwitchesAndIntelPtTraces
- Rename some functions

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

Added: 


Modified: 
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.h
lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp 
b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
index 500e545e0c1e..d4d2a12fe537 100644
--- a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -97,9 +97,7 @@ class LibiptDecoder {
   ///   The status that was result of synchronizing to the most recent PSB.
   ///
   /// \param[in] stop_on_psb_change
-  ///   If \b true, decoding
-  ///   An optional offset to a given PSB. Decoding stops if a 
diff erent PSB is
-  ///   reached.
+  ///   If \b true, decoding stops if a 
diff erent PSB is reached.
   void DecodeInstructionsAndEvents(int status,
bool stop_on_psb_change = false) {
 uint64_t psb_offset;
@@ -310,7 +308,7 @@ static Error SetupMemoryImage(PtInsnDecoderUP &decoder_up, 
Process &process) {
   return Error::success();
 }
 
-void lldb_private::trace_intel_pt::DecodeTrace(DecodedThread &decoded_thread,
+void lldb_private::trace_intel_pt::DecodeSingleTraceForThread(DecodedThread 
&decoded_thread,
TraceIntelPT &trace_intel_pt,
ArrayRef buffer) {
   Expected decoder_up =
@@ -326,7 +324,7 @@ void 
lldb_private::trace_intel_pt::DecodeTrace(DecodedThread &decoded_thread,
   libipt_decoder.DecodeUntilEndOfTrace();
 }
 
-void lldb_private::trace_intel_pt::DecodeTrace(
+void lldb_private::trace_intel_pt::DecodeSystemWideTraceForThread(
 DecodedThread &decoded_thread, TraceIntelPT &trace_intel_pt,
 const DenseMap> &buffers,
 const std::vector &executions) {
@@ -438,8 +436,8 @@ 
lldb_private::trace_intel_pt::SplitTraceInContinuousExecutions(
 &psb_offset); // this can't fail because we got 
here
 
 executions.push_back({
-tsc,
 psb_offset,
+tsc,
 });
   }
   return executions;

diff  --git a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h 
b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
index ffef5c6fcf8f..127e54713770 100644
--- a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
+++ b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
@@ -18,9 +18,13 @@
 namespace lldb_private {
 namespace trace_intel_pt {
 
+/// This struct represents a point in the intel pt trace that the decoder can 
start decoding from without errors.
 struct IntelPTThreadSubtrace {
-  uint64_t tsc;
+  /// The memory offset of a PSB packet that is a synchronization point for 
the decoder. A decoder normally looks first
+  /// for a PSB packet and then it starts decoding.
   uint64_t psb_offset;
+  /// The timestamp associated with the PSB packet above.
+  uint64_t tsc;
 };
 
 /// This struct represents a continuous execution of a thread in a core,
@@ -38,17 +42,43 @@ struct IntelPTThreadContinousExecution {
   bool operator<(const IntelPTThreadContinousExecution &o) const;
 };
 
-/// Decode a raw Intel PT trace given in \p buffer and append the decoded
+/// Decode a raw Intel PT trace for a single thread given in \p buffer and 
append the decoded
 /// instructions and errors in \p decoded_thread. It uses the low level libipt
 /// library underneath.
-void DecodeTrace(DecodedThread &decoded_thread, TraceIntelPT &trace_intel_pt,
+void DecodeSingleTraceForThread(DecodedThread &decoded_thread, TraceIntelPT 
&trace_intel_pt,
  llvm::ArrayRef buffer);
 
-void DecodeTrace(
+/// Decode a raw Intel PT trace for a single thread that was collected in a 
per cpu core basis.
+///
+/// \param[in] decoded_thread
+///   All decoded instructions, errors and events will be appended to this 
object.
+///
+/// \param[in] trace

[Lldb-commits] [PATCH] D127804: [trace][intelpt] Support system-wide tracing [19] - Some other minor improvements

[Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG67c240514501: [trace][intelpt] Support system-wide tracing 
[19] - Some other minor… (authored by Walter Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127804

Files:
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
  lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.h
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp

Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
@@ -171,7 +171,8 @@
   // A list of known threads for the given process. When context switch
   // data is provided, LLDB will automatically create threads for the
   // this process whenever it finds new threads when traversing the
-  // context switches.
+  // context switches, so passing values to this list in this case is
+  // optional.
 {
   "tid": integer,
   "traceBuffer"?: string
@@ -213,10 +214,6 @@
 "timeShift": integer,
 "timeZero": integer,
   }
-  "dontCreateThreadsFromContextSwitches"?: boolean,
-// If this is true, then the automatic creation of threads from context switch
-// data is disabled, and thus only the threads provided in the "processes.threads"
-// section will be created.
 }
 
 Notes:
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
@@ -52,15 +52,19 @@
   size_t GetTotalContinuousExecutionsCount() const;
 
 private:
-  /// Traverse the context switch traces and recover the continuous executions
-  /// by thread.
-  llvm::Error DecodeContextSwitchTraces();
+  /// Traverse the context switch traces and the basic intel pt continuous subtraces
+  /// and produce a list of continuous executions for each process and thread.
+  ///
+  /// See \a DoCorrelateContextSwitchesAndIntelPtTraces.
+  ///
+  /// Any errors are stored in \a m_setup_error.
+  llvm::Error CorrelateContextSwitchesAndIntelPtTraces();
 
   /// Produce a mapping from thread ids to the list of continuos executions with
   /// their associated intel pt subtraces.
   llvm::Expected<
   llvm::DenseMap>>
-  CorrelateContextSwitchesAndIntelPtTraces();
+  DoCorrelateContextSwitchesAndIntelPtTraces();
 
   TraceIntelPT *m_trace;
   std::set m_tids;
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
@@ -31,7 +31,7 @@
 }
 
 DecodedThreadSP TraceIntelPTMultiCoreDecoder::Decode(Thread &thread) {
-  if (Error err = DecodeContextSwitchTraces())
+  if (Error err = CorrelateContextSwitchesAndIntelPtTraces())
 return std::make_shared(thread.shared_from_this(),
std::move(err));
   auto it = m_decoded_threads.find(thread.GetID());
@@ -43,10 +43,10 @@
 
   Error err = m_trace->OnAllCoresBinaryDataRead(
   IntelPTDataKinds::kTraceBuffer,
-  [&](const DenseMap> buffers) -> Error {
+  [&](const DenseMap>& buffers) -> Error {
 auto it = m_continuous_executions_per_thread->find(thread.GetID());
 if (it != m_continuous_executions_per_thread->end())
-  DecodeTrace(*decoded_thread_sp, *m_trace, buffers, it->second);
+  DecodeSystemWideTraceForThread(*decoded_thread_sp, *m_trace, buffers, it->second);
 
 return Error::success();
   });
@@ -57,10 +57,29 @@
   return decoded_thread_sp;
 }
 
-llvm::Expected<
-llvm::DenseMap>>
-TraceIntelPTMultiCoreDecoder::CorrelateContextSwitchesAndIntelPtTraces() {
-  llvm::DenseMap>
+static Expected>
+GetIntelPTSubtracesForCore(TraceIntelPT &trace, core_id_t core_id) {
+  std::vector intel_pt_subtraces;
+  Error err = trace.OnCoreBinaryDataRead(
+  core_id, IntelPTDataKinds::kTraceBuffer,
+  [&](ArrayRef data) -> Error {
+Expected> split_trace =
+SplitTraceInContinuousExecutions(trace, data);
+if (!split_trace)
+   

[Lldb-commits] [lldb] ea37cd5 - [trace][intelpt] Support system-wide tracing [22] - Some final touches

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-16T11:42:22-07:00
New Revision: ea37cd52d147a5ee621418e65db93a3e6102ab89

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

LOG: [trace][intelpt] Support system-wide tracing [22] - Some final touches

Having a member variable TraceIntelPT * makes it look as if it was
optional. I'm using instead a weak_ptr to indicate that it's not
optional and the object is under the ownership of TraceIntelPT.

Besides that, I've simplified the Perf aux and data buffers copying by
using vector.insert.

I'm also renaming Lookup2 to Lookup. The 2 in the name is confusing.

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

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/Perf.cpp
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/source/Plugins/Trace/intel-pt/forward-declarations.h
lldb/source/Target/Trace.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/Perf.cpp 
b/lldb/source/Plugins/Process/Linux/Perf.cpp
index 39ea92f49785..77786664aea4 100644
--- a/lldb/source/Plugins/Process/Linux/Perf.cpp
+++ b/lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -205,15 +205,13 @@ Expected> 
PerfEvent::GetReadOnlyDataBuffer() {
 
   if (data_head > data_size) {
 uint64_t actual_data_head = data_head % data_size;
-// The buffer has wrapped
-for (uint64_t i = actual_data_head; i < data_size; i++)
-  output.push_back(data[i]);
-
-for (uint64_t i = 0; i < actual_data_head; i++)
-  output.push_back(data[i]);
+// The buffer has wrapped, so we first the oldest chunk of data
+output.insert(output.end(), data.begin() + actual_data_head, data.end());
+// And we we read the most recent chunk of data
+output.insert(output.end(), data.begin(), data.begin() + actual_data_head);
   } else {
-for (uint64_t i = 0; i < data_head; i++)
-  output.push_back(data[i]);
+// There's been no wrapping, so we just read linearly
+output.insert(output.end(), data.begin(), data.begin() + data_head);
   }
 
   if (was_enabled) {
@@ -238,7 +236,6 @@ Expected> 
PerfEvent::GetReadOnlyAuxBuffer() {
 
   ArrayRef data = GetAuxBuffer();
   uint64_t aux_head = mmap_metadata.aux_head;
-  uint64_t aux_size = mmap_metadata.aux_size;
   std::vector output;
   output.reserve(data.size());
 
@@ -254,11 +251,8 @@ Expected> 
PerfEvent::GetReadOnlyAuxBuffer() {
*
* */
 
-  for (uint64_t i = aux_head; i < aux_size; i++)
-output.push_back(data[i]);
-
-  for (uint64_t i = 0; i < aux_head; i++)
-output.push_back(data[i]);
+  output.insert(output.end(), data.begin() + aux_head, data.end());
+  output.insert(output.end(), data.begin(), data.begin() + aux_head);
 
   if (was_enabled) {
 if (Error err = EnableWithIoctl())

diff  --git a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h 
b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
index f85e0db6f514..5420c030b8bd 100644
--- a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
+++ b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
@@ -48,21 +48,24 @@ struct IntelPTThreadContinousExecution {
 void DecodeSingleTraceForThread(DecodedThread &decoded_thread, TraceIntelPT 
&trace_intel_pt,
  llvm::ArrayRef buffer);
 
-/// Decode a raw Intel PT trace for a single thread that was collected in a 
per cpu core basis.
+/// Decode a raw Intel PT trace for a single thread that was collected in a per
+/// cpu core basis.
 ///
-/// \param[in] decoded_thread
-///   All decoded instructions, errors and events will be appended to this 
object.
+/// \param[out] decoded_thread
+///   All decoded instructions, errors and events will be appended to this
+///   object.
 ///
 /// \param[in] trace_intel_pt
-///   The main Trace object that contains all the information related to the 
trace session.
+///   The main Trace object that contains all the information related to the
+///   trace session.
 ///
 /// \param[in] buffers
 ///   A map from cpu core id to raw intel pt buffers.
 ///
 /// \param[in] executions
-///   A list of chunks of timed executions of the same given thread. It is 
used to identify if
-///   some executions have missing intel pt data and also to determine in 
which core a certain
-///   part of the execution ocurred.
+///   A list of chunks of timed executions of the same given thread. It is used
+///   to identify if some executions have missing intel pt data and also to
+///   determine in which core a certain part of 

[Lldb-commits] [lldb] 9f45f23 - [trace][intelpt] Support system-wide tracing [21] - Support long numbers in JSON

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-16T11:42:22-07:00
New Revision: 9f45f23d860251edceed6128110855c51af4f39f

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

LOG: [trace][intelpt] Support system-wide tracing [21] - Support long numbers 
in JSON

llvm's JSON parser supports 64 bit integers, but other tools like the
ones written in JS don't support numbers that big, so we need to
represent these possibly big numbers as a string. This diff uses that to
represent addresses and tsc zero. The former is printed in hex for and
the latter in decimal string form. The schema was updated mentioning
that.

Besides that, I fixed some remaining issues and now all test pass. Before I 
wasn't running all tests because for some reason my computer reverted 
perf_paranoid to 1.

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

Added: 

lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_with_string_numbers.json

Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
lldb/source/Plugins/Process/Linux/Perf.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
lldb/test/API/commands/trace/TestTraceLoad.py

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 4f0cfa4585b0..d0f069a5c7a9 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -299,14 +299,14 @@ read packet: {"name":, 
"description":}/E;
 //  intel-pt supports both "thread tracing" and "process tracing".
 //
 //  "Process tracing" is implemented in two 
diff erent ways. If the
-//  "perCoreTracing" option is false, then each thread is traced individually
+//  "perCpuTracing" option is false, then each thread is traced individually
 //  but managed by the same "process trace" instance. This means that the
 //  amount of trace buffers used is proportional to the number of running
 //  threads. This is the recommended option unless the number of threads is
-//  huge. If "perCoreTracing" is true, then each cpu core is traced invidually
+//  huge. If "perCpuTracing" is true, then each cpu core is traced invidually
 //  instead of each thread, which uses a fixed number of trace buffers, but
 //  might result in less data available for less frequent threads. See
-//  "perCoreTracing" below for more information.
+//  "perCpuTracing" below for more information.
 //
 //  Each actual intel pt trace buffer, either from "process tracing" or "thread
 //  tracing", is stored in an in-memory circular buffer, which keeps the most
@@ -352,7 +352,7 @@ read packet: {"name":, 
"description":}/E;
 //  0 if supported.
 //
 // /* process tracing only */
-// "perCoreTracing": 
+// "perCpuTracing": 
 // Instead of having an individual trace buffer per thread, this option
 // triggers the collection on a per cpu core basis. This effectively
 // traces the entire activity on all cores. At decoding time, in order
@@ -374,13 +374,13 @@ read packet: {"name":, 
"description":}/E;
 // buffers for the current process, excluding the ones started with
 // "thread tracing".
 //
-// If "perCoreTracing" is false, whenever a thread is attempted to be
+// If "perCpuTracing" is false, whenever a thread is attempted to be
 // traced due to "process tracing" and the limit would be reached, the
 // process is stopped with a "tracing" reason along with a meaningful
 // description, so that the user can retrace the process if needed.
 //
-// If "perCoreTracing" is true, then starting the system-wide trace
-// session fails if all the individual per-core trace buffers require
+// If "perCpuTracing" is true, then starting the system-wide trace
+// session fails if all the individual per-cpu trace buffers require
 // in total more memory that the limit impossed by this parameter.
 //   }
 //

diff  --git a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h 
b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
index 6a9fde7050ee..36b594613a91 100644
--- a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
@@ -59,6 +59,21 @@ bool fromJSON(const llvm::json::Value &value, 
TraceIntelPTStartRequest &packet,
 llvm:

[Lldb-commits] [PATCH] D127819: [trace][intelpt] Support system-wide tracing [21] - Support long numbers in JSON

[Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9f45f23d8602: [trace][intelpt] Support system-wide tracing 
[21] - Support long numbers in JSON (authored by Walter Erquinigo 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127819

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  lldb/test/API/commands/trace/TestTraceLoad.py
  
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_with_string_numbers.json

Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_with_string_numbers.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_with_string_numbers.json
@@ -0,0 +1,50 @@
+{
+  "cpus": [
+{
+  "contextSwitchTrace": "cores/45.perf_context_switch_trace",
+  "id": 45,
+  "iptTrace": "cores/45.intelpt_trace"
+},
+{
+  "contextSwitchTrace": "cores/51.perf_context_switch_trace",
+  "id": 51,
+  "iptTrace": "cores/51.intelpt_trace"
+}
+  ],
+  "cpuInfo": {
+"family": 6,
+"model": 85,
+"stepping": 4,
+"vendor": "GenuineIntel"
+  },
+  "processes": [
+{
+  "modules": [
+{
+  "file": "modules/m.out",
+  "systemPath": "/tmp/m.out",
+  "loadAddress": "4194304",
+  "uuid": "AEFB0D59-233F-80FF-6D3C-4DED498534CF-11017B3B"
+}
+  ],
+  "pid": 3497234,
+  "threads": [
+{
+  "tid": 3497234
+},
+{
+  "tid": 3497496
+},
+{
+  "tid": 3497497
+}
+  ]
+}
+  ],
+  "tscPerfZeroConversion": {
+"timeMult": 1076264588,
+"timeShift": 31,
+"timeZero": "18433473881008870804"
+  },
+  "type": "intel-pt"
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -21,6 +21,18 @@
   substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl   $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
+def testLoadMultiCoreTraceWithStringNumbers(self):
+src_dir = self.getSourceDir()
+trace_definition_file = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_with_string_numbers.json")
+self.expect("trace load -v " + trace_definition_file, substrs=["intel-pt"])
+self.expect("thread trace dump instructions 2 -t",
+  substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+   "m.out`foo() + 65 at multi_thread.cpp:12:21",
+   "19520: [tsc=0x008fb5211bfbc69e] 0x00400ba7jg 0x400bb3"])
+self.expect("thread trace dump instructions 3 -t",
+  substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl   $0x1, -0x4(%rbp)",
+   "m.out`bar() + 26 at multi_thread.cpp:20:6"])
+
 def testLoadMultiCoreTraceWithMissingThreads(self):
 src_dir = self.getSourceDir()
 trace_definition_file = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_missing_threads.json")
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -22,13 +22,33 @@
   return per_cpu_tracing.getValueOr(false);
 }
 
+json::Value toJSON(const JSONUINT64 &uint64, bool hex) {
+  if (hex)
+return json::Value(formatv("{0:x+}", uint64.value));
+  else
+return json::Value(formatv("{0}", uint64.value));
+}
+
+bool fromJSON(const json::Value &value, JSONUINT64 &uint64, Path path) {
+  if (Optional val = value.getAsUINT64()) {
+uint64.value = *val;
+return true;
+  } else if (Optional val = value.getAsString()) {
+if (!val->getAsInteger(/*radix=*/0, uint64.value))
+  return true;
+path.report("invalid string number");
+  }
+  path.report("invalid number or string number");
+  return false;
+}
+
 bool fromJSON(const json::Value &value, TraceIntelPTStartRequest &packet,
   Path path) {
   ObjectMapper o(value, path);
-  if (!o || !fro

[Lldb-commits] [PATCH] D127881: [trace][intelpt] Support system-wide tracing [22] - Some final touches

[Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGea37cd52d147: [trace][intelpt] Support system-wide tracing 
[22] - Some final touches (authored by Walter Erquinigo ).

Changed prior to commit:
  https://reviews.llvm.org/D127881?vs=437247&id=437643#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127881

Files:
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/forward-declarations.h
  lldb/source/Target/Trace.cpp

Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -63,16 +63,18 @@
   return &it->second;
 }
 
+/// Similar to the methods above but it looks for an item in a map of maps.
 template 
-static Optional Lookup2(DenseMap> &map, K1 k1, K2 k2) {
+static Optional Lookup(DenseMap> &map, K1 k1, K2 k2) {
   auto it = map.find(k1);
   if (it == map.end())
 return None;
   return Lookup(it->second, k2);
 }
 
+/// Similar to the methods above but it looks for an item in a map of maps.
 template 
-static V *Lookup2AsPtr(DenseMap> &map, K1 k1, K2 k2) {
+static V *LookupAsPtr(DenseMap> &map, K1 k1, K2 k2) {
   auto it = map.find(k1);
   if (it == map.end())
 return nullptr;
@@ -159,13 +161,13 @@
 Optional Trace::GetLiveThreadBinaryDataSize(lldb::tid_t tid,
   llvm::StringRef kind) {
   Storage &storage = GetUpdatedStorage();
-  return Lookup2(storage.live_thread_data, tid, ConstString(kind));
+  return Lookup(storage.live_thread_data, tid, ConstString(kind));
 }
 
 Optional Trace::GetLiveCpuBinaryDataSize(lldb::cpu_id_t cpu_id,
llvm::StringRef kind) {
   Storage &storage = GetUpdatedStorage();
-  return Lookup2(storage.live_cpu_data_sizes, cpu_id, ConstString(kind));
+  return Lookup(storage.live_cpu_data_sizes, cpu_id, ConstString(kind));
 }
 
 Optional Trace::GetLiveProcessBinaryDataSize(llvm::StringRef kind) {
@@ -345,7 +347,7 @@
 Trace::GetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind) {
   Storage &storage = GetUpdatedStorage();
   if (Optional file =
-  Lookup2(storage.postmortem_thread_data, tid, ConstString(kind)))
+  Lookup(storage.postmortem_thread_data, tid, ConstString(kind)))
 return *file;
   else
 return createStringError(
@@ -358,7 +360,7 @@
  llvm::StringRef kind) {
   Storage &storage = GetUpdatedStorage();
   if (Optional file =
-  Lookup2(storage.postmortem_cpu_data, cpu_id, ConstString(kind)))
+  Lookup(storage.postmortem_cpu_data, cpu_id, ConstString(kind)))
 return *file;
   else
 return createStringError(
@@ -393,7 +395,7 @@
OnBinaryDataReadCallback callback) {
   Storage &storage = GetUpdatedStorage();
   if (std::vector *cpu_data =
-  Lookup2AsPtr(storage.live_cpu_data, cpu_id, ConstString(kind)))
+  LookupAsPtr(storage.live_cpu_data, cpu_id, ConstString(kind)))
 return callback(*cpu_data);
 
   Expected> data = GetLiveCpuBinaryData(cpu_id, kind);
Index: lldb/source/Plugins/Trace/intel-pt/forward-declarations.h
===
--- lldb/source/Plugins/Trace/intel-pt/forward-declarations.h
+++ lldb/source/Plugins/Trace/intel-pt/forward-declarations.h
@@ -9,12 +9,16 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_FORWARD_DECLARATIONS_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_FORWARD_DECLARATIONS_H
 
+#include 
+
 namespace lldb_private {
 namespace trace_intel_pt {
 
 class TraceIntelPT;
 class ThreadDecoder;
 
+using TraceIntelPTSP = std::shared_ptr;
+
 } // namespace trace_intel_pt
 } // namespace lldb_private
 #endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_FORWARD_DECLARATIONS_H
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
@@ -284,7 +284,8 @@
parsed_process.threads.end());
   }
 
-  TraceSP trace_instance(new TraceIntelPT(session, processes, threads));
+  TraceSP trace_instance = TraceIntelPT::CreateInstanceForPostmortemTrace(
+  session, processes, threads);
   for (c

[Lldb-commits] [PATCH] D127997: Remember whether all owners of the site we hit were internal in StopInfoBreakpoint

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Thanks for getting to the bottom of this one!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127997

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

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

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

- Add `EmitThreadSafe`
- Remove `Flush`


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

https://reviews.llvm.org/D127922

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/Log.h
  lldb/include/lldb/Utility/StreamCallback.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/Log.cpp
  lldb/source/Utility/StreamCallback.cpp
  lldb/tools/lldb-server/LLDBServerUtilities.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/StreamCallbackTest.cpp
  lldb/unittests/Utility/LogTest.cpp

Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -39,13 +39,13 @@
 } // namespace lldb_private
 
 // Wrap enable, disable and list functions to make them easier to test.
-static bool EnableChannel(std::shared_ptr stream_sp,
+static bool EnableChannel(std::shared_ptr log_handler_sp,
   uint32_t log_options, llvm::StringRef channel,
   llvm::ArrayRef categories,
   std::string &error) {
   error.clear();
   llvm::raw_string_ostream error_stream(error);
-  return Log::EnableLogChannel(stream_sp, log_options, channel, categories,
+  return Log::EnableLogChannel(log_handler_sp, log_options, channel, categories,
error_stream);
 }
 
@@ -78,18 +78,27 @@
   }
 };
 
+class TestLogHandler : public LogHandler {
+public:
+  TestLogHandler() : m_messages(), m_stream(m_messages) {}
+
+  void Emit(llvm::StringRef message) override { m_stream << message; }
+
+  llvm::SmallString<0> m_messages;
+  llvm::raw_svector_ostream m_stream;
+};
+
 // A test fixture which provides tests with a pre-registered and pre-enabled
 // channel. Additionally, the messages written to that channel are captured and
 // made available via getMessage().
 class LogChannelEnabledTest : public LogChannelTest {
-  llvm::SmallString<0> m_messages;
-  std::shared_ptr m_stream_sp =
-  std::make_shared(m_messages);
+  std::shared_ptr m_log_handler_sp =
+  std::make_shared();
   Log *m_log;
   size_t m_consumed_bytes = 0;
 
 protected:
-  std::shared_ptr getStream() { return m_stream_sp; }
+  std::shared_ptr getLogHandler() { return m_log_handler_sp; }
   Log *getLog() { return m_log; }
   llvm::StringRef takeOutput();
   llvm::StringRef logAndTakeOutput(llvm::StringRef Message);
@@ -103,14 +112,15 @@
   LogChannelTest::SetUp();
 
   std::string error;
-  ASSERT_TRUE(EnableChannel(m_stream_sp, 0, "chan", {}, error));
+  ASSERT_TRUE(EnableChannel(m_log_handler_sp, 0, "chan", {}, error));
 
   m_log = GetLog(TestChannel::FOO);
   ASSERT_NE(nullptr, m_log);
 }
 
 llvm::StringRef LogChannelEnabledTest::takeOutput() {
-  llvm::StringRef result = m_stream_sp->str().drop_front(m_consumed_bytes);
+  llvm::StringRef result =
+  m_log_handler_sp->m_stream.str().drop_front(m_consumed_bytes);
   m_consumed_bytes+= result.size();
   return result;
 }
@@ -138,9 +148,9 @@
   Log::Register("chan", test_channel);
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
-  std::shared_ptr stream_sp(
-  new llvm::raw_string_ostream(message));
-  EXPECT_TRUE(Log::EnableLogChannel(stream_sp, 0, "chan", {"foo"}, llvm::nulls()));
+  auto log_handler_sp = std::make_shared();
+  EXPECT_TRUE(
+  Log::EnableLogChannel(log_handler_sp, 0, "chan", {"foo"}, llvm::nulls()));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   Log::Unregister("chan");
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
@@ -149,22 +159,21 @@
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
-  std::shared_ptr stream_sp(
-  new llvm::raw_string_ostream(message));
+  auto log_handler_sp = std::make_shared();
   std::string error;
-  ASSERT_FALSE(EnableChannel(stream_sp, 0, "chanchan", {}, error));
+  ASSERT_FALSE(EnableChannel(log_handler_sp, 0, "chanchan", {}, error));
   EXPECT_EQ("Invalid log channel 'chanchan'.\n", error);
 
-  EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {}, error));
+  EXPECT_TRUE(EnableChannel(log_handler_sp, 0, "chan", {}, error));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   EXPECT_EQ(nullptr, GetLog(TestChannel::BAR));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO | TestChannel::BAR));
 
-  EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {"bar"}, error));
+  EXPECT_TRUE(EnableChannel(log_handler_sp, 0, "chan", {"bar"}, error));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   EXPECT_NE(nullptr, GetLog(TestChannel::BAR));
 
-  EXPECT_TRUE(EnableChannel(stream_sp, 0, "chan", {"baz"}, error));
+  EXPECT_TRUE(EnableChannel(log_handler_sp, 0, "chan", {"baz"}, error));
   EXPECT_NE(std::string::npos, error.find("unrecognized log category 'baz'"))

[Lldb-commits] [lldb] dae2faf - Fix TraceGDBRemotePacketsTest

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-16T11:53:13-07:00
New Revision: dae2fafe056c83a802c1f1a1baa3cd48416c06f7

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

LOG: Fix TraceGDBRemotePacketsTest

This test broke, but the fix is simple.

Added: 


Modified: 
lldb/unittests/Utility/TraceGDBRemotePacketsTest.cpp

Removed: 




diff  --git a/lldb/unittests/Utility/TraceGDBRemotePacketsTest.cpp 
b/lldb/unittests/Utility/TraceGDBRemotePacketsTest.cpp
index 47f55181ca38a..06063aa32ded2 100644
--- a/lldb/unittests/Utility/TraceGDBRemotePacketsTest.cpp
+++ b/lldb/unittests/Utility/TraceGDBRemotePacketsTest.cpp
@@ -38,7 +38,7 @@ TEST(TraceGDBRemotePacketsTest, IntelPTGetStateResponse) {
 
   // Create TraceIntelPTGetStateResponse.
   TraceIntelPTGetStateResponse response;
-  response.tsc_perf_zero_conversion = 
LinuxPerfZeroTscConversion{test_time_mult, test_time_shift, test_time_zero};
+  response.tsc_perf_zero_conversion = 
LinuxPerfZeroTscConversion{test_time_mult, test_time_shift, {test_time_zero}};
 
   // Serialize then deserialize.
   Expected deserialized_response =
@@ -56,9 +56,9 @@ TEST(TraceGDBRemotePacketsTest, IntelPTGetStateResponse) {
   const uint64_t EXPECTED_NANOS = 9223372039007304983u;
 
   uint64_t pre_serialization_conversion =
-  response.tsc_perf_zero_conversion->ToNanos(TSC).count();
+  response.tsc_perf_zero_conversion->ToNanos(TSC);
   uint64_t post_serialization_conversion =
-  deserialized_response->tsc_perf_zero_conversion->ToNanos(TSC).count();
+  deserialized_response->tsc_perf_zero_conversion->ToNanos(TSC);
 
   // Check equality:
   // Ensure that both the TraceGetStateResponse and 
TraceIntelPTGetStateResponse



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

[Lldb-commits] [lldb] f22db1f - Fix StopInfoBreakpoint::ShouldNotify when a callback deletes the site we hit.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-06-16T11:54:11-07:00
New Revision: f22db1fabfa165e9b18b1b49c78566d15f22bda1

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

LOG: Fix StopInfoBreakpoint::ShouldNotify when a callback deletes the site we 
hit.

When we hit a breakpoint site all of whose owners are internal, we don't
broadcast that event to the public event queue.  However, we were checking
whether that was true in the ShouldNotify method, which gets run after the
breakpoint callbacks get run.  If the breakpoint callback deletes the site
we just hit, we no longer have the information to make that determination.

This patch just gathers the "was all internal" fact when the StopInfoBreakpoint
gets made, which happens before anyone has a chance to delete the site, and then
uses that cached value.

This bug was causing a couple of tests (including TestStopAtEntry.py) to fail
when using new the macOS Ventura dyld support.

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

Added: 


Modified: 
lldb/source/Target/StopInfo.cpp

Removed: 




diff  --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 08c50ea5e62c..1ab2ad0d34f2 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -88,7 +88,7 @@ class StopInfoBreakpoint : public StopInfo {
   : StopInfo(thread, break_id), m_should_stop(false),
 m_should_stop_is_valid(false), m_should_perform_action(true),
 m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
-m_was_one_shot(false) {
+m_was_all_internal(false), m_was_one_shot(false) {
 StoreBPInfo();
   }
 
@@ -96,7 +96,7 @@ class StopInfoBreakpoint : public StopInfo {
   : StopInfo(thread, break_id), m_should_stop(should_stop),
 m_should_stop_is_valid(true), m_should_perform_action(true),
 m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
-m_was_one_shot(false) {
+m_was_all_internal(false), m_was_one_shot(false) {
 StoreBPInfo();
   }
 
@@ -108,11 +108,22 @@ class StopInfoBreakpoint : public StopInfo {
   BreakpointSiteSP bp_site_sp(
   thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
   if (bp_site_sp) {
-if (bp_site_sp->GetNumberOfOwners() == 1) {
+uint32_t num_owners = bp_site_sp->GetNumberOfOwners();
+if (num_owners == 1) {
   BreakpointLocationSP bp_loc_sp = bp_site_sp->GetOwnerAtIndex(0);
   if (bp_loc_sp) {
-m_break_id = bp_loc_sp->GetBreakpoint().GetID();
-m_was_one_shot = bp_loc_sp->GetBreakpoint().IsOneShot();
+Breakpoint & bkpt = bp_loc_sp->GetBreakpoint();
+m_break_id = bkpt.GetID();
+m_was_one_shot = bkpt.IsOneShot();
+m_was_all_internal = bkpt.IsInternal();
+  }
+} else {
+  m_was_all_internal = true;
+  for (uint32_t i = 0; i < num_owners; i++) {
+if (!bp_site_sp->GetOwnerAtIndex(i)->GetBreakpoint().IsInternal()) 
{
+  m_was_all_internal = false;
+  break;
+}
   }
 }
 m_address = bp_site_sp->GetLoadAddress();
@@ -163,23 +174,7 @@ class StopInfoBreakpoint : public StopInfo {
   }
 
   bool DoShouldNotify(Event *event_ptr) override {
-ThreadSP thread_sp(m_thread_wp.lock());
-if (thread_sp) {
-  BreakpointSiteSP bp_site_sp(
-  thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
-  if (bp_site_sp) {
-bool all_internal = true;
-
-for (uint32_t i = 0; i < bp_site_sp->GetNumberOfOwners(); i++) {
-  if (!bp_site_sp->GetOwnerAtIndex(i)->GetBreakpoint().IsInternal()) {
-all_internal = false;
-break;
-  }
-}
-return !all_internal;
-  }
-}
-return true;
+return !m_was_all_internal;
   }
 
   const char *GetDescription() override {
@@ -603,6 +598,7 @@ class StopInfoBreakpoint : public StopInfo {
   // in case somebody deletes it between the time the StopInfo is made and the
   // description is asked for.
   lldb::break_id_t m_break_id;
+  bool m_was_all_internal;
   bool m_was_one_shot;
 };
 



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

[Lldb-commits] [PATCH] D127997: Remember whether all owners of the site we hit were internal in StopInfoBreakpoint

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf22db1fabfa1: Fix StopInfoBreakpoint::ShouldNotify when a 
callback deletes the site we hit. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127997

Files:
  lldb/source/Target/StopInfo.cpp


Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -88,7 +88,7 @@
   : StopInfo(thread, break_id), m_should_stop(false),
 m_should_stop_is_valid(false), m_should_perform_action(true),
 m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
-m_was_one_shot(false) {
+m_was_all_internal(false), m_was_one_shot(false) {
 StoreBPInfo();
   }
 
@@ -96,7 +96,7 @@
   : StopInfo(thread, break_id), m_should_stop(should_stop),
 m_should_stop_is_valid(true), m_should_perform_action(true),
 m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
-m_was_one_shot(false) {
+m_was_all_internal(false), m_was_one_shot(false) {
 StoreBPInfo();
   }
 
@@ -108,11 +108,22 @@
   BreakpointSiteSP bp_site_sp(
   thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
   if (bp_site_sp) {
-if (bp_site_sp->GetNumberOfOwners() == 1) {
+uint32_t num_owners = bp_site_sp->GetNumberOfOwners();
+if (num_owners == 1) {
   BreakpointLocationSP bp_loc_sp = bp_site_sp->GetOwnerAtIndex(0);
   if (bp_loc_sp) {
-m_break_id = bp_loc_sp->GetBreakpoint().GetID();
-m_was_one_shot = bp_loc_sp->GetBreakpoint().IsOneShot();
+Breakpoint & bkpt = bp_loc_sp->GetBreakpoint();
+m_break_id = bkpt.GetID();
+m_was_one_shot = bkpt.IsOneShot();
+m_was_all_internal = bkpt.IsInternal();
+  }
+} else {
+  m_was_all_internal = true;
+  for (uint32_t i = 0; i < num_owners; i++) {
+if (!bp_site_sp->GetOwnerAtIndex(i)->GetBreakpoint().IsInternal()) 
{
+  m_was_all_internal = false;
+  break;
+}
   }
 }
 m_address = bp_site_sp->GetLoadAddress();
@@ -163,23 +174,7 @@
   }
 
   bool DoShouldNotify(Event *event_ptr) override {
-ThreadSP thread_sp(m_thread_wp.lock());
-if (thread_sp) {
-  BreakpointSiteSP bp_site_sp(
-  thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
-  if (bp_site_sp) {
-bool all_internal = true;
-
-for (uint32_t i = 0; i < bp_site_sp->GetNumberOfOwners(); i++) {
-  if (!bp_site_sp->GetOwnerAtIndex(i)->GetBreakpoint().IsInternal()) {
-all_internal = false;
-break;
-  }
-}
-return !all_internal;
-  }
-}
-return true;
+return !m_was_all_internal;
   }
 
   const char *GetDescription() override {
@@ -603,6 +598,7 @@
   // in case somebody deletes it between the time the StopInfo is made and the
   // description is asked for.
   lldb::break_id_t m_break_id;
+  bool m_was_all_internal;
   bool m_was_one_shot;
 };
 


Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -88,7 +88,7 @@
   : StopInfo(thread, break_id), m_should_stop(false),
 m_should_stop_is_valid(false), m_should_perform_action(true),
 m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
-m_was_one_shot(false) {
+m_was_all_internal(false), m_was_one_shot(false) {
 StoreBPInfo();
   }
 
@@ -96,7 +96,7 @@
   : StopInfo(thread, break_id), m_should_stop(should_stop),
 m_should_stop_is_valid(true), m_should_perform_action(true),
 m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID),
-m_was_one_shot(false) {
+m_was_all_internal(false), m_was_one_shot(false) {
 StoreBPInfo();
   }
 
@@ -108,11 +108,22 @@
   BreakpointSiteSP bp_site_sp(
   thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
   if (bp_site_sp) {
-if (bp_site_sp->GetNumberOfOwners() == 1) {
+uint32_t num_owners = bp_site_sp->GetNumberOfOwners();
+if (num_owners == 1) {
   BreakpointLocationSP bp_loc_sp = bp_site_sp->GetOwnerAtIndex(0);
   if (bp_loc_sp) {
-m_break_id = bp_loc_sp->GetBreakpoint().GetID();
-m_was_one_shot = bp_loc_sp->GetBreakpoint().IsOneShot();
+Breakpoint & bkpt = bp_loc_sp->GetBreakpoint();
+m_break_id = bkpt.GetID();
+m_was_one_shot = bkpt.IsOneShot();
+m_was_all_internal = bkpt.IsInternal();
+  }
+} else {
+  m_was_all_internal = tru

[Lldb-commits] [PATCH] D127999: [lldb] fix stepping through POSIX trampolines

[Michael Daniels via Phabricator via lldb-commits]

mdaniels created this revision.
mdaniels added reviewers: jingham, clayborg, labath.
Herald added a project: All.
mdaniels requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan() function was
doing the symbol lookup using the demangled name. This stopped working
with https://reviews.llvm.org/D118814. To get things working again, just
use the mangled name for the lookup instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127999

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -488,7 +488,7 @@
   if (sym == nullptr || !sym->IsTrampoline())
 return thread_plan_sp;
 
-  ConstString sym_name = sym->GetName();
+  ConstString sym_name = sym->GetMangled().GetName(Mangled::ePreferMangled);
   if (!sym_name)
 return thread_plan_sp;
 


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -488,7 +488,7 @@
   if (sym == nullptr || !sym->IsTrampoline())
 return thread_plan_sp;
 
-  ConstString sym_name = sym->GetName();
+  ConstString sym_name = sym->GetMangled().GetName(Mangled::ePreferMangled);
   if (!sym_name)
 return thread_plan_sp;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

[Greg Clayton via Phabricator via lldb-commits]

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

LGTM!


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

https://reviews.llvm.org/D127922

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

[Lldb-commits] [PATCH] D127999: [lldb] fix stepping through POSIX trampolines

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Is there a test that was failing with this? If not can we add one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127999

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

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I could also imagine having multiple options. For example

--buffered 

would specify to buffer the output and only flush if we go over  bytes. 
This could improve logging speeds.

--circular

would enable circular buffering where things are never flushed unless manually 
done with "log circular dump"

Then we need to manage these settings inside of each LogHandler instance since 
"--buffered" could work for StreamLogHandler and for CallbackLogHandler. I am 
just tossing out ideas here just in case any of them make sense as I am 
thinking about this patch.


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

https://reviews.llvm.org/D127986

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

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 437679.
JDevlieghere added a comment.

Implement Greg's suggestion of integrating this with `log enable`.


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

https://reviews.llvm.org/D127986

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/Log.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/Log.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -1120,7 +1120,7 @@
   /*add_to_history*/ eLazyBoolNo, Result);
 
   if (!opts::Log.empty())
-Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, errs());
+Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, errs());
 
   if (opts::BreakpointSubcommand)
 return opts::breakpoint::evaluateBreakpoints(*Dbg);
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -13,6 +13,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator.h"
 
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
@@ -106,6 +107,13 @@
   }
 }
 
+void Log::Dump(llvm::raw_ostream &output_stream) {
+  llvm::sys::ScopedReader lock(m_mutex);
+  if (RotatingLogHandler *handler =
+  llvm::dyn_cast(m_handler.get()))
+handler->Dump(output_stream);
+}
+
 const Flags Log::GetOptions() const {
   return m_options.load(std::memory_order_relaxed);
 }
@@ -222,6 +230,19 @@
   return true;
 }
 
+bool Log::DumpLogChannel(llvm::StringRef channel,
+ llvm::ArrayRef categories,
+ llvm::raw_ostream &output_stream,
+ llvm::raw_ostream &error_stream) {
+  auto iter = g_channel_map->find(channel);
+  if (iter == g_channel_map->end()) {
+error_stream << llvm::formatv("Invalid log channel '{0}'.\n", channel);
+return false;
+  }
+  iter->second.Dump(output_stream);
+  return true;
+}
+
 bool Log::ListChannelCategories(llvm::StringRef channel,
 llvm::raw_ostream &stream) {
   auto ch = g_channel_map->find(channel);
@@ -403,3 +424,8 @@
 std::shared_ptr RotatingLogHandler::Create(size_t size) {
   return std::make_shared(size);
 }
+
+char LogHandler::ID;
+char StreamLogHandler::ID;
+char CallbackLogHandler::ID;
+char RotatingLogHandler::ID;
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1407,7 +1407,7 @@
 bool Debugger::EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,
- llvm::raw_ostream &error_stream) {
+ size_t buffer_size, llvm::raw_ostream &error_stream) {
   const bool should_close = true;
 
   std::shared_ptr log_handler_sp;
@@ -1416,6 +1416,8 @@
 // For now when using the callback mode you always get thread & timestamp.
 log_options |=
 LLDB_LOG_OPTION_PREPEND_TIMESTAMP | LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
+  } else if (buffer_size) {
+log_handler_sp = RotatingLogHandler::Create(buffer_size);
   } else if (log_file.empty()) {
 log_handler_sp = StreamLogHandler::Create(GetOutputFile().GetDescriptor(),
   !should_close);
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -428,9 +428,11 @@
 Desc<"Clears the current command history.">;
 }
 
-let Command = "log" in {
+let Command = "log enable" in {
   def log_file : Option<"file", "f">, Group<1>, Arg<"Filename">,
-Desc<"Set the destination file to log to.">;
+Desc<"Set the destination file to log to. Cannot be combined with -b.">;
+  def log_buffer_size : Option<"buffer", "b">, Group<1>, Arg<"UnsignedInteger">,
+Desc<"Set the size of the internal buffer to log to. Cannot be combined with -f.">;
   def log_threadsafe : Option<"threadsafe", "t">, Group<1>,
 Desc<"Enable thread safe logging to avoid interweaved log lines.">;
   def log_verbose : Option<"verbose", "v">, Group<1>,
@@ -454,6 +456,11 @@
 Desc<"Prepend the names of files and function that generate the logs.">;
 }
 
+let Command = "log dump" in {
+  def log_dump_file : Option<"file", "f">, Group<1>, Arg<"Filename">,
+Desc<"Set the destination file to dump to.">;
+}
+
 let Command = "reproducer dump" in {
   def reproducer_provider : Option<"provider", "p">, Group<1>,
 EnumArg<"None", "Reproduce

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

The main questions is if we want --buffered for any log channel (file, 
callback, or circular). If we add a --circular flag, then we just let things 
accumulate in the LogHandler class.

The idea would be to add more stuff to the LogHandler base class to support 
this:

class LogHandler {

  bool m_circular = false; // If true, never flush unless "log dump" is called.
  size_t m_buffer_size = 0;
  // If m_buffer_size > 0 then store messages in m_buffer until size is 
exceeded, then flush.
  std::string m_buffer; 

};

Then we don't need "RotatingLogHandler", and then "log dump" can dump to the 
callback or to the file. So when the buffer is full, we flush to the virtual 
Emit(...) call. Just adds some extra logic to the LogHandler class' Emit(...) 
method to take care of the buffering.




Comment at: lldb/source/Commands/CommandObjectLog.cpp:166-169
+if (m_options.log_file && m_options.buffer_size.OptionWasSet()) {
+  result.AppendError("cannot specify both a file and buffer size.");
+  return false;
+}

Can't we build buffer size into a file based LogHandler? Have a std::string 
inside LogHandler and append to it until the bytes left + latest Emit() message 
go over the buffer size and then flush then? 


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

https://reviews.llvm.org/D127986

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

[Lldb-commits] [lldb] 6ff49af - [lldb] Introduce the concept of a log handler (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-16T13:34:28-07:00
New Revision: 6ff49af33d093dd7e0659958185942d0d26a8b90

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

LOG: [lldb] Introduce the concept of a log handler (NFC)

This patch introduces the concept of a log handlers. Log handlers allow
customizing the way log output is emitted. The StreamCallback class
tried to do something conceptually similar. The benefit of the log
handler interface is that you don't need to conform to llvm's
raw_ostream interface.

Differential revision: https://reviews.llvm.org/D127922

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Utility/Log.h
lldb/source/Core/Debugger.cpp
lldb/source/Utility/CMakeLists.txt
lldb/source/Utility/Log.cpp
lldb/tools/lldb-server/LLDBServerUtilities.cpp
lldb/unittests/Core/CMakeLists.txt
lldb/unittests/Utility/LogTest.cpp

Removed: 
lldb/include/lldb/Utility/StreamCallback.h
lldb/source/Utility/StreamCallback.cpp
lldb/unittests/Core/StreamCallbackTest.cpp



diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 8005e03071b24..4cdc6b6237c7e 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -54,7 +54,9 @@ class ThreadPool;
 
 namespace lldb_private {
 class Address;
+class CallbackLogHandler;
 class CommandInterpreter;
+class LogHandler;
 class Process;
 class Stream;
 class SymbolContext;
@@ -553,8 +555,8 @@ class Debugger : public 
std::enable_shared_from_this,
 
   llvm::Optional m_current_event_id;
 
-  llvm::StringMap> m_log_streams;
-  std::shared_ptr m_log_callback_stream_sp;
+  llvm::StringMap> m_stream_handlers;
+  std::shared_ptr m_callback_handler_sp;
   ConstString m_instance_name;
   static LoadPluginCallbackType g_load_plugin_callback;
   typedef std::vector LoadedPluginsList;

diff  --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 1ab12e0cfc1a1..d4ce30dde6c68 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -45,6 +46,42 @@ class raw_ostream;
 // Logging Functions
 namespace lldb_private {
 
+class LogHandler {
+public:
+  virtual ~LogHandler() = default;
+  virtual void Emit(llvm::StringRef message) = 0;
+  void EmitThreadSafe(llvm::StringRef message);
+
+private:
+  std::mutex m_mutex;
+};
+
+class StreamLogHandler : public LogHandler {
+public:
+  StreamLogHandler(int fd, bool should_close, bool unbuffered);
+
+  void Emit(llvm::StringRef message) override;
+
+  static std::shared_ptr Create(int fd, bool unbuffered);
+
+private:
+  llvm::raw_fd_ostream m_stream;
+};
+
+class CallbackLogHandler : public LogHandler {
+public:
+  CallbackLogHandler(lldb::LogOutputCallback callback, void *baton);
+
+  void Emit(llvm::StringRef message) override;
+
+  static std::shared_ptr
+  Create(lldb::LogOutputCallback callback, void *baton);
+
+private:
+  lldb::LogOutputCallback m_callback;
+  void *m_baton;
+};
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:
@@ -111,7 +148,7 @@ class Log final {
   static void Unregister(llvm::StringRef name);
 
   static bool
-  EnableLogChannel(const std::shared_ptr &log_stream_sp,
+  EnableLogChannel(const std::shared_ptr &log_handler_sp,
uint32_t log_options, llvm::StringRef channel,
llvm::ArrayRef categories,
llvm::raw_ostream &error_stream);
@@ -188,7 +225,7 @@ class Log final {
   // Their modification however, is still protected by this mutex.
   llvm::sys::RWMutex m_mutex;
 
-  std::shared_ptr m_stream_sp;
+  std::shared_ptr m_handler;
   std::atomic m_options{0};
   std::atomic m_mask{0};
 
@@ -199,13 +236,13 @@ class Log final {
   void Format(llvm::StringRef file, llvm::StringRef function,
   const llvm::formatv_object_base &payload);
 
-  std::shared_ptr GetStream() {
+  std::shared_ptr GetHandler() {
 llvm::sys::ScopedReader lock(m_mutex);
-return m_stream_sp;
+return m_handler;
   }
 
-  void Enable(const std::shared_ptr &stream_sp,
-  uint32_t options, uint32_t flags);
+  void Enable(const std::shared_ptr &handler_sp, uint32_t options,
+  uint32_t flags);
 
   void Disable(uint32_t flags);
 

diff  --git a/lldb/include/lldb/Utility/StreamCallback.h 
b/lldb/include/lldb/Utility/StreamCallback.h
deleted file mode 100644
index d234cbea85c61..0
--- a/lldb/include/lldb/Utility/StreamCallback.h
+++ /dev/null
@@ -1,35 +0,0 @@
-//===-- StreamCallback.h ---*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with L

[Lldb-commits] [PATCH] D127922: [lldb] Introduce the concept of a log handler (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6ff49af33d09: [lldb] Introduce the concept of a log handler 
(NFC) (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D127922?vs=437651&id=437686#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127922

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/Log.h
  lldb/include/lldb/Utility/StreamCallback.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/Log.cpp
  lldb/source/Utility/StreamCallback.cpp
  lldb/tools/lldb-server/LLDBServerUtilities.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/StreamCallbackTest.cpp
  lldb/unittests/Utility/LogTest.cpp

Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -39,13 +39,13 @@
 } // namespace lldb_private
 
 // Wrap enable, disable and list functions to make them easier to test.
-static bool EnableChannel(std::shared_ptr stream_sp,
+static bool EnableChannel(std::shared_ptr log_handler_sp,
   uint32_t log_options, llvm::StringRef channel,
   llvm::ArrayRef categories,
   std::string &error) {
   error.clear();
   llvm::raw_string_ostream error_stream(error);
-  return Log::EnableLogChannel(stream_sp, log_options, channel, categories,
+  return Log::EnableLogChannel(log_handler_sp, log_options, channel, categories,
error_stream);
 }
 
@@ -68,9 +68,7 @@
 struct LogChannelTest : public ::testing::Test {
   void TearDown() override { Log::DisableAllLogChannels(); }
 
-  static void SetUpTestCase() {
-Log::Register("chan", test_channel);
-  }
+  static void SetUpTestCase() { Log::Register("chan", test_channel); }
 
   static void TearDownTestCase() {
 Log::Unregister("chan");
@@ -78,18 +76,27 @@
   }
 };
 
+class TestLogHandler : public LogHandler {
+public:
+  TestLogHandler() : m_messages(), m_stream(m_messages) {}
+
+  void Emit(llvm::StringRef message) override { m_stream << message; }
+
+  llvm::SmallString<0> m_messages;
+  llvm::raw_svector_ostream m_stream;
+};
+
 // A test fixture which provides tests with a pre-registered and pre-enabled
 // channel. Additionally, the messages written to that channel are captured and
 // made available via getMessage().
 class LogChannelEnabledTest : public LogChannelTest {
-  llvm::SmallString<0> m_messages;
-  std::shared_ptr m_stream_sp =
-  std::make_shared(m_messages);
+  std::shared_ptr m_log_handler_sp =
+  std::make_shared();
   Log *m_log;
   size_t m_consumed_bytes = 0;
 
 protected:
-  std::shared_ptr getStream() { return m_stream_sp; }
+  std::shared_ptr getLogHandler() { return m_log_handler_sp; }
   Log *getLog() { return m_log; }
   llvm::StringRef takeOutput();
   llvm::StringRef logAndTakeOutput(llvm::StringRef Message);
@@ -103,19 +110,21 @@
   LogChannelTest::SetUp();
 
   std::string error;
-  ASSERT_TRUE(EnableChannel(m_stream_sp, 0, "chan", {}, error));
+  ASSERT_TRUE(EnableChannel(m_log_handler_sp, 0, "chan", {}, error));
 
   m_log = GetLog(TestChannel::FOO);
   ASSERT_NE(nullptr, m_log);
 }
 
 llvm::StringRef LogChannelEnabledTest::takeOutput() {
-  llvm::StringRef result = m_stream_sp->str().drop_front(m_consumed_bytes);
-  m_consumed_bytes+= result.size();
+  llvm::StringRef result =
+  m_log_handler_sp->m_stream.str().drop_front(m_consumed_bytes);
+  m_consumed_bytes += result.size();
   return result;
 }
 
-llvm::StringRef LogChannelEnabledTest::logAndTakeOutput(llvm::StringRef Message) {
+llvm::StringRef
+LogChannelEnabledTest::logAndTakeOutput(llvm::StringRef Message) {
   LLDB_LOG(m_log, "{0}", Message);
   return takeOutput();
 }
@@ -138,33 +147,48 @@
   Log::Register("chan", test_channel);
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
-  std::shared_ptr stream_sp(
-  new llvm::raw_string_ostream(message));
-  EXPECT_TRUE(Log::EnableLogChannel(stream_sp, 0, "chan", {"foo"}, llvm::nulls()));
+  auto log_handler_sp = std::make_shared();
+  EXPECT_TRUE(
+  Log::EnableLogChannel(log_handler_sp, 0, "chan", {"foo"}, llvm::nulls()));
   EXPECT_NE(nullptr, GetLog(TestChannel::FOO));
   Log::Unregister("chan");
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
 }
 
+namespace {
+static char test_baton;
+static size_t callback_count = 0;
+static void TestCallback(const char *data, void *baton) {
+  EXPECT_STREQ("Foobar", data);
+  EXPECT_EQ(&test_baton, baton);
+  ++callback_count;
+}
+} // namespace
+
+TEST(LogTest, CallbackLogHandler) {
+  CallbackLogHandler handler(TestCallback, &test_baton);
+  handler.Emit("Foobar");
+  EXPECT_EQ(1u, callback_count);
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPE

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

For this idea to work we would need to change the Emit() function to be 
DoEmit(...), then add logic to LogHandler:

  void LogHandler::Emit(StringRef message) {
if (m_circular) {
  // Fill buffer in circular fashion
  return;
}
if (m_buffer_size > 0) {
  if (m_buffer.size() + message.size() > m_buffer_size) {
// If we exceed the buffer size, flush.
DoEmit(m_buffer);
DoEmit(message);
m_buffer.clear();
  } else {
// Buffer size not exceeded yet.
m_buffer += message.str();
  }
} else {
  DoEmit(message); 
}
  }


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

https://reviews.llvm.org/D127986

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

[Lldb-commits] [PATCH] D127937: [lldb] Add RotatingLogHandler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 437695.
JDevlieghere added a comment.

- Use dynamic array instead of vector.
- Rebase


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

https://reviews.llvm.org/D127937

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/Utility/Log.cpp
  lldb/unittests/Utility/LogTest.cpp

Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -104,6 +104,13 @@
 public:
   void SetUp() override;
 };
+
+static std::string GetDumpAsString(const RotatingLogHandler &handler) {
+  std::string buffer;
+  llvm::raw_string_ostream stream(buffer);
+  handler.Dump(stream);
+  return stream.str();
+}
 } // end anonymous namespace
 
 void LogChannelEnabledTest::SetUp() {
@@ -171,6 +178,21 @@
   EXPECT_EQ(1u, callback_count);
 }
 
+TEST(LogHandlerTest, RotatingLogHandler) {
+  RotatingLogHandler handler(3);
+
+  handler.Emit("foo");
+  handler.Emit("bar");
+  EXPECT_EQ(GetDumpAsString(handler), "foobar");
+
+  handler.Emit("baz");
+  handler.Emit("qux");
+  EXPECT_EQ(GetDumpAsString(handler), "barbazqux");
+
+  handler.Emit("quux");
+  EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -365,3 +365,37 @@
 CallbackLogHandler::Create(lldb::LogOutputCallback callback, void *baton) {
   return std::make_shared(callback, baton);
 }
+
+RotatingLogHandler::RotatingLogHandler(size_t size)
+: m_messages(std::make_unique(size)), m_size(size) {}
+
+void RotatingLogHandler::Emit(llvm::StringRef message) {
+  ++m_total_count;
+  const size_t index = m_next_index;
+  m_next_index = NormalizeIndex(index + 1);
+  m_messages[index] = message.str();
+}
+
+size_t RotatingLogHandler::NormalizeIndex(size_t i) const { return i % m_size; }
+
+size_t RotatingLogHandler::GetNumMessages() const {
+  return m_total_count < m_size ? m_total_count : m_size;
+}
+
+size_t RotatingLogHandler::GetFirstMessageIndex() const {
+  return m_total_count < m_size ? 0 : m_next_index;
+}
+
+void RotatingLogHandler::Dump(llvm::raw_ostream &stream) const {
+  const size_t start_idx = GetFirstMessageIndex();
+  const size_t stop_idx = start_idx + GetNumMessages();
+  for (size_t i = start_idx; i < stop_idx; ++i) {
+const size_t idx = NormalizeIndex(i);
+stream << m_messages[idx];
+  }
+  stream.flush();
+}
+
+std::shared_ptr RotatingLogHandler::Create(size_t size) {
+  return std::make_shared(size);
+}
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -82,6 +82,26 @@
   void *m_baton;
 };
 
+class RotatingLogHandler : public LogHandler {
+public:
+  RotatingLogHandler(size_t size);
+
+  void Emit(llvm::StringRef message) override;
+  void Dump(llvm::raw_ostream &stream) const;
+
+  static std::shared_ptr Create(size_t size);
+
+private:
+  size_t NormalizeIndex(size_t i) const;
+  size_t GetNumMessages() const;
+  size_t GetFirstMessageIndex() const;
+
+  std::unique_ptr m_messages;
+  const size_t m_size = 0;
+  size_t m_next_index = 0;
+  size_t m_total_count = 0;
+};
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D127986#3590157 , @clayborg wrote:

> The main questions is if we want --buffered for any log channel (file, 
> callback, or circular). If we add a --circular flag, then we just let things 
> accumulate in the LogHandler class.

That's wasn't a problem I was trying to solve personally, but if you think that 
would be useful I don't mind supporting it.

> The idea would be to add more stuff to the LogHandler base class to support 
> this:

I don't think this has to (or even should) go into the base class. The idea 
behind the different handlers was to serve as an extension point. I think 
general buffering is a perfect example of that. The way I would implement that 
is by wrapping it into its own handler (e.g. `BufferedLogHandler`) which wraps 
around another Handler. The delegate handler can then be any of the existing 
handlers. The implementation would be pretty similar to what you described:

  class BufferedLogHandler : public LogHandler {
  public:
void Emit(StringRef message) {
if (m_buffer_size > 0) {
  if (m_buffer.size() + message.size() > m_buffer_size) {
// If we exceed the buffer size, flush.
m_delegate.Emit(m_buffer);
m_delegate.DoEmit(message);
m_buffer.clear();
  } else {
// Buffer size not exceeded yet.
m_buffer += message.str();
  }
} else {
  DoEmit(message); 
}
  }
  
  private:
size_t m_buffer_size = 0;
std::string m_buffer; 
LogHandler m_delegate;
  };




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

https://reviews.llvm.org/D127986

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

[Lldb-commits] [PATCH] D127937: [lldb] Add RotatingLogHandler

[Dave Lee via Phabricator via lldb-commits]

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

lgtm




Comment at: lldb/unittests/Utility/LogTest.cpp:112
+  handler.Dump(stream);
+  return stream.str();
+}

minor: `return buffer;`


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

https://reviews.llvm.org/D127937

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

[Lldb-commits] [PATCH] D127937: [lldb] Add RotatingLogHandler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: lldb/unittests/Utility/LogTest.cpp:112
+  handler.Dump(stream);
+  return stream.str();
+}

kastiglione wrote:
> minor: `return buffer;`
I did that on purpose to avoid the call to `flush` but looking at the 
`raw_string_ostream` the stream is unbuffered and `str` doesn't even call flush 
under the hood. TIL. 


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

https://reviews.llvm.org/D127937

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

[Lldb-commits] [lldb] 663612d - [lldb] Remove references to epydoc from the documentation

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-16T15:17:40-07:00
New Revision: 663612dfd8f63e82936f32755fbba2aa219a7ae0

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

LOG: [lldb] Remove references to epydoc from the documentation

We no longer rely on epydoc but instead use a sphinx plugin to generate
the Python API reference.

Added: 


Modified: 
lldb/docs/resources/build.rst

Removed: 




diff  --git a/lldb/docs/resources/build.rst b/lldb/docs/resources/build.rst
index 5e67955c9c104..c44bdeee22c89 100644
--- a/lldb/docs/resources/build.rst
+++ b/lldb/docs/resources/build.rst
@@ -391,10 +391,9 @@ Building the Documentation
 If you wish to build the optional (reference) documentation, additional
 dependencies are required:
 
-* Sphinx (for the website)
+* Sphinx (for the website and the Python API reference)
 * Graphviz (for the 'dot' tool)
 * doxygen (if you wish to build the C++ API reference)
-* epydoc (if you wish to build the Python API reference)
 
 To install the prerequisites for building the documentation (on Debian/Ubuntu)
 do:
@@ -402,7 +401,6 @@ do:
 ::
 
   $ sudo apt-get install doxygen graphviz python3-sphinx
-  $ sudo pip install epydoc
 
 To build the documentation, configure with ``LLVM_ENABLE_SPHINX=ON`` and build 
the desired target(s).
 
@@ -411,7 +409,6 @@ To build the documentation, configure with 
``LLVM_ENABLE_SPHINX=ON`` and build t
   $ ninja docs-lldb-html
   $ ninja docs-lldb-man
   $ ninja lldb-cpp-doc
-  $ ninja lldb-python-doc
 
 Cross-compiling LLDB
 



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

[Lldb-commits] [lldb] 6ac608b - [lldb] Add RotatingLogHandler

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-16T15:17:40-07:00
New Revision: 6ac608b3d897502c987e02667e87315c5fe0e90f

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

LOG: [lldb] Add RotatingLogHandler

Add a log handler that maintains a circular buffer with a fixed size.

Differential revision: https://reviews.llvm.org/D127937

Added: 


Modified: 
lldb/include/lldb/Utility/Log.h
lldb/source/Utility/Log.cpp
lldb/unittests/Utility/LogTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index d4ce30dde6c6..0beb97b4891f 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -82,6 +82,26 @@ class CallbackLogHandler : public LogHandler {
   void *m_baton;
 };
 
+class RotatingLogHandler : public LogHandler {
+public:
+  RotatingLogHandler(size_t size);
+
+  void Emit(llvm::StringRef message) override;
+  void Dump(llvm::raw_ostream &stream) const;
+
+  static std::shared_ptr Create(size_t size);
+
+private:
+  size_t NormalizeIndex(size_t i) const;
+  size_t GetNumMessages() const;
+  size_t GetFirstMessageIndex() const;
+
+  std::unique_ptr m_messages;
+  const size_t m_size = 0;
+  size_t m_next_index = 0;
+  size_t m_total_count = 0;
+};
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:

diff  --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp
index b9b79636d0dc..62547154dbee 100644
--- a/lldb/source/Utility/Log.cpp
+++ b/lldb/source/Utility/Log.cpp
@@ -365,3 +365,37 @@ std::shared_ptr
 CallbackLogHandler::Create(lldb::LogOutputCallback callback, void *baton) {
   return std::make_shared(callback, baton);
 }
+
+RotatingLogHandler::RotatingLogHandler(size_t size)
+: m_messages(std::make_unique(size)), m_size(size) {}
+
+void RotatingLogHandler::Emit(llvm::StringRef message) {
+  ++m_total_count;
+  const size_t index = m_next_index;
+  m_next_index = NormalizeIndex(index + 1);
+  m_messages[index] = message.str();
+}
+
+size_t RotatingLogHandler::NormalizeIndex(size_t i) const { return i % m_size; 
}
+
+size_t RotatingLogHandler::GetNumMessages() const {
+  return m_total_count < m_size ? m_total_count : m_size;
+}
+
+size_t RotatingLogHandler::GetFirstMessageIndex() const {
+  return m_total_count < m_size ? 0 : m_next_index;
+}
+
+void RotatingLogHandler::Dump(llvm::raw_ostream &stream) const {
+  const size_t start_idx = GetFirstMessageIndex();
+  const size_t stop_idx = start_idx + GetNumMessages();
+  for (size_t i = start_idx; i < stop_idx; ++i) {
+const size_t idx = NormalizeIndex(i);
+stream << m_messages[idx];
+  }
+  stream.flush();
+}
+
+std::shared_ptr RotatingLogHandler::Create(size_t size) {
+  return std::make_shared(size);
+}

diff  --git a/lldb/unittests/Utility/LogTest.cpp 
b/lldb/unittests/Utility/LogTest.cpp
index 2b61c99630ea..d89f6df1ce44 100644
--- a/lldb/unittests/Utility/LogTest.cpp
+++ b/lldb/unittests/Utility/LogTest.cpp
@@ -104,6 +104,13 @@ class LogChannelEnabledTest : public LogChannelTest {
 public:
   void SetUp() override;
 };
+
+static std::string GetDumpAsString(const RotatingLogHandler &handler) {
+  std::string buffer;
+  llvm::raw_string_ostream stream(buffer);
+  handler.Dump(stream);
+  return buffer;
+}
 } // end anonymous namespace
 
 void LogChannelEnabledTest::SetUp() {
@@ -171,6 +178,21 @@ TEST(LogTest, CallbackLogHandler) {
   EXPECT_EQ(1u, callback_count);
 }
 
+TEST(LogHandlerTest, RotatingLogHandler) {
+  RotatingLogHandler handler(3);
+
+  handler.Emit("foo");
+  handler.Emit("bar");
+  EXPECT_EQ(GetDumpAsString(handler), "foobar");
+
+  handler.Emit("baz");
+  handler.Emit("qux");
+  EXPECT_EQ(GetDumpAsString(handler), "barbazqux");
+
+  handler.Emit("quux");
+  EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;



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

[Lldb-commits] [PATCH] D127937: [lldb] Add RotatingLogHandler

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.
Closed by commit rG6ac608b3d897: [lldb] Add RotatingLogHandler (authored by 
JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D127937?vs=437695&id=437730#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127937

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/Utility/Log.cpp
  lldb/unittests/Utility/LogTest.cpp

Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -104,6 +104,13 @@
 public:
   void SetUp() override;
 };
+
+static std::string GetDumpAsString(const RotatingLogHandler &handler) {
+  std::string buffer;
+  llvm::raw_string_ostream stream(buffer);
+  handler.Dump(stream);
+  return buffer;
+}
 } // end anonymous namespace
 
 void LogChannelEnabledTest::SetUp() {
@@ -171,6 +178,21 @@
   EXPECT_EQ(1u, callback_count);
 }
 
+TEST(LogHandlerTest, RotatingLogHandler) {
+  RotatingLogHandler handler(3);
+
+  handler.Emit("foo");
+  handler.Emit("bar");
+  EXPECT_EQ(GetDumpAsString(handler), "foobar");
+
+  handler.Emit("baz");
+  handler.Emit("qux");
+  EXPECT_EQ(GetDumpAsString(handler), "barbazqux");
+
+  handler.Emit("quux");
+  EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -365,3 +365,37 @@
 CallbackLogHandler::Create(lldb::LogOutputCallback callback, void *baton) {
   return std::make_shared(callback, baton);
 }
+
+RotatingLogHandler::RotatingLogHandler(size_t size)
+: m_messages(std::make_unique(size)), m_size(size) {}
+
+void RotatingLogHandler::Emit(llvm::StringRef message) {
+  ++m_total_count;
+  const size_t index = m_next_index;
+  m_next_index = NormalizeIndex(index + 1);
+  m_messages[index] = message.str();
+}
+
+size_t RotatingLogHandler::NormalizeIndex(size_t i) const { return i % m_size; }
+
+size_t RotatingLogHandler::GetNumMessages() const {
+  return m_total_count < m_size ? m_total_count : m_size;
+}
+
+size_t RotatingLogHandler::GetFirstMessageIndex() const {
+  return m_total_count < m_size ? 0 : m_next_index;
+}
+
+void RotatingLogHandler::Dump(llvm::raw_ostream &stream) const {
+  const size_t start_idx = GetFirstMessageIndex();
+  const size_t stop_idx = start_idx + GetNumMessages();
+  for (size_t i = start_idx; i < stop_idx; ++i) {
+const size_t idx = NormalizeIndex(i);
+stream << m_messages[idx];
+  }
+  stream.flush();
+}
+
+std::shared_ptr RotatingLogHandler::Create(size_t size) {
+  return std::make_shared(size);
+}
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -82,6 +82,26 @@
   void *m_baton;
 };
 
+class RotatingLogHandler : public LogHandler {
+public:
+  RotatingLogHandler(size_t size);
+
+  void Emit(llvm::StringRef message) override;
+  void Dump(llvm::raw_ostream &stream) const;
+
+  static std::shared_ptr Create(size_t size);
+
+private:
+  size_t NormalizeIndex(size_t i) const;
+  size_t GetNumMessages() const;
+  size_t GetFirstMessageIndex() const;
+
+  std::unique_ptr m_messages;
+  const size_t m_size = 0;
+  size_t m_next_index = 0;
+  size_t m_total_count = 0;
+};
+
 class Log final {
 public:
   /// The underlying type of all log channel enums. Declare them as:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D127999: [lldb] fix stepping through POSIX trampolines

[Michael Daniels via Phabricator via lldb-commits]

mdaniels updated this revision to Diff 437741.
mdaniels added a comment.

Added a simple test case that reproduces the issue.


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

https://reviews.llvm.org/D127999

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/API/lang/cpp/step-into-namespace/Makefile
  lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py
  lldb/test/API/lang/cpp/step-into-namespace/foo.cpp
  lldb/test/API/lang/cpp/step-into-namespace/foo.h
  lldb/test/API/lang/cpp/step-into-namespace/main.cpp


Index: lldb/test/API/lang/cpp/step-into-namespace/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/step-into-namespace/main.cpp
@@ -0,0 +1,6 @@
+#include "foo.h"
+
+int main(void) {
+foo::foo(); // Set a breakpoint here
+}
+
Index: lldb/test/API/lang/cpp/step-into-namespace/foo.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/step-into-namespace/foo.h
@@ -0,0 +1,4 @@
+namespace foo {
+extern void foo();
+}
+
Index: lldb/test/API/lang/cpp/step-into-namespace/foo.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/step-into-namespace/foo.cpp
@@ -0,0 +1,4 @@
+#include "foo.h"
+
+void foo::foo() { } // End up here
+
Index: lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py
@@ -0,0 +1,17 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class StepIntoNamespace(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+(target, process, thread, bkpt) = 
lldbutil.run_to_source_breakpoint(self,
+   "// Set a breakpoint here",
+   lldb.SBFileSpec("main.cpp"),
+   extra_images=["foo"])
+thread.StepInto()
+
+foo_line = line_number("foo.cpp", '// End up here')
+self.expect("frame info", substrs=["foo.cpp:{}:".format(foo_line)])
Index: lldb/test/API/lang/cpp/step-into-namespace/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/step-into-namespace/Makefile
@@ -0,0 +1,6 @@
+DYLIB_NAME := foo
+DYLIB_CXX_SOURCES := foo.cpp
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
+
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -488,7 +488,7 @@
   if (sym == nullptr || !sym->IsTrampoline())
 return thread_plan_sp;
 
-  ConstString sym_name = sym->GetName();
+  ConstString sym_name = sym->GetMangled().GetName(Mangled::ePreferMangled);
   if (!sym_name)
 return thread_plan_sp;
 


Index: lldb/test/API/lang/cpp/step-into-namespace/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/step-into-namespace/main.cpp
@@ -0,0 +1,6 @@
+#include "foo.h"
+
+int main(void) {
+foo::foo(); // Set a breakpoint here
+}
+
Index: lldb/test/API/lang/cpp/step-into-namespace/foo.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/step-into-namespace/foo.h
@@ -0,0 +1,4 @@
+namespace foo {
+extern void foo();
+}
+
Index: lldb/test/API/lang/cpp/step-into-namespace/foo.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/step-into-namespace/foo.cpp
@@ -0,0 +1,4 @@
+#include "foo.h"
+
+void foo::foo() { } // End up here
+
Index: lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py
@@ -0,0 +1,17 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class StepIntoNamespace(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+   "// Set a breakpoint here",
+   lldb.SBFileSpec("main.cpp"),
+   extra_images=["foo"])
+thread.StepInto()
+
+foo_line = line_number("foo.cpp", '// End up here')
+self.expect("frame info", substrs=["foo.cpp:{}:".format(foo_line)])
Index: lldb/test/API/lang/cpp/step-into-namespace/Makefile

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

The delegate idea is fine too! No need to overload the base class with extra 
stuff if not needed. I would just like there to be no predispositions on the 
kinds of logs that can be used when buffering is enabled.


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

https://reviews.llvm.org/D127986

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

[Lldb-commits] [PATCH] D127999: [lldb] fix stepping through POSIX trampolines

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

LGTM, just a question if we want to restrict this test to linux only.




Comment at: 
lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py:8
+
+def test(self):
+self.build()

Do we want to limit this to linux? I am not sure this will pass the windows 
buildbots if we don't restrict it


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

https://reviews.llvm.org/D127999

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

[Lldb-commits] [lldb] af6ec92 - [lldb] Cleanup Python API reference files after building the docs

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-16T16:30:49-07:00
New Revision: af6ec9200b09039573d85e349496c4f5b17c3d7f

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

LOG: [lldb] Cleanup Python API reference files after building the docs

The sphinx-automodapi extension requires that the generated RST files
live next to the index file. This means that we generate them in the
source directory rather than the build directory. This patch ensures
these files are removed again when sphinx finishes its build.

The proper solution to this problem would be to move everything in the
doc folder from the source directory to the build directory before
generating the docs.

I believe that old RST files being kept around is the reason that the
Python API references on the website isn't getting updated. This patch
is meant as a speculative fix and a way to confirm that.

Added: 


Modified: 
lldb/docs/conf.py

Removed: 




diff  --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index cf3af568572b..c9e21fa8b8d5 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -12,7 +12,7 @@
 # serve to show the default.
 from __future__ import print_function
 
-import sys, os, re
+import sys, os, re, shutil
 from datetime import date
 
 building_man_page = tags.has('builder-man')
@@ -293,8 +293,8 @@
 empty_attr_summary = re.compile(r'\.\. rubric:: Attributes Summary\s*\.\. 
autosummary::\s*\.\. rubric::')
 empty_attr_documentation = re.compile(r'\.\. rubric:: Attributes 
Documentation\s*\.\. rubric::')
 
-def cleanup_source(app, docname, source):
-""" Cleans up source files generated by automodapi. """
+def preprocess_source(app, docname, source):
+""" Preprocesses source files generated by automodapi. """
 # Don't cleanup anything beside automodapi-generated sources.
 if not automodapi_toctreedirnm in docname:
   return
@@ -320,5 +320,12 @@ def cleanup_source(app, docname, source):
 # element list).
 source[0] = processed
 
+def cleanup_source(app, exception):
+""" Remove files generated by automodapi in the source tree. """
+if hasattr(app.config, 'automodapi_toctreedirnm'):
+  api_source_dir = os.path.join(app.srcdir, 
app.config.automodapi_toctreedirnm)
+  shutil.rmtree(api_source_dir, ignore_errors=True)
+
 def setup(app):
-app.connect('source-read', cleanup_source)
+app.connect('source-read', preprocess_source)
+app.connect('build-finished', cleanup_source)



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

[Lldb-commits] [PATCH] D128026: [lldb] Add a BufferedLogHandler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: clayborg.
Herald added a project: All.
JDevlieghere requested review of this revision.

Add a BufferedLogHandler as requested by Greg in D127986 
.


https://reviews.llvm.org/D128026

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/Utility/Log.cpp
  lldb/unittests/Utility/LogTest.cpp


Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -193,6 +193,23 @@
   EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
 }
 
+TEST(LogHandlerTest, BufferedLogHandler) {
+  auto delegate_sp = std::make_shared();
+  BufferedLogHandler handler(3, delegate_sp);
+
+  handler.Emit("foo");
+  handler.Emit("bar");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "");
+  handler.Emit("baz");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbaz");
+
+  handler.Emit("qux");
+  handler.Emit("quux");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbaz");
+  handler.Emit("quuz");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbazquxquuxquuz");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -355,6 +355,22 @@
   m_callback(message.data(), m_baton);
 }
 
+BufferedLogHandler::BufferedLogHandler(size_t size,
+   std::shared_ptr delegate)
+: m_delegate(delegate), m_messages(std::make_unique(size)),
+  m_size(size) {}
+
+void BufferedLogHandler::Emit(llvm::StringRef message) {
+  m_messages[m_next_index++] = message.str();
+  if (m_next_index == m_size) {
+for (size_t i = 0; i < m_size; ++i) {
+  m_delegate->Emit(m_messages[i]);
+  m_messages[i].clear();
+}
+m_next_index = 0;
+  }
+}
+
 RotatingLogHandler::RotatingLogHandler(size_t size)
 : m_messages(std::make_unique(size)), m_size(size) {}
 
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -77,6 +77,21 @@
   void *m_baton;
 };
 
+class BufferedLogHandler : public LogHandler {
+public:
+  BufferedLogHandler(size_t size, std::shared_ptr delegate);
+
+  void Emit(llvm::StringRef message) override;
+
+  static std::shared_ptr Create(size_t size);
+
+private:
+  std::shared_ptr m_delegate;
+  std::unique_ptr m_messages;
+  const size_t m_size = 0;
+  size_t m_next_index = 0;
+};
+
 class RotatingLogHandler : public LogHandler {
 public:
   RotatingLogHandler(size_t size);


Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -193,6 +193,23 @@
   EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
 }
 
+TEST(LogHandlerTest, BufferedLogHandler) {
+  auto delegate_sp = std::make_shared();
+  BufferedLogHandler handler(3, delegate_sp);
+
+  handler.Emit("foo");
+  handler.Emit("bar");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "");
+  handler.Emit("baz");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbaz");
+
+  handler.Emit("qux");
+  handler.Emit("quux");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbaz");
+  handler.Emit("quuz");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbazquxquuxquuz");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -355,6 +355,22 @@
   m_callback(message.data(), m_baton);
 }
 
+BufferedLogHandler::BufferedLogHandler(size_t size,
+   std::shared_ptr delegate)
+: m_delegate(delegate), m_messages(std::make_unique(size)),
+  m_size(size) {}
+
+void BufferedLogHandler::Emit(llvm::StringRef message) {
+  m_messages[m_next_index++] = message.str();
+  if (m_next_index == m_size) {
+for (size_t i = 0; i < m_size; ++i) {
+  m_delegate->Emit(m_messages[i]);
+  m_messages[i].clear();
+}
+m_next_index = 0;
+  }
+}
+
 RotatingLogHandler::RotatingLogHandler(size_t size)
 : m_messages(std::make_unique(size)), m_size(size) {}
 
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -77,6 +77,21 @@
   void *m_baton;
 };
 
+class BufferedLogHandler : public LogHandler {
+public:
+  BufferedLogHandler(size_t size, std::shared_ptr delegate);
+
+  void Emit(llvm::StringRef message) override;
+
+  static std::shared_ptr Create(size_t size);
+
+private:
+  std::s

[Lldb-commits] [PATCH] D128026: [lldb] Add a BufferedLogHandler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 437784.

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

https://reviews.llvm.org/D128026

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/Utility/Log.cpp
  lldb/unittests/Utility/LogTest.cpp


Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -193,6 +193,23 @@
   EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
 }
 
+TEST(LogHandlerTest, BufferedLogHandler) {
+  auto delegate_sp = std::make_shared();
+  BufferedLogHandler handler(3, delegate_sp);
+
+  handler.Emit("foo");
+  handler.Emit("bar");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "");
+  handler.Emit("baz");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbaz");
+
+  handler.Emit("qux");
+  handler.Emit("quux");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbaz");
+  handler.Emit("quuz");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbazquxquuxquuz");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -355,6 +355,22 @@
   m_callback(message.data(), m_baton);
 }
 
+BufferedLogHandler::BufferedLogHandler(size_t size,
+   std::shared_ptr delegate)
+: m_delegate(delegate), m_messages(std::make_unique(size)),
+  m_size(size) {}
+
+void BufferedLogHandler::Emit(llvm::StringRef message) {
+  m_messages[m_next_index++] = message.str();
+  if (m_next_index == m_size) {
+for (size_t i = 0; i < m_size; ++i) {
+  m_delegate->Emit(m_messages[i]);
+  m_messages[i].clear();
+}
+m_next_index = 0;
+  }
+}
+
 RotatingLogHandler::RotatingLogHandler(size_t size)
 : m_messages(std::make_unique(size)), m_size(size) {}
 
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -77,6 +77,19 @@
   void *m_baton;
 };
 
+class BufferedLogHandler : public LogHandler {
+public:
+  BufferedLogHandler(size_t size, std::shared_ptr delegate);
+
+  void Emit(llvm::StringRef message) override;
+
+private:
+  std::shared_ptr m_delegate;
+  std::unique_ptr m_messages;
+  const size_t m_size = 0;
+  size_t m_next_index = 0;
+};
+
 class RotatingLogHandler : public LogHandler {
 public:
   RotatingLogHandler(size_t size);


Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -193,6 +193,23 @@
   EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
 }
 
+TEST(LogHandlerTest, BufferedLogHandler) {
+  auto delegate_sp = std::make_shared();
+  BufferedLogHandler handler(3, delegate_sp);
+
+  handler.Emit("foo");
+  handler.Emit("bar");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "");
+  handler.Emit("baz");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbaz");
+
+  handler.Emit("qux");
+  handler.Emit("quux");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbaz");
+  handler.Emit("quuz");
+  EXPECT_EQ(delegate_sp->m_stream.str(), "foobarbazquxquuxquuz");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -355,6 +355,22 @@
   m_callback(message.data(), m_baton);
 }
 
+BufferedLogHandler::BufferedLogHandler(size_t size,
+   std::shared_ptr delegate)
+: m_delegate(delegate), m_messages(std::make_unique(size)),
+  m_size(size) {}
+
+void BufferedLogHandler::Emit(llvm::StringRef message) {
+  m_messages[m_next_index++] = message.str();
+  if (m_next_index == m_size) {
+for (size_t i = 0; i < m_size; ++i) {
+  m_delegate->Emit(m_messages[i]);
+  m_messages[i].clear();
+}
+m_next_index = 0;
+  }
+}
+
 RotatingLogHandler::RotatingLogHandler(size_t size)
 : m_messages(std::make_unique(size)), m_size(size) {}
 
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -77,6 +77,19 @@
   void *m_baton;
 };
 
+class BufferedLogHandler : public LogHandler {
+public:
+  BufferedLogHandler(size_t size, std::shared_ptr delegate);
+
+  void Emit(llvm::StringRef message) override;
+
+private:
+  std::shared_ptr m_delegate;
+  std::unique_ptr m_messages;
+  const size_t m_size = 0;
+  size_t m_next_index = 0;
+};
+
 class RotatingLogHandler : public LogHandler {
 public:
   RotatingLogHandler(size_t size);
__

[Lldb-commits] [PATCH] D127986: [lldb] Support a buffered logging mode

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 437786.
JDevlieghere added a comment.

Generalize support for buffering. You can see it in action below:

  (lldb) log enable -b 10 lldb default
  (lldb) target create ./bin/count
  Current executable set to '/Users/jonas/llvm/build-ra/bin/count' (arm64).
  (lldb) b main
   Processing command: target create ./bin/count
   HandleCommand, cmd_obj : 'target create'
   HandleCommand, (revised) command_string: 'target create ./bin/count'
   HandleCommand, wants_raw_input:'False'
   HandleCommand, command line after removing command name(s): './bin/count'
   Target::Target created with architecture arm64 (arm64-apple-macosx12.4.0)
   HandleCommand, command succeeded
   Processing command: b main
   HandleCommand, cmd_obj : '_regexp-break'
   HandleCommand, (revised) command_string: '_regexp-break main'
   HandleCommand, wants_raw_input:'True'
   HandleCommand, command line after removing command name(s): 'main'
   Processing command: breakpoint set --name 'main'
   ...


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

https://reviews.llvm.org/D127986

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Utility/Log.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Utility/Log.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -1120,7 +1120,7 @@
   /*add_to_history*/ eLazyBoolNo, Result);
 
   if (!opts::Log.empty())
-Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, errs());
+Dbg->EnableLog("lldb", {"all"}, opts::Log, 0, 0, errs());
 
   if (opts::BreakpointSubcommand)
 return opts::breakpoint::evaluateBreakpoints(*Dbg);
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -13,6 +13,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator.h"
 
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
@@ -106,6 +107,13 @@
   }
 }
 
+void Log::Dump(llvm::raw_ostream &output_stream) {
+  llvm::sys::ScopedReader lock(m_mutex);
+  if (RotatingLogHandler *handler =
+  llvm::dyn_cast(m_handler.get()))
+handler->Dump(output_stream);
+}
+
 const Flags Log::GetOptions() const {
   return m_options.load(std::memory_order_relaxed);
 }
@@ -222,6 +230,19 @@
   return true;
 }
 
+bool Log::DumpLogChannel(llvm::StringRef channel,
+ llvm::ArrayRef categories,
+ llvm::raw_ostream &output_stream,
+ llvm::raw_ostream &error_stream) {
+  auto iter = g_channel_map->find(channel);
+  if (iter == g_channel_map->end()) {
+error_stream << llvm::formatv("Invalid log channel '{0}'.\n", channel);
+return false;
+  }
+  iter->second.Dump(output_stream);
+  return true;
+}
+
 bool Log::ListChannelCategories(llvm::StringRef channel,
 llvm::raw_ostream &stream) {
   auto ch = g_channel_map->find(channel);
@@ -400,3 +421,9 @@
   }
   stream.flush();
 }
+
+char LogHandler::ID;
+char StreamLogHandler::ID;
+char CallbackLogHandler::ID;
+char BufferedLogHandler::ID;
+char RotatingLogHandler::ID;
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1409,6 +1409,7 @@
 bool Debugger::EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,
+ size_t buffer_size, bool circular_buffer,
  llvm::raw_ostream &error_stream) {
   const bool should_close = true;
 
@@ -1418,9 +1419,14 @@
 // For now when using the callback mode you always get thread & timestamp.
 log_options |=
 LLDB_LOG_OPTION_PREPEND_TIMESTAMP | LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
+  } else if (buffer_size && circular_buffer) {
+log_handler_sp = std::make_shared(buffer_size);
   } else if (log_file.empty()) {
 log_handler_sp = std::make_shared(
 GetOutputFile().GetDescriptor(), !should_close);
+if (buffer_size)
+  log_handler_sp =
+  std::make_shared(buffer_size, log_handler_sp);
   } else {
 auto pos = m_stream_handlers.find(log_file);
 if (pos != m_stream_handlers.end())
@@ -1442,6 +1448,9 @@
 
   log_handler_sp = std::make_shared(
   (*file)->GetDescriptor(), should_close);
+  if (buffer_size)
+log_handler_sp =
+std::make_shared(buffer_size, log_handler_sp);
   m_stream_handlers[log_file] = log_handler_sp;
 }
   }
Index: lldb/s

[Lldb-commits] [lldb] de74756 - [lldb] Remove LogHandler::Create functions (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-06-16T21:04:08-07:00
New Revision: de7475657156658d16704e956a17a39709de2fdd

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

LOG: [lldb] Remove LogHandler::Create functions (NFC)

Remove the LogHandler::Create functions. Except for the StreamHandler
they were just forwarding their arguments to std::make_shared.

Added: 


Modified: 
lldb/include/lldb/Utility/Log.h
lldb/source/Core/Debugger.cpp
lldb/source/Utility/Log.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 0beb97b4891f1..806eb390773b9 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -58,12 +58,10 @@ class LogHandler {
 
 class StreamLogHandler : public LogHandler {
 public:
-  StreamLogHandler(int fd, bool should_close, bool unbuffered);
+  StreamLogHandler(int fd, bool should_close, bool unbuffered = true);
 
   void Emit(llvm::StringRef message) override;
 
-  static std::shared_ptr Create(int fd, bool unbuffered);
-
 private:
   llvm::raw_fd_ostream m_stream;
 };
@@ -74,9 +72,6 @@ class CallbackLogHandler : public LogHandler {
 
   void Emit(llvm::StringRef message) override;
 
-  static std::shared_ptr
-  Create(lldb::LogOutputCallback callback, void *baton);
-
 private:
   lldb::LogOutputCallback m_callback;
   void *m_baton;
@@ -89,8 +84,6 @@ class RotatingLogHandler : public LogHandler {
   void Emit(llvm::StringRef message) override;
   void Dump(llvm::raw_ostream &stream) const;
 
-  static std::shared_ptr Create(size_t size);
-
 private:
   size_t NormalizeIndex(size_t i) const;
   size_t GetNumMessages() const;

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 08d95c0ea6545..c6f1cbe581dac 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -757,7 +757,8 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, 
void *baton)
   m_forward_listener_sp(), m_clear_once() {
   m_instance_name.SetString(llvm::formatv("debugger_{0}", GetID()).str());
   if (log_callback)
-m_callback_handler_sp = CallbackLogHandler::Create(log_callback, baton);
+m_callback_handler_sp =
+std::make_shared(log_callback, baton);
   m_command_interpreter_up->Initialize();
   // Always add our default platform to the platform list
   PlatformSP default_platform_sp(Platform::GetHostPlatform());
@@ -1290,7 +1291,8 @@ void Debugger::SetLoggingCallback(lldb::LogOutputCallback 
log_callback,
   // For simplicity's sake, I am not going to deal with how to close down any
   // open logging streams, I just redirect everything from here on out to the
   // callback.
-  m_callback_handler_sp = CallbackLogHandler::Create(log_callback, baton);
+  m_callback_handler_sp =
+  std::make_shared(log_callback, baton);
 }
 
 static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id,
@@ -1417,8 +1419,8 @@ bool Debugger::EnableLog(llvm::StringRef channel,
 log_options |=
 LLDB_LOG_OPTION_PREPEND_TIMESTAMP | 
LLDB_LOG_OPTION_PREPEND_THREAD_NAME;
   } else if (log_file.empty()) {
-log_handler_sp = StreamLogHandler::Create(GetOutputFile().GetDescriptor(),
-  !should_close);
+log_handler_sp = std::make_shared(
+GetOutputFile().GetDescriptor(), !should_close);
   } else {
 auto pos = m_stream_handlers.find(log_file);
 if (pos != m_stream_handlers.end())
@@ -1438,8 +1440,8 @@ bool Debugger::EnableLog(llvm::StringRef channel,
 return false;
   }
 
-  log_handler_sp =
-  StreamLogHandler::Create((*file)->GetDescriptor(), should_close);
+  log_handler_sp = std::make_shared(
+  (*file)->GetDescriptor(), should_close);
   m_stream_handlers[log_file] = log_handler_sp;
 }
   }

diff  --git a/lldb/source/Utility/Log.cpp b/lldb/source/Utility/Log.cpp
index 62547154dbee5..6649dc0b9f730 100644
--- a/lldb/source/Utility/Log.cpp
+++ b/lldb/source/Utility/Log.cpp
@@ -347,12 +347,6 @@ void StreamLogHandler::Emit(llvm::StringRef message) {
   m_stream.flush();
 }
 
-std::shared_ptr StreamLogHandler::Create(int fd,
-   bool should_close) {
-  constexpr const bool unbuffered = true;
-  return std::make_shared(fd, should_close, unbuffered);
-}
-
 CallbackLogHandler::CallbackLogHandler(lldb::LogOutputCallback callback,
void *baton)
 : m_callback(callback), m_baton(baton) {}
@@ -361,11 +355,6 @@ void CallbackLogHandler::Emit(llvm::StringRef message) {
   m_callback(message.data(), m_baton);
 }
 
-std::shared_ptr
-CallbackLogHandler::Create(lldb::LogOutputCallback callback, void *baton) {
-  return std::make_shared(callback, bat

[Lldb-commits] [PATCH] D128026: [lldb] Add a BufferedLogHandler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 437788.
JDevlieghere added a comment.

Flush on destruction


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

https://reviews.llvm.org/D128026

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/source/Utility/Log.cpp
  lldb/unittests/Utility/LogTest.cpp


Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -193,6 +193,34 @@
   EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
 }
 
+TEST(LogHandlerTest, BufferedLogHandler) {
+  auto delegate_sp = std::make_shared();
+
+  {
+BufferedLogHandler handler(3, delegate_sp);
+
+handler.Emit("1");
+handler.Emit("2");
+EXPECT_EQ(delegate_sp->m_stream.str(), "");
+
+handler.Emit("3");
+EXPECT_EQ(delegate_sp->m_stream.str(), "123");
+
+handler.Emit("4");
+handler.Emit("5");
+EXPECT_EQ(delegate_sp->m_stream.str(), "123");
+
+handler.Emit("6");
+EXPECT_EQ(delegate_sp->m_stream.str(), "123456");
+
+handler.Emit("7");
+EXPECT_EQ(delegate_sp->m_stream.str(), "123456");
+  }
+
+  // Make sure we flush on destruction.
+  EXPECT_EQ(delegate_sp->m_stream.str(), "1234567");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -355,6 +355,27 @@
   m_callback(message.data(), m_baton);
 }
 
+BufferedLogHandler::BufferedLogHandler(size_t size,
+   std::shared_ptr delegate)
+: m_delegate(delegate), m_messages(std::make_unique(size)),
+  m_size(size) {}
+
+BufferedLogHandler::~BufferedLogHandler() { Flush(); }
+
+void BufferedLogHandler::Emit(llvm::StringRef message) {
+  m_messages[m_next_index++] = message.str();
+  if (m_next_index == m_size)
+Flush();
+}
+
+void BufferedLogHandler::Flush() {
+  for (size_t i = 0; i < m_next_index; ++i) {
+m_delegate->Emit(m_messages[i]);
+m_messages[i].clear();
+  }
+  m_next_index = 0;
+}
+
 RotatingLogHandler::RotatingLogHandler(size_t size)
 : m_messages(std::make_unique(size)), m_size(size) {}
 
Index: lldb/include/lldb/Utility/Log.h
===
--- lldb/include/lldb/Utility/Log.h
+++ lldb/include/lldb/Utility/Log.h
@@ -77,6 +77,21 @@
   void *m_baton;
 };
 
+class BufferedLogHandler : public LogHandler {
+public:
+  BufferedLogHandler(size_t size, std::shared_ptr delegate);
+  ~BufferedLogHandler();
+
+  void Emit(llvm::StringRef message) override;
+  void Flush();
+
+private:
+  std::shared_ptr m_delegate;
+  std::unique_ptr m_messages;
+  const size_t m_size = 0;
+  size_t m_next_index = 0;
+};
+
 class RotatingLogHandler : public LogHandler {
 public:
   RotatingLogHandler(size_t size);


Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -193,6 +193,34 @@
   EXPECT_EQ(GetDumpAsString(handler), "bazquxquux");
 }
 
+TEST(LogHandlerTest, BufferedLogHandler) {
+  auto delegate_sp = std::make_shared();
+
+  {
+BufferedLogHandler handler(3, delegate_sp);
+
+handler.Emit("1");
+handler.Emit("2");
+EXPECT_EQ(delegate_sp->m_stream.str(), "");
+
+handler.Emit("3");
+EXPECT_EQ(delegate_sp->m_stream.str(), "123");
+
+handler.Emit("4");
+handler.Emit("5");
+EXPECT_EQ(delegate_sp->m_stream.str(), "123");
+
+handler.Emit("6");
+EXPECT_EQ(delegate_sp->m_stream.str(), "123456");
+
+handler.Emit("7");
+EXPECT_EQ(delegate_sp->m_stream.str(), "123456");
+  }
+
+  // Make sure we flush on destruction.
+  EXPECT_EQ(delegate_sp->m_stream.str(), "1234567");
+}
+
 TEST_F(LogChannelTest, Enable) {
   EXPECT_EQ(nullptr, GetLog(TestChannel::FOO));
   std::string message;
Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -355,6 +355,27 @@
   m_callback(message.data(), m_baton);
 }
 
+BufferedLogHandler::BufferedLogHandler(size_t size,
+   std::shared_ptr delegate)
+: m_delegate(delegate), m_messages(std::make_unique(size)),
+  m_size(size) {}
+
+BufferedLogHandler::~BufferedLogHandler() { Flush(); }
+
+void BufferedLogHandler::Emit(llvm::StringRef message) {
+  m_messages[m_next_index++] = message.str();
+  if (m_next_index == m_size)
+Flush();
+}
+
+void BufferedLogHandler::Flush() {
+  for (size_t i = 0; i < m_next_index; ++i) {
+m_delegate->Emit(m_messages[i]);
+m_messages[i].clear();
+  }
+  m_next_index = 0;
+}
+
 RotatingLogHandler::RotatingLogHandler(size_t size)
 : m_messages(std::make_unique(size)), m_size(s