[Lldb-commits] [lldb] c912c0e - [LLDB][docs] Remove outdated list of Buildbots

2023-04-13 Thread David Spickett via lldb-commits 

Author: David Spickett
Date: 2023-04-13T08:53:06Z
New Revision: c912c0eef349b0a541c933a1a6ba9dd09dab5ce8

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

LOG: [LLDB][docs] Remove outdated list of Buildbots

This was out of date and the link to the lldb tag will always be up to date.

Added: 


Modified: 
lldb/docs/resources/bots.rst

Removed: 




diff  --git a/lldb/docs/resources/bots.rst b/lldb/docs/resources/bots.rst
index f2deda52851ed..635b3982c3d03 100644
--- a/lldb/docs/resources/bots.rst
+++ b/lldb/docs/resources/bots.rst
@@ -7,11 +7,6 @@ Buildbot
 LLVM Buildbot is the place where volunteers provide build machines. Everyone 
can
 `add a buildbot for LLDB `_.
 
-* `lldb-x64-windows-ninja `_
-* `lldb-x86_64-debian `_
-* `lldb-aarch64-ubuntu `_
-* `lldb-arm-ubuntu `_
-
 An overview of all LLDB builders can be found here:
 
 `https://lab.llvm.org/buildbot/#/builders?tags=lldb 
`_



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

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

2023-04-13 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

Thanks for the test changes, looks good.

There is some documentation that lists what targets we support, MSP430 should 
be added there. `lldb/docs/index.rst` is one of those.

Otherwise, can anyone else think of major debug features that should be (at 
least) smoke tested? (of course you could do this in follow up patches if it 
needs more work)




Comment at: 
lldb/test/API/functionalities/gdb_remote_client/TestMSP430MSPDebug.py:103
+self.assertEqual(reg.GetValueAsUnsigned(),
+ reg_val_dict[reg.GetName()])
+

Write some registers too? Again, not likely to find anything, just ensures we 
have the right offsets and don't take the wrong 16 bits out of the value.


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

https://reviews.llvm.org/D146965

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

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

2023-04-13 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

Also please update the commit message to make it clear that this is for using 
lldb with mspdebug, not with lldb-server.

To help us and also to help anyone who might link to this commit to share the 
good news :)


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

https://reviews.llvm.org/D146965

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

[Lldb-commits] [lldb] 6ea45e3 - [lldb] Add RegisterFlags class

2023-04-13 Thread David Spickett via lldb-commits 

Author: David Spickett
Date: 2023-04-13T10:54:52Z
New Revision: 6ea45e3007b8a489afa56af13a2b8bfcec201a93

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

LOG: [lldb] Add RegisterFlags class

This models the "flags" node from GDB's target XML:
https://sourceware.org/gdb/onlinedocs/gdb/Target-Description-Format.html

This node is used to describe the fields of registers like cpsr on AArch64.

RegisterFlags is a class that contains a list of register fields.
These fields will be extracted from the XML sent by the remote.

We assume that there is at least one field, that the fields are
sorted in descending order and do not overlap. That will be
enforced by the XML processor (the GDB client code in our case).

The fields may not cover the whole register. To account for this
RegisterFields will add anonymous padding fields so that
sizeof(all fields) == sizeof(register). This will save a lot
of hasssle later.

Reviewed By: jasonmolenda, JDevlieghere

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

Added: 
lldb/include/lldb/Target/RegisterFlags.h
lldb/source/Target/RegisterFlags.cpp
lldb/unittests/Target/RegisterFlagsTest.cpp

Modified: 
lldb/source/Target/CMakeLists.txt
lldb/unittests/Target/CMakeLists.txt

Removed: 




