[Lldb-commits] [lldb] 13d0318 - [lldb] Add support for gdb-style 'x' packet (#124733)

2025-01-31 Thread via lldb-commits

Author: Pavel Labath
Date: 2025-01-31T09:07:11+01:00
New Revision: 13d0318a9848ec322ceea4f37fb6b421d70407b0

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

LOG: [lldb] Add support for gdb-style 'x' packet (#124733)

See also
https://discourse.llvm.org/t/rfc-fixing-incompatibilties-of-the-x-packet-w-r-t-gdb/84288
and https://sourceware.org/pipermail/gdb/2025-January/051705.html

Added: 
lldb/test/API/functionalities/gdb_remote_client/TestReadMemory.py

Modified: 
lldb/packages/Python/lldbsuite/test/gdbclientutils.py
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py 
b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
index 1784487323ad6b..4b782b3b470fe2 100644
--- a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
+++ b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
@@ -126,6 +126,9 @@ def respond(self, packet):
 if packet[0] == "m":
 addr, length = [int(x, 16) for x in packet[1:].split(",")]
 return self.readMemory(addr, length)
+if packet[0] == "x":
+addr, length = [int(x, 16) for x in packet[1:].split(",")]
+return self.x(addr, length)
 if packet[0] == "M":
 location, encoded_data = packet[1:].split(":")
 addr, length = [int(x, 16) for x in location.split(",")]
@@ -267,6 +270,9 @@ def writeRegister(self, register, value_hex):
 def readMemory(self, addr, length):
 return "00" * length
 
+def x(self, addr, length):
+return ""
+
 def writeMemory(self, addr, data_hex):
 return "OK"
 

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index b3f1c6f052955b..581dd8f8e0b6b6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -275,7 +275,6 @@ void 
GDBRemoteCommunicationClient::ResetDiscoverableSettings(bool did_exec) {
 m_supports_vCont_s = eLazyBoolCalculate;
 m_supports_vCont_S = eLazyBoolCalculate;
 m_supports_p = eLazyBoolCalculate;
-m_supports_x = eLazyBoolCalculate;
 m_supports_QSaveRegisterState = eLazyBoolCalculate;
 m_qHostInfo_is_valid = eLazyBoolCalculate;
 m_curr_pid_is_valid = eLazyBoolCalculate;
@@ -295,6 +294,7 @@ void 
GDBRemoteCommunicationClient::ResetDiscoverableSettings(bool did_exec) {
 m_supports_qXfer_siginfo_read = eLazyBoolCalculate;
 m_supports_augmented_libraries_svr4_read = eLazyBoolCalculate;
 m_uses_native_signals = eLazyBoolCalculate;
+m_x_packet_state.reset();
 m_supports_qProcessInfoPID = true;
 m_supports_qfProcessInfo = true;
 m_supports_qUserName = true;
@@ -348,6 +348,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   m_supports_memory_tagging = eLazyBoolNo;
   m_supports_qSaveCore = eLazyBoolNo;
   m_uses_native_signals = eLazyBoolNo;
+  m_x_packet_state.reset();
 
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
   // not, we assume no limit
@@ -401,6 +402,8 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
 m_supports_qSaveCore = eLazyBoolYes;
   else if (x == "native-signals+")
 m_uses_native_signals = eLazyBoolYes;
+  else if (x == "binary-upload+")
+m_x_packet_state = xPacketState::Prefixed;
   // Look for a list of compressions in the features list e.g.
   // 
qXfer:features:read+;PacketSize=2;qEcho+;SupportedCompressions=zlib-
   // deflate,lzma
@@ -715,19 +718,20 @@ Status GDBRemoteCommunicationClient::WriteMemoryTags(
   return status;
 }
 
-bool GDBRemoteCommunicationClient::GetxPacketSupported() {
-  if (m_supports_x == eLazyBoolCalculate) {
+GDBRemoteCommunicationClient::xPacketState
+GDBRemoteCommunicationClient::GetxPacketState() {
+  if (!m_x_packet_state)
+GetRemoteQSupported();
+  if (!m_x_packet_state) {
 StringExtractorGDBRemote response;
-m_supports_x = eLazyBoolNo;
-char packet[256];
-snprintf(packet, sizeof(packet), "x0,0");
-if (SendPacketAndWaitForResponse(packet, response) ==
+m_x_packet_state = xPacketState::Unimplemented;
+if (SendPacketAndWaitForResponse("x0,0", response) ==
 PacketResult::Success) {
   if (response.IsOKResponse())
-m_supports_x = eLazyBoolYes;
+m_x_packet_state = xPacketState::Bare;
 }
   }
-  return m_supports_x;
+  return *m_x_packet_s

[Lldb-commits] [lldb] [lldb] Add support for gdb-style 'x' packet (PR #124733)

2025-01-31 Thread Pavel Labath via lldb-commits

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


[Lldb-commits] [lldb] 614f1ab - [lldb] Add RISCV for Makefile.rules (#124758)

2025-01-31 Thread via lldb-commits

Author: kper
Date: 2025-01-31T10:01:35Z
New Revision: 614f1ab7575c83a28811fc6881eba512788101a0

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

LOG: [lldb] Add RISCV for Makefile.rules (#124758)

As discussed with @DavidSpickett in discord. Running the test runner
caused the following issue:

```
gmake: Entering directory 
'/home/kper/oss/llvm-project/build/lldb-test-build.noindex/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.test'
/home/kper/oss/llvm-project/build/bin/clang++  -std=c++11 -g -O0  -mriscv 
-I/home/kper/oss/llvm-project/lldb/packages/Python/lldbsuite/test/make/../../../../..//include
 -I/home/kper/oss/llvm-project/build/tools/lldb/include 
-I/home/kper/oss/llvm-project/lldb/test/API/functionalities/thread/concurrent_events
 -I/home/kper/oss/llvm-project/lldb/packages/Python/lldbsuite/test/make 
-include 
/home/kper/oss/llvm-project/lldb/packages/Python/lldbsuite/test/make/test_common.h
 -fno-limit-debug-info  --driver-mode=g++ -MT main.o -MD -MP -MF main.d -c 
-o main.o 
/home/kper/oss/llvm-project/lldb/test/API/functionalities/thread/concurrent_events/main.cpp
clang++: error: unknown argument: '-mriscv'
gmake: *** [Makefile.rules:619: main.o] Error 1
```

By overriding the flags, we avoid the `-mriscv`.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/make/Makefile.rules

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules 
b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index 2da6ff226b615c3..06959f226066ac8 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -207,6 +207,10 @@ else
override ARCH :=
override ARCHFLAG :=
endif
+   ifeq "$(ARCH)" "riscv"
+   override ARCH :=
+   override ARCHFLAG :=
+   endif
ifeq "$(findstring mips,$(ARCH))" "mips"
override ARCHFLAG := -
endif



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


[Lldb-commits] [lldb] [lldb] Add RISCV for Makefile.rules (PR #124758)

2025-01-31 Thread David Spickett via lldb-commits

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,192 @@
+//===-- DILLexerTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::StringRef;
+
+std::vector>
+ExtractTokenData(lldb_private::dil::DILLexer &lexer) {
+  std::vector> data;
+  if (lexer.NumLexedTokens() == 0)
+return data;
+
+  lexer.ResetTokenIdx(UINT_MAX);
+  do {
+lexer.Advance();
+lldb_private::dil::Token tok = lexer.GetCurrentToken();
+data.push_back(std::make_pair(tok.GetKind(), tok.GetSpelling()));
+  } while (data.back().first != lldb_private::dil::Token::eof);
+  return data;
+}
+
+TEST(DILLexerTests, SimpleTest) {
+  StringRef input_expr("simple_var");
+  uint32_t tok_len = 10;
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "simple_var");
+  EXPECT_EQ(token.GetSpelling().size(), tok_len);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
+}
+
+TEST(DILLexerTests, TokenKindTest) {
+  StringRef input_expr("namespace");
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+
+  EXPECT_TRUE(token.Is(lldb_private::dil::Token::identifier));
+  EXPECT_FALSE(token.Is(lldb_private::dil::Token::l_paren));
+  EXPECT_TRUE(token.IsOneOf(lldb_private::dil::Token::eof,
+lldb_private::dil::Token::identifier));
+  EXPECT_FALSE(token.IsOneOf(
+  lldb_private::dil::Token::l_paren, lldb_private::dil::Token::r_paren,
+  lldb_private::dil::Token::coloncolon, lldb_private::dil::Token::eof));
+}
+
+TEST(DILLexerTests, LookAheadTest) {
+  StringRef input_expr("(anonymous namespace)::some_var");
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  uint32_t expect_loc = 23;
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+
+  // Current token is '('; check the next 4 tokens, to make
+  // sure they are the identifier 'anonymous', the identifier 'namespace'
+  // ')' and '::', in that order.
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::l_paren);
+  EXPECT_EQ(lexer.LookAhead(1).GetKind(), 
lldb_private::dil::Token::identifier);
+  EXPECT_EQ(lexer.LookAhead(1).GetSpelling(), "anonymous");
+  EXPECT_EQ(lexer.LookAhead(2).GetKind(), 
lldb_private::dil::Token::identifier);
+  EXPECT_EQ(lexer.LookAhead(2).GetSpelling(), "namespace");
+  EXPECT_EQ(lexer.LookAhead(3).GetKind(), lldb_private::dil::Token::r_paren);
+  EXPECT_EQ(lexer.LookAhead(4).GetKind(), 
lldb_private::dil::Token::coloncolon);
+
+  // Our current index should still be 0, as we only looked ahead; we are still
+  // officially on the '('.
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)0);
+
+  // Accept the 'lookahead', so our current token is '::', which has the index
+  // 4 in our vector of tokens (which starts at zero).
+  lexer.Advance(4);
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::coloncolon);
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)4);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "some_var");
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)5);
+  // Verify we've advanced our position counter (lexing location) in the
+  // input 23 characters (the length of '(anonymous namespace)::'.
+  EXPECT_EQ(token.GetLocation(), expect_loc);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
+}
+
+TEST(DILLexerTests, MultiTokenLexTest) {
+  StringRef inpu

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits

https://github.com/labath commented:

Apart from the (mainly stylistic) inline comments, the biggest problem I see is 
that the definition of an identifier is still too narrow. The restriction on 
the dollar sign is completely unnecessary as C will let you put that 
[anywhere](https://godbolt.org/z/o7qbfeWve). And it doesn't allow any non-ascii 
characters.

I really think this should be based on an deny- rather than an allow-list. Any 
character we don't claim for ourselves should be fair game for an identifier. 
If someone manages to enter the backspace character (`\x7f`) into the 
expression, then so be it.

The question of "identifiers" starting with digits is interesting. Personally, 
I think it'd be fine to reject those (and require the currenly-vapourware 
quoting syntax), because I suspect you want to accept number suffixes, and I 
think it'd be confusing to explain why `123x` is a valid identifier but `123u` 
is not, but I suspect some might have a different opinion.

We could continue discussing that here, or we could accept everything here, and 
postpone this discussion for the patch which starts parsing numbers. Up to you..

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,131 @@
+//===-- DILLexer.h --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_VALUEOBJECT_DILLEXER_H_
+#define LLDB_VALUEOBJECT_DILLEXER_H_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace lldb_private::dil {
+
+/// Class defining the tokens generated by the DIL lexer and used by the
+/// DIL parser.
+class Token {
+public:
+  enum Kind {
+coloncolon,
+eof,
+identifier,
+l_paren,
+r_paren,
+unknown,

labath wrote:

It looks like the only remaining use of the "unknown" token is to construct 
"empty" tokens in the tests. I think all of those cases could be avoided by 
just declaring the Token variable later in the code.

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,132 @@
+//===-- DILLexer.cpp 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// This implements the recursive descent parser for the Data Inspection
+// Language (DIL), and its helper functions, which will eventually underlie the
+// 'frame variable' command. The language that this parser recognizes is
+// described in lldb/docs/dil-expr-lang.ebnf
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/ADT/StringSwitch.h"
+
+namespace lldb_private::dil {
+
+llvm::StringRef Token::GetTokenName(Kind kind) {
+  switch (kind) {
+  case Kind::coloncolon:
+return "coloncolon";
+  case Kind::eof:
+return "eof";
+  case Kind::identifier:
+return "identifier";
+  case Kind::l_paren:
+return "l_paren";
+  case Kind::r_paren:
+return "r_paren";
+  case Kind::unknown:
+return "unknown";
+  }
+}
+
+static bool IsLetter(char c) {
+  return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z');
+}
+
+static bool IsDigit(char c) { return '0' <= c && c <= '9'; }
+
+// A word starts with a letter, underscore, or dollar sign, followed by
+// letters ('a'..'z','A'..'Z'), digits ('0'..'9'), and/or  underscores.
+static std::optional IsWord(llvm::StringRef expr,
+ llvm::StringRef &remainder) {
+  llvm::StringRef::iterator cur_pos = expr.end() - remainder.size();
+  llvm::StringRef::iterator start = cur_pos;
+  bool dollar_start = false;
+
+  // Must not start with a digit.
+  if (cur_pos == expr.end() || IsDigit(*cur_pos))
+return std::nullopt;
+
+  // First character *may* be a '$', for a register name or convenience
+  // variable.
+  if (*cur_pos == '$') {
+dollar_start = true;
+++cur_pos;
+  }
+
+  // Contains only letters, digits or underscores
+  for (; cur_pos != expr.end(); ++cur_pos) {
+char c = *cur_pos;
+if (!IsLetter(c) && !IsDigit(c) && c != '_')
+  break;
+  }
+
+  // If first char is '$', make sure there's at least one mare char, or it's
+  // invalid.
+  if (dollar_start && (cur_pos - start <= 1)) {
+cur_pos = start;
+return std::nullopt;
+  }
+
+  if (cur_pos == start)
+return std::nullopt;
+
+  llvm::StringRef word = expr.substr(start - expr.begin(), cur_pos - start);
+  if (remainder.consume_front(word))
+return word;
+
+  return std::nullopt;
+}
+
+llvm::Expected DILLexer::Create(llvm::StringRef expr) {
+  std::vector tokens;
+  llvm::StringRef remainder = expr;
+  do {
+if (llvm::Expected t = Lex(expr, remainder)) {
+  tokens.push_back(std::move(*t));
+} else {
+  return t.takeError();
+}
+  } while (tokens.back().GetKind() != Token::eof);
+  return DILLexer(expr, std::move(tokens));
+}
+
+llvm::Expected DILLexer::Lex(llvm::StringRef expr,
+llvm::StringRef &remainder) {
+  // Skip over whitespace (spaces).
+  remainder = remainder.ltrim();
+  llvm::StringRef::iterator cur_pos = expr.end() - remainder.size();
+
+  // Check to see if we've reached the end of our input string.
+  if (remainder.empty() || cur_pos == expr.end())
+return Token(Token::eof, "", (uint32_t)expr.size());
+
+  uint32_t position = cur_pos - expr.begin();
+  std::optional maybe_word = IsWord(expr, remainder);
+  if (maybe_word) {
+llvm::StringRef word = *maybe_word;
+return Token(Token::identifier, word.str(), position);
+  }
+
+  constexpr std::pair operators[] = {
+  {Token::l_paren, "("},
+  {Token::r_paren, ")"},
+  {Token::coloncolon, "::"},
+  };
+  for (auto [kind, str] : operators) {
+if (remainder.consume_front(str)) {
+  cur_pos += strlen(str);
+  return Token(kind, str, position);
+}

labath wrote:

```suggestion
if (remainder.consume_front(str))
  return Token(kind, str, position);
```

(cur_pos isn't used after this)

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,131 @@
+//===-- DILLexer.h --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_VALUEOBJECT_DILLEXER_H_
+#define LLDB_VALUEOBJECT_DILLEXER_H_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace lldb_private::dil {
+
+/// Class defining the tokens generated by the DIL lexer and used by the
+/// DIL parser.
+class Token {
+public:
+  enum Kind {
+coloncolon,
+eof,
+identifier,
+l_paren,
+r_paren,
+unknown,
+  };
+
+  Token(Kind kind, std::string spelling, uint32_t start)
+  : m_kind(kind), m_spelling(spelling), m_start_pos(start) {}
+
+  Kind GetKind() const { return m_kind; }
+
+  std::string GetSpelling() const { return m_spelling; }
+
+  bool Is(Kind kind) const { return m_kind == kind; }
+
+  bool IsNot(Kind kind) const { return m_kind != kind; }
+
+  bool IsOneOf(Kind kind1, Kind kind2) const { return Is(kind1) || Is(kind2); }
+
+  template  bool IsOneOf(Kind kind, Ts... Ks) const {
+return Is(kind) || IsOneOf(Ks...);
+  }
+
+  uint32_t GetLocation() const { return m_start_pos; }
+
+  static llvm::StringRef GetTokenName(Kind kind);
+
+private:
+  Kind m_kind;
+  std::string m_spelling;
+  uint32_t m_start_pos; // within entire expression string
+};
+
+/// Class for doing the simple lexing required by DIL.
+class DILLexer {
+public:
+  /// Lexes all the tokens in expr and calls the private constructor
+  /// with the lexed tokens.
+  static llvm::Expected Create(llvm::StringRef expr);
+
+  /// Return the current token to be handled by the DIL parser.
+  const Token &GetCurrentToken() { return m_lexed_tokens[m_tokens_idx]; }
+
+  /// Advance the current token position by N.
+  void Advance(uint32_t N = 1) {
+// UINT_MAX means uninitialized, no "current" position, so move to start.
+if (m_tokens_idx == UINT_MAX)
+  m_tokens_idx = 0;
+else if (m_tokens_idx + N >= m_lexed_tokens.size())
+  // N is too large; advance to the end of the lexed tokens.
+  m_tokens_idx = m_lexed_tokens.size() - 1;
+else
+  m_tokens_idx += N;
+  }
+
+  /// Return the lexed token N positions ahead of the 'current' token
+  /// being handled by the DIL parser.
+  const Token &LookAhead(uint32_t N) {
+if (m_tokens_idx + N < m_lexed_tokens.size())
+  return m_lexed_tokens[m_tokens_idx + N];
+
+return m_eof_token;

labath wrote:

A valid token stream should always end in and EOF token, right? If, so you can 
just return the last element.

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,192 @@
+//===-- DILLexerTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::StringRef;
+
+std::vector>
+ExtractTokenData(lldb_private::dil::DILLexer &lexer) {
+  std::vector> data;
+  if (lexer.NumLexedTokens() == 0)
+return data;
+
+  lexer.ResetTokenIdx(UINT_MAX);
+  do {
+lexer.Advance();
+lldb_private::dil::Token tok = lexer.GetCurrentToken();
+data.push_back(std::make_pair(tok.GetKind(), tok.GetSpelling()));
+  } while (data.back().first != lldb_private::dil::Token::eof);
+  return data;
+}
+
+TEST(DILLexerTests, SimpleTest) {
+  StringRef input_expr("simple_var");
+  uint32_t tok_len = 10;
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "simple_var");
+  EXPECT_EQ(token.GetSpelling().size(), tok_len);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
+}
+
+TEST(DILLexerTests, TokenKindTest) {
+  StringRef input_expr("namespace");
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();

labath wrote:

```suggestion
  lldb_private::dil::Token token =
  lldb_private::dil::Token(lldb_private::dil::Token::identifier, "ident", 
0);
```

(The reason is that this looks like a test for the token kind testing 
functions. You don't actually need the token to come out of the lexer for that. 
And we already have tests that check that producing a sequence of characters 
produces an "identifier" token.)

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,192 @@
+//===-- DILLexerTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::StringRef;
+
+std::vector>
+ExtractTokenData(lldb_private::dil::DILLexer &lexer) {
+  std::vector> data;
+  if (lexer.NumLexedTokens() == 0)
+return data;
+
+  lexer.ResetTokenIdx(UINT_MAX);
+  do {
+lexer.Advance();
+lldb_private::dil::Token tok = lexer.GetCurrentToken();
+data.push_back(std::make_pair(tok.GetKind(), tok.GetSpelling()));
+  } while (data.back().first != lldb_private::dil::Token::eof);
+  return data;
+}
+
+TEST(DILLexerTests, SimpleTest) {
+  StringRef input_expr("simple_var");
+  uint32_t tok_len = 10;
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "simple_var");
+  EXPECT_EQ(token.GetSpelling().size(), tok_len);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
+}
+
+TEST(DILLexerTests, TokenKindTest) {
+  StringRef input_expr("namespace");
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+
+  EXPECT_TRUE(token.Is(lldb_private::dil::Token::identifier));
+  EXPECT_FALSE(token.Is(lldb_private::dil::Token::l_paren));
+  EXPECT_TRUE(token.IsOneOf(lldb_private::dil::Token::eof,
+lldb_private::dil::Token::identifier));
+  EXPECT_FALSE(token.IsOneOf(
+  lldb_private::dil::Token::l_paren, lldb_private::dil::Token::r_paren,
+  lldb_private::dil::Token::coloncolon, lldb_private::dil::Token::eof));
+}
+
+TEST(DILLexerTests, LookAheadTest) {
+  StringRef input_expr("(anonymous namespace)::some_var");
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  uint32_t expect_loc = 23;
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+
+  // Current token is '('; check the next 4 tokens, to make
+  // sure they are the identifier 'anonymous', the identifier 'namespace'
+  // ')' and '::', in that order.
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::l_paren);
+  EXPECT_EQ(lexer.LookAhead(1).GetKind(), 
lldb_private::dil::Token::identifier);
+  EXPECT_EQ(lexer.LookAhead(1).GetSpelling(), "anonymous");
+  EXPECT_EQ(lexer.LookAhead(2).GetKind(), 
lldb_private::dil::Token::identifier);
+  EXPECT_EQ(lexer.LookAhead(2).GetSpelling(), "namespace");
+  EXPECT_EQ(lexer.LookAhead(3).GetKind(), lldb_private::dil::Token::r_paren);
+  EXPECT_EQ(lexer.LookAhead(4).GetKind(), 
lldb_private::dil::Token::coloncolon);
+
+  // Our current index should still be 0, as we only looked ahead; we are still
+  // officially on the '('.
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)0);
+
+  // Accept the 'lookahead', so our current token is '::', which has the index
+  // 4 in our vector of tokens (which starts at zero).
+  lexer.Advance(4);
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::coloncolon);
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)4);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "some_var");
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)5);
+  // Verify we've advanced our position counter (lexing location) in the
+  // input 23 characters (the length of '(anonymous namespace)::'.
+  EXPECT_EQ(token.GetLocation(), expect_loc);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
+}
+
+TEST(DILLexerTests, MultiTokenLexTest) {
+  StringRef inpu

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,132 @@
+//===-- DILLexer.cpp 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// This implements the recursive descent parser for the Data Inspection
+// Language (DIL), and its helper functions, which will eventually underlie the
+// 'frame variable' command. The language that this parser recognizes is
+// described in lldb/docs/dil-expr-lang.ebnf
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/ADT/StringSwitch.h"
+
+namespace lldb_private::dil {
+
+llvm::StringRef Token::GetTokenName(Kind kind) {
+  switch (kind) {
+  case Kind::coloncolon:
+return "coloncolon";
+  case Kind::eof:
+return "eof";
+  case Kind::identifier:
+return "identifier";
+  case Kind::l_paren:
+return "l_paren";
+  case Kind::r_paren:
+return "r_paren";
+  case Kind::unknown:
+return "unknown";
+  }
+}
+
+static bool IsLetter(char c) {
+  return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z');
+}
+
+static bool IsDigit(char c) { return '0' <= c && c <= '9'; }
+
+// A word starts with a letter, underscore, or dollar sign, followed by
+// letters ('a'..'z','A'..'Z'), digits ('0'..'9'), and/or  underscores.
+static std::optional IsWord(llvm::StringRef expr,
+ llvm::StringRef &remainder) {
+  llvm::StringRef::iterator cur_pos = expr.end() - remainder.size();
+  llvm::StringRef::iterator start = cur_pos;
+  bool dollar_start = false;
+
+  // Must not start with a digit.
+  if (cur_pos == expr.end() || IsDigit(*cur_pos))
+return std::nullopt;
+
+  // First character *may* be a '$', for a register name or convenience
+  // variable.
+  if (*cur_pos == '$') {
+dollar_start = true;
+++cur_pos;
+  }
+
+  // Contains only letters, digits or underscores
+  for (; cur_pos != expr.end(); ++cur_pos) {
+char c = *cur_pos;
+if (!IsLetter(c) && !IsDigit(c) && c != '_')
+  break;
+  }
+
+  // If first char is '$', make sure there's at least one mare char, or it's
+  // invalid.
+  if (dollar_start && (cur_pos - start <= 1)) {
+cur_pos = start;
+return std::nullopt;
+  }
+
+  if (cur_pos == start)
+return std::nullopt;
+
+  llvm::StringRef word = expr.substr(start - expr.begin(), cur_pos - start);
+  if (remainder.consume_front(word))
+return word;
+
+  return std::nullopt;
+}
+
+llvm::Expected DILLexer::Create(llvm::StringRef expr) {
+  std::vector tokens;
+  llvm::StringRef remainder = expr;
+  do {
+if (llvm::Expected t = Lex(expr, remainder)) {
+  tokens.push_back(std::move(*t));
+} else {
+  return t.takeError();
+}
+  } while (tokens.back().GetKind() != Token::eof);
+  return DILLexer(expr, std::move(tokens));
+}
+
+llvm::Expected DILLexer::Lex(llvm::StringRef expr,
+llvm::StringRef &remainder) {
+  // Skip over whitespace (spaces).
+  remainder = remainder.ltrim();
+  llvm::StringRef::iterator cur_pos = expr.end() - remainder.size();

labath wrote:

```suggestion
  llvm::StringRef::iterator cur_pos = remainder.begin();
```

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,131 @@
+//===-- DILLexer.h --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_VALUEOBJECT_DILLEXER_H_
+#define LLDB_VALUEOBJECT_DILLEXER_H_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace lldb_private::dil {
+
+/// Class defining the tokens generated by the DIL lexer and used by the
+/// DIL parser.
+class Token {
+public:
+  enum Kind {
+coloncolon,
+eof,
+identifier,
+l_paren,
+r_paren,
+unknown,
+  };
+
+  Token(Kind kind, std::string spelling, uint32_t start)
+  : m_kind(kind), m_spelling(spelling), m_start_pos(start) {}
+
+  Kind GetKind() const { return m_kind; }
+
+  std::string GetSpelling() const { return m_spelling; }
+
+  bool Is(Kind kind) const { return m_kind == kind; }
+
+  bool IsNot(Kind kind) const { return m_kind != kind; }
+
+  bool IsOneOf(Kind kind1, Kind kind2) const { return Is(kind1) || Is(kind2); }
+
+  template  bool IsOneOf(Kind kind, Ts... Ks) const {
+return Is(kind) || IsOneOf(Ks...);
+  }
+
+  uint32_t GetLocation() const { return m_start_pos; }
+
+  static llvm::StringRef GetTokenName(Kind kind);
+
+private:
+  Kind m_kind;
+  std::string m_spelling;
+  uint32_t m_start_pos; // within entire expression string
+};
+
+/// Class for doing the simple lexing required by DIL.
+class DILLexer {
+public:
+  /// Lexes all the tokens in expr and calls the private constructor
+  /// with the lexed tokens.
+  static llvm::Expected Create(llvm::StringRef expr);
+
+  /// Return the current token to be handled by the DIL parser.
+  const Token &GetCurrentToken() { return m_lexed_tokens[m_tokens_idx]; }
+
+  /// Advance the current token position by N.
+  void Advance(uint32_t N = 1) {
+// UINT_MAX means uninitialized, no "current" position, so move to start.
+if (m_tokens_idx == UINT_MAX)
+  m_tokens_idx = 0;
+else if (m_tokens_idx + N >= m_lexed_tokens.size())
+  // N is too large; advance to the end of the lexed tokens.
+  m_tokens_idx = m_lexed_tokens.size() - 1;
+else
+  m_tokens_idx += N;
+  }
+
+  /// Return the lexed token N positions ahead of the 'current' token
+  /// being handled by the DIL parser.
+  const Token &LookAhead(uint32_t N) {
+if (m_tokens_idx + N < m_lexed_tokens.size())
+  return m_lexed_tokens[m_tokens_idx + N];
+
+return m_eof_token;
+  }
+
+  /// Return the index for the 'current' token being handled by the DIL parser.
+  uint32_t GetCurrentTokenIdx() { return m_tokens_idx; }
+
+  /// Set the index for the 'current' token (to be handled by the parser)
+  /// to a particular position. Used for either committing 'look ahead' parsing
+  /// or rolling back tentative parsing.
+  void ResetTokenIdx(uint32_t new_value) {
+assert(new_value == UINT_MAX || new_value < m_lexed_tokens.size());
+m_tokens_idx = new_value;
+  }
+
+  uint32_t NumLexedTokens() { return m_lexed_tokens.size(); }
+
+private:
+  DILLexer(llvm::StringRef dil_expr, std::vector lexed_tokens)
+  : m_expr(dil_expr), m_lexed_tokens(lexed_tokens), m_tokens_idx(UINT_MAX),

labath wrote:

```suggestion
  : m_expr(dil_expr), m_lexed_tokens(std::move(lexed_tokens)), 
m_tokens_idx(UINT_MAX),
```

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,192 @@
+//===-- DILLexerTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::StringRef;
+
+std::vector>
+ExtractTokenData(lldb_private::dil::DILLexer &lexer) {
+  std::vector> data;
+  if (lexer.NumLexedTokens() == 0)
+return data;
+
+  lexer.ResetTokenIdx(UINT_MAX);
+  do {
+lexer.Advance();
+lldb_private::dil::Token tok = lexer.GetCurrentToken();
+data.push_back(std::make_pair(tok.GetKind(), tok.GetSpelling()));
+  } while (data.back().first != lldb_private::dil::Token::eof);
+  return data;
+}
+
+TEST(DILLexerTests, SimpleTest) {
+  StringRef input_expr("simple_var");
+  uint32_t tok_len = 10;
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "simple_var");
+  EXPECT_EQ(token.GetSpelling().size(), tok_len);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
+}
+
+TEST(DILLexerTests, TokenKindTest) {
+  StringRef input_expr("namespace");
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+
+  EXPECT_TRUE(token.Is(lldb_private::dil::Token::identifier));
+  EXPECT_FALSE(token.Is(lldb_private::dil::Token::l_paren));
+  EXPECT_TRUE(token.IsOneOf(lldb_private::dil::Token::eof,
+lldb_private::dil::Token::identifier));
+  EXPECT_FALSE(token.IsOneOf(
+  lldb_private::dil::Token::l_paren, lldb_private::dil::Token::r_paren,
+  lldb_private::dil::Token::coloncolon, lldb_private::dil::Token::eof));
+}
+
+TEST(DILLexerTests, LookAheadTest) {
+  StringRef input_expr("(anonymous namespace)::some_var");
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  uint32_t expect_loc = 23;
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+
+  // Current token is '('; check the next 4 tokens, to make
+  // sure they are the identifier 'anonymous', the identifier 'namespace'
+  // ')' and '::', in that order.
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::l_paren);
+  EXPECT_EQ(lexer.LookAhead(1).GetKind(), 
lldb_private::dil::Token::identifier);
+  EXPECT_EQ(lexer.LookAhead(1).GetSpelling(), "anonymous");
+  EXPECT_EQ(lexer.LookAhead(2).GetKind(), 
lldb_private::dil::Token::identifier);
+  EXPECT_EQ(lexer.LookAhead(2).GetSpelling(), "namespace");
+  EXPECT_EQ(lexer.LookAhead(3).GetKind(), lldb_private::dil::Token::r_paren);
+  EXPECT_EQ(lexer.LookAhead(4).GetKind(), 
lldb_private::dil::Token::coloncolon);
+
+  // Our current index should still be 0, as we only looked ahead; we are still
+  // officially on the '('.
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)0);
+
+  // Accept the 'lookahead', so our current token is '::', which has the index
+  // 4 in our vector of tokens (which starts at zero).
+  lexer.Advance(4);
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::coloncolon);
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)4);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "some_var");
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)5);
+  // Verify we've advanced our position counter (lexing location) in the
+  // input 23 characters (the length of '(anonymous namespace)::'.
+  EXPECT_EQ(token.GetLocation(), expect_loc);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
+}
+
+TEST(DILLexerTests, MultiTokenLexTest) {
+  StringRef inpu

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,132 @@
+//===-- DILLexer.cpp 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// This implements the recursive descent parser for the Data Inspection
+// Language (DIL), and its helper functions, which will eventually underlie the
+// 'frame variable' command. The language that this parser recognizes is
+// described in lldb/docs/dil-expr-lang.ebnf
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/ADT/StringSwitch.h"
+
+namespace lldb_private::dil {
+
+llvm::StringRef Token::GetTokenName(Kind kind) {
+  switch (kind) {
+  case Kind::coloncolon:
+return "coloncolon";
+  case Kind::eof:
+return "eof";
+  case Kind::identifier:
+return "identifier";
+  case Kind::l_paren:
+return "l_paren";
+  case Kind::r_paren:
+return "r_paren";
+  case Kind::unknown:
+return "unknown";
+  }
+}
+
+static bool IsLetter(char c) {
+  return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z');
+}
+
+static bool IsDigit(char c) { return '0' <= c && c <= '9'; }
+
+// A word starts with a letter, underscore, or dollar sign, followed by
+// letters ('a'..'z','A'..'Z'), digits ('0'..'9'), and/or  underscores.
+static std::optional IsWord(llvm::StringRef expr,
+ llvm::StringRef &remainder) {
+  llvm::StringRef::iterator cur_pos = expr.end() - remainder.size();
+  llvm::StringRef::iterator start = cur_pos;
+  bool dollar_start = false;
+
+  // Must not start with a digit.
+  if (cur_pos == expr.end() || IsDigit(*cur_pos))
+return std::nullopt;
+
+  // First character *may* be a '$', for a register name or convenience
+  // variable.
+  if (*cur_pos == '$') {
+dollar_start = true;
+++cur_pos;
+  }
+
+  // Contains only letters, digits or underscores
+  for (; cur_pos != expr.end(); ++cur_pos) {
+char c = *cur_pos;
+if (!IsLetter(c) && !IsDigit(c) && c != '_')
+  break;
+  }
+
+  // If first char is '$', make sure there's at least one mare char, or it's
+  // invalid.
+  if (dollar_start && (cur_pos - start <= 1)) {
+cur_pos = start;
+return std::nullopt;
+  }
+
+  if (cur_pos == start)
+return std::nullopt;
+
+  llvm::StringRef word = expr.substr(start - expr.begin(), cur_pos - start);
+  if (remainder.consume_front(word))
+return word;
+
+  return std::nullopt;

labath wrote:

This should be equivalent to your code, but I'm only suggesting this to show 
how code like this can be written in a shorter and more stringref-y fashion. I 
think the actual algorithm will have to change, as it has a very narrow 
definition of identifiers.
```suggestion
  const char *start = remainder.data();
  remainder.consume_front("$"); // initial '$' is valid
  remainder = remainder.drop_while([](char c){  return IsDigit(c) || 
IsLetter(c) || c=='_'; });
  llvm::StringRef candidate(start, remainder.data()-start);
  if (candidate.empty() || candidate == "$" || IsDigit(candidate[0]))
return std::nullopt;
  return candidate;
```

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,132 @@
+//===-- DILLexer.cpp 
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// This implements the recursive descent parser for the Data Inspection
+// Language (DIL), and its helper functions, which will eventually underlie the
+// 'frame variable' command. The language that this parser recognizes is
+// described in lldb/docs/dil-expr-lang.ebnf
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/ADT/StringSwitch.h"
+
+namespace lldb_private::dil {
+
+llvm::StringRef Token::GetTokenName(Kind kind) {
+  switch (kind) {
+  case Kind::coloncolon:
+return "coloncolon";
+  case Kind::eof:
+return "eof";
+  case Kind::identifier:
+return "identifier";
+  case Kind::l_paren:
+return "l_paren";
+  case Kind::r_paren:
+return "r_paren";
+  case Kind::unknown:
+return "unknown";
+  }
+}
+
+static bool IsLetter(char c) {
+  return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z');
+}
+
+static bool IsDigit(char c) { return '0' <= c && c <= '9'; }
+
+// A word starts with a letter, underscore, or dollar sign, followed by
+// letters ('a'..'z','A'..'Z'), digits ('0'..'9'), and/or  underscores.
+static std::optional IsWord(llvm::StringRef expr,
+ llvm::StringRef &remainder) {
+  llvm::StringRef::iterator cur_pos = expr.end() - remainder.size();
+  llvm::StringRef::iterator start = cur_pos;
+  bool dollar_start = false;
+
+  // Must not start with a digit.
+  if (cur_pos == expr.end() || IsDigit(*cur_pos))
+return std::nullopt;
+
+  // First character *may* be a '$', for a register name or convenience
+  // variable.
+  if (*cur_pos == '$') {
+dollar_start = true;
+++cur_pos;
+  }
+
+  // Contains only letters, digits or underscores
+  for (; cur_pos != expr.end(); ++cur_pos) {
+char c = *cur_pos;
+if (!IsLetter(c) && !IsDigit(c) && c != '_')
+  break;
+  }
+
+  // If first char is '$', make sure there's at least one mare char, or it's
+  // invalid.
+  if (dollar_start && (cur_pos - start <= 1)) {
+cur_pos = start;
+return std::nullopt;
+  }
+
+  if (cur_pos == start)
+return std::nullopt;
+
+  llvm::StringRef word = expr.substr(start - expr.begin(), cur_pos - start);
+  if (remainder.consume_front(word))
+return word;
+
+  return std::nullopt;
+}
+
+llvm::Expected DILLexer::Create(llvm::StringRef expr) {
+  std::vector tokens;
+  llvm::StringRef remainder = expr;
+  do {
+if (llvm::Expected t = Lex(expr, remainder)) {
+  tokens.push_back(std::move(*t));
+} else {
+  return t.takeError();
+}
+  } while (tokens.back().GetKind() != Token::eof);
+  return DILLexer(expr, std::move(tokens));
+}
+
+llvm::Expected DILLexer::Lex(llvm::StringRef expr,
+llvm::StringRef &remainder) {
+  // Skip over whitespace (spaces).
+  remainder = remainder.ltrim();
+  llvm::StringRef::iterator cur_pos = expr.end() - remainder.size();
+
+  // Check to see if we've reached the end of our input string.
+  if (remainder.empty() || cur_pos == expr.end())
+return Token(Token::eof, "", (uint32_t)expr.size());
+
+  uint32_t position = cur_pos - expr.begin();
+  std::optional maybe_word = IsWord(expr, remainder);
+  if (maybe_word) {
+llvm::StringRef word = *maybe_word;
+return Token(Token::identifier, word.str(), position);
+  }

labath wrote:

(it's shorter, and I don't think the extra local variable helps anything)

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,192 @@
+//===-- DILLexerTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::StringRef;
+
+std::vector>
+ExtractTokenData(lldb_private::dil::DILLexer &lexer) {
+  std::vector> data;
+  if (lexer.NumLexedTokens() == 0)
+return data;
+
+  lexer.ResetTokenIdx(UINT_MAX);
+  do {
+lexer.Advance();
+lldb_private::dil::Token tok = lexer.GetCurrentToken();
+data.push_back(std::make_pair(tok.GetKind(), tok.GetSpelling()));
+  } while (data.back().first != lldb_private::dil::Token::eof);
+  return data;
+}
+
+TEST(DILLexerTests, SimpleTest) {
+  StringRef input_expr("simple_var");
+  uint32_t tok_len = 10;
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "simple_var");
+  EXPECT_EQ(token.GetSpelling().size(), tok_len);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
+}
+
+TEST(DILLexerTests, TokenKindTest) {
+  StringRef input_expr("namespace");
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+
+  EXPECT_TRUE(token.Is(lldb_private::dil::Token::identifier));
+  EXPECT_FALSE(token.Is(lldb_private::dil::Token::l_paren));
+  EXPECT_TRUE(token.IsOneOf(lldb_private::dil::Token::eof,
+lldb_private::dil::Token::identifier));
+  EXPECT_FALSE(token.IsOneOf(
+  lldb_private::dil::Token::l_paren, lldb_private::dil::Token::r_paren,
+  lldb_private::dil::Token::coloncolon, lldb_private::dil::Token::eof));
+}
+
+TEST(DILLexerTests, LookAheadTest) {
+  StringRef input_expr("(anonymous namespace)::some_var");
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  uint32_t expect_loc = 23;
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+
+  // Current token is '('; check the next 4 tokens, to make
+  // sure they are the identifier 'anonymous', the identifier 'namespace'
+  // ')' and '::', in that order.
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::l_paren);
+  EXPECT_EQ(lexer.LookAhead(1).GetKind(), 
lldb_private::dil::Token::identifier);
+  EXPECT_EQ(lexer.LookAhead(1).GetSpelling(), "anonymous");
+  EXPECT_EQ(lexer.LookAhead(2).GetKind(), 
lldb_private::dil::Token::identifier);
+  EXPECT_EQ(lexer.LookAhead(2).GetSpelling(), "namespace");
+  EXPECT_EQ(lexer.LookAhead(3).GetKind(), lldb_private::dil::Token::r_paren);
+  EXPECT_EQ(lexer.LookAhead(4).GetKind(), 
lldb_private::dil::Token::coloncolon);
+
+  // Our current index should still be 0, as we only looked ahead; we are still
+  // officially on the '('.
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)0);
+
+  // Accept the 'lookahead', so our current token is '::', which has the index
+  // 4 in our vector of tokens (which starts at zero).
+  lexer.Advance(4);
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::coloncolon);
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)4);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "some_var");
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)5);
+  // Verify we've advanced our position counter (lexing location) in the
+  // input 23 characters (the length of '(anonymous namespace)::'.
+  EXPECT_EQ(token.GetLocation(), expect_loc);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
+}
+
+TEST(DILLexerTests, MultiTokenLexTest) {
+  StringRef inpu

[Lldb-commits] [lldb] 3736de2 - [lldb] Use Function::GetAddress in Module::FindFunctions (#124938)

2025-01-31 Thread via lldb-commits

Author: Pavel Labath
Date: 2025-01-31T09:12:56+01:00
New Revision: 3736de2e3c20b4ed496a6590bc758ac0e033bd4c

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

LOG: [lldb] Use Function::GetAddress in Module::FindFunctions (#124938)

The original code resulted in a misfire in the symtab vs. debug info
deduplication code, which caused us to return the same function twice
when searching via a regex (for functions whose entry point is also not
the lowest address).

Added: 


Modified: 
lldb/source/Core/Module.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s

Removed: 




diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 9601c834d9b8fe..33668c5d20dde4 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -919,9 +919,8 @@ void Module::FindFunctions(const RegularExpression ®ex,
   const SymbolContext &sc = sc_list[i];
   if (sc.block)
 continue;
-  file_addr_to_index[sc.function->GetAddressRange()
- .GetBaseAddress()
- .GetFileAddress()] = i;
+  file_addr_to_index[sc.function->GetAddress().GetFileAddress()] =
+  i;
 }
 
 FileAddrToIndexMap::const_iterator end = file_addr_to_index.end();

diff  --git a/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s 
b/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s
index 93ea9f33e762df..197ab9aa14910e 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s
@@ -6,17 +6,29 @@
 # The function bar has been placed "in the middle" of foo, and the function
 # entry point is deliberately not its lowest address.
 
-# RUN: llvm-mc -triple x86_64-pc-linux -filetype=obj %s -o %t
-# RUN: %lldb %t -o "image lookup -v -n foo" -o "expr -- &foo" -o exit | 
FileCheck %s
+# RUN: split-file %s %t
+# RUN: llvm-mc -triple x86_64-pc-linux -filetype=obj %t/input.s -o %t/input.o
+# RUN: %lldb %t/input.o -s %t/commands -o exit | FileCheck %s
 
-# CHECK-LABEL: image lookup
+#--- commands
+
+image lookup -v -n foo
+# CHECK-LABEL: image lookup -v -n foo
+# CHECK: 1 match found in {{.*}}
+# CHECK: Summary: input.o`foo
+# CHECK: Function: id = {{.*}}, name = "foo", ranges = 
[0x-0x000e)[0x0014-0x001c)
+
+image lookup -v --regex -n '^foo$'
+# CHECK-LABEL: image lookup -v --regex -n '^foo$'
 # CHECK: 1 match found in {{.*}}
-# CHECK: Summary: {{.*}}`foo
+# CHECK: Summary: input.o`foo
 # CHECK: Function: id = {{.*}}, name = "foo", ranges = 
[0x-0x000e)[0x0014-0x001c)
 
+expr -- &foo
 # CHECK-LABEL: expr -- &foo
 # CHECK: (void (*)()) $0 = 0x0007
 
+#--- input.s
 .text
 
 foo.__part.1:



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


[Lldb-commits] [lldb] [lldb] Use Function::GetAddress in Module::FindFunctions (PR #124938)

2025-01-31 Thread Pavel Labath via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Use Function::GetAddress in Module::FindFunctions (PR #124938)

2025-01-31 Thread Pavel Labath via lldb-commits

labath wrote:

I added a "DEPRECATED" comment, which isn't quite as strong, but sort of 
conveys the same thing. The function is called from a bunch of places, but I 
don't think there's that much churn around it for backsliding to be a problem.

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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,192 @@
+//===-- DILLexerTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::StringRef;
+
+std::vector>
+ExtractTokenData(lldb_private::dil::DILLexer &lexer) {
+  std::vector> data;
+  if (lexer.NumLexedTokens() == 0)
+return data;
+
+  lexer.ResetTokenIdx(UINT_MAX);
+  do {
+lexer.Advance();
+lldb_private::dil::Token tok = lexer.GetCurrentToken();
+data.push_back(std::make_pair(tok.GetKind(), tok.GetSpelling()));
+  } while (data.back().first != lldb_private::dil::Token::eof);
+  return data;
+}
+
+TEST(DILLexerTests, SimpleTest) {
+  StringRef input_expr("simple_var");
+  uint32_t tok_len = 10;
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "simple_var");
+  EXPECT_EQ(token.GetSpelling().size(), tok_len);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
+}
+
+TEST(DILLexerTests, TokenKindTest) {
+  StringRef input_expr("namespace");
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+
+  EXPECT_TRUE(token.Is(lldb_private::dil::Token::identifier));
+  EXPECT_FALSE(token.Is(lldb_private::dil::Token::l_paren));
+  EXPECT_TRUE(token.IsOneOf(lldb_private::dil::Token::eof,
+lldb_private::dil::Token::identifier));
+  EXPECT_FALSE(token.IsOneOf(
+  lldb_private::dil::Token::l_paren, lldb_private::dil::Token::r_paren,
+  lldb_private::dil::Token::coloncolon, lldb_private::dil::Token::eof));
+}
+
+TEST(DILLexerTests, LookAheadTest) {
+  StringRef input_expr("(anonymous namespace)::some_var");
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  uint32_t expect_loc = 23;
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+
+  // Current token is '('; check the next 4 tokens, to make
+  // sure they are the identifier 'anonymous', the identifier 'namespace'
+  // ')' and '::', in that order.
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::l_paren);
+  EXPECT_EQ(lexer.LookAhead(1).GetKind(), 
lldb_private::dil::Token::identifier);
+  EXPECT_EQ(lexer.LookAhead(1).GetSpelling(), "anonymous");
+  EXPECT_EQ(lexer.LookAhead(2).GetKind(), 
lldb_private::dil::Token::identifier);
+  EXPECT_EQ(lexer.LookAhead(2).GetSpelling(), "namespace");
+  EXPECT_EQ(lexer.LookAhead(3).GetKind(), lldb_private::dil::Token::r_paren);
+  EXPECT_EQ(lexer.LookAhead(4).GetKind(), 
lldb_private::dil::Token::coloncolon);
+
+  // Our current index should still be 0, as we only looked ahead; we are still
+  // officially on the '('.
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)0);
+
+  // Accept the 'lookahead', so our current token is '::', which has the index
+  // 4 in our vector of tokens (which starts at zero).
+  lexer.Advance(4);
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::coloncolon);
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)4);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "some_var");
+  EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)5);
+  // Verify we've advanced our position counter (lexing location) in the
+  // input 23 characters (the length of '(anonymous namespace)::'.
+  EXPECT_EQ(token.GetLocation(), expect_loc);

labath wrote:

you might just say `strlen("(anonymous namespace)::")` and skip the constant 
and the comment (this is a test, so it doesn't have to be super-efficient)

[Lldb-commits] [lldb] [lldb] Use llvm::Error instead of CommandReturnObject for error reporting (PR #125125)

2025-01-31 Thread Pavel Labath via lldb-commits

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


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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,192 @@
+//===-- DILLexerTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::StringRef;
+
+std::vector>
+ExtractTokenData(lldb_private::dil::DILLexer &lexer) {
+  std::vector> data;
+  if (lexer.NumLexedTokens() == 0)
+return data;
+
+  lexer.ResetTokenIdx(UINT_MAX);
+  do {
+lexer.Advance();
+lldb_private::dil::Token tok = lexer.GetCurrentToken();
+data.push_back(std::make_pair(tok.GetKind(), tok.GetSpelling()));
+  } while (data.back().first != lldb_private::dil::Token::eof);
+  return data;
+}
+
+TEST(DILLexerTests, SimpleTest) {
+  StringRef input_expr("simple_var");
+  uint32_t tok_len = 10;
+  llvm::Expected maybe_lexer =
+  lldb_private::dil::DILLexer::Create(input_expr);
+  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
+  lldb_private::dil::DILLexer lexer(*maybe_lexer);
+  lldb_private::dil::Token token =
+  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
+
+  lexer.Advance();
+  token = lexer.GetCurrentToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "simple_var");
+  EXPECT_EQ(token.GetSpelling().size(), tok_len);

labath wrote:

Now the size check is unnecessary, as this is just testing the StringRef 
implementation

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


[Lldb-commits] [lldb] [lldb] Add RISCV for Makefile.rules (PR #124758)

2025-01-31 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Please set a valid email address first - 
https://llvm.org/docs/DeveloperPolicy.html#github-email-address

Then this can be merged.

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


[Lldb-commits] [lldb] [lldb] Add RISCV for Makefile.rules (PR #124758)

2025-01-31 Thread David Spickett via lldb-commits

DavidSpickett wrote:

> I am not sure whether I will lose the approval if I rebase.

GitHub seems to let you do almost anything without losing existing approvals, 
for better or worse :)

It would have been fine anyway, it's just doing the rebase onto main that 
"squash and merge" will do anyway.

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


[Lldb-commits] [lldb] 41910f7 - [lldb-dap] Fix build failure on Windows. (#125156)

2025-01-31 Thread via lldb-commits

Author: John Harrison
Date: 2025-01-31T09:48:09Z
New Revision: 41910f72638354cfd27cf7c518dde47e9eb9deab

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

LOG: [lldb-dap] Fix build failure on Windows. (#125156)

A previous change is triggering a failure due to SOCKET not being
defined in IOStream.h. Adjusting the Windows includes to correct the
imports and using a more narrow import (winsock2.h vs windows.h).

Also removed a stale comment.

Tested this on an x86_64 wins 11 vm.

This should fix https://lab.llvm.org/buildbot/#/builders/197/builds/1379
and https://lab.llvm.org/buildbot/#/builders/141/builds/5878

Added: 


Modified: 
lldb/tools/lldb-dap/IOStream.h

Removed: 




diff  --git a/lldb/tools/lldb-dap/IOStream.h b/lldb/tools/lldb-dap/IOStream.h
index 74889eb2e5a866..c91b2f717893c8 100644
--- a/lldb/tools/lldb-dap/IOStream.h
+++ b/lldb/tools/lldb-dap/IOStream.h
@@ -10,13 +10,8 @@
 #define LLDB_TOOLS_LLDB_DAP_IOSTREAM_H
 
 #if defined(_WIN32)
-// We need to #define NOMINMAX in order to skip `min()` and `max()` macro
-// definitions that conflict with other system headers.
-// We also need to #undef GetObject (which is defined to GetObjectW) because
-// the JSON code we use also has methods named `GetObject()` and we conflict
-// against these.
-#define NOMINMAX
-#include 
+#include "lldb/Host/windows/windows.h"
+#include 
 #else
 typedef int SOCKET;
 #endif



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


[Lldb-commits] [lldb] [lldb-dap] Fix build failure on Windows. (PR #125156)

2025-01-31 Thread David Spickett via lldb-commits

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


[Lldb-commits] [lldb] [lldb-dap] Fix build failure on Windows. (PR #125156)

2025-01-31 Thread David Spickett via lldb-commits

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

LGTM

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


[Lldb-commits] [lldb] 2bffa5b - [lldb][Windows] WoA HW Watchpoint support in LLDB (#108072)

2025-01-31 Thread via lldb-commits

Author: Omair Javaid
Date: 2025-01-31T14:11:39+05:00
New Revision: 2bffa5bf7abd4fc7b84f14d029a97c49f4b61130

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

LOG: [lldb][Windows] WoA HW Watchpoint support in LLDB (#108072)

This PR adds support for hardware watchpoints in LLDB for AArch64
Windows targets.

Windows does not provide an API to query the number of available
hardware watchpoints supported by underlying hardware platform.
Therefore, current implementation supports only a single hardware
watchpoint, which has been verified on Windows 11 using Microsoft
SQ2 and Snapdragon Elite X hardware.

LLDB test suite ninja check-lldb still fails watchpoint-related tests.
However, tests that do not require more than a single watchpoint
pass successfully when run individually.

Added: 


Modified: 
lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.cpp
lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.h

lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.cpp

lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp

lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm64.cpp

lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm64.h

lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp

lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp
llvm/docs/ReleaseNotes.md

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp 
b/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
index 9c330ff1186709..79dd46ba319d65 100644
--- a/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
@@ -292,7 +292,8 @@ NativeProcessWindows::GetAuxvData() const {
 
 llvm::Expected>
 NativeProcessWindows::GetSoftwareBreakpointTrapOpcode(size_t size_hint) {
-  static const uint8_t g_aarch64_opcode[] = {0x00, 0x00, 0x3e, 0xd4}; // brk 
#0xf000
+  static const uint8_t g_aarch64_opcode[] = {0x00, 0x00, 0x3e,
+ 0xd4}; // brk #0xf000
   static const uint8_t g_thumb_opcode[] = {0xfe, 0xde}; // udf #0xfe
 
   switch (GetArchitecture().GetMachine()) {
@@ -309,9 +310,9 @@ 
NativeProcessWindows::GetSoftwareBreakpointTrapOpcode(size_t size_hint) {
 }
 
 size_t NativeProcessWindows::GetSoftwareBreakpointPCOffset() {
-// Windows always reports an incremented PC after a breakpoint is hit,
-// even on ARM.
-return cantFail(GetSoftwareBreakpointTrapOpcode(0)).size();
+  // Windows always reports an incremented PC after a breakpoint is hit,
+  // even on ARM.
+  return cantFail(GetSoftwareBreakpointTrapOpcode(0)).size();
 }
 
 bool NativeProcessWindows::FindSoftwareBreakpoint(lldb::addr_t addr) {
@@ -463,6 +464,7 @@ NativeProcessWindows::OnDebugException(bool first_chance,
   switch (record.GetExceptionCode()) {
   case DWORD(STATUS_SINGLE_STEP):
   case STATUS_WX86_SINGLE_STEP: {
+#ifndef __aarch64__
 uint32_t wp_id = LLDB_INVALID_INDEX32;
 if (NativeThreadWindows *thread = GetThreadByID(record.GetThreadID())) {
   NativeRegisterContextWindows ®_ctx = thread->GetRegisterContext();
@@ -483,6 +485,7 @@ NativeProcessWindows::OnDebugException(bool first_chance,
   }
 }
 if (wp_id == LLDB_INVALID_INDEX32)
+#endif
   StopThread(record.GetThreadID(), StopReason::eStopReasonTrace);
 
 SetState(eStateStopped, true);
@@ -492,23 +495,50 @@ NativeProcessWindows::OnDebugException(bool first_chance,
   }
   case DWORD(STATUS_BREAKPOINT):
   case STATUS_WX86_BREAKPOINT:
-if (FindSoftwareBreakpoint(record.GetExceptionAddress())) {
-  LLDB_LOG(log, "Hit non-loader breakpoint at address {0:x}.",
-   record.GetExceptionAddress());
 
-  StopThread(record.GetThreadID(), StopReason::eStopReasonBreakpoint);
+if (NativeThreadWindows *stop_thread =
+GetThreadByID(record.GetThreadID())) {
+  auto ®_ctx = stop_thread->GetRegisterContext();
+  const auto exception_addr = record.GetExceptionAddress();
+  const auto thread_id = record.GetThreadID();
 
-  if (NativeThreadWindows *stop_thread =
-  GetThreadByID(record.GetThreadID())) {
-auto ®ister_context = stop_thread->GetRegisterContext();
-uint32_t breakpoint_size = GetSoftwareBreakpointPCOffset();
+  if (FindSoftwareBreakpoint(exception_addr)) {
+LLDB_LOG(log, "Hit non-loader breakpoint at address {0:x}.",
+ exception_addr);
 // The current PC is AFTER the 

[Lldb-commits] [lldb] [llvm] [lldb][Windows] WoA HW Watchpoint support in LLDB (PR #108072)

2025-01-31 Thread Omair Javaid via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Add RISCV for Makefile.rules (PR #124758)

2025-01-31 Thread via lldb-commits

https://github.com/kper updated https://github.com/llvm/llvm-project/pull/124758

>From 4ea5df2aae805fab0ace107b6f9e4b9b9c19b2f3 Mon Sep 17 00:00:00 2001
From: Kevin Per 
Date: Tue, 28 Jan 2025 14:44:44 +
Subject: [PATCH] lldb: Add RISCV for Makefile.rules

---
 lldb/packages/Python/lldbsuite/test/make/Makefile.rules | 4 
 1 file changed, 4 insertions(+)

diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules 
b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index 2da6ff226b615c..06959f226066ac 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -207,6 +207,10 @@ else
override ARCH :=
override ARCHFLAG :=
endif
+   ifeq "$(ARCH)" "riscv"
+   override ARCH :=
+   override ARCHFLAG :=
+   endif
ifeq "$(findstring mips,$(ARCH))" "mips"
override ARCHFLAG := -
endif

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


[Lldb-commits] [lldb] [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (PR #125203)

2025-01-31 Thread Michael Buch via lldb-commits

https://github.com/Michael137 created 
https://github.com/llvm/llvm-project/pull/125203

I tried using `CompleteEnumType` to replace some duplicated code in 
`DWARFASTParserClang::ParseEnum` but tests started failing.

`CompleteEnumType` parse/attach the child enumerators using the signedness it 
got from `CompilerType::IsIntegerType`. However, this would only report the 
correct signedness for builtin integer types (never for `clang::EnumType`s). We 
have a different API for that in `CompilerType::IsIntegerOrEnumerationType` 
which could've been used there instead. This patch calls 
`IsEnumerationIntegerTypeSigned` to determine signedness because we always pass 
an enum type into `CompleteEnumType` anyway.

Based on git history this has been like the case for a long time, but possibly 
never caused issues because `ParseEnum` was completing the definition manually 
instead of through `CompleteEnumType`.

I couldn't find a good way to test `CompleteEnumType` on its own because it 
expects an enum type to be passed to it, which only gets created in `ParseEnum` 
(at which point we already call `CompleteEnumType`). The only other place we 
call `CompleteEnumType` at is in 
[`CompleteTypeFromDWARF`](https://github.com/llvm/llvm-project/blob/466217eb0334d467ec8e9b96c52ee988aaadc389/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L2260-L2262).
 Though I think we don't actually ever end up calling into that codepath 
because we eagerly complete enum definitions. Maybe we can remove that call to 
`CompleteEnumType` in a follow-up.

>From 620f30f73930a1a8b2edb298f67d555ff3b93e2c Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 31 Jan 2025 11:18:28 +
Subject: [PATCH] [lldb][TypeSystemClang] Fix enum signedness in
 CompleteEnumType

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp   | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 6602dd763ba693..359872c579f35e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1020,15 +1020,7 @@ TypeSP DWARFASTParserClang::ParseEnum(const 
SymbolContext &sc,
   }
 
 
-  if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-if (def_die.HasChildren()) {
-  bool is_signed = false;
-  enumerator_clang_type.IsIntegerType(is_signed);
-  ParseChildEnumerators(clang_type, is_signed,
-type_sp->GetByteSize(nullptr).value_or(0), 
def_die);
-}
-TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
-  } else {
+  if (!CompleteEnumType(def_die, type_sp.get(), clang_type)) {
 dwarf->GetObjectFile()->GetModule()->ReportError(
 "DWARF DIE at {0:x16} named \"{1}\" was not able to start its "
 "definition.\nPlease file a bug and attach the file at the "
@@ -,12 +2214,10 @@ bool DWARFASTParserClang::CompleteEnumType(const 
DWARFDIE &die,
lldb_private::Type *type,
const CompilerType &clang_type) {
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-if (die.HasChildren()) {
-  bool is_signed = false;
-  clang_type.IsIntegerType(is_signed);
-  ParseChildEnumerators(clang_type, is_signed,
+if (die.HasChildren())
+  ParseChildEnumerators(clang_type, 
clang_type.IsEnumerationIntegerTypeSigned(),
 type->GetByteSize(nullptr).value_or(0), die);
-}
+
 TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
   }
   return (bool)clang_type;

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


[Lldb-commits] [lldb] [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (PR #125203)

2025-01-31 Thread Michael Buch via lldb-commits

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/125203

>From 620f30f73930a1a8b2edb298f67d555ff3b93e2c Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 31 Jan 2025 11:18:28 +
Subject: [PATCH 1/3] [lldb][TypeSystemClang] Fix enum signedness in
 CompleteEnumType

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp   | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 6602dd763ba693..359872c579f35e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1020,15 +1020,7 @@ TypeSP DWARFASTParserClang::ParseEnum(const 
SymbolContext &sc,
   }
 
 
-  if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-if (def_die.HasChildren()) {
-  bool is_signed = false;
-  enumerator_clang_type.IsIntegerType(is_signed);
-  ParseChildEnumerators(clang_type, is_signed,
-type_sp->GetByteSize(nullptr).value_or(0), 
def_die);
-}
-TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
-  } else {
+  if (!CompleteEnumType(def_die, type_sp.get(), clang_type)) {
 dwarf->GetObjectFile()->GetModule()->ReportError(
 "DWARF DIE at {0:x16} named \"{1}\" was not able to start its "
 "definition.\nPlease file a bug and attach the file at the "
@@ -,12 +2214,10 @@ bool DWARFASTParserClang::CompleteEnumType(const 
DWARFDIE &die,
lldb_private::Type *type,
const CompilerType &clang_type) {
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-if (die.HasChildren()) {
-  bool is_signed = false;
-  clang_type.IsIntegerType(is_signed);
-  ParseChildEnumerators(clang_type, is_signed,
+if (die.HasChildren())
+  ParseChildEnumerators(clang_type, 
clang_type.IsEnumerationIntegerTypeSigned(),
 type->GetByteSize(nullptr).value_or(0), die);
-}
+
 TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
   }
   return (bool)clang_type;

>From f72106fffe6dd20513134d35e6fbb9b31a06e9cf Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 31 Jan 2025 11:30:46 +
Subject: [PATCH 2/3] fixup! add assert

---
 lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 359872c579f35e..08fe4a29be8eb7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2213,6 +2213,8 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
 bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE &die,
lldb_private::Type *type,
const CompilerType &clang_type) {
+  assert (clang_type.IsEnumerationType());
+
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
 if (die.HasChildren())
   ParseChildEnumerators(clang_type, 
clang_type.IsEnumerationIntegerTypeSigned(),

>From 23cd607c0c8130a783f80d94e98b7b89b70d45f0 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 31 Jan 2025 11:30:57 +
Subject: [PATCH 3/3] fixup! clang-format

---
 lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 08fe4a29be8eb7..ee99fd6f16cc44 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2213,7 +2213,7 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
 bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE &die,
lldb_private::Type *type,
const CompilerType &clang_type) {
-  assert (clang_type.IsEnumerationType());
+  assert(clang_type.IsEnumerationType());
 
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
 if (die.HasChildren())

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


[Lldb-commits] [lldb] [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (PR #125203)

2025-01-31 Thread Michael Buch via lldb-commits

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


[Lldb-commits] [lldb] [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (PR #125203)

2025-01-31 Thread Michael Buch via lldb-commits

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


[Lldb-commits] [lldb] [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (PR #125203)

2025-01-31 Thread via lldb-commits

github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
git-clang-format --diff 2c934dc5e1a3ef7b717400f27d6b9ea21f4e20a0 
23cd607c0c8130a783f80d94e98b7b89b70d45f0 --extensions cpp -- 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ee99fd6f16..dca193fc11 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1019,7 +1019,6 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext 
&sc,
 // Declaration DIE is inserted into the type map in ParseTypeFromDWARF
   }
 
-
   if (!CompleteEnumType(def_die, type_sp.get(), clang_type)) {
 dwarf->GetObjectFile()->GetModule()->ReportError(
 "DWARF DIE at {0:x16} named \"{1}\" was not able to start its "
@@ -2217,7 +2216,8 @@ bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE 
&die,
 
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
 if (die.HasChildren())
-  ParseChildEnumerators(clang_type, 
clang_type.IsEnumerationIntegerTypeSigned(),
+  ParseChildEnumerators(clang_type,
+clang_type.IsEnumerationIntegerTypeSigned(),
 type->GetByteSize(nullptr).value_or(0), die);
 
 TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);

``




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


[Lldb-commits] [lldb] [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (PR #125203)

2025-01-31 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)


Changes

I tried using `CompleteEnumType` to replace some duplicated code in 
`DWARFASTParserClang::ParseEnum` but tests started failing.

`CompleteEnumType` parses/attaches the child enumerators using the signedness 
it got from `CompilerType::IsIntegerType`. However, this would only report the 
correct signedness for builtin integer types (never for `clang::EnumType`s). We 
have a different API for that in `CompilerType::IsIntegerOrEnumerationType` 
which could've been used there instead. This patch calls 
`IsEnumerationIntegerTypeSigned` to determine signedness because we always pass 
an enum type into `CompleteEnumType` anyway.

Based on git history this has been the case for a long time, but possibly never 
caused issues because `ParseEnum` was completing the definition manually 
instead of through `CompleteEnumType`.

I couldn't find a good way to test `CompleteEnumType` on its own because it 
expects an enum type to be passed to it, which only gets created in `ParseEnum` 
(at which point we already call `CompleteEnumType`). The only other place we 
call `CompleteEnumType` at is in 
[`CompleteTypeFromDWARF`](https://github.com/llvm/llvm-project/blob/466217eb0334d467ec8e9b96c52ee988aaadc389/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L2260-L2262).
 Though I think we don't actually ever end up calling into that codepath 
because we eagerly complete enum definitions. Maybe we can remove that call to 
`CompleteEnumType` in a follow-up.

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


1 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(+6-14) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 6602dd763ba6930..ee99fd6f16cc44d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1020,15 +1020,7 @@ TypeSP DWARFASTParserClang::ParseEnum(const 
SymbolContext &sc,
   }
 
 
-  if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-if (def_die.HasChildren()) {
-  bool is_signed = false;
-  enumerator_clang_type.IsIntegerType(is_signed);
-  ParseChildEnumerators(clang_type, is_signed,
-type_sp->GetByteSize(nullptr).value_or(0), 
def_die);
-}
-TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
-  } else {
+  if (!CompleteEnumType(def_die, type_sp.get(), clang_type)) {
 dwarf->GetObjectFile()->GetModule()->ReportError(
 "DWARF DIE at {0:x16} named \"{1}\" was not able to start its "
 "definition.\nPlease file a bug and attach the file at the "
@@ -2221,13 +2213,13 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
 bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE &die,
lldb_private::Type *type,
const CompilerType &clang_type) {
+  assert(clang_type.IsEnumerationType());
+
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-if (die.HasChildren()) {
-  bool is_signed = false;
-  clang_type.IsIntegerType(is_signed);
-  ParseChildEnumerators(clang_type, is_signed,
+if (die.HasChildren())
+  ParseChildEnumerators(clang_type, 
clang_type.IsEnumerationIntegerTypeSigned(),
 type->GetByteSize(nullptr).value_or(0), die);
-}
+
 TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
   }
   return (bool)clang_type;

``




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


[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

2025-01-31 Thread via lldb-commits

jimingham wrote:



> On Jan 31, 2025, at 2:21 AM, Pavel Labath ***@***.***> wrote:
> 
> 
> @labath commented on this pull request.
> 
> Apart from the (mainly stylistic) inline comments, the biggest problem I see 
> is that the definition of an identifier is still too narrow. The restriction 
> on the dollar sign is completely unnecessary as C will let you put that 
> anywhere . And it doesn't allow any 
> non-ascii characters.
> 
> I really think this should be based on an deny- rather than an allow-list. 
> Any character we don't claim for ourselves should be fair game for an 
> identifier. If someone manages to enter the backspace character (\x7f) into 
> the expression, then so be it.
> 
> The question of "identifiers" starting with digits is interesting. 
> Personally, I think it'd be fine to reject those (and require the 
> currenly-vapourware quoting syntax), because I suspect you want to accept 
> number suffixes, and I think it'd be confusing to explain why 123x is a valid 
> identifier but 123u is not, but I suspect some might have a different opinion.
> 
Are there any languages we know of that allow digits as the initial identifier 
character?  Seems like that just makes parsing the language hard for no very 
clear benefit, so I'd be surprised if there are many that allow that.  If there 
aren't any that we're likely to have to support, I wouldn't object to not 
treating tokens beginning with a number as an identifier.

> We could continue discussing that here, or we could accept everything here, 
> and postpone this discussion for the patch which starts parsing numbers. Up 
> to you..
> 
> In lldb/unittests/ValueObject/DILLexerTests.cpp 
> :
> 
> > +TEST(DILLexerTests, SimpleTest) {
> +  StringRef input_expr("simple_var");
> +  uint32_t tok_len = 10;
> +  llvm::Expected maybe_lexer =
> +  lldb_private::dil::DILLexer::Create(input_expr);
> +  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
> +  lldb_private::dil::DILLexer lexer(*maybe_lexer);
> +  lldb_private::dil::Token token =
> +  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
> +  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
> +
> +  lexer.Advance();
> +  token = lexer.GetCurrentToken();
> +  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
> +  EXPECT_EQ(token.GetSpelling(), "simple_var");
> +  EXPECT_EQ(token.GetSpelling().size(), tok_len);
> Now the size check is unnecessary, as this is just testing the StringRef 
> implementation
> 
> In lldb/unittests/ValueObject/DILLexerTests.cpp 
> :
> 
> > +  StringRef input_expr("namespace");
> +  llvm::Expected maybe_lexer =
> +  lldb_private::dil::DILLexer::Create(input_expr);
> +  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
> +  lldb_private::dil::DILLexer lexer(*maybe_lexer);
> +  lldb_private::dil::Token token =
> +  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
> +  lexer.Advance();
> +  token = lexer.GetCurrentToken();
> ⬇️ Suggested change
> -  StringRef input_expr("namespace");
> -  llvm::Expected maybe_lexer =
> -  lldb_private::dil::DILLexer::Create(input_expr);
> -  ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
> -  lldb_private::dil::DILLexer lexer(*maybe_lexer);
> -  lldb_private::dil::Token token =
> -  lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
> -  lexer.Advance();
> -  token = lexer.GetCurrentToken();
> +  lldb_private::dil::Token token =
> +  lldb_private::dil::Token(lldb_private::dil::Token::identifier, 
> "ident", 0);
> (The reason is that this looks like a test for the token kind testing 
> functions. You don't actually need the token to come out of the lexer for 
> that. And we already have tests that check that producing a sequence of 
> characters produces an "identifier" token.)
> 
> In lldb/unittests/ValueObject/DILLexerTests.cpp 
> :
> 
> > +  token = lexer.GetCurrentToken();
> +
> +  // Current token is '('; check the next 4 tokens, to make
> +  // sure they are the identifier 'anonymous', the identifier 'namespace'
> +  // ')' and '::', in that order.
> +  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::l_paren);
> +  EXPECT_EQ(lexer.LookAhead(1).GetKind(), 
> lldb_private::dil::Token::identifier);
> +  EXPECT_EQ(lexer.LookAhead(1).GetSpelling(), "anonymous");
> +  EXPECT_EQ(lexer.LookAhead(2).GetKind(), 
> lldb_private::dil::Token::identifier);
> +  EXPECT_EQ(lexer.LookAhead(2).GetSpelling(), "namespace");
> +  EXPECT_EQ(lexer.LookAhead(3).GetKind(), lldb_private::dil::Token::r_paren);
> +  EXPECT_EQ(lexer.LookAhead(4).GetKind(), 
> lldb_private::dil::Token::coloncolon);
> +
> +  // Our current index should still be 0, as we only looked ahead; we are 
> still
> 

[Lldb-commits] [lldb] [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (PR #125203)

2025-01-31 Thread Michael Buch via lldb-commits

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


[Lldb-commits] [lldb] ae570d5 - [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (#125203)

2025-01-31 Thread via lldb-commits

Author: Michael Buch
Date: 2025-01-31T13:09:46Z
New Revision: ae570d5d77e806784064ee819211868e745d0fbe

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

LOG: [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (#125203)

I tried using `CompleteEnumType` to replace some duplicated code in
`DWARFASTParserClang::ParseEnum` but tests started failing.

`CompleteEnumType` parses/attaches the child enumerators using the
signedness it got from `CompilerType::IsIntegerType`. However, this
would only report the correct signedness for builtin integer types
(never for `clang::EnumType`s). We have a different API for that in
`CompilerType::IsIntegerOrEnumerationType` which could've been used
there instead. This patch calls `IsEnumerationIntegerTypeSigned` to
determine signedness because we always pass an enum type into
`CompleteEnumType` anyway.

Based on git history this has been the case for a long time, but
possibly never caused issues because `ParseEnum` was completing the
definition manually instead of through `CompleteEnumType`.

I couldn't find a good way to test `CompleteEnumType` on its own because
it expects an enum type to be passed to it, which only gets created in
`ParseEnum` (at which point we already call `CompleteEnumType`). The
only other place we call `CompleteEnumType` at is in
[`CompleteTypeFromDWARF`](https://github.com/llvm/llvm-project/blob/466217eb0334d467ec8e9b96c52ee988aaadc389/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L2260-L2262).
Though I think we don't actually ever end up calling into that codepath
because we eagerly complete enum definitions. Maybe we can remove that
call to `CompleteEnumType` in a follow-up.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 6602dd763ba693..dca193fc11b552 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1019,16 +1019,7 @@ TypeSP DWARFASTParserClang::ParseEnum(const 
SymbolContext &sc,
 // Declaration DIE is inserted into the type map in ParseTypeFromDWARF
   }
 
-
-  if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-if (def_die.HasChildren()) {
-  bool is_signed = false;
-  enumerator_clang_type.IsIntegerType(is_signed);
-  ParseChildEnumerators(clang_type, is_signed,
-type_sp->GetByteSize(nullptr).value_or(0), 
def_die);
-}
-TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
-  } else {
+  if (!CompleteEnumType(def_die, type_sp.get(), clang_type)) {
 dwarf->GetObjectFile()->GetModule()->ReportError(
 "DWARF DIE at {0:x16} named \"{1}\" was not able to start its "
 "definition.\nPlease file a bug and attach the file at the "
@@ -2221,13 +2212,14 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
 bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE &die,
lldb_private::Type *type,
const CompilerType &clang_type) {
+  assert(clang_type.IsEnumerationType());
+
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-if (die.HasChildren()) {
-  bool is_signed = false;
-  clang_type.IsIntegerType(is_signed);
-  ParseChildEnumerators(clang_type, is_signed,
+if (die.HasChildren())
+  ParseChildEnumerators(clang_type,
+clang_type.IsEnumerationIntegerTypeSigned(),
 type->GetByteSize(nullptr).value_or(0), die);
-}
+
 TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
   }
   return (bool)clang_type;



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


[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass around enum value as uint64_t (PR #125244)

2025-01-31 Thread Michael Buch via lldb-commits


@@ -8541,12 +8541,10 @@ clang::EnumConstantDecl 
*TypeSystemClang::AddEnumerationValueToEnumerationType(
 
 clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType(
 const CompilerType &enum_type, const Declaration &decl, const char *name,
-int64_t enum_value, uint32_t enum_value_bit_size) {
-  CompilerType underlying_type = GetEnumerationIntegerType(enum_type);
-  bool is_signed = false;
-  underlying_type.IsIntegerType(is_signed);
-
-  llvm::APSInt value(enum_value_bit_size, !is_signed);
+uint64_t enum_value, uint32_t enum_value_bit_size) {
+  assert(enum_type.IsEnumerationType());
+  llvm::APSInt value(enum_value_bit_size,
+ !enum_type.IsEnumerationIntegerTypeSigned());

Michael137 wrote:

this is a drive-by cleanup

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


[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass around enum value as uint64_t (PR #125244)

2025-01-31 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)


Changes

The `DWARFASTParserClang` reads enum values as `int64_t`s regardless of the 
enumerators signedness. Then we pass it to 
`AddEnumerationValueToEnumerationType` and only then do we create an `APSInt` 
from it. However, there are other places where we read/pass around the enum 
value as unsigned. This patch makes sure we consistently use the same integer 
type for the enum value and let `APSInt` take care of signedness. This 
shouldn't have any observable effect.

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


4 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(+3-5) 
- (modified) lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp (+1-1) 
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+4-6) 
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+1-1) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 6602dd763ba693..3492fb472da8d1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2329,8 +2329,7 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
   continue;
 
 const char *name = nullptr;
-bool got_value = false;
-int64_t enum_value = 0;
+std::optional enum_value;
 Declaration decl;
 
 for (size_t i = 0; i < attributes.Size(); ++i) {
@@ -2339,7 +2338,6 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
   if (attributes.ExtractFormValueAtIndex(i, form_value)) {
 switch (attr) {
 case DW_AT_const_value:
-  got_value = true;
   if (is_signed)
 enum_value = form_value.Signed();
   else
@@ -2368,9 +2366,9 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
   }
 }
 
-if (name && name[0] && got_value) {
+if (name && name[0] && enum_value) {
   m_ast.AddEnumerationValueToEnumerationType(
-  clang_type, decl, name, enum_value, enumerator_byte_size * 8);
+  clang_type, decl, name, *enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
 }
   }
diff --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp 
b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
index 990bacd89bf346..c6dd72e22fb4c1 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -1155,7 +1155,7 @@ bool PDBASTParser::AddEnumValue(CompilerType enum_type,
   Variant v = enum_value.getValue();
   std::string name =
   std::string(MSVCUndecoratedNameParser::DropScope(enum_value.getName()));
-  int64_t raw_value;
+  uint64_t raw_value;
   switch (v.Type) {
   case PDB_VariantType::Int8:
 raw_value = v.Value.Int8;
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index cb246fde976c2f..1da8fbe0bcd6dd 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -8541,12 +8541,10 @@ clang::EnumConstantDecl 
*TypeSystemClang::AddEnumerationValueToEnumerationType(
 
 clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType(
 const CompilerType &enum_type, const Declaration &decl, const char *name,
-int64_t enum_value, uint32_t enum_value_bit_size) {
-  CompilerType underlying_type = GetEnumerationIntegerType(enum_type);
-  bool is_signed = false;
-  underlying_type.IsIntegerType(is_signed);
-
-  llvm::APSInt value(enum_value_bit_size, !is_signed);
+uint64_t enum_value, uint32_t enum_value_bit_size) {
+  assert(enum_type.IsEnumerationType());
+  llvm::APSInt value(enum_value_bit_size,
+ !enum_type.IsEnumerationIntegerTypeSigned());
   value = enum_value;
 
   return AddEnumerationValueToEnumerationType(enum_type, decl, name, value);
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 83f954270e3093..e70ad4c2973a5c 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1040,7 +1040,7 @@ class TypeSystemClang : public TypeSystem {
   // Modifying Enumeration types
   clang::EnumConstantDecl *AddEnumerationValueToEnumerationType(
   const CompilerType &enum_type, const Declaration &decl, const char *name,
-  int64_t enum_value, uint32_t enum_value_bit_size);
+  uint64_t enum_value, uint32_t enum_value_bit_size);
   clang::EnumConstantDecl *AddEnumerationValueToEnumerationType(
   const CompilerType &enum_type, const Declaration &decl, const char *name,
   const llvm::APSInt &value);

``




https://github.com/llvm/llvm-project/pull/125244
__

[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass around enum value as uint64_t (PR #125244)

2025-01-31 Thread Michael Buch via lldb-commits

https://github.com/Michael137 created 
https://github.com/llvm/llvm-project/pull/125244

The `DWARFASTParserClang` reads enum values as `int64_t`s regardless of the 
enumerators signedness. Then we pass it to 
`AddEnumerationValueToEnumerationType` and only then do we create an `APSInt` 
from it. However, there are other places where we read/pass around the enum 
value as unsigned. This patch makes sure we consistently use the same integer 
type for the enum value and let `APSInt` take care of signedness. This 
shouldn't have any observable effect.

>From 66369a911209e20ad0ea58700d97301e5a54d714 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 31 Jan 2025 13:35:10 +
Subject: [PATCH] [lldb][TypeSystemClang] Pass around enum value as uint64_t

The `DWARFASTParserClang` read enum values as `int64_t`s regardless of the 
enumerators signedness. Then we pass it to 
`AddEnumerationValueToEnumerationType` and only then do we create an `APSInt` 
from it. However, there are other places where we read/pass around the enum 
value as unsigned. This patch makes sure we consistently use `uint64_t`s for 
the enum value and let `APSInt` take care of signedness. This shouldn't have 
any observable effect.
---
 .../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp   |  8 +++-
 lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp|  2 +-
 .../Plugins/TypeSystem/Clang/TypeSystemClang.cpp   | 10 --
 lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h |  2 +-
 4 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 6602dd763ba693..3492fb472da8d1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2329,8 +2329,7 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
   continue;
 
 const char *name = nullptr;
-bool got_value = false;
-int64_t enum_value = 0;
+std::optional enum_value;
 Declaration decl;
 
 for (size_t i = 0; i < attributes.Size(); ++i) {
@@ -2339,7 +2338,6 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
   if (attributes.ExtractFormValueAtIndex(i, form_value)) {
 switch (attr) {
 case DW_AT_const_value:
-  got_value = true;
   if (is_signed)
 enum_value = form_value.Signed();
   else
@@ -2368,9 +2366,9 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
   }
 }
 
-if (name && name[0] && got_value) {
+if (name && name[0] && enum_value) {
   m_ast.AddEnumerationValueToEnumerationType(
-  clang_type, decl, name, enum_value, enumerator_byte_size * 8);
+  clang_type, decl, name, *enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
 }
   }
diff --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp 
b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
index 990bacd89bf346..c6dd72e22fb4c1 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -1155,7 +1155,7 @@ bool PDBASTParser::AddEnumValue(CompilerType enum_type,
   Variant v = enum_value.getValue();
   std::string name =
   std::string(MSVCUndecoratedNameParser::DropScope(enum_value.getName()));
-  int64_t raw_value;
+  uint64_t raw_value;
   switch (v.Type) {
   case PDB_VariantType::Int8:
 raw_value = v.Value.Int8;
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index cb246fde976c2f..1da8fbe0bcd6dd 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -8541,12 +8541,10 @@ clang::EnumConstantDecl 
*TypeSystemClang::AddEnumerationValueToEnumerationType(
 
 clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType(
 const CompilerType &enum_type, const Declaration &decl, const char *name,
-int64_t enum_value, uint32_t enum_value_bit_size) {
-  CompilerType underlying_type = GetEnumerationIntegerType(enum_type);
-  bool is_signed = false;
-  underlying_type.IsIntegerType(is_signed);
-
-  llvm::APSInt value(enum_value_bit_size, !is_signed);
+uint64_t enum_value, uint32_t enum_value_bit_size) {
+  assert(enum_type.IsEnumerationType());
+  llvm::APSInt value(enum_value_bit_size,
+ !enum_type.IsEnumerationIntegerTypeSigned());
   value = enum_value;
 
   return AddEnumerationValueToEnumerationType(enum_type, decl, name, value);
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 83f954270e3093..e70ad4c2973a5c 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1040,7 +1040,7 @@ class TypeSystemClang : public TypeSys

[Lldb-commits] [lldb] [lldb] Support CommandInterpreter print callbacks (PR #125006)

2025-01-31 Thread via lldb-commits

jimingham wrote:

We have a bunch of other structured output that we could start decorating this 
way.  For instance if `image list` produced a  a JSON-ified version of itself 
(maybe only when there was a callback present) and had a way to "get JSON-ified 
result" then it could render that list inline with active columns and rows that 
you could rearrange, sort by, etc.

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


[Lldb-commits] [lldb] [lldb] Support CommandInterpreter print callbacks (PR #125006)

2025-01-31 Thread Jonas Devlieghere via lldb-commits

JDevlieghere wrote:

Jim & I have been discussing this offline. Thanks for adding the additional 
context to the PR! 

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


[Lldb-commits] [lldb] [lldb] Make ValueObjectDynamicValue::UpdateValue() point to a host b… (PR #125143)

2025-01-31 Thread Augusto Noronha via lldb-commits

augusto2112 wrote:

> I worry a bit about the fact that in the host case, GetValueAsData is going 
> to end up calling:
> 
> ```
>   memcpy(dst, reinterpret_cast(address), byte_size);
> ```
> 
> where address is the host data buffer and byte_size is the size of the new 
> dynamic type. But in the case where the Value has data in the m_data_buffer, 
> address points to a buffer. The code makes sure that the destination buffer 
> (pointed to by dst) is big enough to fit byte_size, but I don't see the 
> guarantee that the original contents are not smaller than the new dynamic 
> type byte size. If we get that wrong, then we'll crash here or worse sample 
> lldb internal memory and present that to the user as a value. How do you know 
> that can't happen?

I implemented that check inside `GetDynamicTypeAndAddress`. If the buffer is 
not big enough that function will set address_type as invalid. I think that's 
the logical place to implement this check, since finding the dynamic type and 
address is its whole responsibility.

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


[Lldb-commits] [lldb] [lldb] Use llvm::Error instead of CommandReturnObject for error reporting (PR #125125)

2025-01-31 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/125125

>From 24ddc550e3ee61b863cbaea05ff49981bc20f7ad Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Thu, 30 Jan 2025 14:25:16 -0800
Subject: [PATCH 1/4] [lldb] Use llvm::Error instead of CommandReturnObject for
 error reporting

Use `llvm::Error` instead of `CommandReturnObject` for error reporting.
The command return objects were populated with errors but never
displayed. With this patch they're at least logged.
---
 lldb/include/lldb/Interpreter/Options.h   |  4 +-
 lldb/source/Interpreter/CommandAlias.cpp  | 49 +--
 lldb/source/Interpreter/CommandObject.cpp |  2 +-
 lldb/source/Interpreter/Options.cpp   | 87 ++-
 .../DarwinLog/StructuredDataDarwinLog.cpp |  9 +-
 5 files changed, 77 insertions(+), 74 deletions(-)

diff --git a/lldb/include/lldb/Interpreter/Options.h 
b/lldb/include/lldb/Interpreter/Options.h
index 9a6a17c2793fa4c..864bda6f24c8cc5 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -76,12 +76,12 @@ class Options {
   // This gets passed the short option as an integer...
   void OptionSeen(int short_option);
 
-  bool VerifyOptions(CommandReturnObject &result);
+  llvm::Error VerifyOptions();
 
   // Verify that the options given are in the options table and can be used
   // together, but there may be some required options that are missing (used to
   // verify options that get folded into command aliases).
-  bool VerifyPartialOptions(CommandReturnObject &result);
+  llvm::Error VerifyPartialOptions();
 
   void OutputFormattedUsageText(Stream &strm,
 const OptionDefinition &option_def,
diff --git a/lldb/source/Interpreter/CommandAlias.cpp 
b/lldb/source/Interpreter/CommandAlias.cpp
index c5971b52f837faf..ac0c7a3279e64b9 100644
--- a/lldb/source/Interpreter/CommandAlias.cpp
+++ b/lldb/source/Interpreter/CommandAlias.cpp
@@ -10,6 +10,7 @@
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatAdapters.h"
 
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandObject.h"
@@ -20,20 +21,17 @@
 using namespace lldb;
 using namespace lldb_private;
 
-static bool ProcessAliasOptionsArgs(lldb::CommandObjectSP &cmd_obj_sp,
+static llvm::Error
+ProcessAliasOptionsArgs(lldb::CommandObjectSP &cmd_obj_sp,
 llvm::StringRef options_args,
 OptionArgVectorSP &option_arg_vector_sp) {
-  bool success = true;
   OptionArgVector *option_arg_vector = option_arg_vector_sp.get();
 
   if (options_args.size() < 1)
-return true;
+return llvm::Error::success();
 
   Args args(options_args);
   std::string options_string(options_args);
-  // TODO: Find a way to propagate errors in this CommandReturnObject up the
-  // stack.
-  CommandReturnObject result(false);
   // Check to see if the command being aliased can take any command options.
   Options *options = cmd_obj_sp->GetOptions();
   if (options) {
@@ -45,34 +43,30 @@ static bool ProcessAliasOptionsArgs(lldb::CommandObjectSP 
&cmd_obj_sp,
 
 llvm::Expected args_or =
 options->ParseAlias(args, option_arg_vector, options_string);
-if (!args_or) {
-  result.AppendError(toString(args_or.takeError()));
-  result.AppendError("Unable to create requested alias.\n");
-  return false;
-}
+if (!args_or)
+  return llvm::createStringError(
+  llvm::formatv("unable to create alias: {0}",
+llvm::fmt_consume(args_or.takeError(;
 args = std::move(*args_or);
-options->VerifyPartialOptions(result);
-if (!result.Succeeded() &&
-result.GetStatus() != lldb::eReturnStatusStarted) {
-  result.AppendError("Unable to create requested alias.\n");
-  return false;
-}
+if (llvm::Error error = options->VerifyPartialOptions())
+  return error;
   }
 
   if (!options_string.empty()) {
-if (cmd_obj_sp->WantsRawCommandString())
-  option_arg_vector->emplace_back(CommandInterpreter::g_argument, 
-  -1, options_string);
-else {
+if (cmd_obj_sp->WantsRawCommandString()) {
+  option_arg_vector->emplace_back(CommandInterpreter::g_argument, -1,
+  options_string);
+} else {
   for (auto &entry : args.entries()) {
 if (!entry.ref().empty())
-  
option_arg_vector->emplace_back(std::string(CommandInterpreter::g_argument), -1,
+  option_arg_vector->emplace_back(
+  std::string(CommandInterpreter::g_argument), -1,
   std::string(entry.ref()));
   }
 }
   }
 
-  return success;
+  return llvm::Error::success();
 }
 
 CommandAlias::CommandAlias(CommandInterpreter &interpreter,
@@ -85,10 +79,15 @@ CommandAlias::CommandAlias(CommandIn

[Lldb-commits] [lldb] [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (PR #125203)

2025-01-31 Thread Pavel Labath via lldb-commits

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


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


[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)

2025-01-31 Thread Pavel Labath via lldb-commits

labath wrote:

Overall, I like the StreamDescriptor changes, but I fear this patch is starting 
to lose focus again. Could you separate the refactor into a separate PR?

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


[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (PR #125242)

2025-01-31 Thread David Spickett via lldb-commits

https://github.com/DavidSpickett created 
https://github.com/llvm/llvm-project/pull/125242

This reverts commit a774de807e56c1147d4630bfec3110c11d41776e.

This is the same changes as last time, plus:
* We load the binary into the target object so that on Windows, we can resolve 
the locations of the functions.
* We now assert that each required breakpoint has at least 1 location, to 
prevent an issue like that in the future.
* We are less strict about the unsupported error message, because it prints 
"error: windows" on Windows instead of "error: gdb-remote".

>From 7461e6164286c3217f892779c72a5e1df44734a9 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Fri, 31 Jan 2025 15:37:46 +
Subject: [PATCH] Reland "[lldb] Implement basic support for reverse-continue"

This reverts commit a774de807e56c1147d4630bfec3110c11d41776e.

This is the same changes as last time, plus:
* We load the binary into the target object so that on Windows,
  we can resolve the locations of the functions.
* We now assert that each required breakpoint has at least 1
  location, to prevent an issue like that in the future.
* We are less strict about the unsupported error message,
  because it prints "error: windows" on Windows instead of
  "error: gdb-remote".
---
 lldb/include/lldb/API/SBProcess.h |   1 +
 lldb/include/lldb/Target/Process.h|  28 +-
 lldb/include/lldb/Target/StopInfo.h   |   7 +
 lldb/include/lldb/Target/Thread.h |   9 +-
 lldb/include/lldb/Target/ThreadList.h |   6 +-
 lldb/include/lldb/Target/ThreadPlan.h |  13 +
 lldb/include/lldb/Target/ThreadPlanBase.h |   2 +
 lldb/include/lldb/lldb-enumerations.h |   6 +
 .../Python/lldbsuite/test/gdbclientutils.py   |   5 +-
 .../Python/lldbsuite/test/lldbgdbproxy.py | 175 ++
 .../Python/lldbsuite/test/lldbreverse.py  | 541 ++
 .../Python/lldbsuite/test/lldbtest.py |   2 +
 .../tools/lldb-server/lldbgdbserverutils.py   |  14 +-
 lldb/source/API/SBProcess.cpp |  12 +
 lldb/source/API/SBThread.cpp  |   2 +
 .../source/Interpreter/CommandInterpreter.cpp |   3 +-
 .../Process/Linux/NativeThreadLinux.cpp   |   3 +
 .../Process/MacOSX-Kernel/ProcessKDP.cpp  |   8 +-
 .../Process/MacOSX-Kernel/ProcessKDP.h|   2 +-
 .../Process/Windows/Common/ProcessWindows.cpp |   9 +-
 .../Process/Windows/Common/ProcessWindows.h   |   2 +-
 .../GDBRemoteCommunicationClient.cpp  |  20 +
 .../gdb-remote/GDBRemoteCommunicationClient.h |   6 +
 .../GDBRemoteCommunicationServerLLGS.cpp  |   1 +
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |  98 +++-
 .../Process/gdb-remote/ProcessGDBRemote.h |   4 +-
 .../Process/scripted/ScriptedProcess.cpp  |   9 +-
 .../Process/scripted/ScriptedProcess.h|   2 +-
 lldb/source/Target/Process.cpp|  24 +-
 lldb/source/Target/StopInfo.cpp   |  28 +
 lldb/source/Target/Thread.cpp |   9 +-
 lldb/source/Target/ThreadList.cpp |  32 +-
 lldb/source/Target/ThreadPlanBase.cpp |   4 +
 .../reverse-execution/Makefile|   3 +
 .../TestReverseContinueBreakpoints.py | 159 +
 .../TestReverseContinueNotSupported.py|  35 ++
 .../TestReverseContinueWatchpoints.py | 136 +
 .../functionalities/reverse-execution/main.c  |  25 +
 lldb/tools/lldb-dap/JSONUtils.cpp |   3 +
 lldb/tools/lldb-dap/LLDBUtils.cpp |   1 +
 40 files changed, 1402 insertions(+), 47 deletions(-)
 create mode 100644 lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py
 create mode 100644 lldb/packages/Python/lldbsuite/test/lldbreverse.py
 create mode 100644 lldb/test/API/functionalities/reverse-execution/Makefile
 create mode 100644 
lldb/test/API/functionalities/reverse-execution/TestReverseContinueBreakpoints.py
 create mode 100644 
lldb/test/API/functionalities/reverse-execution/TestReverseContinueNotSupported.py
 create mode 100644 
lldb/test/API/functionalities/reverse-execution/TestReverseContinueWatchpoints.py
 create mode 100644 lldb/test/API/functionalities/reverse-execution/main.c

diff --git a/lldb/include/lldb/API/SBProcess.h 
b/lldb/include/lldb/API/SBProcess.h
index 1624e02070b1b21..882b8bd837131d1 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -159,6 +159,7 @@ class LLDB_API SBProcess {
   lldb::SBError Destroy();
 
   lldb::SBError Continue();
+  lldb::SBError ContinueInDirection(lldb::RunDirection direction);
 
   lldb::SBError Stop();
 
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index a184e6dd891affa..b14eb3fbd91d000 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1089,6 +1089,13 @@ class Process : public 
std::enable_shared_from_this,
   /// Returns an error object.
   virtual Status WillResume() { return Status(); }
 
+  /// Reports whether this proc

[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (PR #125242)

2025-01-31 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Pull request just in case we have to discuss this more, going to merge right 
now.

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


[Lldb-commits] [lldb] 7e66cf7 - Reland "[lldb] Implement basic support for reverse-continue" (#125242)

2025-01-31 Thread via lldb-commits

Author: David Spickett
Date: 2025-01-31T15:56:33Z
New Revision: 7e66cf74fb4e6a103f923e34700a7b6f20ac2a9b

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

LOG: Reland "[lldb] Implement basic support for reverse-continue" (#125242)

This reverts commit a774de807e56c1147d4630bfec3110c11d41776e.

This is the same changes as last time, plus:
* We load the binary into the target object so that on Windows, we can
resolve the locations of the functions.
* We now assert that each required breakpoint has at least 1 location,
to prevent an issue like that in the future.
* We are less strict about the unsupported error message, because it
prints "error: windows" on Windows instead of "error: gdb-remote".

Added: 
lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py
lldb/packages/Python/lldbsuite/test/lldbreverse.py
lldb/test/API/functionalities/reverse-execution/Makefile

lldb/test/API/functionalities/reverse-execution/TestReverseContinueBreakpoints.py

lldb/test/API/functionalities/reverse-execution/TestReverseContinueNotSupported.py

lldb/test/API/functionalities/reverse-execution/TestReverseContinueWatchpoints.py
lldb/test/API/functionalities/reverse-execution/main.c

Modified: 
lldb/include/lldb/API/SBProcess.h
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/StopInfo.h
lldb/include/lldb/Target/Thread.h
lldb/include/lldb/Target/ThreadList.h
lldb/include/lldb/Target/ThreadPlan.h
lldb/include/lldb/Target/ThreadPlanBase.h
lldb/include/lldb/lldb-enumerations.h
lldb/packages/Python/lldbsuite/test/gdbclientutils.py
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
lldb/source/API/SBProcess.cpp
lldb/source/API/SBThread.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
lldb/source/Plugins/Process/scripted/ScriptedProcess.h
lldb/source/Target/Process.cpp
lldb/source/Target/StopInfo.cpp
lldb/source/Target/Thread.cpp
lldb/source/Target/ThreadList.cpp
lldb/source/Target/ThreadPlanBase.cpp
lldb/tools/lldb-dap/JSONUtils.cpp
lldb/tools/lldb-dap/LLDBUtils.cpp

Removed: 




diff  --git a/lldb/include/lldb/API/SBProcess.h 
b/lldb/include/lldb/API/SBProcess.h
index 1624e02070b1b21..882b8bd837131d1 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -159,6 +159,7 @@ class LLDB_API SBProcess {
   lldb::SBError Destroy();
 
   lldb::SBError Continue();
+  lldb::SBError ContinueInDirection(lldb::RunDirection direction);
 
   lldb::SBError Stop();
 

diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index a184e6dd891affa..b14eb3fbd91d000 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1089,6 +1089,13 @@ class Process : public 
std::enable_shared_from_this,
   /// Returns an error object.
   virtual Status WillResume() { return Status(); }
 
+  /// Reports whether this process supports reverse execution.
+  ///
+  /// \return
+  /// Returns true if the process supports reverse execution (at least
+  /// under some circumstances).
+  virtual bool SupportsReverseDirection() { return false; }
+
   /// Resumes all of a process's threads as configured using the Thread run
   /// control functions.
   ///
@@ -1104,9 +,13 @@ class Process : public 
std::enable_shared_from_this,
   /// \see Thread:Resume()
   /// \see Thread:Step()
   /// \see Thread:Suspend()
-  virtual Status DoResume() {
+  virtual Status DoResume(lldb::RunDirection direction) {
+if (direction == lldb::RunDirection::eRunForward)
+  return Status::FromErrorStringWithFormatv(
+  "error: {0} does not support resuming processes", GetPluginName());
 return Status::FromErrorStringWithFormatv(
-"error: {0} does not support resuming processes", GetPluginName());
+"error: {0} does not support reverse execution of processes",
+GetPluginName());
   }
 
   /// Called after resuming a

[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (PR #125242)

2025-01-31 Thread David Spickett via lldb-commits

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


[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (PR #125242)

2025-01-31 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)


Changes

This reverts commit a774de807e56c1147d4630bfec3110c11d41776e.

This is the same changes as last time, plus:
* We load the binary into the target object so that on Windows, we can resolve 
the locations of the functions.
* We now assert that each required breakpoint has at least 1 location, to 
prevent an issue like that in the future.
* We are less strict about the unsupported error message, because it prints 
"error: windows" on Windows instead of "error: gdb-remote".

---

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


40 Files Affected:

- (modified) lldb/include/lldb/API/SBProcess.h (+1) 
- (modified) lldb/include/lldb/Target/Process.h (+26-2) 
- (modified) lldb/include/lldb/Target/StopInfo.h (+7) 
- (modified) lldb/include/lldb/Target/Thread.h (+4-5) 
- (modified) lldb/include/lldb/Target/ThreadList.h (+5-1) 
- (modified) lldb/include/lldb/Target/ThreadPlan.h (+13) 
- (modified) lldb/include/lldb/Target/ThreadPlanBase.h (+2) 
- (modified) lldb/include/lldb/lldb-enumerations.h (+6) 
- (modified) lldb/packages/Python/lldbsuite/test/gdbclientutils.py (+3-2) 
- (added) lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py (+175) 
- (added) lldb/packages/Python/lldbsuite/test/lldbreverse.py (+541) 
- (modified) lldb/packages/Python/lldbsuite/test/lldbtest.py (+2) 
- (modified) 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py 
(+9-5) 
- (modified) lldb/source/API/SBProcess.cpp (+12) 
- (modified) lldb/source/API/SBThread.cpp (+2) 
- (modified) lldb/source/Interpreter/CommandInterpreter.cpp (+2-1) 
- (modified) lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp (+3) 
- (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (+7-1) 
- (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h (+1-1) 
- (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp 
(+8-1) 
- (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h (+1-1) 
- (modified) 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+20) 
- (modified) 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (+6) 
- (modified) 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
(+1) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
(+85-13) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (+3-1) 
- (modified) lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp (+7-2) 
- (modified) lldb/source/Plugins/Process/scripted/ScriptedProcess.h (+1-1) 
- (modified) lldb/source/Target/Process.cpp (+20-4) 
- (modified) lldb/source/Target/StopInfo.cpp (+28) 
- (modified) lldb/source/Target/Thread.cpp (+6-3) 
- (modified) lldb/source/Target/ThreadList.cpp (+29-3) 
- (modified) lldb/source/Target/ThreadPlanBase.cpp (+4) 
- (added) lldb/test/API/functionalities/reverse-execution/Makefile (+3) 
- (added) 
lldb/test/API/functionalities/reverse-execution/TestReverseContinueBreakpoints.py
 (+159) 
- (added) 
lldb/test/API/functionalities/reverse-execution/TestReverseContinueNotSupported.py
 (+35) 
- (added) 
lldb/test/API/functionalities/reverse-execution/TestReverseContinueWatchpoints.py
 (+136) 
- (added) lldb/test/API/functionalities/reverse-execution/main.c (+25) 
- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+3) 
- (modified) lldb/tools/lldb-dap/LLDBUtils.cpp (+1) 


``diff
diff --git a/lldb/include/lldb/API/SBProcess.h 
b/lldb/include/lldb/API/SBProcess.h
index 1624e02070b1b21..882b8bd837131d1 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -159,6 +159,7 @@ class LLDB_API SBProcess {
   lldb::SBError Destroy();
 
   lldb::SBError Continue();
+  lldb::SBError ContinueInDirection(lldb::RunDirection direction);
 
   lldb::SBError Stop();
 
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index a184e6dd891affa..b14eb3fbd91d000 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1089,6 +1089,13 @@ class Process : public 
std::enable_shared_from_this,
   /// Returns an error object.
   virtual Status WillResume() { return Status(); }
 
+  /// Reports whether this process supports reverse execution.
+  ///
+  /// \return
+  /// Returns true if the process supports reverse execution (at least
+  /// under some circumstances).
+  virtual bool SupportsReverseDirection() { return false; }
+
   /// Resumes all of a process's threads as configured using the Thread run
   /// control functions.
   ///
@@ -1104,9 +,13 @@ class Process : public 
std::enable_shared_from_this,
   /// \see Thread:Resume()
   /// \see Thread:Step()
   /// \see Thread:Suspend()
-  virtual Status DoResume() {
+  virtual Status DoResume(lldb::RunDirection direction) {
+if (direction == lldb::RunDi

[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)

2025-01-31 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Back in again after fixing Windows testing - 
https://github.com/llvm/llvm-project/pull/125242.

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


[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

2025-01-31 Thread Michael Buch via lldb-commits

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


[Lldb-commits] [lldb] 8bd7281 - [lldb][test] explicit-member-function-quals.cpp: fix expected output

2025-01-31 Thread Michael Buch via lldb-commits

Author: Michael Buch
Date: 2025-01-31T12:38:07Z
New Revision: 8bd728180cadbab9fe11cd853670e488827ee302

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

LOG: [lldb][test] explicit-member-function-quals.cpp: fix expected output

The `type lookup` output looks different.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/x86/explicit-member-function-quals.cpp

Removed: 




diff  --git 
a/lldb/test/Shell/SymbolFile/DWARF/x86/explicit-member-function-quals.cpp 
b/lldb/test/Shell/SymbolFile/DWARF/x86/explicit-member-function-quals.cpp
index 103aa0a9684749..5d1222795dd834 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/explicit-member-function-quals.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/explicit-member-function-quals.cpp
@@ -9,9 +9,9 @@
 // CHECK:  (lldb) type lookup Foo
 // CHECK-NEXT: struct Foo {
 // CHECK-NEXT:  void Method(Foo);
-// CHECK-NEXT:  void cMethod(Foo const&);
-// CHECK-NEXT:  void vMethod(Foo volatile&);
-// CHECK-NEXT:  void cvMethod(const Foo volatile&) const volatile;
+// CHECK-NEXT:  void cMethod(const Foo &) const;
+// CHECK-NEXT:  void vMethod(volatile Foo &) volatile;
+// CHECK-NEXT:  void cvMethod(const volatile Foo &) const volatile;
 // CHECK-NEXT: }
 
 struct Foo {



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


[Lldb-commits] [lldb] [lldb] Support CommandInterpreter print callbacks (PR #125006)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -743,3 +743,15 @@ void SBCommand::SetFlags(uint32_t flags) {
   if (IsValid())
 m_opaque_sp->GetFlags().Set(flags);
 }
+
+void SBCommandInterpreter::SetPrintCallback(
+lldb::SBCommandPrintCallback callback, void *baton) {
+  LLDB_INSTRUMENT_VA(this, callback, baton);
+
+  if (m_opaque_ptr)
+return m_opaque_ptr->SetPrintCallback(

labath wrote:

this isn't actually returning anything

```suggestion
m_opaque_ptr->SetPrintCallback(
```

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


[Lldb-commits] [lldb] [lldb] Support CommandInterpreter print callbacks (PR #125006)

2025-01-31 Thread Pavel Labath via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Support CommandInterpreter print callbacks (PR #125006)

2025-01-31 Thread Pavel Labath via lldb-commits


@@ -17,6 +17,7 @@
 
 namespace lldb_private {
 class CommandPluginInterfaceImplementation;
+class CommandPrintCallbackBaton;

labath wrote:

I think these could go away too...

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


[Lldb-commits] [lldb] [lldb] Support CommandInterpreter print callbacks (PR #125006)

2025-01-31 Thread via lldb-commits

jimingham wrote:

There are other ways you could extend this that would be very fun.  For 
instance, we could make any command that conses up its result just by calling 
the ValueObjectPrinter on some ValueObject to also stuff that ValueObject to 
the command return.  Then we could add 
SBCommandResultObject::GetReturnValueObject to return that ValueObject.  Then 
the callback could check whether there was a ValueObject in the command result, 
and if there is it would suppress that textual output and render the 
ValueObject as a live object in the result slot in the command output it is 
building up.

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


[Lldb-commits] [lldb] 1b0becf - [lldb] Add some formatting to variable.rst (NFC)

2025-01-31 Thread Dave Lee via lldb-commits

Author: Dave Lee
Date: 2025-01-31T09:29:22-08:00
New Revision: 1b0becf739ace0e04c57b50ab701b5e3d009ccbb

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

LOG: [lldb] Add some formatting to variable.rst (NFC)

Added: 


Modified: 
lldb/docs/use/variable.rst

Removed: 




diff  --git a/lldb/docs/use/variable.rst b/lldb/docs/use/variable.rst
index 22c2bee73fa597b..3ad71cb93c51d08 100644
--- a/lldb/docs/use/variable.rst
+++ b/lldb/docs/use/variable.rst
@@ -1226,13 +1226,13 @@ By default, several categories are created in LLDB:
 - CoreServices: CS classes
 - VectorTypes: compact display for several vector types
 
-If you want to use a custom category for your formatters, all the type ... add
-provide a --category (-w) option, that names the category to add the formatter
+If you want to use a custom category for your formatters, all the ``type ... 
add``
+provide a ``--category`` (``-w``) option, that names the category to add the 
formatter
 to. To delete the formatter, you then have to specify the correct category.
 
 Categories can be in one of two states: enabled and disabled. A category is
-initially disabled, and can be enabled using the type category enable command.
-To disable an enabled category, the command to use is type category disable.
+initially disabled, and can be enabled using the ``type category enable`` 
command.
+To disable an enabled category, the command to use is ``type category 
disable``.
 
 The order in which categories are enabled or disabled is significant, in that
 LLDB uses that order when looking for formatters. Therefore, when you enable a



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


[Lldb-commits] [lldb] [lldb] Support CommandInterpreter print callbacks (PR #125006)

2025-01-31 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/125006

>From d9c0b0e1e6c18b6710c5b15843e23a96ca0a9dc8 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Tue, 28 Jan 2025 17:08:52 -0800
Subject: [PATCH 1/4] [lldb] Support CommandInterpreter print callbacks

Xcode uses a pseudoterminal for the debugger console.

 - The upside of this apporach is that it means that it can rely on
   LLDB's IOHandlers for multiline and script input.

 - The downside of this approach is that the command output is printed
   to the PTY and you don't get a SBCommandReturnObject. Adrian added
   support for inline diagnostics in #110901 and we'd like to access
   those from the IDE.

This patch adds support for registering a callback in the command
interpreter that gives access to the (SB)CommandReturnObject right
before it will be printed. The callback implementation can choose
whether it likes to handle printing the result or defer to lldb. If the
callback indicated it handled the result, the command interpreter will
skip printing the result.

We considered a few other alternatives to solve this problem:

 - The most obvious one is using `HandleCommand`, which returns a
   `SBCommandReturnObject`. The problem with this approach is the
   multiline input mentioned above. We would need a way to tell the IDE
   that it should expect multiline input, which isn't known until LLDB
   starts handling the command.

 - To address the multiline issue,we considered exposing (some of the)
   IOHandler machinery through the SB API. To solve this particular
   issue, that would require reimplementing a ton of logic that already
   exists today in the CommandInterpeter. Furthermore that seems like
   overkill compared to the proposed solution.

rdar://141254310
---
 lldb/bindings/python/python-swigsafecast.swig |  4 ++
 lldb/bindings/python/python-typemaps.swig | 19 ++
 lldb/bindings/python/python-wrapper.swig  | 24 ++-
 lldb/include/lldb/API/SBCommandInterpreter.h  |  8 ++-
 lldb/include/lldb/API/SBCommandReturnObject.h |  2 +
 lldb/include/lldb/API/SBDefines.h |  3 +
 .../lldb/Interpreter/CommandInterpreter.h | 13 
 lldb/include/lldb/lldb-enumerations.h |  9 +++
 lldb/source/API/SBCommandInterpreter.cpp  | 40 ++-
 .../source/Interpreter/CommandInterpreter.cpp | 60 +++--
 .../Python/SWIGPythonBridge.h | 13 ++--
 .../python_api/interpreter_callback/Makefile  |  3 +
 .../TestCommandInterepterPrintCallback.py | 66 +++
 .../python_api/interpreter_callback/main.c|  6 ++
 14 files changed, 238 insertions(+), 32 deletions(-)
 create mode 100644 lldb/test/API/python_api/interpreter_callback/Makefile
 create mode 100644 
lldb/test/API/python_api/interpreter_callback/TestCommandInterepterPrintCallback.py
 create mode 100644 lldb/test/API/python_api/interpreter_callback/main.c

diff --git a/lldb/bindings/python/python-swigsafecast.swig 
b/lldb/bindings/python/python-swigsafecast.swig
index 429baad158ca5d..4721dfdc17e6a0 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/lldb/bindings/python/python-swigsafecast.swig
@@ -9,6 +9,10 @@ PythonObject 
SWIGBridge::ToSWIGWrapper(std::unique_ptr value_sb)
   return ToSWIGHelper(value_sb.release(), SWIGTYPE_p_lldb__SBValue);
 }
 
+PythonObject 
SWIGBridge::ToSWIGWrapper(std::unique_ptr 
result_up) {
+  return ToSWIGHelper(result_up.release(), 
SWIGTYPE_p_lldb__SBCommandReturnObject);
+}
+
 PythonObject SWIGBridge::ToSWIGWrapper(lldb::ValueObjectSP value_sp) {
   return ToSWIGWrapper(std::unique_ptr(new 
lldb::SBValue(value_sp)));
 }
diff --git a/lldb/bindings/python/python-typemaps.swig 
b/lldb/bindings/python/python-typemaps.swig
index f8c33e15c03e66..88b6cd9ef6b6e7 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -476,6 +476,25 @@ template <> bool SetNumberFromPyObject(double 
&number, PyObject *obj) {
   $1 = $1 || PyCallable_Check(reinterpret_cast($input));
 }
 
+// For lldb::SBCommandPrintCallback
+%typemap(in) (lldb::SBCommandPrintCallback callback, void *baton) {
+  if (!($input == Py_None ||
+PyCallable_Check(reinterpret_cast($input {
+PyErr_SetString(PyExc_TypeError, "Need a callable object or None!");
+SWIG_fail;
+  }
+
+  // Don't lose the callback reference.
+  Py_INCREF($input);
+  $1 = LLDBSwigPythonCallPythonCommandPrintCallback;
+  $2 = $input;
+}
+
+%typemap(typecheck) (lldb::SBCommandPrintCallback callback, void *baton) {
+  $1 = $input == Py_None;
+  $1 = $1 || PyCallable_Check(reinterpret_cast($input));
+}
+
 %typemap(in) (lldb::CommandOverrideCallback callback, void *baton) {
   if (!($input == Py_None ||
 PyCallable_Check(reinterpret_cast($input {
diff --git a/lldb/bindings/python/python-wrapper.swig 
b/lldb/bindings/python/python-wrapper.swig
index b72a462d04643b..fcabf60008b17d 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb

[Lldb-commits] [lldb] [lldb][TypeSystemClang] Pass around enum value as uint64_t (PR #125244)

2025-01-31 Thread Felipe de Azevedo Piovezan via lldb-commits

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

after staring at this long enough, I think this is fine, since the uint->int 
conversion is done by apint

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


[Lldb-commits] [lldb] [lldb] Use validation combination of options in TestAbbreviations (PR #125270)

2025-01-31 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/125270

Name and line number are part of different option groups and are not compatible.

  (lldb) breakpoint set -n foo -l 10
  error: invalid combination of options for the given command

The help output for `breakpoint set` confirms this. This patch updates the test 
to use two compatible options. With the improved error reporting from #125125 
this becomes an issue.

>From a33e43e56e8a58c1d53c566c0b9da7791254d68e Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 31 Jan 2025 11:10:40 -0800
Subject: [PATCH] [lldb] Use validation combination of options in
 TestAbbreviations

Name and line number are part of different option groups and are not
compatible.

  (lldb) breakpoint set -n foo -l 10
  error: invalid combination of options for the given command

The help output for `breakpoint set` confirms this. This patch updates
the test to use two compatible options. With the improved error
reporting from #125125 this becomes an issue.
---
 .../API/functionalities/abbreviation/TestAbbreviations.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py 
b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
index a8cbffbb7ba4a52..bef159501c6cd1b 100644
--- a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
+++ b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
@@ -45,10 +45,10 @@ def test_command_abbreviations_and_aliases(self):
 
 # Make sure an unabbreviated command is not mangled.
 command_interpreter.ResolveCommand(
-"breakpoint set --name main --line 123", result
+"breakpoint set --name main --ignore-count 123", result
 )
 self.assertTrue(result.Succeeded())
-self.assertEqual("breakpoint set --name main --line 123", 
result.GetOutput())
+self.assertEqual("breakpoint set --name main --ignore-count 123", 
result.GetOutput())
 
 # Create some aliases.
 self.runCmd("com a alias com al")
@@ -72,10 +72,10 @@ def test_command_abbreviations_and_aliases(self):
 "process launch -s -o /dev/tty0 -e /dev/tty0", result.GetOutput()
 )
 
-self.runCmd("alias xyzzy breakpoint set -n %1 -l %2")
+self.runCmd("alias xyzzy breakpoint set -n %1 -i %2")
 command_interpreter.ResolveCommand("xyzzy main 123", result)
 self.assertTrue(result.Succeeded())
-self.assertEqual("breakpoint set -n main -l 123", 
result.GetOutput().strip())
+self.assertEqual("breakpoint set -n main -i 123", 
result.GetOutput().strip())
 
 # And again, without enough parameters.
 command_interpreter.ResolveCommand("xyzzy main", result)

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


[Lldb-commits] [lldb] [lldb] Use validation combination of options in TestAbbreviations (PR #125270)

2025-01-31 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

Name and line number are part of different option groups and are not compatible.

```
(lldb) breakpoint set -n foo -l 10
error: invalid combination of options for the given command
```

The help output for `breakpoint set` confirms this. This patch updates the test 
to use two compatible options. With the improved error reporting from #125125 this becomes an issue.

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


1 Files Affected:

- (modified) lldb/test/API/functionalities/abbreviation/TestAbbreviations.py 
(+4-4) 


``diff
diff --git a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py 
b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
index a8cbffbb7ba4a5..bef159501c6cd1 100644
--- a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
+++ b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
@@ -45,10 +45,10 @@ def test_command_abbreviations_and_aliases(self):
 
 # Make sure an unabbreviated command is not mangled.
 command_interpreter.ResolveCommand(
-"breakpoint set --name main --line 123", result
+"breakpoint set --name main --ignore-count 123", result
 )
 self.assertTrue(result.Succeeded())
-self.assertEqual("breakpoint set --name main --line 123", 
result.GetOutput())
+self.assertEqual("breakpoint set --name main --ignore-count 123", 
result.GetOutput())
 
 # Create some aliases.
 self.runCmd("com a alias com al")
@@ -72,10 +72,10 @@ def test_command_abbreviations_and_aliases(self):
 "process launch -s -o /dev/tty0 -e /dev/tty0", result.GetOutput()
 )
 
-self.runCmd("alias xyzzy breakpoint set -n %1 -l %2")
+self.runCmd("alias xyzzy breakpoint set -n %1 -i %2")
 command_interpreter.ResolveCommand("xyzzy main 123", result)
 self.assertTrue(result.Succeeded())
-self.assertEqual("breakpoint set -n main -l 123", 
result.GetOutput().strip())
+self.assertEqual("breakpoint set -n main -i 123", 
result.GetOutput().strip())
 
 # And again, without enough parameters.
 command_interpreter.ResolveCommand("xyzzy main", result)

``




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


[Lldb-commits] [lldb] [lldb] Use validation combination of options in TestAbbreviations (PR #125270)

2025-01-31 Thread Jonas Devlieghere via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Use validation combination of options in TestAbbreviations (PR #125270)

2025-01-31 Thread via lldb-commits

github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
darker --check --diff -r 
1b0becf739ace0e04c57b50ab701b5e3d009ccbb...a33e43e56e8a58c1d53c566c0b9da7791254d68e
 lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
``





View the diff from darker here.


``diff
--- TestAbbreviations.py2025-01-31 19:10:57.00 +
+++ TestAbbreviations.py2025-01-31 19:19:05.426694 +
@@ -46,11 +46,13 @@
 # Make sure an unabbreviated command is not mangled.
 command_interpreter.ResolveCommand(
 "breakpoint set --name main --ignore-count 123", result
 )
 self.assertTrue(result.Succeeded())
-self.assertEqual("breakpoint set --name main --ignore-count 123", 
result.GetOutput())
+self.assertEqual(
+"breakpoint set --name main --ignore-count 123", result.GetOutput()
+)
 
 # Create some aliases.
 self.runCmd("com a alias com al")
 self.runCmd("alias gurp help")
 

``




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


[Lldb-commits] [lldb] [lldb] Use validation combination of options in TestAbbreviations (PR #125270)

2025-01-31 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/125270

>From a33e43e56e8a58c1d53c566c0b9da7791254d68e Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 31 Jan 2025 11:10:40 -0800
Subject: [PATCH 1/2] [lldb] Use validation combination of options in
 TestAbbreviations

Name and line number are part of different option groups and are not
compatible.

  (lldb) breakpoint set -n foo -l 10
  error: invalid combination of options for the given command

The help output for `breakpoint set` confirms this. This patch updates
the test to use two compatible options. With the improved error
reporting from #125125 this becomes an issue.
---
 .../API/functionalities/abbreviation/TestAbbreviations.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py 
b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
index a8cbffbb7ba4a5..bef159501c6cd1 100644
--- a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
+++ b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
@@ -45,10 +45,10 @@ def test_command_abbreviations_and_aliases(self):
 
 # Make sure an unabbreviated command is not mangled.
 command_interpreter.ResolveCommand(
-"breakpoint set --name main --line 123", result
+"breakpoint set --name main --ignore-count 123", result
 )
 self.assertTrue(result.Succeeded())
-self.assertEqual("breakpoint set --name main --line 123", 
result.GetOutput())
+self.assertEqual("breakpoint set --name main --ignore-count 123", 
result.GetOutput())
 
 # Create some aliases.
 self.runCmd("com a alias com al")
@@ -72,10 +72,10 @@ def test_command_abbreviations_and_aliases(self):
 "process launch -s -o /dev/tty0 -e /dev/tty0", result.GetOutput()
 )
 
-self.runCmd("alias xyzzy breakpoint set -n %1 -l %2")
+self.runCmd("alias xyzzy breakpoint set -n %1 -i %2")
 command_interpreter.ResolveCommand("xyzzy main 123", result)
 self.assertTrue(result.Succeeded())
-self.assertEqual("breakpoint set -n main -l 123", 
result.GetOutput().strip())
+self.assertEqual("breakpoint set -n main -i 123", 
result.GetOutput().strip())
 
 # And again, without enough parameters.
 command_interpreter.ResolveCommand("xyzzy main", result)

>From 09cc81731b94aadd7941dcf9e0deae1f8c60272e Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 31 Jan 2025 11:20:12 -0800
Subject: [PATCH 2/2] Format TestAbbreviations.py

---
 .../API/functionalities/abbreviation/TestAbbreviations.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py 
b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
index bef159501c6cd1..b4e770af0e392c 100644
--- a/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
+++ b/lldb/test/API/functionalities/abbreviation/TestAbbreviations.py
@@ -48,7 +48,9 @@ def test_command_abbreviations_and_aliases(self):
 "breakpoint set --name main --ignore-count 123", result
 )
 self.assertTrue(result.Succeeded())
-self.assertEqual("breakpoint set --name main --ignore-count 123", 
result.GetOutput())
+self.assertEqual(
+"breakpoint set --name main --ignore-count 123", result.GetOutput()
+)
 
 # Create some aliases.
 self.runCmd("com a alias com al")

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


[Lldb-commits] [lldb] [lldb][Docs] Expand remote testing instructions (PR #122694)

2025-01-31 Thread Vladislav Dzhidzhoev via lldb-commits

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

Thank you!

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


[Lldb-commits] [lldb] [lldb][test] Link certain libc++ tests with the whole library (PR #118986)

2025-01-31 Thread Vladislav Dzhidzhoev via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Show value for libcxx and libstdcxx summary and remove pointer value in libcxx container summary (PR #125294)

2025-01-31 Thread Zequan Wu via lldb-commits

https://github.com/ZequanWu created 
https://github.com/llvm/llvm-project/pull/125294

This has two changes:
1. Set show value for libcxx and libstdcxx summary provider. This will print 
the pointer value for both pointer type and reference type.
2. Remove pointer value printing in libcxx container summary.

Discussion:
https://discourse.llvm.org/t/lldb-hides-raw-pointer-value-for-libcxx-and-libstdcxx-pointer-types-in-summary-string/84226

>From bbecf2f990c1fdf8ed993c3bba4c68212ebb299c Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Fri, 31 Jan 2025 13:05:14 -0800
Subject: [PATCH] [lldb] Show value for libcxx and libstdcxx summary and remove
 pointer value in libcxx container summary

---
 .../Language/CPlusPlus/CPlusPlusLanguage.cpp  |  4 +-
 .../Plugins/Language/CPlusPlus/LibCxx.cpp |  6 ---
 .../deque/TestDataFormatterLibcxxDeque.py | 49 ---
 .../span/TestDataFormatterLibcxxSpan.py   |  5 +-
 .../variant/TestDataFormatterLibcxxVariant.py |  2 +-
 .../vector/TestDataFormatterLibcxxVector.py   | 49 ---
 .../TestDataFormatterLibStdcxxVariant.py  |  2 +-
 7 files changed, 69 insertions(+), 48 deletions(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 2bf574e97768ec..4b045d12ad4947 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -641,7 +641,7 @@ static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP 
cpp_category_sp) {
   .SetSkipPointers(false)
   .SetSkipReferences(false)
   .SetDontShowChildren(true)
-  .SetDontShowValue(true)
+  .SetDontShowValue(false)
   .SetShowMembersOneLiner(false)
   .SetHideItemNames(false);
 
@@ -1204,7 +1204,7 @@ static void 
LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   .SetSkipPointers(false)
   .SetSkipReferences(false)
   .SetDontShowChildren(true)
-  .SetDontShowValue(true)
+  .SetDontShowValue(false)
   .SetShowMembersOneLiner(false)
   .SetHideItemNames(false);
 
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 6d0ccdbbe4a71d..2aa8fdba706348 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -430,12 +430,6 @@ size_t 
lldb_private::formatters::LibcxxUniquePtrSyntheticFrontEnd::
 
 bool lldb_private::formatters::LibcxxContainerSummaryProvider(
 ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
-  if (valobj.IsPointerType()) {
-uint64_t value = valobj.GetValueAsUnsigned(0);
-if (!value)
-  return false;
-stream.Printf("0x%016" PRIx64 " ", value);
-  }
   return FormatEntity::FormatStringRef("size=${svar%#}", stream, nullptr,
nullptr, nullptr, &valobj, false, 
false);
 }
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
index 3596b546be306a..8f8af60d4e9728 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
@@ -9,22 +9,37 @@
 
 
 class LibcxxDequeDataFormatterTestCase(TestBase):
-def check_numbers(self, var_name):
-self.expect(
-"frame variable " + var_name,
-substrs=[
-var_name + " = size=7",
-"[0] = 1",
-"[1] = 12",
-"[2] = 123",
-"[3] = 1234",
-"[4] = 12345",
-"[5] = 123456",
-"[6] = 1234567",
-"}",
-],
-)
-
+def check_numbers(self, var_name, show_ptr = False):
+if show_ptr:
+self.expect(
+"frame variable " + var_name,
+patterns=[var_name + " = 0x.* size=7"],
+substrs=[
+"[0] = 1",
+"[1] = 12",
+"[2] = 123",
+"[3] = 1234",
+"[4] = 12345",
+"[5] = 123456",
+"[6] = 1234567",
+"}",
+],
+)
+else:
+self.expect(
+"frame variable " + var_name,
+substrs=[
+var_name + " = size=7",
+"[0] = 1",
+"[1] = 12",
+"[2] = 123",
+"[3] = 1234",
+"[4] = 12345",
+"[5] = 123456",
+"[6] = 1234567",
+"}",
+ 

[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)

2025-01-31 Thread Adrian Prantl via lldb-commits

adrian-prantl wrote:

This is breaking macOS bots: 
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/19569/

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


[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)

2025-01-31 Thread Adrian Prantl via lldb-commits

adrian-prantl wrote:

I'm going to revert this for now, let me know if you need help understanding 
the failure cause!

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


[Lldb-commits] [lldb] [lldb] Show value for libcxx and libstdcxx summary and remove pointer value in libcxx container summary (PR #125294)

2025-01-31 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Zequan Wu (ZequanWu)


Changes

This has two changes:
1. Set show value for libcxx and libstdcxx summary provider. This will print 
the pointer value for both pointer type and reference type.
2. Remove pointer value printing in libcxx container summary.

Discussion:
https://discourse.llvm.org/t/lldb-hides-raw-pointer-value-for-libcxx-and-libstdcxx-pointer-types-in-summary-string/84226

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


7 Files Affected:

- (modified) lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
(+2-2) 
- (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp (-6) 
- (modified) 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
 (+32-17) 
- (modified) 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
 (+1-4) 
- (modified) 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
 (+1-1) 
- (modified) 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/vector/TestDataFormatterLibcxxVector.py
 (+32-17) 
- (modified) 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
 (+1-1) 


``diff
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 2bf574e97768ec..4b045d12ad4947 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -641,7 +641,7 @@ static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP 
cpp_category_sp) {
   .SetSkipPointers(false)
   .SetSkipReferences(false)
   .SetDontShowChildren(true)
-  .SetDontShowValue(true)
+  .SetDontShowValue(false)
   .SetShowMembersOneLiner(false)
   .SetHideItemNames(false);
 
@@ -1204,7 +1204,7 @@ static void 
LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   .SetSkipPointers(false)
   .SetSkipReferences(false)
   .SetDontShowChildren(true)
-  .SetDontShowValue(true)
+  .SetDontShowValue(false)
   .SetShowMembersOneLiner(false)
   .SetHideItemNames(false);
 
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 6d0ccdbbe4a71d..2aa8fdba706348 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -430,12 +430,6 @@ size_t 
lldb_private::formatters::LibcxxUniquePtrSyntheticFrontEnd::
 
 bool lldb_private::formatters::LibcxxContainerSummaryProvider(
 ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
-  if (valobj.IsPointerType()) {
-uint64_t value = valobj.GetValueAsUnsigned(0);
-if (!value)
-  return false;
-stream.Printf("0x%016" PRIx64 " ", value);
-  }
   return FormatEntity::FormatStringRef("size=${svar%#}", stream, nullptr,
nullptr, nullptr, &valobj, false, 
false);
 }
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
index 3596b546be306a..8f8af60d4e9728 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/deque/TestDataFormatterLibcxxDeque.py
@@ -9,22 +9,37 @@
 
 
 class LibcxxDequeDataFormatterTestCase(TestBase):
-def check_numbers(self, var_name):
-self.expect(
-"frame variable " + var_name,
-substrs=[
-var_name + " = size=7",
-"[0] = 1",
-"[1] = 12",
-"[2] = 123",
-"[3] = 1234",
-"[4] = 12345",
-"[5] = 123456",
-"[6] = 1234567",
-"}",
-],
-)
-
+def check_numbers(self, var_name, show_ptr = False):
+if show_ptr:
+self.expect(
+"frame variable " + var_name,
+patterns=[var_name + " = 0x.* size=7"],
+substrs=[
+"[0] = 1",
+"[1] = 12",
+"[2] = 123",
+"[3] = 1234",
+"[4] = 12345",
+"[5] = 123456",
+"[6] = 1234567",
+"}",
+],
+)
+else:
+self.expect(
+"frame variable " + var_name,
+substrs=[
+var_name + " = size=7",
+"[0] = 1",
+"[1] = 12",

[Lldb-commits] [lldb] [lldb] Make ValueObjectDynamicValue::UpdateValue() point to a host b… (PR #125143)

2025-01-31 Thread via lldb-commits

jimingham wrote:

I think it's worth making it as hard as possible to get the length you're 
supposed to read from that buffer right.

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


[Lldb-commits] [lldb] [lldb] Make ValueObjectDynamicValue::UpdateValue() point to a host b… (PR #125143)

2025-01-31 Thread via lldb-commits

jimingham wrote:

This is a very common crash point.  I suspect that in most of the cases it's 
because we flub whether something is a load or a host address, but I still 
don't want to add other mistakes we could make here.


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


[Lldb-commits] [lldb] LLDB: WatchAddress ignores modify option (PR #124847)

2025-01-31 Thread Greg Clayton via lldb-commits

clayborg wrote:

Seeing as you are working on watchpoints here, I found that `void 
WatchpointList::RemoveAll(bool notify)` is sending the wrong event of 
`Target::eBroadcastBitBreakpointChanged` instead of sending 
`Target::eBroadcastBitWatchpointChanged`. Might be a good fix to get in to 
ensure watchpoints are solid.

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


[Lldb-commits] [lldb] [lldb] Store the command in the CommandReturnObject (PR #125132)

2025-01-31 Thread via lldb-commits

jimingham wrote:

This looks good.  I wonder if we ought to do something a little less ad hoc 
about the cases where our command return objects are from HandleCommands, so 
they won't necessarily have the result of a single command.  In the case where 
a UI is parsing these, it might very well need to know that fact.  They all 
currently have `<>` around the command name, but sadly we don't outlaw `<` as 
the initial character of a command, so you can't 100% tell from that. 

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


[Lldb-commits] [lldb] [lldb] Show value for libcxx and libstdcxx summary and remove pointer value in libcxx container summary (PR #125294)

2025-01-31 Thread via lldb-commits

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

LGTM, better to be consistent, and the pointer value is useful.

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


[Lldb-commits] [lldb] [lldb] Implement bidirectional access for backing<->backed thread relationship (PR #125300)

2025-01-31 Thread Felipe de Azevedo Piovezan via lldb-commits


@@ -470,6 +470,18 @@ class Thread : public std::enable_shared_from_this,
 
   virtual void ClearStackFrames();
 
+  /// Derived classes implementing SetBackingThread should use this to provide
+  /// bidirectional access to the Backing-Backed relationship.
+  void SetBackedThread(Thread &backed_thread) {

felipepiovezan wrote:

Good point, this also addresses my concern of people calling `SetBackedThread` 
without also calling `SetBackingThread`. Will update.

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


[Lldb-commits] [lldb] [lldb] Implement bidirectional access for backing<->backed thread relationship (PR #125300)

2025-01-31 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/125300

>From 0c9d9ed5b1aa78f397e95c894def54ee627bea62 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Fri, 31 Jan 2025 12:07:45 -0800
Subject: [PATCH 1/2] [lldb] Implement bidirectional access for
 backing<->backed thread relationship

This enables finding the backed thread from the backing thread without
going through the thread list, and it will be useful for subsequent
commits.
---
 lldb/include/lldb/Target/Thread.h | 15 +++
 lldb/include/lldb/Target/ThreadList.h |  2 --
 .../source/Plugins/Process/Utility/ThreadMemory.h |  7 ++-
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |  2 +-
 lldb/source/Target/ThreadList.cpp | 14 --
 5 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index ef66fa11574db9..d5fe33586fa3cf 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -470,6 +470,18 @@ class Thread : public std::enable_shared_from_this,
 
   virtual void ClearStackFrames();
 
+  /// Derived classes implementing SetBackingThread should use this to provide
+  /// bidirectional access to the Backing-Backed relationship.
+  void SetBackedThread(Thread &backed_thread) {
+assert(backed_thread.GetBackingThread().get() == this);
+m_backed_thread = backed_thread.shared_from_this();
+  }
+
+  void ClearBackedThread() { m_backed_thread.reset(); }
+
+  /// Returns the thread that is backed by this thread, if any.
+  lldb::ThreadSP GetBackedThread() const { return m_backed_thread; }
+
   virtual bool SetBackingThread(const lldb::ThreadSP &thread_sp) {
 return false;
   }
@@ -1349,6 +1361,9 @@ class Thread : public 
std::enable_shared_from_this,
   LazyBool m_override_should_notify;
   mutable std::unique_ptr m_null_plan_stack_up;
 
+  /// The Thread backed by this thread, if any.
+  lldb::ThreadSP m_backed_thread;
+
 private:
   bool m_extended_info_fetched; // Have we tried to retrieve the 
m_extended_info
 // for this thread?
diff --git a/lldb/include/lldb/Target/ThreadList.h 
b/lldb/include/lldb/Target/ThreadList.h
index f931bb83a8ceaf..bca01f5fe2083e 100644
--- a/lldb/include/lldb/Target/ThreadList.h
+++ b/lldb/include/lldb/Target/ThreadList.h
@@ -101,8 +101,6 @@ class ThreadList : public ThreadCollection {
 
   lldb::ThreadSP GetThreadSPForThreadPtr(Thread *thread_ptr);
 
-  lldb::ThreadSP GetBackingThread(const lldb::ThreadSP &real_thread);
-
   bool ShouldStop(Event *event_ptr);
 
   Vote ShouldReportStop(Event *event_ptr);
diff --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.h 
b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
index d124f5780ea9bd..1e309671e85c65 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.h
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
@@ -72,12 +72,17 @@ class ThreadMemory : public lldb_private::Thread {
 
   void ClearStackFrames() override;
 
-  void ClearBackingThread() override { m_backing_thread_sp.reset(); }
+  void ClearBackingThread() override {
+if (m_backing_thread_sp)
+  m_backing_thread_sp->ClearBackedThread();
+m_backing_thread_sp.reset();
+  }
 
   bool SetBackingThread(const lldb::ThreadSP &thread_sp) override {
 // printf ("Thread 0x%llx is being backed by thread 0x%llx\n", GetID(),
 // thread_sp->GetID());
 m_backing_thread_sp = thread_sp;
+thread_sp->SetBackedThread(*this);
 return (bool)thread_sp;
   }
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 21a0fa283644d6..3f34ea2930a66a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1728,7 +1728,7 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
 thread_sp->SetStopInfo(StopInfoSP());
 // If there's a memory thread backed by this thread, we need to use it to
 // calculate StopInfo.
-if (ThreadSP memory_thread_sp = m_thread_list.GetBackingThread(thread_sp))
+if (ThreadSP memory_thread_sp = thread_sp->GetBackedThread())
   thread_sp = memory_thread_sp;
 
 if (exc_type != 0) {
diff --git a/lldb/source/Target/ThreadList.cpp 
b/lldb/source/Target/ThreadList.cpp
index 6cbef330bf4888..c0440d82fd1ffc 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -191,20 +191,6 @@ ThreadSP ThreadList::GetThreadSPForThreadPtr(Thread 
*thread_ptr) {
   return thread_sp;
 }
 
-ThreadSP ThreadList::GetBackingThread(const ThreadSP &real_thread) {
-  std::lock_guard guard(GetMutex());
-
-  ThreadSP thread_sp;
-  const uint32_t num_threads = m_threads.size();
-  for (uint32_t idx = 0; idx < num_threads; ++idx) {
-if (m_threads[idx]->GetBackingThread() == real_thread) {
-  thread_sp = m_

[Lldb-commits] [lldb] [lldb] Implement bidirectional access for backing<->backed thread relationship (PR #125300)

2025-01-31 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/125300

>From 0c9d9ed5b1aa78f397e95c894def54ee627bea62 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Fri, 31 Jan 2025 12:07:45 -0800
Subject: [PATCH 1/3] [lldb] Implement bidirectional access for
 backing<->backed thread relationship

This enables finding the backed thread from the backing thread without
going through the thread list, and it will be useful for subsequent
commits.
---
 lldb/include/lldb/Target/Thread.h | 15 +++
 lldb/include/lldb/Target/ThreadList.h |  2 --
 .../source/Plugins/Process/Utility/ThreadMemory.h |  7 ++-
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |  2 +-
 lldb/source/Target/ThreadList.cpp | 14 --
 5 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index ef66fa11574db9..d5fe33586fa3cf 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -470,6 +470,18 @@ class Thread : public std::enable_shared_from_this,
 
   virtual void ClearStackFrames();
 
+  /// Derived classes implementing SetBackingThread should use this to provide
+  /// bidirectional access to the Backing-Backed relationship.
+  void SetBackedThread(Thread &backed_thread) {
+assert(backed_thread.GetBackingThread().get() == this);
+m_backed_thread = backed_thread.shared_from_this();
+  }
+
+  void ClearBackedThread() { m_backed_thread.reset(); }
+
+  /// Returns the thread that is backed by this thread, if any.
+  lldb::ThreadSP GetBackedThread() const { return m_backed_thread; }
+
   virtual bool SetBackingThread(const lldb::ThreadSP &thread_sp) {
 return false;
   }
@@ -1349,6 +1361,9 @@ class Thread : public 
std::enable_shared_from_this,
   LazyBool m_override_should_notify;
   mutable std::unique_ptr m_null_plan_stack_up;
 
+  /// The Thread backed by this thread, if any.
+  lldb::ThreadSP m_backed_thread;
+
 private:
   bool m_extended_info_fetched; // Have we tried to retrieve the 
m_extended_info
 // for this thread?
diff --git a/lldb/include/lldb/Target/ThreadList.h 
b/lldb/include/lldb/Target/ThreadList.h
index f931bb83a8ceaf..bca01f5fe2083e 100644
--- a/lldb/include/lldb/Target/ThreadList.h
+++ b/lldb/include/lldb/Target/ThreadList.h
@@ -101,8 +101,6 @@ class ThreadList : public ThreadCollection {
 
   lldb::ThreadSP GetThreadSPForThreadPtr(Thread *thread_ptr);
 
-  lldb::ThreadSP GetBackingThread(const lldb::ThreadSP &real_thread);
-
   bool ShouldStop(Event *event_ptr);
 
   Vote ShouldReportStop(Event *event_ptr);
diff --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.h 
b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
index d124f5780ea9bd..1e309671e85c65 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.h
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
@@ -72,12 +72,17 @@ class ThreadMemory : public lldb_private::Thread {
 
   void ClearStackFrames() override;
 
-  void ClearBackingThread() override { m_backing_thread_sp.reset(); }
+  void ClearBackingThread() override {
+if (m_backing_thread_sp)
+  m_backing_thread_sp->ClearBackedThread();
+m_backing_thread_sp.reset();
+  }
 
   bool SetBackingThread(const lldb::ThreadSP &thread_sp) override {
 // printf ("Thread 0x%llx is being backed by thread 0x%llx\n", GetID(),
 // thread_sp->GetID());
 m_backing_thread_sp = thread_sp;
+thread_sp->SetBackedThread(*this);
 return (bool)thread_sp;
   }
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 21a0fa283644d6..3f34ea2930a66a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1728,7 +1728,7 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
 thread_sp->SetStopInfo(StopInfoSP());
 // If there's a memory thread backed by this thread, we need to use it to
 // calculate StopInfo.
-if (ThreadSP memory_thread_sp = m_thread_list.GetBackingThread(thread_sp))
+if (ThreadSP memory_thread_sp = thread_sp->GetBackedThread())
   thread_sp = memory_thread_sp;
 
 if (exc_type != 0) {
diff --git a/lldb/source/Target/ThreadList.cpp 
b/lldb/source/Target/ThreadList.cpp
index 6cbef330bf4888..c0440d82fd1ffc 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -191,20 +191,6 @@ ThreadSP ThreadList::GetThreadSPForThreadPtr(Thread 
*thread_ptr) {
   return thread_sp;
 }
 
-ThreadSP ThreadList::GetBackingThread(const ThreadSP &real_thread) {
-  std::lock_guard guard(GetMutex());
-
-  ThreadSP thread_sp;
-  const uint32_t num_threads = m_threads.size();
-  for (uint32_t idx = 0; idx < num_threads; ++idx) {
-if (m_threads[idx]->GetBackingThread() == real_thread) {
-  thread_sp = m_

[Lldb-commits] [lldb] [lldb] Implement bidirectional access for backing<->backed thread relationship (PR #125300)

2025-01-31 Thread via lldb-commits

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

LGTM

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


[Lldb-commits] [lldb] [lldb] Support CommandInterpreter print callbacks (PR #125006)

2025-01-31 Thread Pavel Labath via lldb-commits

https://github.com/labath commented:

> Yes, part of the requirement is that the output isn't printed by lldb itself, 
> although the implementer is still in control with the return value.

Okay. I was afraid of that. :)

The code looks good, though I must admit that the functionality seems a bit.. 
random. Like, you're putting lldb in a terminal, but then you don't want it to 
behave like lldb in a terminal. I don't suppose there's anything you can say 
about what you intend to do with it?

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


[Lldb-commits] [lldb] [lldb][TypeSystemClang] Fix enum signedness in CompleteEnumType (PR #125203)

2025-01-31 Thread Michael Buch via lldb-commits

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/125203

>From 620f30f73930a1a8b2edb298f67d555ff3b93e2c Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 31 Jan 2025 11:18:28 +
Subject: [PATCH 1/4] [lldb][TypeSystemClang] Fix enum signedness in
 CompleteEnumType

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp   | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 6602dd763ba693..359872c579f35e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1020,15 +1020,7 @@ TypeSP DWARFASTParserClang::ParseEnum(const 
SymbolContext &sc,
   }
 
 
-  if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-if (def_die.HasChildren()) {
-  bool is_signed = false;
-  enumerator_clang_type.IsIntegerType(is_signed);
-  ParseChildEnumerators(clang_type, is_signed,
-type_sp->GetByteSize(nullptr).value_or(0), 
def_die);
-}
-TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
-  } else {
+  if (!CompleteEnumType(def_die, type_sp.get(), clang_type)) {
 dwarf->GetObjectFile()->GetModule()->ReportError(
 "DWARF DIE at {0:x16} named \"{1}\" was not able to start its "
 "definition.\nPlease file a bug and attach the file at the "
@@ -,12 +2214,10 @@ bool DWARFASTParserClang::CompleteEnumType(const 
DWARFDIE &die,
lldb_private::Type *type,
const CompilerType &clang_type) {
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
-if (die.HasChildren()) {
-  bool is_signed = false;
-  clang_type.IsIntegerType(is_signed);
-  ParseChildEnumerators(clang_type, is_signed,
+if (die.HasChildren())
+  ParseChildEnumerators(clang_type, 
clang_type.IsEnumerationIntegerTypeSigned(),
 type->GetByteSize(nullptr).value_or(0), die);
-}
+
 TypeSystemClang::CompleteTagDeclarationDefinition(clang_type);
   }
   return (bool)clang_type;

>From f72106fffe6dd20513134d35e6fbb9b31a06e9cf Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 31 Jan 2025 11:30:46 +
Subject: [PATCH 2/4] fixup! add assert

---
 lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 359872c579f35e..08fe4a29be8eb7 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2213,6 +2213,8 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
 bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE &die,
lldb_private::Type *type,
const CompilerType &clang_type) {
+  assert (clang_type.IsEnumerationType());
+
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
 if (die.HasChildren())
   ParseChildEnumerators(clang_type, 
clang_type.IsEnumerationIntegerTypeSigned(),

>From 23cd607c0c8130a783f80d94e98b7b89b70d45f0 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 31 Jan 2025 11:30:57 +
Subject: [PATCH 3/4] fixup! clang-format

---
 lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 08fe4a29be8eb7..ee99fd6f16cc44 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2213,7 +2213,7 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
 bool DWARFASTParserClang::CompleteEnumType(const DWARFDIE &die,
lldb_private::Type *type,
const CompilerType &clang_type) {
-  assert (clang_type.IsEnumerationType());
+  assert(clang_type.IsEnumerationType());
 
   if (TypeSystemClang::StartTagDeclarationDefinition(clang_type)) {
 if (die.HasChildren())

>From f5b774a6260349778a92dcd852b76cccda1407c5 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 31 Jan 2025 11:42:40 +
Subject: [PATCH 4/4] fixup! clang-format

---
 lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ee99fd6f16cc44..dca193fc11b552 100644
--- a/lldb/so

[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)

2025-01-31 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Windows on Arm passed, Arm and AArch64 Linux passed. No complaints so far.

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


[Lldb-commits] [lldb] [lldb] Support CommandInterpreter print callbacks (PR #125006)

2025-01-31 Thread via lldb-commits

jimingham wrote:

> > Yes, part of the requirement is that the output isn't printed by lldb 
> > itself, although the implementer is still in control with the return value.
> 
> Okay. I was afraid of that. :)
> 
> The code looks good, though I must admit that the functionality seems a bit.. 
> random. Like, you're putting lldb in a terminal, but then you don't want it 
> to behave like lldb in a terminal. I don't suppose there's anything you can 
> say about what you intend to do with it?

One use case is that a UI might want to make a nicer presentation for some 
class of errors - for in particular the expression syntax errors that have more 
rich structure - making them disclosable or whatnot. Since those can be huge - 
particularly compile errors from expressions - you really don't want to 
duplicate the printing in that case.


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


[Lldb-commits] [lldb] LLDB: WatchAddress ignores modify option (PR #124847)

2025-01-31 Thread Ben Jackson via lldb-commits

puremourning wrote:

I think the issue is that... we just don't support read-only watchpoints on 
intel because the [hardware doesn't support 
it](https://en.wikipedia.org/wiki/X86_debug_register#cite_note-brkpt_type-19).

I guess I'll mark this test as "skip on x86"

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


[Lldb-commits] [lldb] LLDB: WatchAddress ignores modify option (PR #124847)

2025-01-31 Thread Ben Jackson via lldb-commits

https://github.com/puremourning updated 
https://github.com/llvm/llvm-project/pull/124847

>From 6b3ba3f6dcf11d8b18ff72bc736b24ac75231626 Mon Sep 17 00:00:00 2001
From: Ben Jackson 
Date: Tue, 28 Jan 2025 21:47:24 +
Subject: [PATCH] LLDB: WatchAddress ignores modify option

The WatchAddress API includes a flag to indicate if watchpoint should be
for read, modify or both. This API uses 2 booleans, but the 'modify'
flag was ignored and WatchAddress unconditionally watched write
(actually modify).

We now only watch for modify when the modify flag is true.
---
 lldb/source/API/SBTarget.cpp  |   3 +-
 .../watchpoint/TestWatchpointRead.py  | 129 ++
 .../watchlocation/TestTargetWatchAddress.py   |  71 +-
 3 files changed, 201 insertions(+), 2 deletions(-)
 create mode 100644 lldb/test/API/python_api/watchpoint/TestWatchpointRead.py

diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 2a33161bc21edc7..dd9caa724ea3628 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1342,7 +1342,8 @@ lldb::SBWatchpoint SBTarget::WatchAddress(lldb::addr_t 
addr, size_t size,
 
   SBWatchpointOptions options;
   options.SetWatchpointTypeRead(read);
-  options.SetWatchpointTypeWrite(eWatchpointWriteTypeOnModify);
+  if (modify)
+options.SetWatchpointTypeWrite(eWatchpointWriteTypeOnModify);
   return WatchpointCreateByAddress(addr, size, options, error);
 }
 
diff --git a/lldb/test/API/python_api/watchpoint/TestWatchpointRead.py 
b/lldb/test/API/python_api/watchpoint/TestWatchpointRead.py
new file mode 100644
index 000..f482ebe5b1ecee6
--- /dev/null
+++ b/lldb/test/API/python_api/watchpoint/TestWatchpointRead.py
@@ -0,0 +1,129 @@
+"""
+Use lldb Python SBTarget API to set read watchpoints
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class SetReadOnlyWatchpointTestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+# Our simple source filename.
+self.source = "main.c"
+# Find the line number to break inside main().
+self.line = line_number(self.source, "// Set break point at this 
line.")
+self.build()
+
+# Intel hardware does not support read-only watchpoints
+@expectedFailureAll(archs=["i386", "x86_64"])
+def test_read_watchpoint_watch_address(self):
+exe = self.getBuildArtifact("a.out")
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Now create a breakpoint on main.c.
+breakpoint = target.BreakpointCreateByLocation(self.source, self.line)
+self.assertTrue(
+breakpoint and breakpoint.GetNumLocations() == 1, VALID_BREAKPOINT
+)
+
+# Now launch the process, and do not stop at the entry point.
+process = target.LaunchSimple(None, None, 
self.get_process_working_directory())
+
+# We should be stopped due to the breakpoint.  Get frame #0.
+process = target.GetProcess()
+self.assertState(process.GetState(), lldb.eStateStopped, 
PROCESS_STOPPED)
+thread = lldbutil.get_stopped_thread(process, 
lldb.eStopReasonBreakpoint)
+frame0 = thread.GetFrameAtIndex(0)
+
+value = frame0.FindValue("global", lldb.eValueTypeVariableGlobal)
+local = frame0.FindValue("local", lldb.eValueTypeVariableLocal)
+error = lldb.SBError()
+
+watchpoint = target.WatchAddress(value.GetLoadAddress(), 1, True, 
False, error)
+self.assertTrue(
+value and local and watchpoint,
+"Successfully found the values and set a watchpoint",
+)
+self.DebugSBValue(value)
+self.DebugSBValue(local)
+
+# Hide stdout if not running with '-t' option.
+if not self.TraceOn():
+self.HideStdout()
+
+print(watchpoint)
+
+# Continue.  Expect the program to stop due to the variable being
+# read, but *not* written to.
+process.Continue()
+
+if self.TraceOn():
+lldbutil.print_stacktraces(process)
+
+self.assertTrue(
+local.GetValueAsSigned() > 0, "The local variable has been 
incremented"
+)
+
+# Intel hardware does not support read-only watchpoints
+@expectedFailureAll(archs=["i386", "x86_64"])
+def test_read_watchpoint_watch_create_by_address(self):
+exe = self.getBuildArtifact("a.out")
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Now create a breakpoint on main.c.
+breakpoint = target.BreakpointCreateByLocation(self.source, self.line)
+self.assertTrue(
+breakpoint and breakpoint.GetNumLocations() == 1, VALID_BREAKPOINT
+)
+
+# Now launch the process, and do not stop at

[Lldb-commits] [lldb] [lldb] Implement bidirectional access for backing<->backed thread relationship (PR #125300)

2025-01-31 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan created 
https://github.com/llvm/llvm-project/pull/125300

This enables finding the backed thread from the backing thread without going 
through the thread list, and it will be useful for subsequent commits.

>From 0c9d9ed5b1aa78f397e95c894def54ee627bea62 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Fri, 31 Jan 2025 12:07:45 -0800
Subject: [PATCH] [lldb] Implement bidirectional access for backing<->backed
 thread relationship

This enables finding the backed thread from the backing thread without
going through the thread list, and it will be useful for subsequent
commits.
---
 lldb/include/lldb/Target/Thread.h | 15 +++
 lldb/include/lldb/Target/ThreadList.h |  2 --
 .../source/Plugins/Process/Utility/ThreadMemory.h |  7 ++-
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |  2 +-
 lldb/source/Target/ThreadList.cpp | 14 --
 5 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index ef66fa11574db96..d5fe33586fa3cf9 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -470,6 +470,18 @@ class Thread : public std::enable_shared_from_this,
 
   virtual void ClearStackFrames();
 
+  /// Derived classes implementing SetBackingThread should use this to provide
+  /// bidirectional access to the Backing-Backed relationship.
+  void SetBackedThread(Thread &backed_thread) {
+assert(backed_thread.GetBackingThread().get() == this);
+m_backed_thread = backed_thread.shared_from_this();
+  }
+
+  void ClearBackedThread() { m_backed_thread.reset(); }
+
+  /// Returns the thread that is backed by this thread, if any.
+  lldb::ThreadSP GetBackedThread() const { return m_backed_thread; }
+
   virtual bool SetBackingThread(const lldb::ThreadSP &thread_sp) {
 return false;
   }
@@ -1349,6 +1361,9 @@ class Thread : public 
std::enable_shared_from_this,
   LazyBool m_override_should_notify;
   mutable std::unique_ptr m_null_plan_stack_up;
 
+  /// The Thread backed by this thread, if any.
+  lldb::ThreadSP m_backed_thread;
+
 private:
   bool m_extended_info_fetched; // Have we tried to retrieve the 
m_extended_info
 // for this thread?
diff --git a/lldb/include/lldb/Target/ThreadList.h 
b/lldb/include/lldb/Target/ThreadList.h
index f931bb83a8ceaf3..bca01f5fe2083e4 100644
--- a/lldb/include/lldb/Target/ThreadList.h
+++ b/lldb/include/lldb/Target/ThreadList.h
@@ -101,8 +101,6 @@ class ThreadList : public ThreadCollection {
 
   lldb::ThreadSP GetThreadSPForThreadPtr(Thread *thread_ptr);
 
-  lldb::ThreadSP GetBackingThread(const lldb::ThreadSP &real_thread);
-
   bool ShouldStop(Event *event_ptr);
 
   Vote ShouldReportStop(Event *event_ptr);
diff --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.h 
b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
index d124f5780ea9bd0..1e309671e85c653 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.h
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
@@ -72,12 +72,17 @@ class ThreadMemory : public lldb_private::Thread {
 
   void ClearStackFrames() override;
 
-  void ClearBackingThread() override { m_backing_thread_sp.reset(); }
+  void ClearBackingThread() override {
+if (m_backing_thread_sp)
+  m_backing_thread_sp->ClearBackedThread();
+m_backing_thread_sp.reset();
+  }
 
   bool SetBackingThread(const lldb::ThreadSP &thread_sp) override {
 // printf ("Thread 0x%llx is being backed by thread 0x%llx\n", GetID(),
 // thread_sp->GetID());
 m_backing_thread_sp = thread_sp;
+thread_sp->SetBackedThread(*this);
 return (bool)thread_sp;
   }
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 21a0fa283644d60..3f34ea2930a66a0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1728,7 +1728,7 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
 thread_sp->SetStopInfo(StopInfoSP());
 // If there's a memory thread backed by this thread, we need to use it to
 // calculate StopInfo.
-if (ThreadSP memory_thread_sp = m_thread_list.GetBackingThread(thread_sp))
+if (ThreadSP memory_thread_sp = thread_sp->GetBackedThread())
   thread_sp = memory_thread_sp;
 
 if (exc_type != 0) {
diff --git a/lldb/source/Target/ThreadList.cpp 
b/lldb/source/Target/ThreadList.cpp
index 6cbef330bf4888b..c0440d82fd1ffcb 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -191,20 +191,6 @@ ThreadSP ThreadList::GetThreadSPForThreadPtr(Thread 
*thread_ptr) {
   return thread_sp;
 }
 
-ThreadSP ThreadList::GetBackingThread(const ThreadSP &real_thread) {
-  std::lock_guard guard(GetMutex());
-
-  ThreadSP thread_sp;
-  const uint32_t num_threads =

[Lldb-commits] [lldb] [lldb] Use llvm::Error instead of CommandReturnObject for error reporting (PR #125125)

2025-01-31 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/125125

>From 24ddc550e3ee61b863cbaea05ff49981bc20f7ad Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Thu, 30 Jan 2025 14:25:16 -0800
Subject: [PATCH 1/4] [lldb] Use llvm::Error instead of CommandReturnObject for
 error reporting

Use `llvm::Error` instead of `CommandReturnObject` for error reporting.
The command return objects were populated with errors but never
displayed. With this patch they're at least logged.
---
 lldb/include/lldb/Interpreter/Options.h   |  4 +-
 lldb/source/Interpreter/CommandAlias.cpp  | 49 +--
 lldb/source/Interpreter/CommandObject.cpp |  2 +-
 lldb/source/Interpreter/Options.cpp   | 87 ++-
 .../DarwinLog/StructuredDataDarwinLog.cpp |  9 +-
 5 files changed, 77 insertions(+), 74 deletions(-)

diff --git a/lldb/include/lldb/Interpreter/Options.h 
b/lldb/include/lldb/Interpreter/Options.h
index 9a6a17c2793fa4..864bda6f24c8cc 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -76,12 +76,12 @@ class Options {
   // This gets passed the short option as an integer...
   void OptionSeen(int short_option);
 
-  bool VerifyOptions(CommandReturnObject &result);
+  llvm::Error VerifyOptions();
 
   // Verify that the options given are in the options table and can be used
   // together, but there may be some required options that are missing (used to
   // verify options that get folded into command aliases).
-  bool VerifyPartialOptions(CommandReturnObject &result);
+  llvm::Error VerifyPartialOptions();
 
   void OutputFormattedUsageText(Stream &strm,
 const OptionDefinition &option_def,
diff --git a/lldb/source/Interpreter/CommandAlias.cpp 
b/lldb/source/Interpreter/CommandAlias.cpp
index c5971b52f837fa..ac0c7a3279e64b 100644
--- a/lldb/source/Interpreter/CommandAlias.cpp
+++ b/lldb/source/Interpreter/CommandAlias.cpp
@@ -10,6 +10,7 @@
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatAdapters.h"
 
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandObject.h"
@@ -20,20 +21,17 @@
 using namespace lldb;
 using namespace lldb_private;
 
-static bool ProcessAliasOptionsArgs(lldb::CommandObjectSP &cmd_obj_sp,
+static llvm::Error
+ProcessAliasOptionsArgs(lldb::CommandObjectSP &cmd_obj_sp,
 llvm::StringRef options_args,
 OptionArgVectorSP &option_arg_vector_sp) {
-  bool success = true;
   OptionArgVector *option_arg_vector = option_arg_vector_sp.get();
 
   if (options_args.size() < 1)
-return true;
+return llvm::Error::success();
 
   Args args(options_args);
   std::string options_string(options_args);
-  // TODO: Find a way to propagate errors in this CommandReturnObject up the
-  // stack.
-  CommandReturnObject result(false);
   // Check to see if the command being aliased can take any command options.
   Options *options = cmd_obj_sp->GetOptions();
   if (options) {
@@ -45,34 +43,30 @@ static bool ProcessAliasOptionsArgs(lldb::CommandObjectSP 
&cmd_obj_sp,
 
 llvm::Expected args_or =
 options->ParseAlias(args, option_arg_vector, options_string);
-if (!args_or) {
-  result.AppendError(toString(args_or.takeError()));
-  result.AppendError("Unable to create requested alias.\n");
-  return false;
-}
+if (!args_or)
+  return llvm::createStringError(
+  llvm::formatv("unable to create alias: {0}",
+llvm::fmt_consume(args_or.takeError(;
 args = std::move(*args_or);
-options->VerifyPartialOptions(result);
-if (!result.Succeeded() &&
-result.GetStatus() != lldb::eReturnStatusStarted) {
-  result.AppendError("Unable to create requested alias.\n");
-  return false;
-}
+if (llvm::Error error = options->VerifyPartialOptions())
+  return error;
   }
 
   if (!options_string.empty()) {
-if (cmd_obj_sp->WantsRawCommandString())
-  option_arg_vector->emplace_back(CommandInterpreter::g_argument, 
-  -1, options_string);
-else {
+if (cmd_obj_sp->WantsRawCommandString()) {
+  option_arg_vector->emplace_back(CommandInterpreter::g_argument, -1,
+  options_string);
+} else {
   for (auto &entry : args.entries()) {
 if (!entry.ref().empty())
-  
option_arg_vector->emplace_back(std::string(CommandInterpreter::g_argument), -1,
+  option_arg_vector->emplace_back(
+  std::string(CommandInterpreter::g_argument), -1,
   std::string(entry.ref()));
   }
 }
   }
 
-  return success;
+  return llvm::Error::success();
 }
 
 CommandAlias::CommandAlias(CommandInterpreter &interpreter,
@@ -85,10 +79,15 @@ CommandAlias::CommandAlias(CommandInterp

[Lldb-commits] [lldb] [lldb] Use validation combination of options in TestAbbreviations (PR #125270)

2025-01-31 Thread Jonas Devlieghere via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Use validation combination of options in TestAbbreviations (PR #125270)

2025-01-31 Thread via lldb-commits

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

I'm happy that you can make ResolveCommand fail on commands that aren't valid!  
These tests clearly thought they were making up a legal command string, so this 
change expresses better what the test intends anyway.

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


[Lldb-commits] [lldb] [lldb] Implement bidirectional access for backing<->backed thread relationship (PR #125300)

2025-01-31 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)


Changes

This enables finding the backed thread from the backing thread without going 
through the thread list, and it will be useful for subsequent commits.

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


5 Files Affected:

- (modified) lldb/include/lldb/Target/Thread.h (+15) 
- (modified) lldb/include/lldb/Target/ThreadList.h (-2) 
- (modified) lldb/source/Plugins/Process/Utility/ThreadMemory.h (+6-1) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+1-1) 
- (modified) lldb/source/Target/ThreadList.cpp (-14) 


``diff
diff --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index ef66fa11574db9..d5fe33586fa3cf 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -470,6 +470,18 @@ class Thread : public std::enable_shared_from_this,
 
   virtual void ClearStackFrames();
 
+  /// Derived classes implementing SetBackingThread should use this to provide
+  /// bidirectional access to the Backing-Backed relationship.
+  void SetBackedThread(Thread &backed_thread) {
+assert(backed_thread.GetBackingThread().get() == this);
+m_backed_thread = backed_thread.shared_from_this();
+  }
+
+  void ClearBackedThread() { m_backed_thread.reset(); }
+
+  /// Returns the thread that is backed by this thread, if any.
+  lldb::ThreadSP GetBackedThread() const { return m_backed_thread; }
+
   virtual bool SetBackingThread(const lldb::ThreadSP &thread_sp) {
 return false;
   }
@@ -1349,6 +1361,9 @@ class Thread : public 
std::enable_shared_from_this,
   LazyBool m_override_should_notify;
   mutable std::unique_ptr m_null_plan_stack_up;
 
+  /// The Thread backed by this thread, if any.
+  lldb::ThreadSP m_backed_thread;
+
 private:
   bool m_extended_info_fetched; // Have we tried to retrieve the 
m_extended_info
 // for this thread?
diff --git a/lldb/include/lldb/Target/ThreadList.h 
b/lldb/include/lldb/Target/ThreadList.h
index f931bb83a8ceaf..bca01f5fe2083e 100644
--- a/lldb/include/lldb/Target/ThreadList.h
+++ b/lldb/include/lldb/Target/ThreadList.h
@@ -101,8 +101,6 @@ class ThreadList : public ThreadCollection {
 
   lldb::ThreadSP GetThreadSPForThreadPtr(Thread *thread_ptr);
 
-  lldb::ThreadSP GetBackingThread(const lldb::ThreadSP &real_thread);
-
   bool ShouldStop(Event *event_ptr);
 
   Vote ShouldReportStop(Event *event_ptr);
diff --git a/lldb/source/Plugins/Process/Utility/ThreadMemory.h 
b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
index d124f5780ea9bd..1e309671e85c65 100644
--- a/lldb/source/Plugins/Process/Utility/ThreadMemory.h
+++ b/lldb/source/Plugins/Process/Utility/ThreadMemory.h
@@ -72,12 +72,17 @@ class ThreadMemory : public lldb_private::Thread {
 
   void ClearStackFrames() override;
 
-  void ClearBackingThread() override { m_backing_thread_sp.reset(); }
+  void ClearBackingThread() override {
+if (m_backing_thread_sp)
+  m_backing_thread_sp->ClearBackedThread();
+m_backing_thread_sp.reset();
+  }
 
   bool SetBackingThread(const lldb::ThreadSP &thread_sp) override {
 // printf ("Thread 0x%llx is being backed by thread 0x%llx\n", GetID(),
 // thread_sp->GetID());
 m_backing_thread_sp = thread_sp;
+thread_sp->SetBackedThread(*this);
 return (bool)thread_sp;
   }
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 21a0fa283644d6..3f34ea2930a66a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1728,7 +1728,7 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
 thread_sp->SetStopInfo(StopInfoSP());
 // If there's a memory thread backed by this thread, we need to use it to
 // calculate StopInfo.
-if (ThreadSP memory_thread_sp = m_thread_list.GetBackingThread(thread_sp))
+if (ThreadSP memory_thread_sp = thread_sp->GetBackedThread())
   thread_sp = memory_thread_sp;
 
 if (exc_type != 0) {
diff --git a/lldb/source/Target/ThreadList.cpp 
b/lldb/source/Target/ThreadList.cpp
index 6cbef330bf4888..c0440d82fd1ffc 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -191,20 +191,6 @@ ThreadSP ThreadList::GetThreadSPForThreadPtr(Thread 
*thread_ptr) {
   return thread_sp;
 }
 
-ThreadSP ThreadList::GetBackingThread(const ThreadSP &real_thread) {
-  std::lock_guard guard(GetMutex());
-
-  ThreadSP thread_sp;
-  const uint32_t num_threads = m_threads.size();
-  for (uint32_t idx = 0; idx < num_threads; ++idx) {
-if (m_threads[idx]->GetBackingThread() == real_thread) {
-  thread_sp = m_threads[idx];
-  break;
-}
-  }
-  return thread_sp;
-}
-
 ThreadSP ThreadList::FindThreadByIndexID(uint32_t index_id, bool can_update) {
   std::lock_guard guard(GetMutex());
 

[Lldb-commits] [lldb] [lldb] Improve isolation between Process plugins and OS plugins (PR #125302)

2025-01-31 Thread Felipe de Azevedo Piovezan via lldb-commits

https://github.com/felipepiovezan created 
https://github.com/llvm/llvm-project/pull/125302

Generally speaking, process plugins (e.g. ProcessGDBRemote) should not be aware 
of OS plugin threads. However, ProcessGDBRemote attempts to check for the 
existence of OS threads when calculating stop info. When OS threads are 
present, it sets the stop info directly on the OS plugin thread and leaves the 
ThreadGDBRemote without a StopInfo.

This is problematic for a few reasons:

1. No other process plugins do this, as they shouldn't. They should set the 
stop info for their own process threads, and let the abstractions built on top 
propagate StopInfos.

2. This conflicts with the expectations of ThreadMemory, which checks for the 
backing threads's info, and then attempts to propagate it (in the future, it 
should probably ask the plugin itself too...). We see this happening in the 
code below. The `if` condition will not trigger, because `backing_stop_info_sp` 
will be null (remember, ProcessGDB remote is ignoring its own threads), and 
then this method returns false.

```
bool ThreadMemory::CalculateStopInfo() {
...
  lldb::StopInfoSP backing_stop_info_sp(
  m_backing_thread_sp->GetPrivateStopInfo());
  if (backing_stop_info_sp &&
  backing_stop_info_sp->IsValidForOperatingSystemThread(*this)) {
backing_stop_info_sp->SetThread(shared_from_this());
```

```
Thread::GetPrivateStopInfo
...
if (!CalculateStopInfo())
  SetStopInfo(StopInfoSP());
```

To solve this, we change ProcessGDB remote so that it does the principled 
thing: it now only sets the stop info of its own threads. This change by itself 
breaks the tests TestPythonOSPlugin.py and TestOSPluginStepping.py and probably 
explains why ProcessGDB had originally "violated" this isolation of layers.

To make this work, BreakpointSites must be aware of BackingThreads when 
answering the question: "Is this breakpoint valid for this thread?". Why? 
Breakpoints are created on top of the OS threads (that's what the user sees), 
but breakpoints are hit by process threads. In the presence of OS threads, a 
TID-specific breakpoint is valid for a process thread if it is backing an OS 
thread with that TID.

>From 126a5bec73ed83ed2294817b0a30f0bb503ed757 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Fri, 31 Jan 2025 12:28:45 -0800
Subject: [PATCH] [lldb] Improve isolation between Process plugins and OS
 plugins

Generally speaking, process plugins (e.g. ProcessGDBRemote) should not
be aware of OS plugin threads. However, ProcessGDBRemote attempts to
check for the existence of OS threads when calculating stop info.
When OS threads are present, it sets the stop info directly on the OS
plugin thread and leaves the ThreadGDBRemote without a StopInfo.

This is problematic for a few reasons:

1. No other process plugins do this, as they shouldn't. They should set
the stop info for their own process threads, and let the abstractions
built on top propagate StopInfos.

2. This conflicts with the expectations of ThreadMemory, which checks
for the backing threads's info, and then attempts to propagate it (in
the future, it should probably ask the plugin itself too...). We see
this happening in the code below. The `if` condition will not trigger,
because `backing_stop_info_sp` will be null (remember, ProcessGDB remote
is ignoring its own threads), and then this method returns false.

```
bool ThreadMemory::CalculateStopInfo() {
...
  lldb::StopInfoSP backing_stop_info_sp(
  m_backing_thread_sp->GetPrivateStopInfo());
  if (backing_stop_info_sp &&
  backing_stop_info_sp->IsValidForOperatingSystemThread(*this)) {
backing_stop_info_sp->SetThread(shared_from_this());
```

```
Thread::GetPrivateStopInfo
...
if (!CalculateStopInfo())
  SetStopInfo(StopInfoSP());
```

To solve this, we change ProcessGDB remote so that it does the
principled thing: it now only sets the stop info of its own threads.
This change by itself breaks the tests TestPythonOSPlugin.py and
TestOSPluginStepping.py and probably explains why ProcessGDB had
originally "violated" this isolation of layers.

To make this work, BreakpointSites must be aware of BackingThreads when
answering the question: "Is this breakpoint valid for this thread?".
Why? Breakpoints are created on top of the OS threads (that's what the
user sees), but breakpoints are hit by process threads. In the presence
of OS threads, a TID-specific breakpoint is valid for a process thread
if it is backing an OS thread with that TID.
---
 lldb/source/Breakpoint/BreakpointSite.cpp   | 3 +++
 lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | 4 
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Breakpoint/BreakpointSite.cpp 
b/lldb/source/Breakpoint/BreakpointSite.cpp
index 9700a57d3346e0..8b964c5711468c 100644
--- a/lldb/source/Breakpoint/BreakpointSite.cpp
+++ b/lldb/source/Breakpoint/BreakpointSite.cpp
@@ -12,6 +12,7 @@

[Lldb-commits] [lldb] [lldb] Improve isolation between Process plugins and OS plugins (PR #125302)

2025-01-31 Thread Felipe de Azevedo Piovezan via lldb-commits

felipepiovezan wrote:

Depends on https://github.com/llvm/llvm-project/pull/125300

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


[Lldb-commits] [lldb] LLDB: correct event when removing all watchpoints (PR #125312)

2025-01-31 Thread Jonas Devlieghere via lldb-commits

JDevlieghere wrote:

Can you write or modify a test? I glanced over `TestWatchpointEvents.py` and it 
looks like it's listening to the wrong bit (even thought the test is passing 
with your change). 

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


[Lldb-commits] [lldb] [lldb] Implement bidirectional access for backing<->backed thread relationship (PR #125300)

2025-01-31 Thread Felipe de Azevedo Piovezan via lldb-commits


@@ -72,12 +72,17 @@ class ThreadMemory : public lldb_private::Thread {
 
   void ClearStackFrames() override;
 
-  void ClearBackingThread() override { m_backing_thread_sp.reset(); }
+  void ClearBackingThread() override {
+if (m_backing_thread_sp)
+  m_backing_thread_sp->ClearBackedThread();
+m_backing_thread_sp.reset();
+  }

felipepiovezan wrote:

I really wanted to make `ClearBackedThread` a protected member of Thread, but 
then we can't access it here. Protected only works if you're going through 
`this`.

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


  1   2   >