diff  --git a/lldb/include/lldb/Target/RegisterFlags.h 
b/lldb/include/lldb/Target/RegisterFlags.h
new file mode 100644
index 0..89058d582e0fb
--- /dev/null
+++ b/lldb/include/lldb/Target/RegisterFlags.h
@@ -0,0 +1,88 @@
+//===-- RegisterFlags.h -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_TARGET_REGISTERFLAGS_H
+#define LLDB_TARGET_REGISTERFLAGS_H
+
+#include "lldb/Utility/Log.h"
+
+namespace lldb_private {
+
+class RegisterFlags {
+public:
+  class Field {
+  public:
+Field(std::string name, unsigned start, unsigned end)
+: m_name(std::move(name)), m_start(start), m_end(end) {
+  assert(m_start <= m_end && "Start bit must be <= end bit.");
+}
+
+/// Get size of the field in bits. Will always be at least 1.
+unsigned GetSizeInBits() const { return m_end - m_start + 1; }
+
+/// A mask that covers all bits of the field.
+uint64_t GetMask() const {
+  return (((uint64_t)1 << (GetSizeInBits())) - 1) << m_start;
+}
+
+/// Extract value of the field from a whole register value.
+uint64_t GetValue(uint64_t register_value) const {
+  return (register_value & GetMask()) >> m_start;
+}
+
+const std::string &GetName() const { return m_name; }
+unsigned GetStart() const { return m_start; }
+unsigned GetEnd() const { return m_end; }
+bool Overlaps(const Field &other) const;
+void log(Log *log) const;
+
+/// Return the number of bits between this field and the other, that are 
not
+/// covered by either field.
+unsigned PaddingDistance(const Field &other) const;
+
+bool operator<(const Field &rhs) const {
+  return GetStart() < rhs.GetStart();
+}
+
+bool operator==(const Field &rhs) const {
+  return (m_name == rhs.m_name) && (m_start == rhs.m_start) &&
+ (m_end == rhs.m_end);
+}
+
+  private:
+std::string m_name;
+/// Start/end bit positions. Where start N, end N means a single bit
+/// field at position N. We expect that start <= end. Bit positions begin
+/// at 0.
+/// Start is the LSB, end is the MSB.
+unsigned m_start;
+unsigned m_end;
+  };
+
+  /// This assumes that:
+  /// * There is at least one field.
+  /// * The fields are sorted in descending order.
+  /// Gaps are allowed, they will be filled with anonymous padding fields.
+  RegisterFlags(std::string id, unsigned size,
+const std::vector &fields);
+
+  const std::vector &GetFields() const { return m_fields; }
+  const std::string &GetID() const { return m_id; }
+  unsigned GetSize() const { return m_size; }
+  void log(Log *log) const;
+
+private:
+  const std::string m_id;
+  /// Size in bytes
+  const unsigned m_size;
+  std::vector m_fields;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_REGISTERFLAGS_H

diff  --git a/lldb/source/Target/CMakeLists.txt 
b/lldb/source/Target/CMakeLists.txt
index d3a922c0ffb5b..cf4818eae3eb8 100644
--- a/lldb/source/Target/CMakeLists.txt
+++ b/lldb/source/Target/CMakeLists.txt
@@ -33,6 +33,7 @@ add_lldb_library(lldbTarget
   QueueList.cpp
   RegisterContext.cpp
   RegisterContextUnwind.cpp
+  RegisterFlags.cpp
   RegisterNumber.cpp
   RemoteAwarePlat

[Lldb-commits] [PATCH] D145566: [lldb] Add RegisterFlags class

2023-04-13 Thread David Spickett via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6ea45e3007b8: [lldb] Add RegisterFlags class (authored by 
DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145566

Files:
  lldb/include/lldb/Target/RegisterFlags.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/RegisterFlags.cpp
  lldb/unittests/Target/CMakeLists.txt
  lldb/unittests/Target/RegisterFlagsTest.cpp

Index: lldb/unittests/Target/RegisterFlagsTest.cpp
===
--- /dev/null
+++ lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -0,0 +1,123 @@
+//===-- RegisterFlagsTest.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/Target/RegisterFlags.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb;
+
+TEST(RegisterFlagsTest, Field) {
+  // We assume that start <= end is always true, so that is not tested here.
+
+  RegisterFlags::Field f1("abc", 0, 0);
+  ASSERT_EQ(f1.GetName(), "abc");
+  // start == end means a 1 bit field.
+  ASSERT_EQ(f1.GetSizeInBits(), (unsigned)1);
+  ASSERT_EQ(f1.GetMask(), (uint64_t)1);
+  ASSERT_EQ(f1.GetValue(0), (uint64_t)0);
+  ASSERT_EQ(f1.GetValue(3), (uint64_t)1);
+
+  // End is inclusive meaning that start 0 to end 1 includes bit 1
+  // to make a 2 bit field.
+  RegisterFlags::Field f2("", 0, 1);
+  ASSERT_EQ(f2.GetSizeInBits(), (unsigned)2);
+  ASSERT_EQ(f2.GetMask(), (uint64_t)3);
+  ASSERT_EQ(f2.GetValue(UINT64_MAX), (uint64_t)3);
+  ASSERT_EQ(f2.GetValue(UINT64_MAX & ~(uint64_t)3), (uint64_t)0);
+
+  // If the field doesn't start at 0 we need to shift up/down
+  // to account for it.
+  RegisterFlags::Field f3("", 2, 5);
+  ASSERT_EQ(f3.GetSizeInBits(), (unsigned)4);
+  ASSERT_EQ(f3.GetMask(), (uint64_t)0x3c);
+  ASSERT_EQ(f3.GetValue(UINT64_MAX), (uint64_t)0xf);
+  ASSERT_EQ(f3.GetValue(UINT64_MAX & ~(uint64_t)0x3c), (uint64_t)0);
+
+  // Fields are sorted lowest starting bit first.
+  ASSERT_TRUE(f2 < f3);
+  ASSERT_FALSE(f3 < f1);
+  ASSERT_FALSE(f1 < f2);
+  ASSERT_FALSE(f1 < f1);
+}
+
+static RegisterFlags::Field make_field(unsigned start, unsigned end) {
+  return RegisterFlags::Field("", start, end);
+}
+
+TEST(RegisterFlagsTest, FieldOverlaps) {
+  // Single bit fields
+  ASSERT_FALSE(make_field(0, 0).Overlaps(make_field(1, 1)));
+  ASSERT_TRUE(make_field(1, 1).Overlaps(make_field(1, 1)));
+  ASSERT_FALSE(make_field(1, 1).Overlaps(make_field(3, 3)));
+
+  ASSERT_TRUE(make_field(0, 1).Overlaps(make_field(1, 2)));
+  ASSERT_TRUE(make_field(1, 2).Overlaps(make_field(0, 1)));
+  ASSERT_FALSE(make_field(0, 1).Overlaps(make_field(2, 3)));
+  ASSERT_FALSE(make_field(2, 3).Overlaps(make_field(0, 1)));
+
+  ASSERT_FALSE(make_field(1, 5).Overlaps(make_field(10, 20)));
+  ASSERT_FALSE(make_field(15, 30).Overlaps(make_field(7, 12)));
+}
+
+TEST(RegisterFlagsTest, PaddingDistance) {
+  // We assume that this method is always called with a more significant
+  // (start bit is higher) field first and that they do not overlap.
+
+  // [field 1][field 2]
+  ASSERT_EQ(make_field(1, 1).PaddingDistance(make_field(0, 0)), 0ULL);
+  // [field 1][..][field 2]
+  ASSERT_EQ(make_field(2, 2).PaddingDistance(make_field(0, 0)), 1ULL);
+  // [field 1][field 1][field 2]
+  ASSERT_EQ(make_field(1, 2).PaddingDistance(make_field(0, 0)), 0ULL);
+  // [field 1][30 bits free][field 2]
+  ASSERT_EQ(make_field(31, 31).PaddingDistance(make_field(0, 0)), 30ULL);
+}
+
+static void test_padding(const std::vector &fields,
+ const std::vector &expected) {
+  RegisterFlags rf("", 4, fields);
+  EXPECT_THAT(expected, ::testing::ContainerEq(rf.GetFields()));
+}
+
+TEST(RegisterFlagsTest, RegisterFlagsPadding) {
+  // When creating a set of flags we assume that:
+  // * There are >= 1 fields.
+  // * They are sorted in descending order.
+  // * There may be gaps between each field.
+
+  // Needs no padding
+  auto fields =
+  std::vector{make_field(16, 31), make_field(0, 15)};
+  test_padding(fields, fields);
+
+  // Needs padding in between the fields, single bit.
+  test_padding({make_field(17, 31), make_field(0, 15)},
+   {make_field(17, 31), make_field(16, 16), make_field(0, 15)});
+  // Multiple bits of padding.
+  test_padding({make_field(17, 31), make_field(0, 14)},
+   {make_field(17, 31), make_field(15, 16), make_field(0, 14)});
+
+  // Padding before first field, single bit.
+  test_padding({make_field(0, 30)}, {make_field(31, 31), make_field(0, 30)});
+  // Multiple bits.
+  test_padding({make_field(0, 15)}, {make_fiel

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

2023-04-13 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

I just landed 
https://github.com/llvm/llvm-project/commit/57c8fee1b97eb7e70513b935b765f8381a941b18
 which will give you build errors. All you have to do is add an extra `nullptr` 
to your register infos.


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

https://reviews.llvm.org/D146965

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

[Lldb-commits] [PATCH] D147642: [lldb][ObjectFileELF] Support AArch32 in ApplyRelocations

2023-04-13 Thread Stefan Gränitz via Phabricator via lldb-commits 
sgraenitz updated this revision to Diff 513178.
sgraenitz marked an inline comment as done.
sgraenitz added a comment.

Rename new function to `ApplyELF32ABS32RelRelocation()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147642

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml

Index: lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml
@@ -0,0 +1,62 @@
+# RUN: yaml2obj %s -o %t
+# RUN: lldb-test object-file -contents %t | FileCheck %s
+
+## Test that R_ARM_ABS32 relocations are resolved in .debug_info sections on aarch32.
+## REL-type relocations store implicit addend as signed values inline.
+## We relocate the symbol foo with 4 different addends and bar once in the .debug_info section.
+## Results that exceed the 32-bit range or overflow are logged and ignored.
+
+# CHECK:  Name: .debug_info
+# CHECK:  Data:  (
+#
+#  Addends: Zero Positive Negative Overflow Out-of-range
+#    04030201 D6FF D5FF FF7F
+# CHECK-NEXT: : 2A00 2E030201  D5FF FF7F
+# CHECK-NEXT: )
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_ARM
+  Flags:   [ EF_ARM_EABI_VER5 ]
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+  - Name:.debug_info
+Type:SHT_PROGBITS
+Content: 04030201D6FFD57F
+  - Name:.rel.debug_info
+Type:SHT_REL
+Info:.debug_info
+Relocations:
+  - Offset:  0x0
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x4
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x8
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0xC
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x10
+Symbol:  bar
+Type:R_ARM_ABS32
+Symbols:
+  - Name:.debug_info
+Type:STT_SECTION
+Section: .debug_info
+  - Name:foo
+Type:STT_FUNC
+Section: .debug_info
+Value:   0x002A
+  - Name:bar
+Type:STT_FUNC
+Section: .debug_info
+Value:   0xFF00
+...
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2637,6 +2637,43 @@
   }
 }
 
+static void ApplyELF32ABS32RelRelocation(Symtab *symtab, ELFRelocation &rel,
+ DataExtractor &debug_data,
+ Section *rel_section) {
+  Log *log = GetLog(LLDBLog::Modules);
+  Symbol *symbol = symtab->FindSymbolByID(ELFRelocation::RelocSymbol32(rel));
+  if (symbol) {
+addr_t value = symbol->GetAddressRef().GetFileAddress();
+if (value == LLDB_INVALID_ADDRESS) {
+  const char *name = symbol->GetName().GetCString();
+  LLDB_LOGF(log, "Debug info symbol invalid: %s", name);
+  return;
+}
+assert(llvm::isUInt<32>(value) && "Valid addresses are 32-bit");
+DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+// ObjectFileELF creates a WritableDataBuffer in CreateInstance.
+WritableDataBuffer *data_buffer =
+llvm::cast(data_buffer_sp.get());
+uint8_t *dst = data_buffer->GetBytes() + rel_section->GetFileOffset() +
+   ELFRelocation::RelocOffset32(rel);
+// Implicit addend is stored inline as a signed value.
+int32_t addend = *reinterpret_cast(dst);
+// The sum must be positive. This extra check prevents UB from overflow in
+// the actual range check below.
+if (addend < 0 && static_cast(std::abs(addend)) > value) {
+  LLDB_LOGF(log, "Debug info relocation overflow: 0x%" PRIx64,
+static_cast(value) + addend);
+  return;
+}
+if (!llvm::isUInt<32>(value + addend)) {
+  LLDB_LOGF(log, "Debug info relocation out of range: 0x%" PRIx64, value);
+  return;
+}
+uint32_t addr = value + addend;
+memcpy(dst, &addr, sizeof(uint32_t));
+  }
+}
+
 unsigned ObjectFileELF::ApplyRelocations(
 Symtab *symtab, const ELFHeader *hdr, const ELFSectionHeader *rel_hdr,
 const ELFSectionHeader *symtab_hdr, const ELFSectionHeader *debug_hdr,
@@ -2667,6 +2704,15 @@
 
 if (hdr->Is32Bit()) {

[Lldb-commits] [PATCH] D147642: [lldb][ObjectFileELF] Support AArch32 in ApplyRelocations

2023-04-13 Thread Stefan Gränitz via Phabricator via lldb-commits 
sgraenitz updated this revision to Diff 513179.
sgraenitz added a comment.

Add error reporting for `R_ARM_REL32` relocations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147642

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml

Index: lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml
===
--- /dev/null
+++ lldb/test/Shell/ObjectFile/ELF/aarch32-relocations.yaml
@@ -0,0 +1,62 @@
+# RUN: yaml2obj %s -o %t
+# RUN: lldb-test object-file -contents %t | FileCheck %s
+
+## Test that R_ARM_ABS32 relocations are resolved in .debug_info sections on aarch32.
+## REL-type relocations store implicit addend as signed values inline.
+## We relocate the symbol foo with 4 different addends and bar once in the .debug_info section.
+## Results that exceed the 32-bit range or overflow are logged and ignored.
+
+# CHECK:  Name: .debug_info
+# CHECK:  Data:  (
+#
+#  Addends: Zero Positive Negative Overflow Out-of-range
+#    04030201 D6FF D5FF FF7F
+# CHECK-NEXT: : 2A00 2E030201  D5FF FF7F
+# CHECK-NEXT: )
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_ARM
+  Flags:   [ EF_ARM_EABI_VER5 ]
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+  - Name:.debug_info
+Type:SHT_PROGBITS
+Content: 04030201D6FFD57F
+  - Name:.rel.debug_info
+Type:SHT_REL
+Info:.debug_info
+Relocations:
+  - Offset:  0x0
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x4
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x8
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0xC
+Symbol:  foo
+Type:R_ARM_ABS32
+  - Offset:  0x10
+Symbol:  bar
+Type:R_ARM_ABS32
+Symbols:
+  - Name:.debug_info
+Type:STT_SECTION
+Section: .debug_info
+  - Name:foo
+Type:STT_FUNC
+Section: .debug_info
+Value:   0x002A
+  - Name:bar
+Type:STT_FUNC
+Section: .debug_info
+Value:   0xFF00
+...
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2637,6 +2637,43 @@
   }
 }
 
+static void ApplyELF32ABS32RelRelocation(Symtab *symtab, ELFRelocation &rel,
+ DataExtractor &debug_data,
+ Section *rel_section) {
+  Log *log = GetLog(LLDBLog::Modules);
+  Symbol *symbol = symtab->FindSymbolByID(ELFRelocation::RelocSymbol32(rel));
+  if (symbol) {
+addr_t value = symbol->GetAddressRef().GetFileAddress();
+if (value == LLDB_INVALID_ADDRESS) {
+  const char *name = symbol->GetName().GetCString();
+  LLDB_LOGF(log, "Debug info symbol invalid: %s", name);
+  return;
+}
+assert(llvm::isUInt<32>(value) && "Valid addresses are 32-bit");
+DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+// ObjectFileELF creates a WritableDataBuffer in CreateInstance.
+WritableDataBuffer *data_buffer =
+llvm::cast(data_buffer_sp.get());
+uint8_t *dst = data_buffer->GetBytes() + rel_section->GetFileOffset() +
+   ELFRelocation::RelocOffset32(rel);
+// Implicit addend is stored inline as a signed value.
+int32_t addend = *reinterpret_cast(dst);
+// The sum must be positive. This extra check prevents UB from overflow in
+// the actual range check below.
+if (addend < 0 && static_cast(std::abs(addend)) > value) {
+  LLDB_LOGF(log, "Debug info relocation overflow: 0x%" PRIx64,
+static_cast(value) + addend);
+  return;
+}
+if (!llvm::isUInt<32>(value + addend)) {
+  LLDB_LOGF(log, "Debug info relocation out of range: 0x%" PRIx64, value);
+  return;
+}
+uint32_t addr = value + addend;
+memcpy(dst, &addr, sizeof(uint32_t));
+  }
+}
+
 unsigned ObjectFileELF::ApplyRelocations(
 Symtab *symtab, const ELFHeader *hdr, const ELFSectionHeader *rel_hdr,
 const ELFSectionHeader *symtab_hdr, const ELFSectionHeader *debug_hdr,
@@ -2667,6 +2704,21 @@
 
 if (hdr->Is32Bit()) {
   switch (hdr->e_machine) {
+  case llvm::ELF::

[Lldb-commits] [PATCH] D147642: [lldb][ObjectFileELF] Support AArch32 in ApplyRelocations

2023-04-13 Thread Stefan Gränitz via Phabricator via lldb-commits 
sgraenitz added a comment.

Addressed feedback and added handling for `R_ARM_REL32`. I didn't see a 
real-world case for it yet. Hope it's ok to have the error reported for the 
time being.




Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2640
 
+static void ApplyELF32ABS32Relocation(Symtab *symtab, ELFRelocation &rel,
+  DataExtractor &debug_data,

peter.smith wrote:
> IIUC the  largest difference between this and ApplyELF64ABS32Relocation is 
> the use of REL rather than RELA relocations. Perhaps worth naming it as 
> `ApplyELF32ABS32RelRelocation` 
Thanks, very good point. Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147642

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

[Lldb-commits] [PATCH] D147627: [lldb][ObjectFileELF] Improve error output for unsupported arch/relocations

2023-04-13 Thread Stefan Gränitz via Phabricator via lldb-commits 
sgraenitz updated this revision to Diff 513187.
sgraenitz added a comment.

Include `R_386_NONE` for error reporting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147627

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp


Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2666,38 +2666,49 @@
 Symbol *symbol = nullptr;
 
 if (hdr->Is32Bit()) {
-  switch (reloc_type(rel)) {
-  case R_386_32:
-symbol = symtab->FindSymbolByID(reloc_symbol(rel));
-if (symbol) {
-  addr_t f_offset =
-  rel_section->GetFileOffset() + ELFRelocation::RelocOffset32(rel);
-  DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
-  // ObjectFileELF creates a WritableDataBuffer in CreateInstance.
-  WritableDataBuffer *data_buffer =
-  llvm::cast(data_buffer_sp.get());
-  uint32_t *dst = reinterpret_cast(
-  data_buffer->GetBytes() + f_offset);
-
-  addr_t value = symbol->GetAddressRef().GetFileAddress();
-  if (rel.IsRela()) {
-value += ELFRelocation::RelocAddend32(rel);
+  switch (hdr->e_machine) {
+  case llvm::ELF::EM_386:
+switch (reloc_type(rel)) {
+case R_386_32:
+  symbol = symtab->FindSymbolByID(reloc_symbol(rel));
+  if (symbol) {
+addr_t f_offset =
+rel_section->GetFileOffset() + 
ELFRelocation::RelocOffset32(rel);
+DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+// ObjectFileELF creates a WritableDataBuffer in CreateInstance.
+WritableDataBuffer *data_buffer =
+llvm::cast(data_buffer_sp.get());
+uint32_t *dst = reinterpret_cast(
+data_buffer->GetBytes() + f_offset);
+
+addr_t value = symbol->GetAddressRef().GetFileAddress();
+if (rel.IsRela()) {
+  value += ELFRelocation::RelocAddend32(rel);
+} else {
+  value += *dst;
+}
+*dst = value;
   } else {
-value += *dst;
+GetModule()->ReportError(".rel{0}[{1}] unknown symbol id: {2:d}",
+rel_section->GetName().AsCString(), i,
+reloc_symbol(rel));
   }
-  *dst = value;
-} else {
-  GetModule()->ReportError(".rel{0}[{1}] unknown symbol id: {2:d}",
+  break;
+case R_386_NONE:
+case R_386_PC32:
+  GetModule()->ReportError("unsupported i386 relocation:"
+   " .rel{0}[{1}], type {2}",
rel_section->GetName().AsCString(), i,
-   reloc_symbol(rel));
+   reloc_type(rel));
+  break;
+default:
+  assert(false && "unexpected relocation type");
+  break;
 }
 break;
-  case R_386_PC32:
   default:
-GetModule()->ReportError("unsupported 32-bit relocation:"
- " .rel{0}[{1}], type {2}",
- rel_section->GetName().AsCString(), i,
- reloc_type(rel));
+GetModule()->ReportError("unsupported 32-bit ELF machine arch: {0}", 
hdr->e_machine);
+break;
   }
 } else {
   switch (hdr->e_machine) {
@@ -2743,7 +2754,8 @@
 }
 break;
   default:
-assert(false && "unsupported machine");
+GetModule()->ReportError("unsupported 64-bit ELF machine arch: {0}", 
hdr->e_machine);
+break;
   }
 }
   }


Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2666,38 +2666,49 @@
 Symbol *symbol = nullptr;
 
 if (hdr->Is32Bit()) {
-  switch (reloc_type(rel)) {
-  case R_386_32:
-symbol = symtab->FindSymbolByID(reloc_symbol(rel));
-if (symbol) {
-  addr_t f_offset =
-  rel_section->GetFileOffset() + ELFRelocation::RelocOffset32(rel);
-  DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
-  // ObjectFileELF creates a WritableDataBuffer in CreateInstance.
-  WritableDataBuffer *data_buffer =
-  llvm::cast(data_buffer_sp.get());
-  uint32_t *dst = reinterpret_cast(
-  data_buffer->GetBytes() + f_offset);
-
-  addr_t value = symbol->GetAddressRef().GetFileAddress();
-  if (rel.IsRela()) {
-

[Lldb-commits] [PATCH] D147627: [lldb][ObjectFileELF] Improve error output for unsupported arch/relocations

2023-04-13 Thread Stefan Gränitz via Phabricator via lldb-commits 
sgraenitz marked an inline comment as done.
sgraenitz added a comment.

Thanks for your feedback




Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2704
+default:
+  assert(false && "unexpected relocation type");
+  break;

DavidSpickett wrote:
> Should this report error also? Given that you want it to not crash.
> 
> Though it did that already, you must have had reason to change it.
I think the relocation types that can occur in debug sections are considered an 
invariant. If we see other relocation types, we got on a wrong track long 
before. The relocation resolver that exists in LLVM nowadays follows a 
support/resolve approach, which gives it a cleaner structure, but it comes down 
to the same behavior eventually: 
https://github.com/llvm/llvm-project/blob/release/16.x/llvm/lib/Object/RelocationResolver.cpp#L287-L299

I think it's best to keep this as is: Bail out with the assertion in 
development builds and ignore it silently in release builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147627

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

[Lldb-commits] [lldb] cc6ab26 - [lldb][ObjectFileELF] Improve error output for unsupported arch/relocations

2023-04-13 Thread Stefan Gränitz via lldb-commits 

Author: Stefan Gränitz
Date: 2023-04-13T14:32:15+02:00
New Revision: cc6ab268d89b27723f62517a07e86df85e999c89

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

LOG: [lldb][ObjectFileELF] Improve error output for unsupported arch/relocations

ObjectFileELF::ApplyRelocations() considered all 32-bit input objects to be 
i386 and didn't provide good error messages for AArch32 objects. Please find an 
example in https://github.com/llvm/llvm-project/issues/61948
While we are here, let' improve the situation for unsupported architectures as 
well. I think we should report the error here too and not silently fail (or 
crash with assertions enabled).

Reviewed By: SixWeining

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

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 5b75738e070c3..0c39a4f68e878 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2666,38 +2666,49 @@ unsigned ObjectFileELF::ApplyRelocations(
 Symbol *symbol = nullptr;
 
 if (hdr->Is32Bit()) {
-  switch (reloc_type(rel)) {
-  case R_386_32:
-symbol = symtab->FindSymbolByID(reloc_symbol(rel));
-if (symbol) {
-  addr_t f_offset =
-  rel_section->GetFileOffset() + ELFRelocation::RelocOffset32(rel);
-  DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
-  // ObjectFileELF creates a WritableDataBuffer in CreateInstance.
-  WritableDataBuffer *data_buffer =
-  llvm::cast(data_buffer_sp.get());
-  uint32_t *dst = reinterpret_cast(
-  data_buffer->GetBytes() + f_offset);
-
-  addr_t value = symbol->GetAddressRef().GetFileAddress();
-  if (rel.IsRela()) {
-value += ELFRelocation::RelocAddend32(rel);
+  switch (hdr->e_machine) {
+  case llvm::ELF::EM_386:
+switch (reloc_type(rel)) {
+case R_386_32:
+  symbol = symtab->FindSymbolByID(reloc_symbol(rel));
+  if (symbol) {
+addr_t f_offset =
+rel_section->GetFileOffset() + 
ELFRelocation::RelocOffset32(rel);
+DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+// ObjectFileELF creates a WritableDataBuffer in CreateInstance.
+WritableDataBuffer *data_buffer =
+llvm::cast(data_buffer_sp.get());
+uint32_t *dst = reinterpret_cast(
+data_buffer->GetBytes() + f_offset);
+
+addr_t value = symbol->GetAddressRef().GetFileAddress();
+if (rel.IsRela()) {
+  value += ELFRelocation::RelocAddend32(rel);
+} else {
+  value += *dst;
+}
+*dst = value;
   } else {
-value += *dst;
+GetModule()->ReportError(".rel{0}[{1}] unknown symbol id: {2:d}",
+rel_section->GetName().AsCString(), i,
+reloc_symbol(rel));
   }
-  *dst = value;
-} else {
-  GetModule()->ReportError(".rel{0}[{1}] unknown symbol id: {2:d}",
+  break;
+case R_386_NONE:
+case R_386_PC32:
+  GetModule()->ReportError("unsupported i386 relocation:"
+   " .rel{0}[{1}], type {2}",
rel_section->GetName().AsCString(), i,
-   reloc_symbol(rel));
+   reloc_type(rel));
+  break;
+default:
+  assert(false && "unexpected relocation type");
+  break;
 }
 break;
-  case R_386_PC32:
   default:
-GetModule()->ReportError("unsupported 32-bit relocation:"
- " .rel{0}[{1}], type {2}",
- rel_section->GetName().AsCString(), i,
- reloc_type(rel));
+GetModule()->ReportError("unsupported 32-bit ELF machine arch: {0}", 
hdr->e_machine);
+break;
   }
 } else {
   switch (hdr->e_machine) {
@@ -2743,7 +2754,8 @@ unsigned ObjectFileELF::ApplyRelocations(
 }
 break;
   default:
-assert(false && "unsupported machine");
+GetModule()->ReportError("unsupported 64-bit ELF machine arch: {0}", 
hdr->e_machine);
+break;
   }
 }
   }



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

[Lldb-commits] [PATCH] D147627: [lldb][ObjectFileELF] Improve error output for unsupported arch/relocations

2023-04-13 Thread Stefan Gränitz via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
sgraenitz marked an inline comment as done.
Closed by commit rGcc6ab268d89b: [lldb][ObjectFileELF] Improve error output for 
unsupported arch/relocations (authored by sgraenitz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147627

Files:
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp


Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2666,38 +2666,49 @@
 Symbol *symbol = nullptr;
 
 if (hdr->Is32Bit()) {
-  switch (reloc_type(rel)) {
-  case R_386_32:
-symbol = symtab->FindSymbolByID(reloc_symbol(rel));
-if (symbol) {
-  addr_t f_offset =
-  rel_section->GetFileOffset() + ELFRelocation::RelocOffset32(rel);
-  DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
-  // ObjectFileELF creates a WritableDataBuffer in CreateInstance.
-  WritableDataBuffer *data_buffer =
-  llvm::cast(data_buffer_sp.get());
-  uint32_t *dst = reinterpret_cast(
-  data_buffer->GetBytes() + f_offset);
-
-  addr_t value = symbol->GetAddressRef().GetFileAddress();
-  if (rel.IsRela()) {
-value += ELFRelocation::RelocAddend32(rel);
+  switch (hdr->e_machine) {
+  case llvm::ELF::EM_386:
+switch (reloc_type(rel)) {
+case R_386_32:
+  symbol = symtab->FindSymbolByID(reloc_symbol(rel));
+  if (symbol) {
+addr_t f_offset =
+rel_section->GetFileOffset() + 
ELFRelocation::RelocOffset32(rel);
+DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
+// ObjectFileELF creates a WritableDataBuffer in CreateInstance.
+WritableDataBuffer *data_buffer =
+llvm::cast(data_buffer_sp.get());
+uint32_t *dst = reinterpret_cast(
+data_buffer->GetBytes() + f_offset);
+
+addr_t value = symbol->GetAddressRef().GetFileAddress();
+if (rel.IsRela()) {
+  value += ELFRelocation::RelocAddend32(rel);
+} else {
+  value += *dst;
+}
+*dst = value;
   } else {
-value += *dst;
+GetModule()->ReportError(".rel{0}[{1}] unknown symbol id: {2:d}",
+rel_section->GetName().AsCString(), i,
+reloc_symbol(rel));
   }
-  *dst = value;
-} else {
-  GetModule()->ReportError(".rel{0}[{1}] unknown symbol id: {2:d}",
+  break;
+case R_386_NONE:
+case R_386_PC32:
+  GetModule()->ReportError("unsupported i386 relocation:"
+   " .rel{0}[{1}], type {2}",
rel_section->GetName().AsCString(), i,
-   reloc_symbol(rel));
+   reloc_type(rel));
+  break;
+default:
+  assert(false && "unexpected relocation type");
+  break;
 }
 break;
-  case R_386_PC32:
   default:
-GetModule()->ReportError("unsupported 32-bit relocation:"
- " .rel{0}[{1}], type {2}",
- rel_section->GetName().AsCString(), i,
- reloc_type(rel));
+GetModule()->ReportError("unsupported 32-bit ELF machine arch: {0}", 
hdr->e_machine);
+break;
   }
 } else {
   switch (hdr->e_machine) {
@@ -2743,7 +2754,8 @@
 }
 break;
   default:
-assert(false && "unsupported machine");
+GetModule()->ReportError("unsupported 64-bit ELF machine arch: {0}", 
hdr->e_machine);
+break;
   }
 }
   }


Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2666,38 +2666,49 @@
 Symbol *symbol = nullptr;
 
 if (hdr->Is32Bit()) {
-  switch (reloc_type(rel)) {
-  case R_386_32:
-symbol = symtab->FindSymbolByID(reloc_symbol(rel));
-if (symbol) {
-  addr_t f_offset =
-  rel_section->GetFileOffset() + ELFRelocation::RelocOffset32(rel);
-  DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
-  // ObjectFileELF creates a WritableDataBuffer in CreateInstance.
-  WritableDataBuffer *data_buffer =
-  llvm::cast(data_buffer_sp.get());
-  uint32_t *dst = reinterpret_cast(
-  data_buffer->Ge

[Lldb-commits] [lldb] 54981bb - [lldb] Read register fields from target XML

2023-04-13 Thread David Spickett via lldb-commits 

Author: David Spickett
Date: 2023-04-13T12:34:14Z
New Revision: 54981bb75d374863c6a15530018c6d7ac5be6478

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

LOG: [lldb] Read register fields from target XML

This teaches ProcessGDBRemote to look for "flags" nodes
in the target XML that tell you what fields a register has.

https://sourceware.org/gdb/onlinedocs/gdb/Target-Description-Format.html

It will check for various invalid inputs like:
* Flags nodes with 0 fields in them.
* Start or end being > the register size.
* Fields that overlap.
* Required properties not being present (e.g. no name).
* Flag sets being redefined.

If anything untoward is found, we'll just drop the field or the
flag set altogether. Register fields are a "nice to have" so LLDB
shouldn't be crashing because of them, instead just log anything
we throw away. So the user can fix their XML/file a bug with their
vendor.

Once that is done it will sort the fields and pass them to
the RegisterFields class I added previously.

There is no way to see these fields yet, so tests for this code
will come later when the formatting code is added.

The fields are stored in a map of unique pointers on the
ProcessGDBRemote class. It will give out raw pointers on the
assumption that the GDB process lives longer than the users
of those pointers do. Which means RegisterInfo is still a trivial struct
but we are properly destroying the fields when the GDB process ends.

We can't store the fields directly in the map because adding new
items may cause its storage to be reallocated, which would invalidate
pointers we've already given out.

Reviewed By: jasonmolenda, JDevlieghere

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

Added: 


Modified: 
lldb/include/lldb/Target/DynamicRegisterInfo.h
lldb/include/lldb/lldb-private-types.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Target/DynamicRegisterInfo.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/DynamicRegisterInfo.h 
b/lldb/include/lldb/Target/DynamicRegisterInfo.h
index 20f442529da8e..1b1df848315ce 100644
--- a/lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ b/lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 
+#include "lldb/Target/RegisterFlags.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/StructuredData.h"
 #include "lldb/lldb-private.h"
@@ -39,6 +40,8 @@ class DynamicRegisterInfo {
 std::vector value_regs;
 std::vector invalidate_regs;
 uint32_t value_reg_offset = 0;
+// Non-null if there is an XML provided type.
+const RegisterFlags *flags_type = nullptr;
   };
 
   DynamicRegisterInfo() = default;

diff  --git a/lldb/include/lldb/lldb-private-types.h 
b/lldb/include/lldb/lldb-private-types.h
index 94f08496426f4..b89b8eae0d124 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/lldb/include/lldb/lldb-private-types.h
@@ -11,6 +11,7 @@
 
 #if defined(__cplusplus)
 
+#include "lldb/Target/RegisterFlags.h"
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/ArrayRef.h"
@@ -62,8 +63,8 @@ struct RegisterInfo {
   /// this register changes. For example, the invalidate list for eax would be
   /// rax ax, ah, and al.
   uint32_t *invalidate_regs;
-  // Will be replaced with register flags info in the next patch.
-  void *unused;
+  /// If not nullptr, a type defined by XML descriptions.
+  const RegisterFlags *flags_type;
 
   llvm::ArrayRef data(const uint8_t *context_base) const {
 return llvm::ArrayRef(context_base + byte_offset, byte_size);

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 41d453a4acb3a..95bc79ebdfc9d 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -53,6 +53,7 @@
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Target/RegisterFlags.h"
 #include "lldb/Target/SystemRuntime.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/TargetList.h"
@@ -84,6 +85,7 @@
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/Threading.h"
@@ -4059,15 +4061,213 @@ struct GdbServerTargetInfo {
   RegisterSetMap reg_set_map;
 };
 
-bool ParseRegisters(XMLNode feature_node, GdbServerTargetInfo &target_info,
-std::vector ®isters) {
+static std::vector ParseFlagsFields(XMLNode flags_node,
+

[Lldb-commits] [PATCH] D145574: [lldb] Read register fields from target XML

2023-04-13 Thread David Spickett via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG54981bb75d37: [lldb] Read register fields from target XML 
(authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145574

Files:
  lldb/include/lldb/Target/DynamicRegisterInfo.h
  lldb/include/lldb/lldb-private-types.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/DynamicRegisterInfo.cpp

Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -407,7 +407,7 @@
   {reg.regnum_ehframe, reg.regnum_dwarf, reg.regnum_generic,
reg.regnum_remote, local_regnum},
   // value_regs and invalidate_regs are filled by Finalize()
-  nullptr, nullptr, nullptr
+  nullptr, nullptr, reg.flags_type
 };
 
 m_regs.push_back(reg_info);
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -38,6 +38,7 @@
 #include "GDBRemoteRegisterContext.h"
 
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringMap.h"
 
 namespace lldb_private {
 namespace repro {
@@ -466,6 +467,15 @@
   // fork helpers
   void DidForkSwitchSoftwareBreakpoints(bool enable);
   void DidForkSwitchHardwareTraps(bool enable);
+
+  // Lists of register fields generated from the remote's target XML.
+  // Pointers to these RegisterFlags will be set in the register info passed
+  // back to the upper levels of lldb. Doing so is safe because this class will
+  // live at least as long as the debug session. We therefore do not store the
+  // data directly in the map because the map may reallocate it's storage as new
+  // entries are added. Which would invalidate any pointers set in the register
+  // info up to that point.
+  llvm::StringMap> m_registers_flags_types;
 };
 
 } // namespace process_gdb_remote
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -53,6 +53,7 @@
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Target/RegisterFlags.h"
 #include "lldb/Target/SystemRuntime.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/TargetList.h"
@@ -84,6 +85,7 @@
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/Threading.h"
@@ -4059,15 +4061,213 @@
   RegisterSetMap reg_set_map;
 };
 
-bool ParseRegisters(XMLNode feature_node, GdbServerTargetInfo &target_info,
-std::vector ®isters) {
+static std::vector ParseFlagsFields(XMLNode flags_node,
+  unsigned size) {
+  Log *log(GetLog(GDBRLog::Process));
+  const unsigned max_start_bit = size * 8 - 1;
+
+  // Process the fields of this set of flags.
+  std::vector fields;
+  flags_node.ForEachChildElementWithName("field", [&fields, max_start_bit,
+   &log](const XMLNode
+ &field_node) {
+std::optional name;
+std::optional start;
+std::optional end;
+
+field_node.ForEachAttribute([&name, &start, &end, max_start_bit,
+ &log](const llvm::StringRef &attr_name,
+   const llvm::StringRef &attr_value) {
+  // Note that XML in general requires that each of these attributes only
+  // appears once, so we don't have to handle that here.
+  if (attr_name == "name") {
+LLDB_LOG(log,
+ "ProcessGDBRemote::ParseFlags Found field node name \"{0}\"",
+ attr_value.data());
+name = attr_value;
+  } else if (attr_name == "start") {
+unsigned parsed_start = 0;
+if (llvm::to_integer(attr_value, parsed_start)) {
+  if (parsed_start > max_start_bit) {
+LLDB_LOG(
+log,
+"ProcessGDBRemote::ParseFlags Invalid start {0} in field node, "
+"cannot be > {1}",
+parsed_start, max_start_bit);
+  } else
+start = parsed_start;
+} else {
+  LLDB_LOG(log,
+   "ProcessGDBRemote::ParseFlags Invalid start \"{0}\" in "
+

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

2023-04-13 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2212
 m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
-if (record_decl)
+if (record_decl) {
+  bool is_empty = true;

Generally I'm not sure if attaching a `clang::NoUniqueAddressAttr` to every 
empty field is the right approach. That goes slightly against our attempts to 
construct an AST that's faithful to the source to avoid unpredictable behaviour 
(which isn't always possible but for the most part we try). This approach was 
considered in https://reviews.llvm.org/D101237 but concern was raised about it 
affecting ABI, etc., leading to subtle issues down the line.

Based on the the discussion in https://reviews.llvm.org/D101237 it seemed to me 
like the only two viable solutions are:
1. Add a `DW_AT_byte_size` of `0` to the empty field
2. Add a `DW_AT_no_unique_address`

AFAICT Jan tried to implement (1) but never seemed to be able to fully add 
support for this in the ASTImporter/LLDB. Another issue I see with this is that 
sometimes the byte-size of said field is not `0`, depending on the context in 
which the structure is used.

I'm still leaning towards proposing a `DW_AT_no_unique_address`. Which is 
pretty easy to implement and also reason about from LLDB's perspective. 
@dblaikie @aprantl does that sound reasonable to you?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2219
+if (is_empty_field)
+  field->addAttr(clang::NoUniqueAddressAttr::Create(
+  m_ast.getASTContext(), clang::SourceRange()));

Typically the call to `record_decl->fields()` below would worry me, because if 
the decl `!hasLoadedFieldsFromExternalStorage()` then we'd start another 
`ASTImport` process, which could lead to some unpredictable behaviour if we are 
already in the middle of an import. But since 
`CompleteTagDeclarationDefinition` sets 
`setHasLoadedFieldsFromExternalStorage(true)` I *think* we'd be ok. Might 
warrant a comment.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2231
+if (is_empty)
+  record_decl->markEmpty();
+  }

Why do we need to mark the parents empty here again? Wouldn't they have been 
marked in `ParseStructureLikeDIE`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D145580: [lldb] Show register fields using bitfield struct types

2023-04-13 Thread David Spickett via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe07a421dd587: [lldb] Show register fields using bitfield 
struct types (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145580

Files:
  lldb/include/lldb/Core/DumpRegisterValue.h
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
  lldb/include/lldb/Target/RegisterFlags.h
  lldb/include/lldb/Target/RegisterTypeBuilder.h
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Core/DumpRegisterValue.cpp
  lldb/source/Core/PluginManager.cpp
  lldb/source/DataFormatters/DumpValueObjectOptions.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/CMakeLists.txt
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
  lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
  lldb/unittests/Target/RegisterFlagsTest.cpp
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -251,6 +251,12 @@
 * LLDB is now able to show the subtype of signals found in a core file. For example
   memory tagging specific segfaults such as ``SIGSEGV: sync tag check fault``.
 
+* LLDB can now display register fields if they are described in target XML sent
+  by a debug server such as ``gdbserver`` (``lldb-server`` does not currently produce
+  this information). Fields are only printed when reading named registers, for
+  example ``register read cpsr``. They are not shown when reading a register set,
+  ``register read -s 0``.
+
 Changes to Sanitizers
 -
 * For Darwin users that override weak symbols, note that the dynamic linker will
Index: lldb/unittests/Target/RegisterFlagsTest.cpp
===
--- lldb/unittests/Target/RegisterFlagsTest.cpp
+++ lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -121,3 +121,19 @@
 make_field(20, 21), make_field(12, 19), make_field(8, 11),
 make_field(0, 7)});
 }
+
+TEST(RegisterFieldsTest, ReverseFieldOrder) {
+  // Unchanged
+  RegisterFlags rf("", 4, {make_field(0, 31)});
+  ASSERT_EQ(0x12345678ULL, rf.ReverseFieldOrder(0x12345678));
+
+  // Swap the two halves around.
+  RegisterFlags rf2("", 4, {make_field(16, 31), make_field(0, 15)});
+  ASSERT_EQ(0x56781234ULL, rf2.ReverseFieldOrder(0x12345678));
+
+  // Many small fields.
+  RegisterFlags rf3("", 4,
+{make_field(31, 31), make_field(30, 30), make_field(29, 29),
+ make_field(28, 28)});
+  ASSERT_EQ(0x0005ULL, rf3.ReverseFieldOrder(0xA000));
+}
Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -0,0 +1,497 @@
+""" Check that register fields found in target XML are properly processed.
+
+These tests make XML out of string substitution. This can lead to some strange
+failures. Check that the final XML is valid and each child is indented more than
+the parent tag.
+"""
+
+from textwrap import dedent
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+class MultiDocResponder(MockGDBServerResponder):
+# docs is a dictionary of filename -> file content.
+def __init__(self, docs):
+super().__init__()
+self.docs = docs
+
+def qXferRead(self, obj, annex, offset, length):
+try:
+return self.docs[annex], False
+except KeyError:
+return None,
+
+def readRegister(self, regnum):
+return "E01"
+
+def readRegisters(self):
+return ''.join([
+  # Data for all registers requested by the tests below.
+  # 0x7 and 0xE are used because their lsb and msb are opposites, which
+  # is needed for a byte order test.
+  '', # 64 bit x0/r0
+  '', # 32 bit cpsr/fpc
+  '', # 64 bit pc/pswa
+])
+
+class TestXMLRegisterFlags(GDBRemoteTestBase):
+def setup_multidoc_test(self, docs):
+self.server.responder = MultiDocResponder(docs)
+target = self.dbg.CreateTarget('')
+
+if self.TraceOn():
+self.runCmd("log enable gdb-remote packets process")
+

[Lldb-commits] [lldb] e07a421 - [lldb] Show register fields using bitfield struct types

2023-04-13 Thread David Spickett via lldb-commits 

Author: David Spickett
Date: 2023-04-13T13:04:34Z
New Revision: e07a421dd587f596b3b34ac2f79081402089f878

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

LOG: [lldb] Show register fields using bitfield struct types

This change uses the information from target.xml sent by
the GDB stub to produce C types that we can use to print
register fields.

lldb-server *does not* produce this information yet. This will
only work with GDB stubs that do. gdbserver or qemu
are 2 I know of. Testing is added that uses a mocked lldb-server.
```
(lldb) register read cpsr x0 fpcr fpsr x1
cpsr = 0x60001000
 = (N = 0, Z = 1, C = 1, V = 0, TCO = 0, DIT = 0, UAO = 0, PAN = 0, SS 
= 0, IL = 0, SSBS = 1, BTYPE = 0, D = 0, A = 0, I = 0, F = 0, nRW = 0, EL = 0, 
SP = 0)
```

Only "register read" will display fields, and only when
we are not printing a register block.

For example, cpsr is a 32 bit register. Using the target's scratch type
system we construct a type:
```
struct __attribute__((__packed__)) cpsr {
  uint32_t N : 1;
  uint32_t Z : 1;
  ...
  uint32_t EL : 2;
  uint32_t SP : 1;
};
```

If this register had unallocated bits in it, those would
have been filled in by RegisterFlags as anonymous fields.
A new option "SetChildPrintingDecider" is added so we
can disable printing those.

Important things about this type:
* It is packed so that sizeof(struct cpsr) == sizeof(the real register).
  (this will hold for all flags types we create)
* Each field has the same storage type, which is the same as the type
  of the raw register value. This prevents fields being spilt over
  into more storage units, as is allowed by most ABIs.
* Each bitfield size matches that of its register field.
* The most significant field is first.

The last point is required because the most significant bit (MSB)
being on the left/top of a print out matches what you'd expect to
see in an architecture manual. In addition, having lldb print a
different field order on big/little endian hosts is not acceptable.

As a consequence, if the target is little endian we have to
reverse the order of the fields in the value. The value of each field
remains the same. For example 0b01 doesn't become 0b10, it just shifts
up or down.

This is needed because clang's type system assumes that for a struct
like the one above, the least significant bit (LSB) will be first
for a little endian target. We need the MSB to be first.

Finally, if lldb's host is a different endian to the target we have
to byte swap the host endian value to match the endian of the target's
typesystem.

| Host Endian | Target Endian | Field Order Swap | Byte Order Swap |
|-|---|--|-|
| Little  | Little| Yes  | No  |
| Big | Little| Yes  | Yes |
| Little  | Big   | No   | Yes |
| Big | Big   | No   | No  |

Testing was done as follows:
* Little -> Little
  * LE AArch64 native debug.
* Big -> Little
  * s390x lldb running under QEMU, connected to LE AArch64 target.
* Little -> Big
  * LE AArch64 lldb connected to QEMU's GDB stub, which is running
an s390x program.
* Big -> Big
 * s390x lldb running under QEMU, connected to another QEMU's GDB
   stub, which is running an s390x program.

As we are not allowed to link core code to plugins directly,
I have added a new plugin RegisterTypeBuilder. There is one implementation
of this, RegisterTypeBuilderClang, which uses TypeSystemClang to build
the CompilerType from the register fields.

Reviewed By: jasonmolenda

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

Added: 
lldb/include/lldb/Target/RegisterTypeBuilder.h
lldb/source/Plugins/RegisterTypeBuilder/CMakeLists.txt
lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py

Modified: 
lldb/include/lldb/Core/DumpRegisterValue.h
lldb/include/lldb/Core/PluginManager.h
lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
lldb/include/lldb/Target/RegisterFlags.h
lldb/include/lldb/Target/Target.h
lldb/include/lldb/lldb-forward.h
lldb/include/lldb/lldb-private-interfaces.h
lldb/source/Commands/CommandObjectRegister.cpp
lldb/source/Core/DumpRegisterValue.cpp
lldb/source/Core/PluginManager.cpp
lldb/source/DataFormatters/DumpValueObjectOptions.cpp
lldb/source/DataFormatters/ValueObjectPrinter.cpp
lldb/source/Plugins/CMakeLists.txt
lldb/source/Target/Target.cpp
lldb/unittests/Target/RegisterFlagsTest.cpp
llvm/docs/ReleaseNotes.rst

Removed: 



###

[Lldb-commits] [PATCH] D148228: [lldb] Change some pointers to refs in register printing code

2023-04-13 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

No one was passing nullptr for these.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148228

Files:
  lldb/include/lldb/Core/DumpRegisterValue.h
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/source/Core/DumpRegisterValue.cpp
  lldb/source/Core/EmulateInstruction.cpp
  lldb/source/Core/FormatEntity.cpp
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Target/ThreadPlanCallFunction.cpp
  lldb/source/Target/ThreadPlanTracer.cpp

Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -223,7 +223,7 @@
   reg_value != m_register_values[reg_num]) {
 if (reg_value.GetType() != RegisterValue::eTypeInvalid) {
   stream->PutCString("\n\t");
-  DumpRegisterValue(reg_value, stream, reg_info, true, false,
+  DumpRegisterValue(reg_value, *stream, *reg_info, true, false,
 eFormatDefault);
 }
   }
Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -163,7 +163,7 @@
  reg_idx < num_registers; ++reg_idx) {
   const RegisterInfo *reg_info = reg_ctx->GetRegisterInfoAtIndex(reg_idx);
   if (reg_ctx->ReadRegister(reg_info, reg_value)) {
-DumpRegisterValue(reg_value, &strm, reg_info, true, false,
+DumpRegisterValue(reg_value, strm, *reg_info, true, false,
   eFormatDefault);
 strm.EOL();
   }
Index: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===
--- lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -500,7 +500,7 @@
 strm.Printf("UnwindAssemblyInstEmulation::ReadRegister  (name = \"%s\") => "
 "synthetic_value = %i, value = ",
 reg_info->name, synthetic);
-DumpRegisterValue(reg_value, &strm, reg_info, false, false, eFormatDefault);
+DumpRegisterValue(reg_value, strm, *reg_info, false, false, eFormatDefault);
 log->PutString(strm.GetString());
   }
   return true;
@@ -526,7 +526,7 @@
 strm.Printf(
 "UnwindAssemblyInstEmulation::WriteRegister (name = \"%s\", value = ",
 reg_info->name);
-DumpRegisterValue(reg_value, &strm, reg_info, false, false, eFormatDefault);
+DumpRegisterValue(reg_value, strm, *reg_info, false, false, eFormatDefault);
 strm.PutCString(", context = ");
 context.Dump(strm, instruction);
 log->PutString(strm.GetString());
Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -605,7 +605,7 @@
 if (reg_info) {
   RegisterValue reg_value;
   if (reg_ctx->ReadRegister(reg_info, reg_value)) {
-DumpRegisterValue(reg_value, &s, reg_info, false, false, format);
+DumpRegisterValue(reg_value, s, *reg_info, false, false, format);
 return true;
   }
 }
@@ -985,7 +985,7 @@
   if (reg_info) {
 RegisterValue reg_value;
 if (reg_ctx->ReadRegister(reg_info, reg_value)) {
-  DumpRegisterValue(reg_value, &s, reg_info, false, false, format);
+  DumpRegisterValue(reg_value, s, *reg_info, false, false, format);
   return true;
 }
   }
Index: lldb/source/Core/EmulateInstruction.cpp
===
--- lldb/source/Core/EmulateInstruction.cpp
+++ lldb/source/Core/EmulateInstruction.cpp
@@ -363,7 +363,7 @@
   const RegisterValue ®_value) {
   StreamFile strm(stdout, false);
   strm.Printf("Write to Register (name = %s, value = ", reg_info->name);
-  DumpRegisterValue(reg_value, &strm, reg_info, false, false, eFormatDefault);
+  DumpRegisterValue(reg_value, strm, *reg_info, false, false, eFormatDefault);
   strm.PutCString(", context = ");
   context.Dump(strm, instruction);
   strm.EOL();
Index: lldb/source/Core/DumpRegisterValue.cpp
===
--- lldb/source/Core/DumpRegisterValue.cpp
+++ lldb/source/Core/DumpRegisterValue.cpp
@@ -58,8 +58,8 @@
   vobj_sp->Dump(strm, dump_options);
 }
 
-void lldb_private::DumpRegisterValue(const RegisterValue ®_val, Stream *s,
-

[Lldb-commits] [lldb] be58b42 - [lldb] Add 'CHECK' to class-type-nullptr-deref.s test.

2023-04-13 Thread Caroline Tice via lldb-commits 

Author: Caroline Tice
Date: 2023-04-13T08:17:53-07:00
New Revision: be58b42a7510e3fe204e015bcf5faa9ed35d1766

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

LOG: [lldb] Add 'CHECK' to class-type-nullptr-deref.s test.

This test previously relied on just segfaulting or not. This commit adds
a CHECK statement to the test.

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

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s 
b/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
index 610b45823458..c7aea06bf909 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
@@ -2,7 +2,9 @@
 # null), LLDB does not try to dereference the null pointer.
 
 # RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj -o %t
-# RUN: %lldb %t -o "target variable x" -o exit 2>&1
+# RUN: %lldb %t -o "target variable x" -o exit 2>&1 | FileCheck %s
+
+# CHECK: 'Unable to determine byte size.'
 
 # This tests a fix for a crash. If things are working we don't get a segfault.
 



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

[Lldb-commits] [PATCH] D148151: [lldb] Add 'CHECK' to class-type-nullptr-deref.s test.

2023-04-13 Thread Caroline Tice via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbe58b42a7510: [lldb] Add 'CHECK' to 
class-type-nullptr-deref.s test. (authored by cmtice).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148151

Files:
  lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s


Index: lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
@@ -2,7 +2,9 @@
 # null), LLDB does not try to dereference the null pointer.
 
 # RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj -o %t
-# RUN: %lldb %t -o "target variable x" -o exit 2>&1
+# RUN: %lldb %t -o "target variable x" -o exit 2>&1 | FileCheck %s
+
+# CHECK: 'Unable to determine byte size.'
 
 # This tests a fix for a crash. If things are working we don't get a segfault.
 


Index: lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s
@@ -2,7 +2,9 @@
 # null), LLDB does not try to dereference the null pointer.
 
 # RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj -o %t
-# RUN: %lldb %t -o "target variable x" -o exit 2>&1
+# RUN: %lldb %t -o "target variable x" -o exit 2>&1 | FileCheck %s
+
+# CHECK: 'Unable to determine byte size.'
 
 # This tests a fix for a crash. If things are working we don't get a segfault.
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D146965: [lldb] Add support for MSP430 in LLDB.

2023-04-13 Thread Ilia Kuklin via Phabricator via lldb-commits 
kuilpd updated this revision to Diff 513245.
kuilpd added a comment.

Rebased and updated register info initialization.


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

https://reviews.llvm.org/D146965

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/include/lldb/Utility/DataExtractor.h
  lldb/source/Expression/IRMemoryMap.cpp
  lldb/source/Expression/LLVMUserExpression.cpp
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/ABI/CMakeLists.txt
  lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.cpp
  lldb/source/Plugins/ABI/MSP430/ABISysV_msp430.h
  lldb/source/Plugins/ABI/MSP430/CMakeLists.txt
  lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterFallback.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Utility/ArchSpec.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestMSP430MSPDebug.py
  lldb/test/API/functionalities/gdb_remote_client/msp430.yaml
  lldb/unittests/Utility/ArchSpecTest.cpp

Index: lldb/unittests/Utility/ArchSpecTest.cpp
===
--- lldb/unittests/Utility/ArchSpecTest.cpp
+++ lldb/unittests/Utility/ArchSpecTest.cpp
@@ -123,6 +123,12 @@
   EXPECT_STREQ("i686", AS.GetArchitectureName());
   EXPECT_EQ(ArchSpec::eCore_x86_32_i686, AS.GetCore());
 
+  AS = ArchSpec();
+  EXPECT_TRUE(AS.SetTriple("msp430---elf"));
+  EXPECT_EQ(llvm::Triple::msp430, AS.GetTriple().getArch());
+  EXPECT_STREQ("msp430", AS.GetArchitectureName());
+  EXPECT_EQ(ArchSpec::eCore_msp430, AS.GetCore());
+
   // Various flavors of invalid triples.
   AS = ArchSpec();
   EXPECT_FALSE(AS.SetTriple("unknown-unknown-unknown"));
Index: lldb/test/API/functionalities/gdb_remote_client/msp430.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/msp430.yaml
@@ -0,0 +1,426 @@
+# File test.c, compiled with flags "-O0 -g"
+# Source code:
+#
+# int foo = 0;
+#
+# int func() {
+# foo = 1234;
+# return foo;
+# }
+#
+# int main() {
+# return func();
+# }
+#
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  OSABI:   ELFOSABI_STANDALONE
+  Type:ET_EXEC
+  Machine: EM_MSP430
+  Flags:   [  ]
+  Entry:   0x500
+ProgramHeaders:
+  - Type:PT_LOAD
+Flags:   [ PF_X, PF_R ]
+FirstSec:.text
+LastSec: .bss
+VAddr:   0x46C
+Align:   0x4
+  - Type:PT_LOAD
+Flags:   [ PF_W, PF_R ]
+FirstSec:.data
+LastSec: .bss
+VAddr:   0x53C
+Align:   0x4
+  - Type:PT_LOAD
+Flags:   [ PF_R ]
+FirstSec:__interrupt_vector_31
+LastSec: __interrupt_vector_31
+VAddr:   0xFFFE
+Align:   0x4
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x500
+AddressAlign:0x4
+Content: 3140C0FF0C43B0121C05B0128101B240D2043C051C423C053041318002008143B01210053150020030411C4330413C402A0030410C433041
+  - Name:.data
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x53C
+AddressAlign:0x1
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x53C
+AddressAlign:0x2
+Size:0x2
+  - Name:__interrupt_vector_31
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC ]
+Address: 0xFFFE
+AddressAlign:0x1
+Offset:  0xD2
+Content: '0005'
+  - Name:.rodata
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x500
+AddressAlign:0x1
+  - Name:.rodata2
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE ]
+Address: 0x500
+AddressAlign:0x1
+  - Name:.noinit
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE ]
+Address: 0x53E
+AddressAlign:0x1
+  - Name:.persistent
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE ]
+Address: 0x53E
+AddressAlign:0x1
+  - Name:.MSP430.attributes
+Type:SHT_MSP430_ATTRIBUTES
+AddressAlign:0x1
+Content: 4116006D737061626900010B00040106010801
+  - Name:.comment
+Type:SHT_PROGBITS
+Flags:   [ SHF_MERGE, SHF_STRINGS ]
+AddressAlign:0x1
+EntSize: 0x1
+Content: 4743433A20284D6974746F2053797374656D73204C

[Lldb-commits] [PATCH] D145574: [lldb] Read register fields from target XML

2023-04-13 Thread Fangrui Song via Phabricator via lldb-commits 
MaskRay added inline comments.



Comment at: lldb/include/lldb/lldb-private-types.h:14
 
+#include "lldb/Target/RegisterFlags.h"
 #include "lldb/lldb-private.h"

Is there a library layering violation?

I assume that `lldb/Target/*.h` files depend on 
`lldb/include/lldb/lldb-private-types.h`, so 
`lldb/include/lldb/lldb-private-types.h` cannot include `lldb/Target/*.h` 
files...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145574

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

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-13 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib added inline comments.



Comment at: lldb/examples/python/crashlog.py:515
 def parse_images(self, json_images):
-idx = 0
-for json_image in json_images:
+for idx, json_image in enumerate(json_images):
 img_uuid = uuid.UUID(json_image['uuid'])

JDevlieghere wrote:
> mib wrote:
> > What do we use `idx` for ?
> You're right, this isn't necessary anymore.
I'm really not a big fan of having very similar image lists ... may be we could 
use the from the crashlog object and skip the first entry (since we know it's 
the main executable).
What do you think ?



Comment at: lldb/examples/python/crashlog.py:555
+"type": "code",
+"size": 0x0,
+"address": frame_offset - location

Does this needs to be initialized ?



Comment at: lldb/examples/python/symbolication.py:400-401
+})
+for symbol in self.symbols.values():
+data['symbols'].append(symbol)
+with open(tf.name, 'w') as f:

What about :


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

https://reviews.llvm.org/D148172

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

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-13 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib added inline comments.



Comment at: lldb/examples/python/crashlog.py:515
 def parse_images(self, json_images):
-idx = 0
-for json_image in json_images:
+for idx, json_image in enumerate(json_images):
 img_uuid = uuid.UUID(json_image['uuid'])

mib wrote:
> JDevlieghere wrote:
> > mib wrote:
> > > What do we use `idx` for ?
> > You're right, this isn't necessary anymore.
> I'm really not a big fan of having very similar image lists ... may be we 
> could use the from the crashlog object and skip the first entry (since we 
> know it's the main executable).
> What do you think ?
Otherwise, we could hoist the main executable image from the image list and 
handle it separately.


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

https://reviews.llvm.org/D148172

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

[Lldb-commits] [lldb] 4c8662e - [lldb] Fix library layering after D145574

2023-04-13 Thread Fangrui Song via lldb-commits 

Author: Fangrui Song
Date: 2023-04-13T10:55:15-07:00
New Revision: 4c8662e3fe304005d1a64c1c8bfb0c0d71e21324

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

LOG: [lldb] Fix library layering after D145574

Added: 


Modified: 
lldb/include/lldb/lldb-private-types.h
lldb/source/Core/DumpRegisterValue.cpp
lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp

Removed: 




diff  --git a/lldb/include/lldb/lldb-private-types.h 
b/lldb/include/lldb/lldb-private-types.h
index b89b8eae0d124..9ff85ab162ada 100644
--- a/lldb/include/lldb/lldb-private-types.h
+++ b/lldb/include/lldb/lldb-private-types.h
@@ -11,7 +11,6 @@
 
 #if defined(__cplusplus)
 
-#include "lldb/Target/RegisterFlags.h"
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/ArrayRef.h"
@@ -27,6 +26,7 @@ class DynamicLibrary;
 namespace lldb_private {
 class Platform;
 class ExecutionContext;
+class RegisterFlags;
 
 typedef llvm::sys::DynamicLibrary (*LoadPluginCallbackType)(
 const lldb::DebuggerSP &debugger_sp, const FileSpec &spec, Status &error);

diff  --git a/lldb/source/Core/DumpRegisterValue.cpp 
b/lldb/source/Core/DumpRegisterValue.cpp
index d1bde2c8f9156..deeddfc96f3d4 100644
--- a/lldb/source/Core/DumpRegisterValue.cpp
+++ b/lldb/source/Core/DumpRegisterValue.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/DumpValueObjectOptions.h"
+#include "lldb/Target/RegisterFlags.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/RegisterValue.h"

diff  --git 
a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp 
b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
index abd6afbd916ca..49348ed697014 100644
--- a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
+++ b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
@@ -11,6 +11,7 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "RegisterTypeBuilderClang.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/RegisterFlags.h"
 #include "lldb/lldb-enumerations.h"
 
 using namespace lldb_private;



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

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-13 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added inline comments.



Comment at: lldb/examples/python/crashlog.py:515
 def parse_images(self, json_images):
-idx = 0
-for json_image in json_images:
+for idx, json_image in enumerate(json_images):
 img_uuid = uuid.UUID(json_image['uuid'])

mib wrote:
> mib wrote:
> > JDevlieghere wrote:
> > > mib wrote:
> > > > What do we use `idx` for ?
> > > You're right, this isn't necessary anymore.
> > I'm really not a big fan of having very similar image lists ... may be we 
> > could use the from the crashlog object and skip the first entry (since we 
> > know it's the main executable).
> > What do you think ?
> Otherwise, we could hoist the main executable image from the image list and 
> handle it separately.
I understand the concern. To be fair, I didn't check whether the main 
executable coming first is something we rely on, but I'm pretty sure we are: 
we'll need it to create the target. I didn't want to mess with that and risk 
introducing a bug that way. It took me quite some time to figure out this was 
an issue when parsing the symbol data. If we don't want to break that 
assumption, there's nothing more efficient than keeping a second list of 
references. I also think it makes sense to keep that in the JSON parser, 
because the index of (parsed) image is only something that makes sense for that 
format because it cross references images based on their index. That's not the 
case in the textual or parser crashlogs.  

FWIW this is the code that moves the main image to the top, invalidating the 
image indexes of every image before it:

```
def set_main_image(self, identifier):
for i, image in enumerate(self.images):
if image.identifier == identifier:
self.images.insert(0, self.images.pop(i))
break
```



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

https://reviews.llvm.org/D148172

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

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-13 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 513327.
JDevlieghere marked 4 inline comments as done.

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

https://reviews.llvm.org/D148172

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/symbolication.py
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
@@ -41,3 +41,6 @@
 # CHECK-NEXT:frame #2: 0x000100ec5a87 multithread-test`compute_pow{{.*}} [artificial]
 # CHECK:frame #{{[0-9]+}}: 0x00019cc7e06b{{.*}} [artificial]
 # CHECK:frame #{{[0-9]+}}: 0x00019cc78e2b{{.*}} [artificial]
+
+image list
+# CHECK: ---- {{.*}}bogus.dylib
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
@@ -478,6 +478,15 @@
   "source" : "A",
   "base" : 0,
   "uuid" : "----"
+},
+{
+  "arch": "arm64",
+  "base": 12345,
+  "name": "bogus.dylib",
+  "path": "/usr/lib/system/bogus.dylib",
+  "size": 1000,
+  "source": "P",
+  "uuid": "----"
 }
   ],
   "userID": 501,
Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -35,6 +35,8 @@
 import sys
 import time
 import uuid
+import json
+import tempfile
 
 
 class Address:
@@ -230,6 +232,7 @@
 def __init__(self, path, uuid=None):
 self.path = path
 self.resolved_path = None
+self.resolve = False
 self.resolved = False
 self.unavailable = False
 self.uuid = uuid
@@ -240,6 +243,7 @@
 self.module = None
 self.symfile = None
 self.slide = None
+self.symbols = dict()
 
 @classmethod
 def InitWithSBTargetAndSBModule(cls, target, module):
@@ -372,14 +376,32 @@
 uuid_str = self.get_normalized_uuid_string()
 if uuid_str:
 self.module = target.AddModule(None, None, uuid_str)
-if not self.module:
+if not self.module and self.resolve:
 self.locate_module_and_debug_symbols()
-if self.unavailable:
-return None
-resolved_path = self.get_resolved_path()
-self.module = target.AddModule(
-resolved_path, None, uuid_str, self.symfile)
-if not self.module:
+if not self.unavailable:
+resolved_path = self.get_resolved_path()
+self.module = target.AddModule(
+resolved_path, None, uuid_str, self.symfile)
+if not self.module and self.section_infos:
+name = os.path.basename(self.path)
+with tempfile.NamedTemporaryFile(suffix='.' + name) as tf:
+data = {
+'triple': target.triple,
+'uuid': uuid_str,
+'type': 'sharedlibrary',
+'sections': list(),
+'symbols': list()
+}
+for section in self.section_infos:
+data['sections'].append({
+'name' : section.name,
+'size': section.end_addr - section.start_addr
+})
+data['symbols'] = list(self.symbols.values())
+with open(tf.name, 'w') as f:
+f.write(json.dumps(data, indent=4))
+self.module = target.AddModule(tf.name, None, uuid_str)
+if not self.module and not self.unavailable:
 return 'error: unable to get module for (%s) "%s"' % (
 self.arch, self.get_resolved_path())
 if self.has_section_load_info():
Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scr

[Lldb-commits] [PATCH] D147669: PECOFF: consume errors properly

2023-04-13 Thread Stefan Gränitz via Phabricator via lldb-commits 
sgraenitz added a comment.

I think it's a very good practice that objects in moved-from state will only 
ever get destroyed. While the move ctor itself has no preconditions and thus 
might be fine to call again, the implementation of `consumeError()` does 
inspect the object. This adds an unnecessary risk for UB. Please simply check 
the log state and call either `fmt_consume()` or `consumeError()`. Thanks.


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

https://reviews.llvm.org/D147669

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

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-13 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: lldb/examples/python/crashlog.py:515
 def parse_images(self, json_images):
-idx = 0
-for json_image in json_images:
+for idx, json_image in enumerate(json_images):
 img_uuid = uuid.UUID(json_image['uuid'])

JDevlieghere wrote:
> mib wrote:
> > mib wrote:
> > > JDevlieghere wrote:
> > > > mib wrote:
> > > > > What do we use `idx` for ?
> > > > You're right, this isn't necessary anymore.
> > > I'm really not a big fan of having very similar image lists ... may be we 
> > > could use the from the crashlog object and skip the first entry (since we 
> > > know it's the main executable).
> > > What do you think ?
> > Otherwise, we could hoist the main executable image from the image list and 
> > handle it separately.
> I understand the concern. To be fair, I didn't check whether the main 
> executable coming first is something we rely on, but I'm pretty sure we are: 
> we'll need it to create the target. I didn't want to mess with that and risk 
> introducing a bug that way. It took me quite some time to figure out this was 
> an issue when parsing the symbol data. If we don't want to break that 
> assumption, there's nothing more efficient than keeping a second list of 
> references. I also think it makes sense to keep that in the JSON parser, 
> because the index of (parsed) image is only something that makes sense for 
> that format because it cross references images based on their index. That's 
> not the case in the textual or parser crashlogs.  
> 
> FWIW this is the code that moves the main image to the top, invalidating the 
> image indexes of every image before it:
> 
> ```
> def set_main_image(self, identifier):
> for i, image in enumerate(self.images):
> if image.identifier == identifier:
> self.images.insert(0, self.images.pop(i))
> break
> ```
> 
Fair


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

https://reviews.llvm.org/D148172

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

[Lldb-commits] [lldb] 0e5cdbf - [lldb] Make ObjectFileJSON loadable as a module

2023-04-13 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2023-04-13T14:08:19-07:00
New Revision: 0e5cdbf07e6d45ca168a76b2bc19b6e385cfae17

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

LOG: [lldb] Make ObjectFileJSON loadable as a module

This patch adds support for creating modules from JSON object files.
This is necessary for the crashlog use case where we don't have either a
module or a symbol file. In that case the ObjectFileJSON serves as both.

The patch adds support for an object file type (i.e. executable, shared
library, etc). It also adds the ability to specify sections, which is
necessary in order specify symbols by address. Finally, this patch
improves error handling and fixes a bug where we wouldn't read more than
the initial 512 bytes in GetModuleSpecifications.

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

Added: 
lldb/test/API/macosx/symbols/TestObjectFileJSON.py

Modified: 
lldb/include/lldb/Core/Section.h
lldb/include/lldb/Symbol/ObjectFile.h
lldb/source/Core/Section.cpp
lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.h
lldb/source/Symbol/ObjectFile.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Section.h 
b/lldb/include/lldb/Core/Section.h
index 8a9fea3743141..4bf73e2e5a068 100644
--- a/lldb/include/lldb/Core/Section.h
+++ b/lldb/include/lldb/Core/Section.h
@@ -17,6 +17,7 @@
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
+#include "llvm/Support/JSON.h"
 
 #include 
 #include 
@@ -99,6 +100,13 @@ class SectionList {
   collection m_sections;
 };
 
+struct JSONSection {
+  std::string name;
+  std::optional type;
+  std::optional address;
+  std::optional size;
+};
+
 class Section : public std::enable_shared_from_this,
 public ModuleChild,
 public UserID,
@@ -287,4 +295,16 @@ class Section : public 
std::enable_shared_from_this,
 
 } // namespace lldb_private
 
+namespace llvm {
+namespace json {
+
+bool fromJSON(const llvm::json::Value &value,
+  lldb_private::JSONSection §ion, llvm::json::Path path);
+
+bool fromJSON(const llvm::json::Value &value, lldb::SectionType &type,
+  llvm::json::Path path);
+
+} // namespace json
+} // namespace llvm
+
 #endif // LLDB_CORE_SECTION_H

diff  --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index 8f7806f682ede..8862630495e60 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -768,6 +768,11 @@ template <> struct 
format_provider {
   static void format(const lldb_private::ObjectFile::Strata &strata,
  raw_ostream &OS, StringRef Style);
 };
+
+namespace json {
+bool fromJSON(const llvm::json::Value &value, lldb_private::ObjectFile::Type &,
+  llvm::json::Path path);
+} // namespace json
 } // namespace llvm
 
 #endif // LLDB_SYMBOL_OBJECTFILE_H

diff  --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index 50c1562f75619..808d25875cfc6 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -673,3 +673,36 @@ uint64_t SectionList::GetDebugInfoSize() const {
   }
   return debug_info_size;
 }
+
+namespace llvm {
+namespace json {
+
+bool fromJSON(const llvm::json::Value &value,
+  lldb_private::JSONSection §ion, llvm::json::Path path) {
+  llvm::json::ObjectMapper o(value, path);
+  return o && o.map("name", section.name) && o.map("type", section.type) &&
+ o.map("size", section.address) && o.map("size", section.size);
+}
+
+bool fromJSON(const llvm::json::Value &value, lldb::SectionType &type,
+  llvm::json::Path path) {
+  if (auto str = value.getAsString()) {
+type = llvm::StringSwitch(*str)
+   .Case("code", eSectionTypeCode)
+   .Case("container", eSectionTypeContainer)
+   .Case("data", eSectionTypeData)
+   .Case("debug", eSectionTypeDebug)
+   .Default(eSectionTypeInvalid);
+
+if (type == eSectionTypeInvalid) {
+  path.report("invalid section type");
+  return false;
+}
+
+return true;
+  }
+  path.report("expected string");
+  return false;
+}
+} // namespace json
+} // namespace llvm

diff  --git a/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp 
b/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
index 7cd836ab2460c..ffbd87714242c 100644
--- a/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
+++ b/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
@@ -49,6 +49,7 @@ ObjectFileJSON::CreateInstance(const ModuleSP &module_sp, 
DataBufferSP data_sp,
   if (!MagicBytesMatch(data_sp, 0, data_sp->GetByteSize()))
 return nullptr;
 
+  // Update

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

2023-04-13 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e5cdbf07e6d: [lldb] Make ObjectFileJSON loadable as a 
module (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148062

Files:
  lldb/include/lldb/Core/Section.h
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Core/Section.cpp
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
  lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.h
  lldb/source/Symbol/ObjectFile.cpp
  lldb/test/API/macosx/symbols/TestObjectFileJSON.py

Index: lldb/test/API/macosx/symbols/TestObjectFileJSON.py
===
--- /dev/null
+++ lldb/test/API/macosx/symbols/TestObjectFileJSON.py
@@ -0,0 +1,100 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+import json
+import uuid
+import os
+import shutil
+
+
+class TestObjectFileJSON(TestBase):
+TRIPLE = "arm64-apple-macosx13.0.0"
+
+def setUp(self):
+TestBase.setUp(self)
+self.source = "main.c"
+
+def emitJSON(self, data, path):
+json_object = json.dumps(data, indent=4)
+json_object_file = self.getBuildArtifact("a.json")
+with open(json_object_file, "w") as outfile:
+outfile.write(json_object)
+
+def toModuleSpec(self, path):
+module_spec = lldb.SBModuleSpec()
+module_spec.SetFileSpec(lldb.SBFileSpec(path))
+return module_spec
+
+@no_debug_info_test
+def test_target(self):
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+"type": "executable",
+}
+
+json_object_file = self.getBuildArtifact("a.json")
+self.emitJSON(data, json_object_file)
+
+target = self.dbg.CreateTarget(json_object_file)
+self.assertTrue(target.IsValid())
+self.assertEqual(target.GetTriple(), self.TRIPLE)
+
+@no_debug_info_test
+def test_module(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+target = self.dbg.CreateTarget(exe)
+
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+}
+
+json_object_file = self.getBuildArtifact("a.json")
+self.emitJSON(data, json_object_file)
+
+module = target.AddModule(self.toModuleSpec(json_object_file))
+self.assertFalse(module.IsValid())
+
+data = {
+"triple": self.TRIPLE,
+"uuid": str(uuid.uuid4()),
+"type": "sharedlibrary",
+"sections": [
+{
+"name": "__TEXT",
+"type": "code",
+"address": 0,
+"size": 0x222,
+}
+],
+"symbols": [
+{
+"name": "foo",
+"address": 0x100,
+"size": 0x11,
+}
+],
+}
+self.emitJSON(data, json_object_file)
+
+module = target.AddModule(self.toModuleSpec(json_object_file))
+self.assertTrue(module.IsValid())
+
+section = module.GetSectionAtIndex(0)
+self.assertTrue(section.IsValid())
+self.assertEqual(section.GetName(), "__TEXT")
+self.assertEqual(section.file_addr, 0x0)
+self.assertEqual(section.size, 0x222)
+
+symbol = module.FindSymbol("foo")
+self.assertTrue(symbol.IsValid())
+self.assertEqual(symbol.addr.GetFileAddress(), 0x100)
+self.assertEqual(symbol.GetSize(), 0x11)
+
+error = target.SetSectionLoadAddress(section, 0x1000)
+self.assertSuccess(error)
+self.assertEqual(symbol.addr.GetLoadAddress(target), 0x1100)
Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -761,3 +761,34 @@
   m_cache_hash = llvm::djbHash(strm.GetString());
   return *m_cache_hash;
 }
+
+namespace llvm {
+namespace json {
+
+bool fromJSON(const llvm::json::Value &value,
+  lldb_private::ObjectFile::Type &type, llvm::json::Path path) {
+  if (auto str = value.getAsString()) {
+type = llvm::StringSwitch(*str)
+   .Case("corefile", ObjectFile::eTypeCoreFile)
+   .Case("executable", ObjectFile::eTypeExecutable)
+   .Case("debuginfo", ObjectFile::eTypeDebugInfo)
+   .Case("dynamiclinker", ObjectFile::eTypeDynamicLinker)
+   .Case("objectfile", ObjectFile::eTypeObjectFile)
+   .Case("sharedlibrary", ObjectFile::eTypeSharedLibrary)
+   .Case("stublibrary", ObjectFile::eTypeStubLibrary)
+   .Case("jit", ObjectFile::eTypeJIT)
+

[Lldb-commits] [lldb] 6f8360a - [lldb] Use the host's target triple in TestObjectFileJSON

2023-04-13 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2023-04-13T14:20:58-07:00
New Revision: 6f8360a0e15f3affe3f711130c693059930bc317

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

LOG: [lldb] Use the host's target triple in TestObjectFileJSON

Use the target's triple when adding JSON modules in TestObjectFileJSON.
This should fix the test failure on non-Darwin bots.

Added: 


Modified: 
lldb/test/API/macosx/symbols/TestObjectFileJSON.py

Removed: 




diff  --git a/lldb/test/API/macosx/symbols/TestObjectFileJSON.py 
b/lldb/test/API/macosx/symbols/TestObjectFileJSON.py
index d51cac8c376b2..04f91ec62f88b 100644
--- a/lldb/test/API/macosx/symbols/TestObjectFileJSON.py
+++ b/lldb/test/API/macosx/symbols/TestObjectFileJSON.py
@@ -29,8 +29,9 @@ def toModuleSpec(self, path):
 
 @no_debug_info_test
 def test_target(self):
+triple = "arm64-apple-macosx13.0.0"
 data = {
-"triple": self.TRIPLE,
+"triple": triple,
 "uuid": str(uuid.uuid4()),
 "type": "executable",
 }
@@ -40,7 +41,7 @@ def test_target(self):
 
 target = self.dbg.CreateTarget(json_object_file)
 self.assertTrue(target.IsValid())
-self.assertEqual(target.GetTriple(), self.TRIPLE)
+self.assertEqual(target.GetTriple(), triple)
 
 @no_debug_info_test
 def test_module(self):
@@ -49,7 +50,7 @@ def test_module(self):
 target = self.dbg.CreateTarget(exe)
 
 data = {
-"triple": self.TRIPLE,
+"triple": target.GetTriple(),
 "uuid": str(uuid.uuid4()),
 }
 
@@ -60,7 +61,7 @@ def test_module(self):
 self.assertFalse(module.IsValid())
 
 data = {
-"triple": self.TRIPLE,
+"triple": target.GetTriple(),
 "uuid": str(uuid.uuid4()),
 "type": "sharedlibrary",
 "sections": [



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

[Lldb-commits] [PATCH] D148175: [lldb] Add `operator StringRef` to ConstString

2023-04-13 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

In D148175#4263217 , @aprantl wrote:

> I understand that the opposite direction is explicit because actual work is 
> being done. In this direction, it shouldn't affect memory management (since 
> ConstStrings live forever) or performance, so I think this is good (and very 
> convenient!).
> Does anyone else see a problem with this?

I think this is safe and helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148175

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

[Lldb-commits] [PATCH] D148175: [lldb] Add `operator StringRef` to ConstString

2023-04-13 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.

+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148175

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

[Lldb-commits] [PATCH] D148175: [lldb] Add `operator StringRef` to ConstString

2023-04-13 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl accepted this revision.
aprantl added a comment.

Okay, then let the refactor fest begin!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148175

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

[Lldb-commits] [lldb] 27f27d1 - [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-13 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2023-04-13T16:56:29-07:00
New Revision: 27f27d15f6c90b026eca23b8ee238fdbf772fd80

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

LOG: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

Create an artificial module using a JSON object file when we can't
locate the module and dSYM through dsymForUUID (or however
locate_module_and_debug_symbols is implemented). By parsing the symbols
from the crashlog and making them part of the JSON object file, LLDB can
symbolicate frames it otherwise wouldn't be able to, as there is no
module for it.

For non-interactive crashlogs, that never was a problem because we could
simply show the "pre-symbolicated" frame from the input. For interactive
crashlogs, we need a way to pass the symbol information to LLDB so that
it can symbolicate the frames, which is what motivated the JSON object
file format.

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

Added: 


Modified: 
lldb/examples/python/crashlog.py
lldb/examples/python/scripted_process/crashlog_scripted_process.py
lldb/examples/python/symbolication.py

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 68ead43ce8692..dadab3cd8b7f4 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -80,6 +80,7 @@ class Thread:
 def __init__(self, index, app_specific_backtrace):
 self.index = index
 self.id = index
+self.images = list()
 self.frames = list()
 self.idents = list()
 self.registers = dict()
@@ -456,6 +457,11 @@ def parse_json(buffer):
 except:
 return None
 
+def __init__(self, debugger, path, verbose):
+super().__init__(debugger, path, verbose)
+# List of DarwinImages sorted by their index.
+self.images = list()
+
 def parse(self):
 try:
 self.parse_process_info(self.data)
@@ -506,7 +512,6 @@ def parse_crash_reason(self, json_exception):
   exception_extra)
 
 def parse_images(self, json_images):
-idx = 0
 for json_image in json_images:
 img_uuid = uuid.UUID(json_image['uuid'])
 low = int(json_image['base'])
@@ -518,8 +523,8 @@ def parse_images(self, json_images):
 darwin_image = self.crashlog.DarwinImage(low, high, name, version,
  img_uuid, path,
  self.verbose)
+self.images.append(darwin_image)
 self.crashlog.images.append(darwin_image)
-idx += 1
 
 def parse_main_image(self, json_data):
 if 'procName' in json_data:
@@ -539,6 +544,17 @@ def parse_frames(self, thread, json_frames):
 frame_offset = int(json_frame['imageOffset'])
 image_addr = self.get_used_image(image_id)['base']
 pc = image_addr + frame_offset
+
+if 'symbol' in json_frame:
+symbol = json_frame['symbol']
+location = int(json_frame['symbolLocation'])
+image = self.images[image_id]
+image.symbols[symbol] = {
+"name": symbol,
+"type": "code",
+"address": frame_offset - location
+}
+
 thread.frames.append(self.crashlog.Frame(idx, pc, frame_offset))
 
 # on arm64 systems, if it jump through a null function pointer,
@@ -1015,40 +1031,25 @@ def SymbolicateCrashLog(crash_log, options):
 target = crash_log.create_target()
 if not target:
 return
-exe_module = target.GetModuleAtIndex(0)
-images_to_load = list()
-loaded_images = list()
+
+
 if options.load_all_images:
-# --load-all option was specified, load everything up
 for image in crash_log.images:
-images_to_load.append(image)
-else:
-# Only load the images found in stack frames for the crashed threads
-if options.crashed_only:
-for thread in crash_log.threads:
-if thread.did_crash():
-for ident in thread.idents:
-images = crash_log.find_images_with_identifier(ident)
-if images:
-for image in images:
-images_to_load.append(image)
-else:
-print('error: can\'t

[Lldb-commits] [PATCH] D148172: [lldb] Use ObjectFileJSON to create modules for interactive crashlogs

2023-04-13 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG27f27d15f6c9: [lldb] Use ObjectFileJSON to create modules 
for interactive crashlogs (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148172

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/symbolication.py
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
  
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test
@@ -41,3 +41,6 @@
 # CHECK-NEXT:frame #2: 0x000100ec5a87 multithread-test`compute_pow{{.*}} [artificial]
 # CHECK:frame #{{[0-9]+}}: 0x00019cc7e06b{{.*}} [artificial]
 # CHECK:frame #{{[0-9]+}}: 0x00019cc78e2b{{.*}} [artificial]
+
+image list
+# CHECK: ---- {{.*}}bogus.dylib
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/interactive_crashlog/multithread-test.ips
@@ -478,6 +478,15 @@
   "source" : "A",
   "base" : 0,
   "uuid" : "----"
+},
+{
+  "arch": "arm64",
+  "base": 12345,
+  "name": "bogus.dylib",
+  "path": "/usr/lib/system/bogus.dylib",
+  "size": 1000,
+  "source": "P",
+  "uuid": "----"
 }
   ],
   "userID": 501,
Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -35,6 +35,8 @@
 import sys
 import time
 import uuid
+import json
+import tempfile
 
 
 class Address:
@@ -230,6 +232,7 @@
 def __init__(self, path, uuid=None):
 self.path = path
 self.resolved_path = None
+self.resolve = False
 self.resolved = False
 self.unavailable = False
 self.uuid = uuid
@@ -240,6 +243,7 @@
 self.module = None
 self.symfile = None
 self.slide = None
+self.symbols = dict()
 
 @classmethod
 def InitWithSBTargetAndSBModule(cls, target, module):
@@ -372,14 +376,32 @@
 uuid_str = self.get_normalized_uuid_string()
 if uuid_str:
 self.module = target.AddModule(None, None, uuid_str)
-if not self.module:
+if not self.module and self.resolve:
 self.locate_module_and_debug_symbols()
-if self.unavailable:
-return None
-resolved_path = self.get_resolved_path()
-self.module = target.AddModule(
-resolved_path, None, uuid_str, self.symfile)
-if not self.module:
+if not self.unavailable:
+resolved_path = self.get_resolved_path()
+self.module = target.AddModule(
+resolved_path, None, uuid_str, self.symfile)
+if not self.module and self.section_infos:
+name = os.path.basename(self.path)
+with tempfile.NamedTemporaryFile(suffix='.' + name) as tf:
+data = {
+'triple': target.triple,
+'uuid': uuid_str,
+'type': 'sharedlibrary',
+'sections': list(),
+'symbols': list()
+}
+for section in self.section_infos:
+data['sections'].append({
+'name' : section.name,
+'size': section.end_addr - section.start_addr
+})
+data['symbols'] = list(self.symbols.values())
+with open(tf.name, 'w') as f:
+f.write(json.dumps(data, indent=4))
+self.module = target.AddModule(tf.name, None, uuid_str)
+if not self.module and not self.unavailable:
 return 'error: unable to get module for (%s) "%s"' % (
 self.arch, self.get_resolved_path())
 if self.has_section_load_info():
Index: lldb/example

[Lldb-commits] [PATCH] D148282: Fix the help for "type X delete" to make the -a & -w behaviors clear

2023-04-13 Thread Jim Ingham via Phabricator via lldb-commits 
jingham created this revision.
jingham added reviewers: JDevlieghere, aprantl.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

If you run:

(lldb) type summary delete Foo

that will only search for the type summary for Foo in the "default" category.  
That's by design, since this is "delete" you wouldn't want to accidentally 
delete a format it might be hard to get back, so making you specify the 
category or say "all" explicitly is a good safeguard.  But the help is not at 
all clear that that's how it works and it has confused several people now.

This patch fixes the help strings.  Since all the `type X delete` commands go 
through a common base, I centralized the help output in the base class so the 
help would be consistent.  I also removed an unused macro, and changed where we 
were calling these `formatter_kind_mask` to be `formatter_kind` since we only 
use them as a single enum element, we never use them as masks in this context.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148282

Files:
  lldb/source/Commands/CommandObjectType.cpp

Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -39,9 +39,6 @@
 #include 
 #include 
 
-#define CHECK_FORMATTER_KIND_MASK(VAL) \
-  ((m_formatter_kind_mask & (VAL)) == (VAL))
-
 using namespace lldb;
 using namespace lldb_private;
 
@@ -100,6 +97,21 @@
   return false;
 }
 
+const char *FormatCategoryToString(FormatCategoryItem item, bool long_name) {
+  switch (item) {
+  case eFormatCategoryItemSummary:
+return "summary";
+  case eFormatCategoryItemFilter:
+return "filter";
+  case eFormatCategoryItemSynth:
+if (long_name)
+  return "synthetic child provider";
+return "synthetic";
+  case eFormatCategoryItemFormat:
+return "format";
+  }
+};
+
 #define LLDB_OPTIONS_type_summary_add
 #include "CommandOptions.inc"
 
@@ -754,16 +766,19 @@
   };
 
   CommandOptions m_options;
-  uint32_t m_formatter_kind_mask;
+  FormatCategoryItem m_formatter_kind;
 
   Options *GetOptions() override { return &m_options; }
+  
+  static const char *g_short_help_template;
+  static const char *g_long_help_template;
 
 public:
   CommandObjectTypeFormatterDelete(CommandInterpreter &interpreter,
-   uint32_t formatter_kind_mask,
-   const char *name, const char *help)
-  : CommandObjectParsed(interpreter, name, help, nullptr),
-m_formatter_kind_mask(formatter_kind_mask) {
+   FormatCategoryItem formatter_kind)
+  : CommandObjectParsed(interpreter, 
+FormatCategoryToString(formatter_kind, false)),
+m_formatter_kind(formatter_kind) {
 CommandArgumentEntry type_arg;
 CommandArgumentData type_style_arg;
 
@@ -773,6 +788,19 @@
 type_arg.push_back(type_style_arg);
 
 m_arguments.push_back(type_arg);
+
+const char *kind = FormatCategoryToString(formatter_kind, true);
+const char *short_kind = FormatCategoryToString(formatter_kind, false);
+  
+StreamString s;
+s.Printf(g_short_help_template, kind);
+SetHelp(s.GetData());
+s.Clear();
+s.Printf(g_long_help_template, kind, short_kind);
+SetHelpLong(s.GetData());
+s.Clear();
+s.Printf("type %s delete", short_kind);
+SetCommandName(s.GetData());
   }
 
   ~CommandObjectTypeFormatterDelete() override = default;
@@ -785,7 +813,7 @@
 
 DataVisualization::Categories::ForEach(
 [this, &request](const lldb::TypeCategoryImplSP &category_sp) {
-  category_sp->AutoComplete(request, m_formatter_kind_mask);
+  category_sp->AutoComplete(request, m_formatter_kind);
   return true;
 });
   }
@@ -812,7 +840,7 @@
 if (m_options.m_delete_all) {
   DataVisualization::Categories::ForEach(
   [this, typeCS](const lldb::TypeCategoryImplSP &category_sp) -> bool {
-category_sp->Delete(typeCS, m_formatter_kind_mask);
+category_sp->Delete(typeCS, m_formatter_kind);
 return true;
   });
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
@@ -827,14 +855,14 @@
   DataVisualization::Categories::GetCategory(m_options.m_language,
  category);
   if (category)
-delete_category = category->Delete(typeCS, m_formatter_kind_mask);
+delete_category = category->Delete(typeCS, m_formatter_kind);
   extra_deletion = FormatterSpecificDeletion(typeCS);
 } else {
   lldb::TypeCategoryImplSP category;
   DataVisualization::Categories::GetCategory(
   ConstString(m_options.m_category.c_str()), category);
   if

[Lldb-commits] [PATCH] D148282: Fix the help for "type X delete" to make the -a & -w behaviors clear

2023-04-13 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord added inline comments.



Comment at: lldb/source/Commands/CommandObjectType.cpp:112-113
+return "format";
+  }
+};
+

maybe add an `llvm_unreachable` after the switch statement? 



Comment at: lldb/source/Commands/CommandObjectType.cpp:879-885
+const char *CommandObjectTypeFormatterDelete::g_short_help_template = 
+"Delete an existing %s for a type.";
+const char *CommandObjectTypeFormatterDelete::g_long_help_template = 
+"Delete an existing %s for a type.  Unless you specify a "
+"specific category or all categories, only the "
+"'default' category is searched.  The names must be exactly as "
+"shown in the 'type %s list' output";

You could define these in the class and not out-of-line if you use constexpr. 
So for example, above in the class you can do:

```
  static constexpr const char *g_short_help_template = "Delete an existing %s 
for a type.";
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148282

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

[Lldb-commits] [lldb] 0ae342f - [lldb][test] Fix -Wsign-compare in RegisterFlagsTest.cpp (NFC)

2023-04-13 Thread Jie Fu via lldb-commits 

Author: Jie Fu
Date: 2023-04-14T09:47:21+08:00
New Revision: 0ae342f45bedd29e34690de087011a9da4db6a65

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

LOG: [lldb][test] Fix -Wsign-compare in RegisterFlagsTest.cpp (NFC)

/data/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1526:11:
 error: comparison of integers of different signs: 'const unsigned long long' 
and 'const int' [-Werror,-Wsign-compare]
  if (lhs == rhs) {
  ~~~ ^  ~~~
/data/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1553:12:
 note: in instantiation of function template specialization 
'testing::internal::CmpHelperEQ' requested here
return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
   ^
/data/llvm-project/lldb/unittests/Target/RegisterFlagsTest.cpp:128:3: note: in 
instantiation of function template specialization 
'testing::internal::EqHelper::Compare' 
requested here
  ASSERT_EQ(0x12345678ULL, rf.ReverseFieldOrder(0x12345678));
  ^
/data/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:2056:32:
 note: expanded from macro 'ASSERT_EQ'
   ^
/data/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:2040:54:
 note: expanded from macro 'GTEST_ASSERT_EQ'
  ASSERT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
 ^
1 error generated.

Added: 


Modified: 
lldb/unittests/Target/RegisterFlagsTest.cpp

Removed: 




diff  --git a/lldb/unittests/Target/RegisterFlagsTest.cpp 
b/lldb/unittests/Target/RegisterFlagsTest.cpp
index 3819c6fda6ceb..c24a6d9bf6c32 100644
--- a/lldb/unittests/Target/RegisterFlagsTest.cpp
+++ b/lldb/unittests/Target/RegisterFlagsTest.cpp
@@ -125,11 +125,11 @@ TEST(RegisterFlagsTest, RegisterFlagsPadding) {
 TEST(RegisterFieldsTest, ReverseFieldOrder) {
   // Unchanged
   RegisterFlags rf("", 4, {make_field(0, 31)});
-  ASSERT_EQ(0x12345678ULL, rf.ReverseFieldOrder(0x12345678));
+  ASSERT_EQ(0x12345678ULL, (unsigned long 
long)rf.ReverseFieldOrder(0x12345678));
 
   // Swap the two halves around.
   RegisterFlags rf2("", 4, {make_field(16, 31), make_field(0, 15)});
-  ASSERT_EQ(0x56781234ULL, rf2.ReverseFieldOrder(0x12345678));
+  ASSERT_EQ(0x56781234ULL, (unsigned long 
long)rf2.ReverseFieldOrder(0x12345678));
 
   // Many small fields.
   RegisterFlags rf3("", 4,



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