[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData JSON Array parsing (PR #101929)
https://github.com/medismailben updated
https://github.com/llvm/llvm-project/pull/101929
>From 18fcfda3baadceca0ec666bd86948baf991f8800 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Wed, 7 Aug 2024 00:17:36 -0700
Subject: [PATCH] [lldb/API] Fix SBStructuredData support any JSON type
This patch loosen the parsing requirement to allow parsing not only
JSON dictionaries but also valid JSON type (integer, float, string,
bool, array, null).
Signed-off-by: Med Ismail Bennani
---
lldb/source/API/SBStructuredData.cpp | 9 +++--
.../sbstructureddata/TestStructuredDataAPI.py | 36 +++
2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/lldb/source/API/SBStructuredData.cpp
b/lldb/source/API/SBStructuredData.cpp
index b18fc5655fc81..6361aa6babca7 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -86,7 +86,12 @@ lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream
&stream) {
StructuredData::ParseJSON(stream.GetData());
m_impl_up->SetObjectSP(json_obj);
- if (!json_obj || json_obj->GetType() != eStructuredDataTypeDictionary)
+ static constexpr StructuredDataType unsupported_type[] = {
+ eStructuredDataTypeInvalid,
+ eStructuredDataTypeGeneric,
+ };
+
+ if (!json_obj || llvm::is_contained(unsupported_type, json_obj->GetType()))
error.SetErrorString("Invalid Syntax");
return error;
}
@@ -106,7 +111,7 @@ bool SBStructuredData::IsValid() const {
SBStructuredData::operator bool() const {
LLDB_INSTRUMENT_VA(this);
- return m_impl_up->IsValid();
+ return m_impl_up && m_impl_up->IsValid();
}
void SBStructuredData::Clear() {
diff --git a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
index b3db3bc61e4dc..4493961d612a0 100644
--- a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -110,6 +110,42 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example = lldb.SBStructuredData()
+self.assertSuccess(example.SetFromJSON("1"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeInteger)
+self.assertEqual(example.GetIntegerValue(), 1)
+
+self.assertSuccess(example.SetFromJSON("4.19"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeFloat)
+self.assertEqual(example.GetFloatValue(), 4.19)
+
+self.assertSuccess(example.SetFromJSON('"Bonjour, 123!"'))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeString)
+self.assertEqual(example.GetStringValue(42), "Bonjour, 123!")
+
+self.assertSuccess(example.SetFromJSON("true"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeBoolean)
+self.assertTrue(example.GetBooleanValue())
+
+self.assertSuccess(example.SetFromJSON("null"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeNull)
+
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+
+# Check SetFromJSON API for dictionaries, integers, floating point
+# values, strings and arrays
+error = example.SetFromJSON(s)
+if not error.Success():
+self.fail("FAILED: " + error.GetCString())
+
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+sb_data = json.loads(s.GetData())
+self.assertEqual(sb_data, example_arr)
+
def invalid_struct_test(self, example):
invalid_struct = lldb.SBStructuredData()
invalid_struct = example.GetValueForKey("invalid_key")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData JSON Array parsing (PR #101929)
https://github.com/medismailben edited https://github.com/llvm/llvm-project/pull/101929 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData support any JSON type (PR #101929)
https://github.com/medismailben edited https://github.com/llvm/llvm-project/pull/101929 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData support any JSON type (PR #101929)
medismailben wrote: > Shouldn't any valid JSON be acceptable? A dictionary, array, string, number, > "true", "false", or "null"? Sure, fixed that @clayborg :) https://github.com/llvm/llvm-project/pull/101929 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData support any JSON type (PR #101929)
https://github.com/medismailben updated
https://github.com/llvm/llvm-project/pull/101929
>From cfceebbf9ffa7abe1eadcb5fade88f3eb359757c Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Wed, 7 Aug 2024 00:20:12 -0700
Subject: [PATCH] [lldb/API] Fix SBStructuredData support any JSON type
This patch loosen the parsing requirement to allow parsing not only
JSON dictionaries but also valid JSON type (integer, float, string,
bool, array, null).
Signed-off-by: Med Ismail Bennani
---
lldb/source/API/SBStructuredData.cpp | 7 +++-
.../sbstructureddata/TestStructuredDataAPI.py | 36 +++
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/lldb/source/API/SBStructuredData.cpp
b/lldb/source/API/SBStructuredData.cpp
index b18fc5655fc81..78afdc69fe0d2 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -86,7 +86,12 @@ lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream
&stream) {
StructuredData::ParseJSON(stream.GetData());
m_impl_up->SetObjectSP(json_obj);
- if (!json_obj || json_obj->GetType() != eStructuredDataTypeDictionary)
+ static constexpr StructuredDataType unsupported_type[] = {
+ eStructuredDataTypeInvalid,
+ eStructuredDataTypeGeneric,
+ };
+
+ if (!json_obj || llvm::is_contained(unsupported_type, json_obj->GetType()))
error.SetErrorString("Invalid Syntax");
return error;
}
diff --git a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
index b3db3bc61e4dc..4493961d612a0 100644
--- a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -110,6 +110,42 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example = lldb.SBStructuredData()
+self.assertSuccess(example.SetFromJSON("1"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeInteger)
+self.assertEqual(example.GetIntegerValue(), 1)
+
+self.assertSuccess(example.SetFromJSON("4.19"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeFloat)
+self.assertEqual(example.GetFloatValue(), 4.19)
+
+self.assertSuccess(example.SetFromJSON('"Bonjour, 123!"'))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeString)
+self.assertEqual(example.GetStringValue(42), "Bonjour, 123!")
+
+self.assertSuccess(example.SetFromJSON("true"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeBoolean)
+self.assertTrue(example.GetBooleanValue())
+
+self.assertSuccess(example.SetFromJSON("null"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeNull)
+
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+
+# Check SetFromJSON API for dictionaries, integers, floating point
+# values, strings and arrays
+error = example.SetFromJSON(s)
+if not error.Success():
+self.fail("FAILED: " + error.GetCString())
+
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+sb_data = json.loads(s.GetData())
+self.assertEqual(sb_data, example_arr)
+
def invalid_struct_test(self, example):
invalid_struct = lldb.SBStructuredData()
invalid_struct = example.GetValueForKey("invalid_key")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -336,3 +336,89 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
0xab, 0xad, 0xca, 0xfe}),
*ExpectedContext);
}
+
+TEST(MinidumpYAML, MemoryRegion_64bit) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile &File = **ExpectedFile;
+
+ ASSERT_THAT(1u, File.streams().size());
+
+ Error Err = Error::success();
+ // Explicit Err check
+ ASSERT_FALSE(Err);
+ Expected>
+ ExpectedMemoryList = File.getMemory64List(Err);
+
+ ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
+
+ iterator_range MemoryList =
+ *ExpectedMemoryList;
+ auto Iterator = MemoryList.begin();
+
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
+ auto DescOnePair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescOne = DescOnePair.first;
+ ASSERT_THAT(0x7FCF0818283u, DescOne.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescOne.DataSize);
+
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
+ auto DescTwoPair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescTwo = DescTwoPair.first;
+ ASSERT_THAT(0x7FFF0818283u, DescTwo.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescTwo.DataSize);
+ const std::optional> ExpectedContent =
+ File.getRawStream(StreamType::Memory64List);
+ ASSERT_TRUE(ExpectedContent);
+ const size_t ExpectedStreamSize =
+ sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
+ ASSERT_THAT(ExpectedStreamSize, ExpectedContent->size());
+
+ Expected ExpectedHeader =
+ File.getMemoryList64Header();
+ ASSERT_THAT_EXPECTED(ExpectedHeader, Succeeded());
+ ASSERT_THAT(ExpectedHeader->BaseRVA, 92u);
+
+ Expected> DescOneExpectedContentSlice = DescOnePair.second;
+ ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
+ ASSERT_THAT(5u, DescOneExpectedContentSlice->size());
+ ASSERT_THAT(arrayRefFromStringRef("hello"), *DescOneExpectedContentSlice);
+
+ Expected> DescTwoExpectedContentSlice = DescTwoPair.second;
+ ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
+ ASSERT_THAT(arrayRefFromStringRef("world"), *DescTwoExpectedContentSlice);
+
+ ASSERT_TRUE(Iterator == MemoryList.end());
labath wrote:
could this be ASSERT_EQ, or does that not compile (if it doesn't, it's fine, I
just want to know)?
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
https://github.com/labath commented: I can see you struggled to get this to work, and I appreciate the effort you put in. I think some of the inline comments could make this flow better. https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,95 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ /// Class the provides an iterator over the memory64 memory ranges. Only the
+ /// the first descriptor is validated as readable beforehand.
+ class Memory64Iterator {
+ public:
+static Memory64Iterator
+begin(ArrayRef Storage,
+ ArrayRef Descriptors,
+ uint64_t BaseRVA) {
+ return Memory64Iterator(Storage, Descriptors, BaseRVA);
+}
+
+static Memory64Iterator end() { return Memory64Iterator(); }
+
+std::pair> Current;
+
+bool operator==(const Memory64Iterator &R) const {
+ return IsEnd == R.IsEnd;
+}
+
+bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
+
+const std::pair> &
+operator*() {
+ return Current;
+}
+
+const std::pair> *
+operator&() {
+ return &Current;
+}
labath wrote:
This sounds wrong. There are very few good reasons to override operator&. Did
you maybe mean to implement operator-> instead?
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -494,6 +528,32 @@ Stream::create(const Directory &StreamDesc, const
object::MinidumpFile &File) {
}
return std::make_unique(std::move(Ranges));
}
+ case StreamKind::Memory64List: {
+// Error, unlike expected is true in failure state
+Error Err = Error::success();
+// Explicit check on Err so that if we return due to getmemory64list
+// getting an error, it's not destructed when unchecked.
+if (Err)
+ return Err;
+auto ExpectedList = File.getMemory64List(Err);
+if (!ExpectedList)
+ return ExpectedList.takeError();
labath wrote:
This is pretty clumsy, so yeah, I'm going to agree with myself that we should
only use a single error object for returning getMemory64List errors.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -336,3 +336,89 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
0xab, 0xad, 0xca, 0xfe}),
*ExpectedContext);
}
+
+TEST(MinidumpYAML, MemoryRegion_64bit) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile &File = **ExpectedFile;
+
+ ASSERT_THAT(1u, File.streams().size());
+
+ Error Err = Error::success();
+ // Explicit Err check
+ ASSERT_FALSE(Err);
+ Expected>
+ ExpectedMemoryList = File.getMemory64List(Err);
+
+ ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
+
+ iterator_range MemoryList =
+ *ExpectedMemoryList;
+ auto Iterator = MemoryList.begin();
+
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
+ auto DescOnePair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescOne = DescOnePair.first;
+ ASSERT_THAT(0x7FCF0818283u, DescOne.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescOne.DataSize);
+
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
+ auto DescTwoPair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescTwo = DescTwoPair.first;
+ ASSERT_THAT(0x7FFF0818283u, DescTwo.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescTwo.DataSize);
+ const std::optional> ExpectedContent =
+ File.getRawStream(StreamType::Memory64List);
+ ASSERT_TRUE(ExpectedContent);
+ const size_t ExpectedStreamSize =
+ sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
+ ASSERT_THAT(ExpectedStreamSize, ExpectedContent->size());
+
+ Expected ExpectedHeader =
+ File.getMemoryList64Header();
+ ASSERT_THAT_EXPECTED(ExpectedHeader, Succeeded());
+ ASSERT_THAT(ExpectedHeader->BaseRVA, 92u);
+
+ Expected> DescOneExpectedContentSlice = DescOnePair.second;
+ ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
+ ASSERT_THAT(5u, DescOneExpectedContentSlice->size());
+ ASSERT_THAT(arrayRefFromStringRef("hello"), *DescOneExpectedContentSlice);
+
+ Expected> DescTwoExpectedContentSlice = DescTwoPair.second;
+ ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
+ ASSERT_THAT(arrayRefFromStringRef("world"), *DescTwoExpectedContentSlice);
+
+ ASSERT_TRUE(Iterator == MemoryList.end());
+}
+
+TEST(MinidumpYAML, MemoryRegion_DataSize_TooSmall) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Data Size: 4 1
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Failed());
labath wrote:
What is this checking? Are you sure this isn't failing due to a yaml syntax
error (`Data Size: 4 1`). Could we assert a specific error message?
If not, maybe you could make that a FileCheck test instead (I suspect that's
why `test/tools/yaml2obj/Minidump/raw-stream-small-size.yaml` is not a unit
test)
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -154,3 +155,33 @@ MinidumpFile::create(MemoryBufferRef Source) {
return std::unique_ptr(
new MinidumpFile(Source, Hdr, *ExpectedStreams, std::move(StreamMap)));
}
+
+Expected>
labath wrote:
Instead of returning another error inside the `Expected`, maybe we could
reuse the error out parameter -- just set that to whatever we encounter and
return an empty range?
Something like:
```
ErrorAsOutParameter ErrAsOut(&Err);
Expected ListHeader = ...;
if (!ListHeader) {
Err = ListHeader.takeError();
return make_fallible_range(Memory64Iterator::end(), Memory64Iterator::end(),
Err);
}
```
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,95 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ /// Class the provides an iterator over the memory64 memory ranges. Only the
+ /// the first descriptor is validated as readable beforehand.
+ class Memory64Iterator {
+ public:
+static Memory64Iterator
+begin(ArrayRef Storage,
+ ArrayRef Descriptors,
+ uint64_t BaseRVA) {
+ return Memory64Iterator(Storage, Descriptors, BaseRVA);
+}
+
+static Memory64Iterator end() { return Memory64Iterator(); }
+
+std::pair> Current;
+
+bool operator==(const Memory64Iterator &R) const {
+ return IsEnd == R.IsEnd;
+}
+
+bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
+
+const std::pair> &
+operator*() {
+ return Current;
+}
+
+const std::pair> *
+operator&() {
+ return &Current;
+}
+
+Error inc() {
+ if (Storage.size() == 0 || Descriptors.size() == 0)
+return make_error("No Memory64List Stream",
+ object_error::parse_failed);
+
+ if (Index >= Descriptors.size())
+return createError("Can't read past of Memory64List Stream");
+
+ const minidump::MemoryDescriptor_64 &Descriptor = Descriptors[Index];
+ if (RVA + Descriptor.DataSize > Storage.size())
+return make_error(
+"Memory64 Descriptor exceeds end of file.",
+object_error::unexpected_eof);
+
+ ArrayRef Content = Storage.slice(RVA, Descriptor.DataSize);
+ Current = std::make_pair(Descriptor, Content);
+ Index++;
+ RVA += Descriptor.DataSize;
labath wrote:
Just an idea that I think could make this slightly neater: Instead of
incrementing the RVA, just drop the relevant portion of the data. I.e.
something like
```
Content = Storage.slice(Descriptor.DataSize);
Storage = Storage.drop_front(Descriptor.Datasize);
```
And then (I think) you don't need the RVA member.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,95 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ /// Class the provides an iterator over the memory64 memory ranges. Only the
+ /// the first descriptor is validated as readable beforehand.
+ class Memory64Iterator {
+ public:
+static Memory64Iterator
+begin(ArrayRef Storage,
+ ArrayRef Descriptors,
+ uint64_t BaseRVA) {
+ return Memory64Iterator(Storage, Descriptors, BaseRVA);
+}
+
+static Memory64Iterator end() { return Memory64Iterator(); }
+
+std::pair> Current;
+
+bool operator==(const Memory64Iterator &R) const {
+ return IsEnd == R.IsEnd;
+}
+
+bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
+
+const std::pair> &
+operator*() {
+ return Current;
+}
+
+const std::pair> *
+operator&() {
+ return &Current;
+}
+
+Error inc() {
+ if (Storage.size() == 0 || Descriptors.size() == 0)
+return make_error("No Memory64List Stream",
+ object_error::parse_failed);
+
+ if (Index >= Descriptors.size())
+return createError("Can't read past of Memory64List Stream");
labath wrote:
This should probably be an assertion. The caller shouldn't be incrementing an
`end()` iterator, right?
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,95 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ /// Class the provides an iterator over the memory64 memory ranges. Only the
+ /// the first descriptor is validated as readable beforehand.
+ class Memory64Iterator {
+ public:
+static Memory64Iterator
+begin(ArrayRef Storage,
+ ArrayRef Descriptors,
+ uint64_t BaseRVA) {
+ return Memory64Iterator(Storage, Descriptors, BaseRVA);
+}
+
+static Memory64Iterator end() { return Memory64Iterator(); }
+
+std::pair> Current;
+
+bool operator==(const Memory64Iterator &R) const {
+ return IsEnd == R.IsEnd;
+}
+
+bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
+
+const std::pair> &
+operator*() {
+ return Current;
+}
+
+const std::pair> *
+operator&() {
+ return &Current;
+}
+
+Error inc() {
+ if (Storage.size() == 0 || Descriptors.size() == 0)
labath wrote:
These should be caught by the checks below: for `Descriptors` the assertion for
incrementing end iterators, for `Storage`, the bounds check on line 183.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,95 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ /// Class the provides an iterator over the memory64 memory ranges. Only the
+ /// the first descriptor is validated as readable beforehand.
+ class Memory64Iterator {
+ public:
+static Memory64Iterator
+begin(ArrayRef Storage,
+ ArrayRef Descriptors,
+ uint64_t BaseRVA) {
+ return Memory64Iterator(Storage, Descriptors, BaseRVA);
+}
+
+static Memory64Iterator end() { return Memory64Iterator(); }
+
+std::pair> Current;
labath wrote:
This should be private.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -336,3 +336,89 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
0xab, 0xad, 0xca, 0xfe}),
*ExpectedContext);
}
+
+TEST(MinidumpYAML, MemoryRegion_64bit) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile &File = **ExpectedFile;
+
+ ASSERT_THAT(1u, File.streams().size());
+
+ Error Err = Error::success();
+ // Explicit Err check
+ ASSERT_FALSE(Err);
+ Expected>
+ ExpectedMemoryList = File.getMemory64List(Err);
+
+ ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
+
+ iterator_range MemoryList =
+ *ExpectedMemoryList;
+ auto Iterator = MemoryList.begin();
+
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
+ auto DescOnePair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescOne = DescOnePair.first;
+ ASSERT_THAT(0x7FCF0818283u, DescOne.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescOne.DataSize);
+
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
+ auto DescTwoPair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescTwo = DescTwoPair.first;
+ ASSERT_THAT(0x7FFF0818283u, DescTwo.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescTwo.DataSize);
+ const std::optional> ExpectedContent =
+ File.getRawStream(StreamType::Memory64List);
+ ASSERT_TRUE(ExpectedContent);
+ const size_t ExpectedStreamSize =
+ sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
+ ASSERT_THAT(ExpectedStreamSize, ExpectedContent->size());
+
+ Expected ExpectedHeader =
+ File.getMemoryList64Header();
+ ASSERT_THAT_EXPECTED(ExpectedHeader, Succeeded());
+ ASSERT_THAT(ExpectedHeader->BaseRVA, 92u);
+
+ Expected> DescOneExpectedContentSlice = DescOnePair.second;
+ ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
+ ASSERT_THAT(5u, DescOneExpectedContentSlice->size());
labath wrote:
and here
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,95 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ /// Class the provides an iterator over the memory64 memory ranges. Only the
+ /// the first descriptor is validated as readable beforehand.
+ class Memory64Iterator {
+ public:
+static Memory64Iterator
+begin(ArrayRef Storage,
+ ArrayRef Descriptors,
+ uint64_t BaseRVA) {
+ return Memory64Iterator(Storage, Descriptors, BaseRVA);
+}
+
+static Memory64Iterator end() { return Memory64Iterator(); }
+
+std::pair> Current;
+
+bool operator==(const Memory64Iterator &R) const {
+ return IsEnd == R.IsEnd;
+}
+
+bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
+
+const std::pair> &
+operator*() {
+ return Current;
+}
+
+const std::pair> *
+operator&() {
+ return &Current;
+}
+
+Error inc() {
+ if (Storage.size() == 0 || Descriptors.size() == 0)
+return make_error("No Memory64List Stream",
+ object_error::parse_failed);
+
+ if (Index >= Descriptors.size())
+return createError("Can't read past of Memory64List Stream");
+
+ const minidump::MemoryDescriptor_64 &Descriptor = Descriptors[Index];
+ if (RVA + Descriptor.DataSize > Storage.size())
+return make_error(
+"Memory64 Descriptor exceeds end of file.",
+object_error::unexpected_eof);
+
+ ArrayRef Content = Storage.slice(RVA, Descriptor.DataSize);
+ Current = std::make_pair(Descriptor, Content);
+ Index++;
+ RVA += Descriptor.DataSize;
+ if (Index >= Descriptors.size())
+IsEnd = true;
+ return Error::success();
+}
+
+ private:
+// This constructor will only happen after a validation check to see
+// if the first descriptor is readable.
+Memory64Iterator(ArrayRef Storage,
+ ArrayRef Descriptors,
+ uint64_t BaseRVA)
+: RVA(BaseRVA), Storage(Storage), Descriptors(Descriptors),
+ IsEnd(false) {
+ assert(Descriptors.size() > 0);
+ assert(Storage.size() >= BaseRVA + Descriptors.front().DataSize);
labath wrote:
Instead of the assertions, maybe just pass the correct member values directly.
Something like:
```
Memory64Iterator(minidump::MemoryDescriptor_64 CurrentDescriptors,
ArrayRef CurrentData,
ArrayRef
RemainingDescriptors,
ArrayRef RemainingData);
```
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -494,6 +528,32 @@ Stream::create(const Directory &StreamDesc, const
object::MinidumpFile &File) {
}
return std::make_unique(std::move(Ranges));
}
+ case StreamKind::Memory64List: {
+// Error, unlike expected is true in failure state
+Error Err = Error::success();
+// Explicit check on Err so that if we return due to getmemory64list
+// getting an error, it's not destructed when unchecked.
+if (Err)
+ return Err;
+auto ExpectedList = File.getMemory64List(Err);
+if (!ExpectedList)
+ return ExpectedList.takeError();
+std::vector Ranges;
+for (auto It = ExpectedList->begin(); It != ExpectedList->end(); ++It) {
labath wrote:
It should be possible to use a range based for to implement this loop, and the
fact that you needed to check the error inside the loop makes me suspect there
is something wrong with the loop termination condition. Maybe you need to
explicitly set the iterator to the "end" state so that this condition fires?
```
friend bool operator==(const fallible_iterator &LHS,
const fallible_iterator &RHS) {
// If both iterators are in the end state they compare
// equal, regardless of whether either is valid.
if (LHS.isEnd() && RHS.isEnd())
return true;
```
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -336,3 +336,89 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
0xab, 0xad, 0xca, 0xfe}),
*ExpectedContext);
}
+
+TEST(MinidumpYAML, MemoryRegion_64bit) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile &File = **ExpectedFile;
+
+ ASSERT_THAT(1u, File.streams().size());
labath wrote:
The order here should be reversed.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -336,3 +336,89 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
0xab, 0xad, 0xca, 0xfe}),
*ExpectedContext);
}
+
+TEST(MinidumpYAML, MemoryRegion_64bit) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile &File = **ExpectedFile;
+
+ ASSERT_THAT(1u, File.streams().size());
+
+ Error Err = Error::success();
+ // Explicit Err check
+ ASSERT_FALSE(Err);
+ Expected>
+ ExpectedMemoryList = File.getMemory64List(Err);
+
+ ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
+
+ iterator_range MemoryList =
+ *ExpectedMemoryList;
+ auto Iterator = MemoryList.begin();
+
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
+ auto DescOnePair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescOne = DescOnePair.first;
+ ASSERT_THAT(0x7FCF0818283u, DescOne.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescOne.DataSize);
+
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
+ auto DescTwoPair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescTwo = DescTwoPair.first;
+ ASSERT_THAT(0x7FFF0818283u, DescTwo.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescTwo.DataSize);
+ const std::optional> ExpectedContent =
+ File.getRawStream(StreamType::Memory64List);
+ ASSERT_TRUE(ExpectedContent);
+ const size_t ExpectedStreamSize =
+ sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
+ ASSERT_THAT(ExpectedStreamSize, ExpectedContent->size());
+
+ Expected ExpectedHeader =
+ File.getMemoryList64Header();
+ ASSERT_THAT_EXPECTED(ExpectedHeader, Succeeded());
+ ASSERT_THAT(ExpectedHeader->BaseRVA, 92u);
+
+ Expected> DescOneExpectedContentSlice = DescOnePair.second;
+ ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
+ ASSERT_THAT(5u, DescOneExpectedContentSlice->size());
+ ASSERT_THAT(arrayRefFromStringRef("hello"), *DescOneExpectedContentSlice);
+
+ Expected> DescTwoExpectedContentSlice = DescTwoPair.second;
+ ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
+ ASSERT_THAT(arrayRefFromStringRef("world"), *DescTwoExpectedContentSlice);
labath wrote:
and here.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
labath wrote:
> I haven't found the mechanism that an armv7 lldb-server would report
> `reason:trace` when it emulated the current instruction, put a breakpoint on
> the next instruction we'll execute, and 'continued' the thread when we
> resume. But I might have just missed something in ProcessNativeLinux where it
> knows it was instruction stepping. (but in the old stepping model in lldb,
> when you're at a BreakpointSite, the stop reason would be "breakpoint hit"
> even if it hadn't actually been hit)
I think that's this code here (I have missed it at first as well):
```
void NativeProcessLinux::MonitorBreakpoint(NativeThreadLinux &thread) {
Log *log = GetLog(LLDBLog::Process | LLDBLog::Breakpoints);
LLDB_LOG(log, "received breakpoint event, pid = {0}", thread.GetID());
// Mark the thread as stopped at breakpoint.
thread.SetStoppedByBreakpoint();
FixupBreakpointPCAsNeeded(thread);
if (m_threads_stepping_with_breakpoint.find(thread.GetID()) !=
m_threads_stepping_with_breakpoint.end())
thread.SetStoppedByTrace();
StopRunningThreads(thread.GetID());
}
```
I think this is the correct thing to do, since lldb is completely unaware of
whether software single stepping is taking place(*) I got lost in all of the
mach exceptions, so it's not clear to me what is the conclusion. Are you saying
that debugserver does not do this (i.e., it reports the stop as a "breakpoint
hit", in the mach exception form)?
Just as a curiosity the linux kernel does allow you do use the hardware single
step capability if debugging a 32 bit application on a 64-bit kernel (I'm not
sure if the debugger process needs to be 64-bit or not). At one point, I wanted
to make use of that, which would make our stepping behavior more reliable in
these situations. However it would also make it harder to reproduce situations
like these, as one would have to find actual 32-bit hardware...
(*) The code is probably too casual about this check though, in that it does
not check whether it was the actual single-stepping breakpoint that got hit. If
there was a breakpoint under the current PC, it would report this as "trace"
even though it hasn't actually stepped anything. Could this maybe be the
problem that you ran into with this patch?
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData support any JSON type (PR #101929)
@@ -110,6 +110,23 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+example = lldb.SBStructuredData()
+
+# Check SetFromJSON API for dictionaries, integers, floating point
+# values, strings and arrays
+error = example.SetFromJSON(s)
+if not error.Success():
+self.fail("FAILED: " + error.GetCString())
labath wrote:
I see you've resolved this comment, but I don't actually understand how. Is
your code in some way better than assertSuccess (which exists precisely so it
can print the error message when the SBError is in failure state)?
https://github.com/llvm/llvm-project/pull/101929
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData support any JSON type (PR #101929)
@@ -110,6 +110,23 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+example = lldb.SBStructuredData()
+
+# Check SetFromJSON API for dictionaries, integers, floating point
+# values, strings and arrays
+error = example.SetFromJSON(s)
+if not error.Success():
+self.fail("FAILED: " + error.GetCString())
labath wrote:
Ah, maybe I see... You've changed one instance to assertSucess, but then added
another one with the old pattern, right?
If so, then just change the other one as well, and this is good to go :)
https://github.com/llvm/llvm-project/pull/101929
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/Plugins] Introduce Scripted Platform Plugin (PR #99814)
labath wrote: > I'd also love to hear other people opinion on extending platforms through > ScriptedPlatform (cc @labath @jimingham @JDevlieghere ) I think that having the ability to forward operations to existing platform instances would be very useful. If you look at PlatformQemuUser, most of its functions just forward operations to be host platform. We also have another downstream plugin which looks very similar to that. I would love if both of these could be lightweight python wrappers. That said, I'm not exactly sure how Greg's idea would work, or if it's even needed. Some of this functionality without any special support, by just having an `SBPlatform` member inside the python class. This would be more flexible as you could call it only when it suits you (going with the Qemu example, the emulator mostly exposes the host filesystem, but it has some special remapping logic for system libraries (like LD_LIBRARY_PATH, but implemented in the emulator), so we may want to implement get/putfile mostly by deferring to the host, and only doing something custom for special paths). And if you really wanted to you could make a python wrapper class that makes all of this looks like (python) inheritance -- without any special support in lldb. The main flaw in this idea is that the SBPlatform interface is pretty small compared to the internal Platform interface. I'm guessing the python class will be implementing something like the internal interface, while it will only have the external API to work with... https://github.com/llvm/llvm-project/pull/99814 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
https://github.com/labath commented: I could of high-level comments, I don't want to get too involved in the design of this feature. https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
@@ -0,0 +1,63 @@
+//===-- RealpathPrefixes.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_CORE_REALPATHPREFIXES_H
+#define LLDB_CORE_REALPATHPREFIXES_H
+
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include
+#include
+#include
+
+namespace lldb_private {
+class FileSpec;
+class FileSpecList;
+class Target;
+} // namespace lldb_private
+
+namespace lldb_private {
+
+class RealpathPrefixes {
+public:
+ // Prefixes are obtained from FileSpecList, through FileSpec::GetPath(),
which
+ // ensures that the paths are normalized. For example:
+ // "./foo/.." -> ""
+ // "./foo/../bar" -> "bar"
+ explicit RealpathPrefixes(const FileSpecList &file_spec_list);
+
+ // Sets an optional filesystem to use for realpath'ing. If not set, the real
+ // filesystem will be used.
+ void SetFileSystem(llvm::IntrusiveRefCntPtr fs);
+
+ // Sets an optional Target instance to gather statistics.
+ void SetTarget(Target *target) { m_target = target; }
labath wrote:
Could these be set directly in the constructor (maybe as optional arguments)?
From what I can tell they're only used to override values in tests, and
immutable classes are much easier to reason about.
https://github.com/llvm/llvm-project/pull/102223
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
@@ -0,0 +1,66 @@ +//===-- RealpathPrefixes.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/Utility/RealpathPrefixes.h" + +#include "lldb/Target/Statistics.h" +#include "lldb/Target/Target.h" labath wrote: This is a layering violation. Code in Utility should not depend on Target stuff. If you want this to be a Utility class, you should use some kind of dependency inversion technique to break this dependency. https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem] Pass ClangASTMetadata by-value when creating record/objc types (PR #102296)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/102296
This is a follow-up to https://github.com/llvm/llvm-project/pull/102161 where
we changed the `GetMetadata`/`SetMetadata` APIs to pass `ClangASTMetadata`
by-value, instead of `ClangASTMetadata *`, which wasn't a very friendly API.
This patch continues from there and changes
`CreateRecordType`/`CreateObjCClass` to take the metadata by-value as well.
As a drive-by change, I also changed `DelayedAddObjCClassProperty` to store the
metadata by-value, instead of in a `std::unique_ptr`, which AFAICT, was done
solely due to the TypeSystemClang APIs taking the metadata by pointer. This
meant we could also get rid off the user-provided copy constructors.
>From 45c8f9534ec0ddc50c825fa7a3d048746b74360c Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 7 Aug 2024 11:03:25 +0100
Subject: [PATCH] [lldb][TypeSystem] Pass ClangASTMetadata by-value when
creating record/objc types
This is a follow-up to https://github.com/llvm/llvm-project/pull/102161
where we changed the `GetMetadata`/`SetMetadata` APIs to pass
`ClangASTMetadata` by-value, instead of `ClangASTMetadata *`, which
wasn't a very friendly API.
This patch continues from there and changes
`CreateRecordType`/`CreateObjCClass` to take the metadata by-value as well.
As a drive-by change, I also changed `DelayedAddObjCClassProperty` to
store the metadata by-value, instead of in a `std::unique_ptr`, which
AFAICT, was done solely due to the TypeSystemClang APIs taking the
metadata by pointer.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 39 ---
.../SymbolFile/NativePDB/PdbAstBuilder.cpp| 2 +-
.../NativePDB/UdtRecordCompleter.cpp | 2 +-
.../Plugins/SymbolFile/PDB/PDBASTParser.cpp | 2 +-
.../TypeSystem/Clang/TypeSystemClang.cpp | 16
.../TypeSystem/Clang/TypeSystemClang.h| 28 ++---
6 files changed, 31 insertions(+), 58 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a4dcde1629fc21..1a13725b99804a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1803,7 +1803,7 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
if (!clang_type) {
clang_type = m_ast.CreateRecordType(
containing_decl_ctx, GetOwningClangModule(die), attrs.accessibility,
-attrs.name.GetCString(), tag_decl_kind, attrs.class_language,
&metadata,
+attrs.name.GetCString(), tag_decl_kind, attrs.class_language, metadata,
attrs.exports_symbols);
}
@@ -1883,43 +1883,18 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
// required if you don't have
an
// ivar decl
const char *property_setter_name, const char *property_getter_name,
- uint32_t property_attributes, const ClangASTMetadata *metadata)
+ uint32_t property_attributes, ClangASTMetadata metadata)
: m_class_opaque_type(class_opaque_type), m_property_name(property_name),
m_property_opaque_type(property_opaque_type),
m_property_setter_name(property_setter_name),
m_property_getter_name(property_getter_name),
-m_property_attributes(property_attributes) {
-if (metadata != nullptr) {
- m_metadata_up = std::make_unique();
- *m_metadata_up = *metadata;
-}
- }
-
- DelayedAddObjCClassProperty(const DelayedAddObjCClassProperty &rhs) {
-*this = rhs;
- }
-
- DelayedAddObjCClassProperty &
- operator=(const DelayedAddObjCClassProperty &rhs) {
-m_class_opaque_type = rhs.m_class_opaque_type;
-m_property_name = rhs.m_property_name;
-m_property_opaque_type = rhs.m_property_opaque_type;
-m_property_setter_name = rhs.m_property_setter_name;
-m_property_getter_name = rhs.m_property_getter_name;
-m_property_attributes = rhs.m_property_attributes;
-
-if (rhs.m_metadata_up) {
- m_metadata_up = std::make_unique();
- *m_metadata_up = *rhs.m_metadata_up;
-}
-return *this;
- }
+m_property_attributes(property_attributes), m_metadata(metadata) {}
bool Finalize() {
return TypeSystemClang::AddObjCClassProperty(
m_class_opaque_type, m_property_name, m_property_opaque_type,
/*ivar_decl=*/nullptr, m_property_setter_name, m_property_getter_name,
-m_property_attributes, m_metadata_up.get());
+m_property_attributes, m_metadata);
}
private:
@@ -1929,7 +1904,7 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
const char *m_property_setter_name;
const char *m_property_getter_name;
uint32_t m_property_attributes;
- std::unique_ptr m_metadata_up;
+ ClangASTMetadata m_metadata;
};
bool DWARFASTParserClang::ParseTemplateDIE(
@@ -2721,10 +2
[Lldb-commits] [lldb] [lldb][TypeSystem] Pass ClangASTMetadata by-value when creating record/objc types (PR #102296)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/102296 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem] Pass ClangASTMetadata by-value when creating record/objc types (PR #102296)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
Changes
This is a follow-up to https://github.com/llvm/llvm-project/pull/102161 where
we changed the `GetMetadata`/`SetMetadata` APIs to pass `ClangASTMetadata`
by-value, instead of `ClangASTMetadata *`, which wasn't a very friendly API.
This patch continues from there and changes
`CreateRecordType`/`CreateObjCClass` to take the metadata by-value as well.
As a drive-by change, I also changed `DelayedAddObjCClassProperty` to store the
metadata by-value, instead of in a `std::unique_ptr`, which AFAICT, was done
solely due to the TypeSystemClang APIs taking the metadata by pointer. This
meant we could also get rid off the user-provided copy constructors.
---
Full diff: https://github.com/llvm/llvm-project/pull/102296.diff
6 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
(+7-32)
- (modified) lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp (+1-1)
- (modified) lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
(+1-1)
- (modified) lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp (+1-1)
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+7-9)
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+14-14)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a4dcde1629fc21..1a13725b99804a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1803,7 +1803,7 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
if (!clang_type) {
clang_type = m_ast.CreateRecordType(
containing_decl_ctx, GetOwningClangModule(die), attrs.accessibility,
-attrs.name.GetCString(), tag_decl_kind, attrs.class_language,
&metadata,
+attrs.name.GetCString(), tag_decl_kind, attrs.class_language, metadata,
attrs.exports_symbols);
}
@@ -1883,43 +1883,18 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
// required if you don't have
an
// ivar decl
const char *property_setter_name, const char *property_getter_name,
- uint32_t property_attributes, const ClangASTMetadata *metadata)
+ uint32_t property_attributes, ClangASTMetadata metadata)
: m_class_opaque_type(class_opaque_type), m_property_name(property_name),
m_property_opaque_type(property_opaque_type),
m_property_setter_name(property_setter_name),
m_property_getter_name(property_getter_name),
-m_property_attributes(property_attributes) {
-if (metadata != nullptr) {
- m_metadata_up = std::make_unique();
- *m_metadata_up = *metadata;
-}
- }
-
- DelayedAddObjCClassProperty(const DelayedAddObjCClassProperty &rhs) {
-*this = rhs;
- }
-
- DelayedAddObjCClassProperty &
- operator=(const DelayedAddObjCClassProperty &rhs) {
-m_class_opaque_type = rhs.m_class_opaque_type;
-m_property_name = rhs.m_property_name;
-m_property_opaque_type = rhs.m_property_opaque_type;
-m_property_setter_name = rhs.m_property_setter_name;
-m_property_getter_name = rhs.m_property_getter_name;
-m_property_attributes = rhs.m_property_attributes;
-
-if (rhs.m_metadata_up) {
- m_metadata_up = std::make_unique();
- *m_metadata_up = *rhs.m_metadata_up;
-}
-return *this;
- }
+m_property_attributes(property_attributes), m_metadata(metadata) {}
bool Finalize() {
return TypeSystemClang::AddObjCClassProperty(
m_class_opaque_type, m_property_name, m_property_opaque_type,
/*ivar_decl=*/nullptr, m_property_setter_name, m_property_getter_name,
-m_property_attributes, m_metadata_up.get());
+m_property_attributes, m_metadata);
}
private:
@@ -1929,7 +1904,7 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
const char *m_property_setter_name;
const char *m_property_getter_name;
uint32_t m_property_attributes;
- std::unique_ptr m_metadata_up;
+ ClangASTMetadata m_metadata;
};
bool DWARFASTParserClang::ParseTemplateDIE(
@@ -2721,10 +2696,10 @@ void DWARFASTParserClang::ParseObjCProperty(
ClangASTMetadata metadata;
metadata.SetUserID(die.GetID());
- delayed_properties.push_back(DelayedAddObjCClassProperty(
+ delayed_properties.emplace_back(
class_clang_type, propAttrs.prop_name,
member_type->GetLayoutCompilerType(), propAttrs.prop_setter_name,
- propAttrs.prop_getter_name, propAttrs.prop_attributes, &metadata));
+ propAttrs.prop_getter_name, propAttrs.prop_attributes, metadata);
}
llvm::Expected DWARFASTParserClang::ExtractIntFromFormValue(
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
b/lldb/source/Plu
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
https://github.com/Michael137 approved this pull request. LGTM, thanks! So the actual problem is that the ASTImporter will only copy over the definition to the destination decl if the source decl `isCompleteDefinition`. So then when we tried to add the typedef decl to the `CXXRecordDecl` that the ASTImporter created for `A`, it would try to query its definition which didn't get copied over, and hence we assert. https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -0,0 +1,49 @@ +// REQUIRES: system-linux, native labath wrote: This test still requires linux a bunch of system dependencies. Here's a self-contained version that can be run anywhere (just run the input through yaml2obj and give it to lldb). I took `test/tools/llvm-readobj/ELF/needed-libs.test:` and adapted it for our use case -- I added DT_DEBUG as that's important for lldb, but you can also easily add any other tag you deem necessary ``` --- !ELF FileHeader: Class: ELFCLASS64 Data:ELFDATA2LSB Type:ET_EXEC Machine: EM_X86_64 Sections: - Type: SectionHeaderTable NoHeaders: true - Name: .dynstr Type: SHT_STRTAB Flags: [ SHF_ALLOC ] Content: '00616161006262620063636300' ## 0,a,a,a,0,b,b,b,0,c,c,c,0 Size:0x100 - Name:.dynamic Type:SHT_DYNAMIC Address: 0x100 Entries: - Tag: DT_STRTAB Value: 0x - Tag: DT_NEEDED Value: 9 - Tag: DT_NEEDED Value: 1 - Tag: DT_NEEDED Value: 5 - Tag: DT_STRSZ Value: 0x100 - Tag: DT_DEBUG Value: 0xdeadbeef - Tag: DT_NULL Value: 0x0 ProgramHeaders: - Type: PT_LOAD VAddr: 0x0 FirstSec: .dynstr LastSec: .dynamic - Type:PT_DYNAMIC FirstSec:.dynamic LastSec: .dynamic VAddr: 0x100 Align: 0x8 ``` https://github.com/llvm/llvm-project/pull/101237 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -3704,3 +3802,88 @@ ObjectFileELF::MapFileDataWritable(const FileSpec &file,
uint64_t Size,
return FileSystem::Instance().CreateWritableDataBuffer(file.GetPath(), Size,
Offset);
}
+
+std::optional ObjectFileELF::GetDynstrData() {
+ if (SectionList *section_list = GetSectionList()) {
+// Find the SHT_DYNAMIC section.
+if (Section *dynamic =
+section_list
+->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true)
+.get()) {
+ assert(dynamic->GetObjectFile() == this);
+ if (const ELFSectionHeaderInfo *header =
+ GetSectionHeaderByIndex(dynamic->GetID())) {
+// sh_link: section header index of string table used by entries in
+// the section.
+if (Section *dynstr =
+section_list->FindSectionByID(header->sh_link).get()) {
+DataExtractor data;
+ if (ReadSectionData(dynstr, data))
+ return data;
+}
+ }
+}
+ }
+
+ // Every ELF file which represents an executable or shared library has
+ // mandatory .dynamic entries. Two of these values are DT_STRTAB and DT_STRSZ
+ // and represent the dynamic symbol tables's string table. These are needed
+ // by the dynamic loader and we can read them from a process' address space.
+ //
+ // When loading and ELF file from memory, only the program headers end up
+ // being mapped into memory, and we can find these values in the PT_DYNAMIC
+ // segment.
+ const ELFDynamic *strtab = FindDynamicSymbol(DT_STRTAB);
+ const ELFDynamic *strsz = FindDynamicSymbol(DT_STRSZ);
+ if (strtab == nullptr || strsz == nullptr)
+return std::nullopt;
+
+ if (ProcessSP process_sp = m_process_wp.lock()) {
+if (DataBufferSP data_sp =
+ReadMemory(process_sp, strtab->d_ptr, strsz->d_val))
+ return DataExtractor(data_sp, GetByteOrder(), GetAddressByteSize());
+ } else {
+// We have an ELF file with no section headers or we didn't find the
+// .dynamic section. Try and find the .dynstr section.
+Address addr;
+if (addr.ResolveAddressUsingFileSections(strtab->d_ptr, GetSectionList()))
{
+ DataExtractor data;
+ addr.GetSection()->GetSectionData(data);
+ return DataExtractor(data,
+strtab->d_ptr - addr.GetSection()->GetFileOffset(),
labath wrote:
```suggestion
strtab->d_ptr - addr.GetSection()->GetFileAddress(),
```
(fixes a bug I found while working on the test case, unlikely to have been
caught for real object files, as their first segment always starts at zero)
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -3802,9 +3817,9 @@ std::optional
ObjectFileELF::GetDynstrData() {
// the section.
if (Section *dynstr =
section_list->FindSectionByID(header->sh_link).get()) {
- DataExtractor data;
+DataExtractor data;
if (ReadSectionData(dynstr, data))
-return data;
+ return data;
labath wrote:
It looks like something went wrong with the formatting here.
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
@@ -788,7 +808,10 @@ Type::GetTypeScopeAndBasename(llvm::StringRef name) {
switch (pos.value()) {
case ':':
if (prev_is_colon && template_depth == 0) {
-result.scope.push_back(name.slice(name_begin, pos.index() - 1));
+llvm::StringRef scope_name = name.slice(name_begin, pos.index() - 1);
labath wrote:
> Do I understand correctly that we could've kept this as-is, but would then
> have to check for the (anonymous namespace) string in the ContextMatches
> function?
That's correct, except that we would need to check for this string and still
also the empty string (as that's how it comes out of DWARF). I figured its best
to standardize this as early as possible (and an empty string seems like a less
language-specific version).
https://github.com/llvm/llvm-project/pull/102111
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
@@ -145,10 +159,16 @@ bool TypeQuery::ContextMatches(
++pat;
}
- // Skip over any remaining module entries if we were asked to do that.
- while (GetIgnoreModules() && ctx != ctx_end &&
- ctx->kind == CompilerContextKind::Module)
-++ctx;
+ // Skip over any remaining module and anonymous namespace entries if we were
+ // asked to do that.
+ auto should_skip = [this](const CompilerContext &ctx) {
+if (ctx.kind == CompilerContextKind::Module)
+ return GetIgnoreModules();
+if (ctx.kind == CompilerContextKind::Namespace && ctx.name.IsEmpty())
labath wrote:
In the first version of the patch I had an `IsAnonymousNamespace` and
`MatchesAnonymousNamespace` (the difference being in how they test the `kind`
bits) . This was before I changed everything to use an empty string, after
which it did not seem that useful...
https://github.com/llvm/llvm-project/pull/102111
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
@@ -266,6 +269,11 @@ class TypeQuery {
bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; }
void SetIgnoreModules() { m_options &= ~e_ignore_modules; }
+ bool GetStrictNamespaces() const {
+return (m_options & e_strict_namespaces) != 0;
+ }
+ void SetStrictNamespaces() { m_options &= ~e_strict_namespaces; }
labath wrote:
Oops :facepalm:
I don't know what I was thinking. I'll fix that and the two other
implementations.
https://github.com/llvm/llvm-project/pull/102111
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem] Pass ClangASTMetadata by-value when creating record/objc types (PR #102296)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/102296 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
labath wrote: > LGTM, thanks! > > So the actual problem is that the ASTImporter will only copy over the > definition to the destination decl if the source decl `isCompleteDefinition`. > So then when we tried to add the typedef decl to the `CXXRecordDecl` that the > ASTImporter created for `A`, it would try to query its definition which > didn't get copied over (cause we only ever started the definition for `A`, > but nothing completed it, especially since it had no external storage), and > hence we assert. No, the problem was that `A` was stuck in this half-complete state, where we've begun -- but not finished -- it's definition, and it also didn't have an external ast source which could complete it (because we've already tried completing it, and CompleteTypeFromDWARF has cleared the "external" flag). AFAICT, the ast importer is just not prepared to handle this situation. The crash happened when it tried to query some property (probably to copy it over) of the source Decl, and this crashed because the decl didn't have the DefinitionData field set up. https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem] Pass ClangASTMetadata by-value when creating record/objc types (PR #102296)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/102296
>From 45c8f9534ec0ddc50c825fa7a3d048746b74360c Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 7 Aug 2024 11:03:25 +0100
Subject: [PATCH 1/2] [lldb][TypeSystem] Pass ClangASTMetadata by-value when
creating record/objc types
This is a follow-up to https://github.com/llvm/llvm-project/pull/102161
where we changed the `GetMetadata`/`SetMetadata` APIs to pass
`ClangASTMetadata` by-value, instead of `ClangASTMetadata *`, which
wasn't a very friendly API.
This patch continues from there and changes
`CreateRecordType`/`CreateObjCClass` to take the metadata by-value as well.
As a drive-by change, I also changed `DelayedAddObjCClassProperty` to
store the metadata by-value, instead of in a `std::unique_ptr`, which
AFAICT, was done solely due to the TypeSystemClang APIs taking the
metadata by pointer.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 39 ---
.../SymbolFile/NativePDB/PdbAstBuilder.cpp| 2 +-
.../NativePDB/UdtRecordCompleter.cpp | 2 +-
.../Plugins/SymbolFile/PDB/PDBASTParser.cpp | 2 +-
.../TypeSystem/Clang/TypeSystemClang.cpp | 16
.../TypeSystem/Clang/TypeSystemClang.h| 28 ++---
6 files changed, 31 insertions(+), 58 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a4dcde1629fc2..1a13725b99804 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1803,7 +1803,7 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
if (!clang_type) {
clang_type = m_ast.CreateRecordType(
containing_decl_ctx, GetOwningClangModule(die), attrs.accessibility,
-attrs.name.GetCString(), tag_decl_kind, attrs.class_language,
&metadata,
+attrs.name.GetCString(), tag_decl_kind, attrs.class_language, metadata,
attrs.exports_symbols);
}
@@ -1883,43 +1883,18 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
// required if you don't have
an
// ivar decl
const char *property_setter_name, const char *property_getter_name,
- uint32_t property_attributes, const ClangASTMetadata *metadata)
+ uint32_t property_attributes, ClangASTMetadata metadata)
: m_class_opaque_type(class_opaque_type), m_property_name(property_name),
m_property_opaque_type(property_opaque_type),
m_property_setter_name(property_setter_name),
m_property_getter_name(property_getter_name),
-m_property_attributes(property_attributes) {
-if (metadata != nullptr) {
- m_metadata_up = std::make_unique();
- *m_metadata_up = *metadata;
-}
- }
-
- DelayedAddObjCClassProperty(const DelayedAddObjCClassProperty &rhs) {
-*this = rhs;
- }
-
- DelayedAddObjCClassProperty &
- operator=(const DelayedAddObjCClassProperty &rhs) {
-m_class_opaque_type = rhs.m_class_opaque_type;
-m_property_name = rhs.m_property_name;
-m_property_opaque_type = rhs.m_property_opaque_type;
-m_property_setter_name = rhs.m_property_setter_name;
-m_property_getter_name = rhs.m_property_getter_name;
-m_property_attributes = rhs.m_property_attributes;
-
-if (rhs.m_metadata_up) {
- m_metadata_up = std::make_unique();
- *m_metadata_up = *rhs.m_metadata_up;
-}
-return *this;
- }
+m_property_attributes(property_attributes), m_metadata(metadata) {}
bool Finalize() {
return TypeSystemClang::AddObjCClassProperty(
m_class_opaque_type, m_property_name, m_property_opaque_type,
/*ivar_decl=*/nullptr, m_property_setter_name, m_property_getter_name,
-m_property_attributes, m_metadata_up.get());
+m_property_attributes, m_metadata);
}
private:
@@ -1929,7 +1904,7 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
const char *m_property_setter_name;
const char *m_property_getter_name;
uint32_t m_property_attributes;
- std::unique_ptr m_metadata_up;
+ ClangASTMetadata m_metadata;
};
bool DWARFASTParserClang::ParseTemplateDIE(
@@ -2721,10 +2696,10 @@ void DWARFASTParserClang::ParseObjCProperty(
ClangASTMetadata metadata;
metadata.SetUserID(die.GetID());
- delayed_properties.push_back(DelayedAddObjCClassProperty(
+ delayed_properties.emplace_back(
class_clang_type, propAttrs.prop_name,
member_type->GetLayoutCompilerType(), propAttrs.prop_setter_name,
- propAttrs.prop_getter_name, propAttrs.prop_attributes, &metadata));
+ propAttrs.prop_getter_name, propAttrs.prop_attributes, metadata);
}
llvm::Expected DWARFASTParserClang::ExtractIntFromFormValue(
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
b/lldb/source/Plugins/SymbolFile
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
https://github.com/labath updated
https://github.com/llvm/llvm-project/pull/102111
>From c6936f3d5d0592babe9082e867b179af594c447b Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Tue, 6 Aug 2024 09:51:20 +0200
Subject: [PATCH 1/3] [lldb] Better matching of types in anonymous namespaces
This patch extends TypeQuery matching to support anonymous namespaces. A
new flag is added to control the behavior. In the "strict" mode, the
query must match the type exactly -- all anonymous namespaces included.
The dynamic type resolver in the itanium abi (the motivating use case
for this) uses this flag, as it queries using the name from the
demangles, which includes anonymous namespaces.
This ensures we don't confuse a type with a same-named type in an
anonymous namespace. However, this does *not* ensure we don't confuse
two types in anonymous namespacs (in different CUs). To resolve this, we
would need to use a completely different lookup algorithm, which
probably also requires a DWARF extension.
In the "lax" mode (the default), the anonymous namespaces in the query
are optional, and this allows one search for the type using the usual
language rules (`::A` matches `::(anonymous namespace)::A`).
This patch also changes the type context computation algorithm in
DWARFDIE, so that it includes anonymous namespace information. This
causes a slight change in behavior: the algorithm previously stopped
computing the context after encountering an anonymous namespace, which
caused the outer namespaces to be ignored. This meant that a type like
`NS::(anonymous namespace)::A` would be (incorrectly) recognized as
`::A`). This can cause code depending on the old behavior to misbehave.
The fix is to specify all the enclosing namespaces in the query, or use
a non-exact match.
---
lldb/include/lldb/Symbol/Type.h | 10 -
.../ItaniumABI/ItaniumABILanguageRuntime.cpp | 1 +
.../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 8 +---
lldb/source/Symbol/Type.cpp | 33 +---
lldb/test/API/lang/cpp/dynamic-value/Makefile | 2 +-
.../cpp/dynamic-value/TestDynamicValue.py | 17 -
lldb/test/API/lang/cpp/dynamic-value/a.h | 25
.../lang/cpp/dynamic-value/anonymous-b.cpp| 15
.../lang/cpp/dynamic-value/pass-to-base.cpp | 38 +--
lldb/unittests/Symbol/TestType.cpp| 22 +++
.../SymbolFile/DWARF/DWARFDIETest.cpp | 24 +++-
11 files changed, 149 insertions(+), 46 deletions(-)
create mode 100644 lldb/test/API/lang/cpp/dynamic-value/a.h
create mode 100644 lldb/test/API/lang/cpp/dynamic-value/anonymous-b.cpp
diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h
index 420307e0dbcf02..021e27b52fca71 100644
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -77,10 +77,13 @@ FLAGS_ENUM(TypeQueryOptions){
/// If set, the query will ignore all Module entries in the type context,
/// even for exact matches.
e_ignore_modules = (1u << 2),
+/// If set, all anonymous namespaces in the context must be matched exactly
+/// by the pattern. Otherwise, superfluous namespaces are skipped.
+e_strict_namespaces = (1u << 3),
/// When true, the find types call should stop the query as soon as a
single
/// matching type is found. When false, the type query should find all
/// matching types.
-e_find_one = (1u << 3),
+e_find_one = (1u << 4),
};
LLDB_MARK_AS_BITMASK_ENUM(TypeQueryOptions)
@@ -266,6 +269,11 @@ class TypeQuery {
bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; }
void SetIgnoreModules() { m_options &= ~e_ignore_modules; }
+ bool GetStrictNamespaces() const {
+return (m_options & e_strict_namespaces) != 0;
+ }
+ void SetStrictNamespaces() { m_options &= ~e_strict_namespaces; }
+
/// The \a m_context can be used in two ways: normal types searching with
/// the context containing a stanadard declaration context for a type, or
/// with the context being more complete for exact matches in clang modules.
diff --git
a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
index 7af768aad0bc19..4c547afe30fe81 100644
---
a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
+++
b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
@@ -90,6 +90,7 @@ TypeAndOrName ItaniumABILanguageRuntime::GetTypeInfo(
TypeResults results;
TypeQuery query(const_lookup_name.GetStringRef(),
TypeQueryOptions::e_exact_match |
+ TypeQueryOptions::e_strict_namespaces |
TypeQueryOptions::e_find_one);
if (module_sp) {
module_sp->FindTypes(query, results);
diff --git a/lldb/source/Plugins/Symbol
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
@@ -111,4 +115,22 @@ TEST(Type, CompilerContextPattern) {
Matches(std::vector{make_class("C")}));
EXPECT_THAT((std::vector{make_namespace("NS"), make_class("C")}),
Not(Matches(std::vector{make_any_type("C")})));
+
+ EXPECT_THAT((std::vector{make_namespace(""), make_class("C")}),
+ Matches(std::vector{make_class("C")}));
+ EXPECT_THAT((std::vector{make_namespace(""), make_class("C")}),
+ Not(MatchesWithStrictNamespaces(std::vector{make_class("C")})));
+ EXPECT_THAT((std::vector{make_namespace(""), make_class("C")}),
+ Matches(std::vector{make_namespace(""), make_class("C")}));
+ EXPECT_THAT((std::vector{make_namespace(""), make_class("C")}),
+ MatchesWithStrictNamespaces(
+ std::vector{make_namespace(""), make_class("C")}));
+ EXPECT_THAT((std::vector{make_class("C")}),
+ Not(Matches(std::vector{make_namespace(""), make_class("C")})));
+ EXPECT_THAT((std::vector{make_class("C")}),
+ Not(MatchesWithStrictNamespaces(
+ std::vector{make_namespace(""), make_class("C")})));
+ EXPECT_THAT((std::vector{make_namespace(""), make_namespace("NS"),
+ make_namespace(""), make_class("C")}),
+ Matches(std::vector{make_namespace("NS"), make_class("C")}));
labath wrote:
I sprinkled some module and namespace tests. Let me know if you think something
is missing.
I think nested anonymous namespaces in mangled names are necessary to support
code like [this](https://godbolt.org/z/M3EW6n4s1). I hope noone writes that for
real, though.
https://github.com/llvm/llvm-project/pull/102111
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
Michael137 wrote: > > LGTM, thanks! > > So the actual problem is that the ASTImporter will only copy over the > > definition to the destination decl if the source decl > > `isCompleteDefinition`. So then when we tried to add the typedef decl to > > the `CXXRecordDecl` that the ASTImporter created for `A`, it would try to > > query its definition which didn't get copied over (cause we only ever > > started the definition for `A`, but nothing completed it, especially since > > it had no external storage), and hence we assert. > > No, the problem was that `A` was stuck in this half-complete state, where > we've begun -- but not finished -- it's definition, and it also didn't have > an external ast source which could complete it (because we've already tried > completing it, and CompleteTypeFromDWARF has cleared the "external" flag). > AFAICT, the ast importer is just not prepared to handle this situation. The > crash happened when it tried to query some property (probably to copy it > over) of the source Decl, and this crashed because the decl didn't have the > DefinitionData field set up. Yea agreed. I was just curious where exactly the difference in the ASTImporter codepath came from with/without the patch. And it looks like without completing the definition, we lose the `DefinitionData` during the import process. What I meant was: we do always allocate `DefinitionData` for `A` in `PrepareContextToRecieveMembers`, but when we imported/copied the decl for `A::X` into the expression AST, the ASTImporter didn't import the definition for `A` because the `isCompleteDefinition` bit wasn't set (which happens [here](https://github.com/llvm/llvm-project/blob/f05e8186a439cf538f383dfbde68eca364a5acec/clang/lib/AST/ASTImporter.cpp#L3322-L3324)). With this patch, we make sure that the call to `ImportDefinition` happens before we try adding the `TypedefDecl` to the `A` context. https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix crash when adding members to an "incomplete" type (PR #102116)
labath wrote: You're right -- I misremembered. I thought the DefinitionData is set when completing the definition, but happens when the definition is started. In that case, yes, the crash happened because the ast importer didn't copy over the definition data, but then tried to access it when adding the typedef. https://github.com/llvm/llvm-project/pull/102116 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 3ac5f5e - [lldb][TypeSystem] Pass ClangASTMetadata by-value when creating record/objc types (#102296)
Author: Michael Buch
Date: 2024-08-07T13:05:45+01:00
New Revision: 3ac5f5e1fc189d0cca169a409199fdaec13112d1
URL:
https://github.com/llvm/llvm-project/commit/3ac5f5e1fc189d0cca169a409199fdaec13112d1
DIFF:
https://github.com/llvm/llvm-project/commit/3ac5f5e1fc189d0cca169a409199fdaec13112d1.diff
LOG: [lldb][TypeSystem] Pass ClangASTMetadata by-value when creating
record/objc types (#102296)
This is a follow-up to https://github.com/llvm/llvm-project/pull/102161
where we changed the `GetMetadata`/`SetMetadata` APIs to pass
`ClangASTMetadata` by-value, instead of `ClangASTMetadata *`, which
wasn't a very friendly API.
This patch continues from there and changes
`CreateRecordType`/`CreateObjCClass` to take the metadata by-value as
well.
As a drive-by change, I also changed `DelayedAddObjCClassProperty` to
store the metadata by-value, instead of in a `std::unique_ptr`, which
AFAICT, was done solely due to the TypeSystemClang APIs taking the
metadata by pointer. This meant we could also get rid of the
user-provided copy constructors.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/unittests/Symbol/TestTypeSystemClang.cpp
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a4dcde1629fc2..1a13725b99804 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1803,7 +1803,7 @@ DWARFASTParserClang::ParseStructureLikeDIE(const
SymbolContext &sc,
if (!clang_type) {
clang_type = m_ast.CreateRecordType(
containing_decl_ctx, GetOwningClangModule(die), attrs.accessibility,
-attrs.name.GetCString(), tag_decl_kind, attrs.class_language,
&metadata,
+attrs.name.GetCString(), tag_decl_kind, attrs.class_language, metadata,
attrs.exports_symbols);
}
@@ -1883,43 +1883,18 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
// required if you don't have
an
// ivar decl
const char *property_setter_name, const char *property_getter_name,
- uint32_t property_attributes, const ClangASTMetadata *metadata)
+ uint32_t property_attributes, ClangASTMetadata metadata)
: m_class_opaque_type(class_opaque_type), m_property_name(property_name),
m_property_opaque_type(property_opaque_type),
m_property_setter_name(property_setter_name),
m_property_getter_name(property_getter_name),
-m_property_attributes(property_attributes) {
-if (metadata != nullptr) {
- m_metadata_up = std::make_unique();
- *m_metadata_up = *metadata;
-}
- }
-
- DelayedAddObjCClassProperty(const DelayedAddObjCClassProperty &rhs) {
-*this = rhs;
- }
-
- DelayedAddObjCClassProperty &
- operator=(const DelayedAddObjCClassProperty &rhs) {
-m_class_opaque_type = rhs.m_class_opaque_type;
-m_property_name = rhs.m_property_name;
-m_property_opaque_type = rhs.m_property_opaque_type;
-m_property_setter_name = rhs.m_property_setter_name;
-m_property_getter_name = rhs.m_property_getter_name;
-m_property_attributes = rhs.m_property_attributes;
-
-if (rhs.m_metadata_up) {
- m_metadata_up = std::make_unique();
- *m_metadata_up = *rhs.m_metadata_up;
-}
-return *this;
- }
+m_property_attributes(property_attributes), m_metadata(metadata) {}
bool Finalize() {
return TypeSystemClang::AddObjCClassProperty(
m_class_opaque_type, m_property_name, m_property_opaque_type,
/*ivar_decl=*/nullptr, m_property_setter_name, m_property_getter_name,
-m_property_attributes, m_metadata_up.get());
+m_property_attributes, m_metadata);
}
private:
@@ -1929,7 +1904,7 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
const char *m_property_setter_name;
const char *m_property_getter_name;
uint32_t m_property_attributes;
- std::unique_ptr m_metadata_up;
+ ClangASTMetadata m_metadata;
};
bool DWARFASTParserClang::ParseTemplateDIE(
@@ -2721,10 +2696,10 @@ void DWARFASTParserClang::ParseObjCProperty(
ClangASTMetadata metadata;
metadata.SetUserID(die.GetID());
- delayed_properties.push_back(DelayedAddObjCClassProperty(
+ delayed_properties.emplace_back(
class_clang_type, propAttrs.prop_name,
member_type->GetLayoutCompilerType(), propAttrs.prop_setter_name,
- propAttrs.prop_getter_name, propAttrs.pr
[Lldb-commits] [lldb] [lldb][TypeSystem] Pass ClangASTMetadata by-value when creating record/objc types (PR #102296)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/102296 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
https://github.com/Michael137 approved this pull request. Latest test additions LGTM, thanks! https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] f05efd0 - [lldb][ClangExpressionParser][NFC] Remove unused local vars
Author: Michael Buch Date: 2024-08-07T13:39:52+01:00 New Revision: f05efd034c0993c7aa606bfbaaf575d94d75b43f URL: https://github.com/llvm/llvm-project/commit/f05efd034c0993c7aa606bfbaaf575d94d75b43f DIFF: https://github.com/llvm/llvm-project/commit/f05efd034c0993c7aa606bfbaaf575d94d75b43f.diff LOG: [lldb][ClangExpressionParser][NFC] Remove unused local vars These got removed in `d6cbcf93b227befaad00957a56acd63c837c26ff` but mistakenly added back when rebasing `12e3a06cb7615fbd91031420f3dec2a85d7877d6` Added: Modified: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp Removed: diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 2a8bdf29314e4..f41323d32ac86 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -625,10 +625,6 @@ ClangExpressionParser::ClangExpressionParser( // Make sure clang uses the same VFS as LLDB. m_compiler->createFileManager(FileSystem::Instance().GetVirtualFileSystem()); - std::string abi; - ArchSpec target_arch; - target_arch = target_sp->GetArchitecture(); - // 2. Configure the compiler with a set of default options that are // appropriate for most situations. SetupTargetOpts(*m_compiler, *target_sp); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
@@ -44,6 +47,14 @@ using namespace llvm; // option descriptors for getopt_long_only() +#ifdef _WIN32 +typedef pipe_t fd_t; +#define kInvalidFD (LLDB_INVALID_PIPE) labath wrote: Lets make this a real constant (not a macro). https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
@@ -254,6 +445,65 @@ int main_platform(int argc, char *argv[]) {
lldb_private::Args inferior_arguments;
inferior_arguments.SetArguments(argc, const_cast(argv));
+ if (fd != kInvalidFD) {
+// Child process will handle the connection and exit.
+Log *log = GetLog(LLDBLog::Platform);
+if (!listen_host_port.empty()) {
+ LLDB_LOGF(log, "lldb-platform child: "
+ "ambiguous parameters --listen and --fd");
+ return socket_error;
+}
+
+NativeSocket socket;
+#ifdef _WIN32
labath wrote:
And I think this could be a static method on the class I described above.
`CompleteReceiving` ?
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
@@ -114,6 +126,175 @@ static Status save_socket_id_to_file(const std::string
&socket_id,
return status;
}
+static void client_handle(GDBRemoteCommunicationServerPlatform &platform,
+ const lldb_private::Args &args) {
+ if (!platform.IsConnected())
+return;
+
+ if (args.GetArgumentCount() > 0) {
+lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
+std::optional port;
+std::string socket_name;
+Status error = platform.LaunchGDBServer(args,
+"", // hostname
+pid, port, socket_name);
+if (error.Success())
+ platform.SetPendingGdbServer(pid, *port, socket_name);
+else
+ fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString());
+ }
+
+ bool interrupt = false;
+ bool done = false;
+ Status error;
+ while (!interrupt && !done) {
+if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt,
+ done) !=
+GDBRemoteCommunication::PacketResult::Success)
+ break;
+ }
+
+ printf("Disconnected.\n");
+}
+
+static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
+static std::mutex gdbserver_portmap_mutex;
+
+static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
+ std::lock_guard guard(gdbserver_portmap_mutex);
+ gdbserver_portmap.FreePortForProcess(pid);
+}
+
+static Status spawn_process(const char *progname, Connection *conn,
+uint16_t gdb_port, uint16_t port_offset,
+const lldb_private::Args &args,
+const std::string &log_file,
+const StringRef log_channels) {
+ Status error;
+ const TCPSocket &tcpSocket =
+ static_cast(*conn->GetReadObject());
+ NativeSocket socket = tcpSocket.GetNativeSocket();
+
+ ProcessLaunchInfo launch_info;
+
+ fd_t fd;
+#ifdef _WIN32
+ // Create a pipe to transfer WSAPROTOCOL_INFO to the child process.
+ Pipe socket_pipe;
+ error = socket_pipe.CreateNew(true);
+ if (error.Fail())
+return error;
+
+ // Seems it will not work anyway. ProcessLauncherWindows ignores FileActions.
+ // And it is necessary to pass HANDLE instead of FD on Widows.
+ launch_info.AppendCloseFileAction(socket_pipe.GetWriteFileDescriptor());
+
+ fd = socket_pipe.GetReadPipe();
+#else
+ fd = socket;
+#endif
+
+ FileSpec self_spec(progname, FileSpec::Style::native);
+ launch_info.SetExecutableFile(self_spec, true);
+ Args &self_args = launch_info.GetArguments();
+ self_args.AppendArgument(llvm::StringRef("platform"));
+ self_args.AppendArgument(llvm::StringRef("--fd"));
+ self_args.AppendArgument(llvm::to_string(fd));
+ if (gdb_port) {
+self_args.AppendArgument(llvm::StringRef("--gdbserver-port"));
+self_args.AppendArgument(llvm::to_string(gdb_port));
+ }
+ if (port_offset > 0) {
+self_args.AppendArgument(llvm::StringRef("--port-offset"));
+self_args.AppendArgument(llvm::to_string(port_offset));
+ }
+ if (!log_file.empty()) {
+self_args.AppendArgument(llvm::StringRef("--log-file"));
+self_args.AppendArgument(log_file);
+ }
+ if (!log_channels.empty()) {
+self_args.AppendArgument(llvm::StringRef("--log-channels"));
+self_args.AppendArgument(log_channels);
+ }
+ if (args.GetArgumentCount() > 0) {
+self_args.AppendArgument("--");
+self_args.AppendArguments(args);
+ }
+
+ launch_info.SetLaunchInSeparateProcessGroup(false);
+ launch_info.SetMonitorProcessCallback(&spawn_process_reaped);
+
+ // Copy the current environment.
+ // WSASocket(FROM_PROTOCOL_INFO) will fail in the child process
+ // with the error WSAEPROVIDERFAILEDINIT if the SystemRoot is missing
+ // in the environment.
+ launch_info.GetEnvironment() = Host::GetEnvironment();
+
+ launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO);
+
+ // Close STDIN, STDOUT and STDERR.
+ launch_info.AppendCloseFileAction(STDIN_FILENO);
+ launch_info.AppendCloseFileAction(STDOUT_FILENO);
+ launch_info.AppendCloseFileAction(STDERR_FILENO);
+
+ // Redirect STDIN, STDOUT and STDERR to "/dev/null".
+ launch_info.AppendSuppressFileAction(STDIN_FILENO, true, false);
+ launch_info.AppendSuppressFileAction(STDOUT_FILENO, false, true);
+ launch_info.AppendSuppressFileAction(STDERR_FILENO, false, true);
+
+ std::string cmd;
+ self_args.GetCommandString(cmd);
+
+ error = Host::LaunchProcess(launch_info);
+ if (error.Fail())
+return error;
+
+ lldb::pid_t child_pid = launch_info.GetProcessID();
+ if (child_pid == LLDB_INVALID_PROCESS_ID)
+return Status("invalid pid");
+
+ LLDB_LOG(GetLog(LLDBLog::Platform), "lldb-platform launched '{0}', pid={1}",
+ cmd, child_pid);
+
+ {
+std::lock_guard guard(gdbserver_portmap_mutex);
+gdbserver_portmap.AssociatePortWithProcess(gdb_port, child_pid);
+ }
+
+#ifdef _WIN32
+ // Transfer WSAPROTOCOL_INFO to the child process.
+ if (socket
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
@@ -114,6 +126,175 @@ static Status save_socket_id_to_file(const std::string
&socket_id,
return status;
}
+static void client_handle(GDBRemoteCommunicationServerPlatform &platform,
+ const lldb_private::Args &args) {
+ if (!platform.IsConnected())
+return;
+
+ if (args.GetArgumentCount() > 0) {
+lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
+std::optional port;
+std::string socket_name;
+Status error = platform.LaunchGDBServer(args,
+"", // hostname
+pid, port, socket_name);
+if (error.Success())
+ platform.SetPendingGdbServer(pid, *port, socket_name);
+else
+ fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString());
+ }
+
+ bool interrupt = false;
+ bool done = false;
+ Status error;
+ while (!interrupt && !done) {
+if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt,
+ done) !=
+GDBRemoteCommunication::PacketResult::Success)
+ break;
+ }
+
+ printf("Disconnected.\n");
+}
+
+static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
+static std::mutex gdbserver_portmap_mutex;
+
+static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
+ std::lock_guard guard(gdbserver_portmap_mutex);
+ gdbserver_portmap.FreePortForProcess(pid);
+}
+
+static Status spawn_process(const char *progname, Connection *conn,
+uint16_t gdb_port, uint16_t port_offset,
+const lldb_private::Args &args,
+const std::string &log_file,
+const StringRef log_channels) {
+ Status error;
+ const TCPSocket &tcpSocket =
+ static_cast(*conn->GetReadObject());
+ NativeSocket socket = tcpSocket.GetNativeSocket();
+
+ ProcessLaunchInfo launch_info;
+
+ fd_t fd;
+#ifdef _WIN32
labath wrote:
I think these two ifdefs in this function could be extracted to a separate
class. I'm imagining something with a constructor which takes the `Connection`
object (maybe also the ProcessLaunchInfo), and two methods that:
- return the number that should be passed to the subprocess via arguments (the
unix version will just pass the connection fd). `GetSendableFD` ?
- finish the sending after the new process is started (unix version would be a
noop). `CompleteSending` ?
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
@@ -60,6 +71,7 @@ static struct option g_long_options[] = {
{"max-gdbserver-port", required_argument, nullptr, 'M'},
{"socket-file", required_argument, nullptr, 'f'},
{"server", no_argument, &g_server, 1},
+{"fd", required_argument, nullptr, 'd'},
labath wrote:
We don't expect this option to be used by the end user (at least not on
windows, where the passing algorithm is quite complex), right?
If so, then I think we should drop the short option (the field is `int`, so I
believe you can just use number not corresponding to a printable char), and use
something longer for the long option -- to reduce the likelyhood of someone
using this by mistake. I'd go with something like `--child-platform-fd`..
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
@@ -114,6 +126,175 @@ static Status save_socket_id_to_file(const std::string
&socket_id,
return status;
}
+static void client_handle(GDBRemoteCommunicationServerPlatform &platform,
+ const lldb_private::Args &args) {
+ if (!platform.IsConnected())
+return;
+
+ if (args.GetArgumentCount() > 0) {
+lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
+std::optional port;
+std::string socket_name;
+Status error = platform.LaunchGDBServer(args,
+"", // hostname
+pid, port, socket_name);
+if (error.Success())
+ platform.SetPendingGdbServer(pid, *port, socket_name);
+else
+ fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString());
+ }
+
+ bool interrupt = false;
+ bool done = false;
+ Status error;
+ while (!interrupt && !done) {
+if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt,
+ done) !=
+GDBRemoteCommunication::PacketResult::Success)
+ break;
+ }
+
+ printf("Disconnected.\n");
+}
+
+static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
+static std::mutex gdbserver_portmap_mutex;
+
+static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
+ std::lock_guard guard(gdbserver_portmap_mutex);
+ gdbserver_portmap.FreePortForProcess(pid);
+}
+
+static Status spawn_process(const char *progname, Connection *conn,
+uint16_t gdb_port, uint16_t port_offset,
+const lldb_private::Args &args,
+const std::string &log_file,
+const StringRef log_channels) {
+ Status error;
+ const TCPSocket &tcpSocket =
+ static_cast(*conn->GetReadObject());
labath wrote:
Other types of connections can be used here (at least on unixen, anyway). The
cast should only be necessary on windows (and even there, the code should
probably check the type first).
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
@@ -114,6 +126,175 @@ static Status save_socket_id_to_file(const std::string
&socket_id,
return status;
}
+static void client_handle(GDBRemoteCommunicationServerPlatform &platform,
+ const lldb_private::Args &args) {
+ if (!platform.IsConnected())
+return;
+
+ if (args.GetArgumentCount() > 0) {
+lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
+std::optional port;
+std::string socket_name;
+Status error = platform.LaunchGDBServer(args,
+"", // hostname
+pid, port, socket_name);
+if (error.Success())
+ platform.SetPendingGdbServer(pid, *port, socket_name);
+else
+ fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString());
+ }
+
+ bool interrupt = false;
+ bool done = false;
+ Status error;
+ while (!interrupt && !done) {
+if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt,
+ done) !=
+GDBRemoteCommunication::PacketResult::Success)
+ break;
+ }
+
+ printf("Disconnected.\n");
+}
+
+static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
+static std::mutex gdbserver_portmap_mutex;
+
+static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
+ std::lock_guard guard(gdbserver_portmap_mutex);
+ gdbserver_portmap.FreePortForProcess(pid);
+}
+
+static Status spawn_process(const char *progname, Connection *conn,
+uint16_t gdb_port, uint16_t port_offset,
+const lldb_private::Args &args,
+const std::string &log_file,
+const StringRef log_channels) {
+ Status error;
+ const TCPSocket &tcpSocket =
+ static_cast(*conn->GetReadObject());
+ NativeSocket socket = tcpSocket.GetNativeSocket();
+
+ ProcessLaunchInfo launch_info;
+
+ fd_t fd;
+#ifdef _WIN32
+ // Create a pipe to transfer WSAPROTOCOL_INFO to the child process.
+ Pipe socket_pipe;
+ error = socket_pipe.CreateNew(true);
+ if (error.Fail())
+return error;
+
+ // Seems it will not work anyway. ProcessLauncherWindows ignores FileActions.
+ // And it is necessary to pass HANDLE instead of FD on Widows.
+ launch_info.AppendCloseFileAction(socket_pipe.GetWriteFileDescriptor());
+
+ fd = socket_pipe.GetReadPipe();
+#else
+ fd = socket;
+#endif
+
+ FileSpec self_spec(progname, FileSpec::Style::native);
+ launch_info.SetExecutableFile(self_spec, true);
+ Args &self_args = launch_info.GetArguments();
+ self_args.AppendArgument(llvm::StringRef("platform"));
+ self_args.AppendArgument(llvm::StringRef("--fd"));
+ self_args.AppendArgument(llvm::to_string(fd));
+ if (gdb_port) {
+self_args.AppendArgument(llvm::StringRef("--gdbserver-port"));
+self_args.AppendArgument(llvm::to_string(gdb_port));
+ }
+ if (port_offset > 0) {
+self_args.AppendArgument(llvm::StringRef("--port-offset"));
+self_args.AppendArgument(llvm::to_string(port_offset));
+ }
+ if (!log_file.empty()) {
+self_args.AppendArgument(llvm::StringRef("--log-file"));
+self_args.AppendArgument(log_file);
+ }
+ if (!log_channels.empty()) {
+self_args.AppendArgument(llvm::StringRef("--log-channels"));
+self_args.AppendArgument(log_channels);
+ }
+ if (args.GetArgumentCount() > 0) {
+self_args.AppendArgument("--");
+self_args.AppendArguments(args);
+ }
+
+ launch_info.SetLaunchInSeparateProcessGroup(false);
+ launch_info.SetMonitorProcessCallback(&spawn_process_reaped);
+
+ // Copy the current environment.
+ // WSASocket(FROM_PROTOCOL_INFO) will fail in the child process
+ // with the error WSAEPROVIDERFAILEDINIT if the SystemRoot is missing
+ // in the environment.
+ launch_info.GetEnvironment() = Host::GetEnvironment();
+
+ launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO);
+
+ // Close STDIN, STDOUT and STDERR.
+ launch_info.AppendCloseFileAction(STDIN_FILENO);
+ launch_info.AppendCloseFileAction(STDOUT_FILENO);
+ launch_info.AppendCloseFileAction(STDERR_FILENO);
+
+ // Redirect STDIN, STDOUT and STDERR to "/dev/null".
+ launch_info.AppendSuppressFileAction(STDIN_FILENO, true, false);
+ launch_info.AppendSuppressFileAction(STDOUT_FILENO, false, true);
+ launch_info.AppendSuppressFileAction(STDERR_FILENO, false, true);
+
+ std::string cmd;
+ self_args.GetCommandString(cmd);
+
+ error = Host::LaunchProcess(launch_info);
+ if (error.Fail())
+return error;
+
+ lldb::pid_t child_pid = launch_info.GetProcessID();
+ if (child_pid == LLDB_INVALID_PROCESS_ID)
+return Status("invalid pid");
+
+ LLDB_LOG(GetLog(LLDBLog::Platform), "lldb-platform launched '{0}', pid={1}",
+ cmd, child_pid);
+
+ {
+std::lock_guard guard(gdbserver_portmap_mutex);
+gdbserver_portmap.AssociatePortWithProcess(gdb_port, child_pid);
+ }
+
+#ifdef _WIN32
+ // Transfer WSAPROTOCOL_INFO to the child process.
+ if (socket
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
https://github.com/labath commented: I actually quite like this. I think we should further merge the platform specific code into one place (which, among other things, will reduce the total number of ifdefs to ~one, and put all of the socket handling code next to each other), but apart from that, I think this is great (all things considered, etc). https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #102309)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/102309
This patch changes the way we initialize `BuiltinHeadersInSystemModules` which
is one of the flags controlling Clang's behaviour when the Darwin module is
split into more fine-grained modules. The ClangExpressionParser currently
unconditionally sets `-fbuiltin-headers-in-system-modules` when evaluating
expressions with the `target.import-std-module` setting. This flag should,
however, be set depending on the SDK version (which is what the Clang Darwin
toolchain does).
Unfortunately, the compiler instance that we create with
`ClangExpressionParser` never consults the Clang driver, and thus doesn't
correctly infer `BuiltinHeadersInSystemModules`. Note, this isn't an issue with
the `CompilerInstance` that the `ClangModulesDeclVendor` creates because it
uses the `createInovcation` API, which calls into
`Darwin::addClangTargetOptions`.
This patch mimicks how `sdkSupportsBuiltinModules` is used in
`Darwin::addClangTargetOptions`.
This ensures that the `import-std-module` API tests run cleanly regardless of
SDK version.
The plan is to eventually make the `CompilerInstance` construction in
`ClangExpressionParser` go through the driver, so we can avoid duplicating the
logic in LLDB. But we aren't there yet.
**Implementation**
* We look for the `SDKSettings.json` in the sysroot directory that we found in
DWARF (via `DW_AT_LLVM_sysroot`)
* Then parse this file and extract the SDK version number out of it
* Then mimick `sdkSupportsBuiltinModules` from `Toolchains/Darwin.cpp` and set
`BuiltinHeadersInSystemModules` based on it
rdar://116490281
>From acc93237396ce8de0e9cbbb2716c1017dffec630 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 7 Aug 2024 12:46:24 +0100
Subject: [PATCH] [lldb][ClangExpressionParser] Set
BuiltinHeadersInSystemModules depending on SDK version
---
lldb/include/lldb/Utility/XcodeSDK.h | 14 +
.../Clang/ClangExpressionParser.cpp | 52 +--
lldb/source/Utility/XcodeSDK.cpp | 21
3 files changed, 84 insertions(+), 3 deletions(-)
diff --git a/lldb/include/lldb/Utility/XcodeSDK.h
b/lldb/include/lldb/Utility/XcodeSDK.h
index 673ea578ffce8..69c7e23d3d0f6 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -85,6 +85,20 @@ class XcodeSDK {
/// Whether LLDB feels confident importing Clang modules from this SDK.
static bool SDKSupportsModules(Type type, llvm::VersionTuple version);
static bool SDKSupportsModules(Type desired_type, const FileSpec &sdk_path);
+
+ /// Returns true if the SDK for the specified triple supports
+ /// builtin modules in system headers.
+ ///
+ /// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
+ /// Toolchains/Darwin.cpp
+ ///
+ /// FIXME: this function will be removed once LLDB's ClangExpressionParser
+ /// constructs the compiler instance through the driver/toolchain. See \ref
+ /// SetupImportStdModuleLangOpts
+ ///
+ static bool SDKSupportsBuiltinModules(const llvm::Triple &triple,
+llvm::VersionTuple sdk_version);
+
/// Return the canonical SDK name, such as "macosx" for the macOS SDK.
static std::string GetCanonicalName(Info info);
/// Return the best-matching SDK type for a specific triple.
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index f41323d32ac86..3b67d28203c3d 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -39,6 +39,7 @@
#include "llvm/ExecutionEngine/ExecutionEngine.h"
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/Debug.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/TargetSelect.h"
@@ -91,6 +92,8 @@
#include "lldb/Utility/StringList.h"
#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
+#include "Plugins/Platform/MacOSX/PlatformDarwin.h"
+#include "lldb/Utility/XcodeSDK.h"
#include
#include
@@ -279,6 +282,48 @@ class ClangDiagnosticManagerAdapter : public
clang::DiagnosticConsumer {
std::string m_output;
};
+/// Returns true if the SDK for the specified triple supports
+/// builtin modules in system headers. This is used to decide
+/// whether to pass -fbuiltin-headers-in-system-modules to
+/// the compiler instance when compiling the `std` module.
+///
+/// \param[in] target The target triple.
+///
+static llvm::Expected
+sdkSupportsBuiltinModules(lldb_private::Target &target) {
+ auto arch_spec = target.GetArchitecture();
+ auto const &triple = arch_spec.GetTriple();
+ auto module_sp = target.GetExecutableModule();
+ if (!module_sp)
+return llvm::createStringError("Executable module not found.");
+
+ // Get SDK path tha
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #102309)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
Changes
This patch changes the way we initialize `BuiltinHeadersInSystemModules` which
is one of the flags controlling Clang's behaviour when the Darwin module is
split into more fine-grained modules. The ClangExpressionParser currently
unconditionally sets `-fbuiltin-headers-in-system-modules` when evaluating
expressions with the `target.import-std-module` setting. This flag should,
however, be set depending on the SDK version (which is what the Clang Darwin
toolchain does).
Unfortunately, the compiler instance that we create with
`ClangExpressionParser` never consults the Clang driver, and thus doesn't
correctly infer `BuiltinHeadersInSystemModules`. Note, this isn't an issue with
the `CompilerInstance` that the `ClangModulesDeclVendor` creates because it
uses the `createInovcation` API, which calls into
`Darwin::addClangTargetOptions`.
This patch mimicks how `sdkSupportsBuiltinModules` is used in
`Darwin::addClangTargetOptions`.
This ensures that the `import-std-module` API tests run cleanly regardless of
SDK version.
The plan is to eventually make the `CompilerInstance` construction in
`ClangExpressionParser` go through the driver, so we can avoid duplicating the
logic in LLDB. But we aren't there yet.
**Implementation**
* We look for the `SDKSettings.json` in the sysroot directory that we found in
DWARF (via `DW_AT_LLVM_sysroot`)
* Then parse this file and extract the SDK version number out of it
* Then mimick `sdkSupportsBuiltinModules` from `Toolchains/Darwin.cpp` and set
`BuiltinHeadersInSystemModules` based on it
rdar://116490281
---
Full diff: https://github.com/llvm/llvm-project/pull/102309.diff
3 Files Affected:
- (modified) lldb/include/lldb/Utility/XcodeSDK.h (+14)
- (modified)
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+49-3)
- (modified) lldb/source/Utility/XcodeSDK.cpp (+21)
``diff
diff --git a/lldb/include/lldb/Utility/XcodeSDK.h
b/lldb/include/lldb/Utility/XcodeSDK.h
index 673ea578ffce85..69c7e23d3d0f63 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -85,6 +85,20 @@ class XcodeSDK {
/// Whether LLDB feels confident importing Clang modules from this SDK.
static bool SDKSupportsModules(Type type, llvm::VersionTuple version);
static bool SDKSupportsModules(Type desired_type, const FileSpec &sdk_path);
+
+ /// Returns true if the SDK for the specified triple supports
+ /// builtin modules in system headers.
+ ///
+ /// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
+ /// Toolchains/Darwin.cpp
+ ///
+ /// FIXME: this function will be removed once LLDB's ClangExpressionParser
+ /// constructs the compiler instance through the driver/toolchain. See \ref
+ /// SetupImportStdModuleLangOpts
+ ///
+ static bool SDKSupportsBuiltinModules(const llvm::Triple &triple,
+llvm::VersionTuple sdk_version);
+
/// Return the canonical SDK name, such as "macosx" for the macOS SDK.
static std::string GetCanonicalName(Info info);
/// Return the best-matching SDK type for a specific triple.
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index f41323d32ac863..3b67d28203c3de 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -39,6 +39,7 @@
#include "llvm/ExecutionEngine/ExecutionEngine.h"
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/Debug.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/TargetSelect.h"
@@ -91,6 +92,8 @@
#include "lldb/Utility/StringList.h"
#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
+#include "Plugins/Platform/MacOSX/PlatformDarwin.h"
+#include "lldb/Utility/XcodeSDK.h"
#include
#include
@@ -279,6 +282,48 @@ class ClangDiagnosticManagerAdapter : public
clang::DiagnosticConsumer {
std::string m_output;
};
+/// Returns true if the SDK for the specified triple supports
+/// builtin modules in system headers. This is used to decide
+/// whether to pass -fbuiltin-headers-in-system-modules to
+/// the compiler instance when compiling the `std` module.
+///
+/// \param[in] target The target triple.
+///
+static llvm::Expected
+sdkSupportsBuiltinModules(lldb_private::Target &target) {
+ auto arch_spec = target.GetArchitecture();
+ auto const &triple = arch_spec.GetTriple();
+ auto module_sp = target.GetExecutableModule();
+ if (!module_sp)
+return llvm::createStringError("Executable module not found.");
+
+ // Get SDK path that the target was compiled against.
+ auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module_sp);
+ if (!sdk_or_err)
+return sdk_or_err.takeError();
+
+
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #102309)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/102309
>From 3dfbfc759f61827f010f4a65241c8464c5815484 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 7 Aug 2024 12:46:24 +0100
Subject: [PATCH] [lldb][ClangExpressionParser] Set
BuiltinHeadersInSystemModules depending on SDK version
---
lldb/include/lldb/Utility/XcodeSDK.h | 14 +
.../Clang/ClangExpressionParser.cpp | 53 +--
lldb/source/Utility/XcodeSDK.cpp | 21
3 files changed, 85 insertions(+), 3 deletions(-)
diff --git a/lldb/include/lldb/Utility/XcodeSDK.h
b/lldb/include/lldb/Utility/XcodeSDK.h
index 673ea578ffce85..69c7e23d3d0f63 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -85,6 +85,20 @@ class XcodeSDK {
/// Whether LLDB feels confident importing Clang modules from this SDK.
static bool SDKSupportsModules(Type type, llvm::VersionTuple version);
static bool SDKSupportsModules(Type desired_type, const FileSpec &sdk_path);
+
+ /// Returns true if the SDK for the specified triple supports
+ /// builtin modules in system headers.
+ ///
+ /// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
+ /// Toolchains/Darwin.cpp
+ ///
+ /// FIXME: this function will be removed once LLDB's ClangExpressionParser
+ /// constructs the compiler instance through the driver/toolchain. See \ref
+ /// SetupImportStdModuleLangOpts
+ ///
+ static bool SDKSupportsBuiltinModules(const llvm::Triple &triple,
+llvm::VersionTuple sdk_version);
+
/// Return the canonical SDK name, such as "macosx" for the macOS SDK.
static std::string GetCanonicalName(Info info);
/// Return the best-matching SDK type for a specific triple.
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index f41323d32ac863..4a4063996fe18c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ExternalASTSource.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/Basic/Builtins.h"
+#include "clang/Basic/DarwinSDKInfo.h"
#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/TargetInfo.h"
@@ -39,6 +40,7 @@
#include "llvm/ExecutionEngine/ExecutionEngine.h"
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/Debug.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/TargetSelect.h"
@@ -91,6 +93,8 @@
#include "lldb/Utility/StringList.h"
#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
+#include "Plugins/Platform/MacOSX/PlatformDarwin.h"
+#include "lldb/Utility/XcodeSDK.h"
#include
#include
@@ -279,6 +283,48 @@ class ClangDiagnosticManagerAdapter : public
clang::DiagnosticConsumer {
std::string m_output;
};
+/// Returns true if the SDK for the specified triple supports
+/// builtin modules in system headers. This is used to decide
+/// whether to pass -fbuiltin-headers-in-system-modules to
+/// the compiler instance when compiling the `std` module.
+///
+/// \param[in] target The target triple.
+///
+static llvm::Expected
+sdkSupportsBuiltinModules(lldb_private::Target &target) {
+ auto arch_spec = target.GetArchitecture();
+ auto const &triple = arch_spec.GetTriple();
+ auto module_sp = target.GetExecutableModule();
+ if (!module_sp)
+return llvm::createStringError("Executable module not found.");
+
+ // Get SDK path that the target was compiled against.
+ auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module_sp);
+ if (!sdk_or_err)
+return sdk_or_err.takeError();
+
+ // Use the SDK path from debug-info to find a local matching SDK directory.
+ auto sdk_path_or_err =
+ HostInfo::GetSDKRoot(HostInfo::SDKOptions{std::move(sdk_or_err->first)});
+ if (!sdk_path_or_err)
+return sdk_path_or_err.takeError();
+
+ auto VFS = FileSystem::Instance().GetVirtualFileSystem();
+ if (!VFS)
+return llvm::createStringError("No virtual filesystem available.");
+
+ // Extract SDK version from the /path/to/some.sdk/SDKSettings.json
+ auto parsed_or_err = clang::parseDarwinSDKInfo(*VFS, *sdk_path_or_err);
+ if (!parsed_or_err)
+return parsed_or_err.takeError();
+
+ auto maybe_sdk = *parsed_or_err;
+ if (!maybe_sdk)
+return llvm::createStringError("Couldn't find Darwin SDK info.");
+
+ return XcodeSDK::SDKSupportsBuiltinModules(triple, maybe_sdk->getVersion());
+}
+
static void SetupModuleHeaderPaths(CompilerInstance *compiler,
std::vector
include_directories,
lldb::TargetSP target_sp) {
@@ -561,7 +607,8 @@ static void SetupLangOpts(CompilerInstance &compiler,
lang_opts.NoBuiltin
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
labath wrote: Also, if any of this code is not tested (outside of running the entire test suite in the remote mode) we should add a dedicated test for it. A quick search found at least one (test/API/tools/lldb-server/TestGdbRemoteCompletion.py) test which would appear to exercise this, but maybe there's room for more. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
@@ -0,0 +1,66 @@ +//===-- RealpathPrefixes.cpp --===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/Utility/RealpathPrefixes.h" + +#include "lldb/Target/Statistics.h" +#include "lldb/Target/Target.h" royitaqi wrote: > If you want this to be a Utility class What is the general guidance of when a class should be in "Target" vs. in "Utility"? > dependency inversion technique Like how? I tried to make an interface `RealpathPrefixStats` which contains the two increment functions, then have `TargetStats` implement this interface, in the hope of the Target can create the `RealpathPrefix` and feed its own `TargetStats` to it as `RealpathPrefixStats` (where `RealpathPrefixStats` only sees `RealpathPrefixStats`, which is also in `Utility`). However, there is the lifecycle problem Jim pointed out earlier, that what guarantees that the `RealpathPrefixStats` object lives longer than the `RealpathPrefix` object. In order to guarantee this, I will need to make `Target::m_stats` a shared pointer, then give a weak pointer of `RealpathPrefixStats` to `RealpathPrefix`. Is the above a good plan? Do you see a better approach? --- FWIW, I did search for the inclusion of Target stuff in Utility, but did it wrong and thought it's a common case. This involves a VS Code bug in "Find in Folder...". https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
https://github.com/royitaqi edited https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
https://github.com/royitaqi edited https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
https://github.com/royitaqi edited https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Realpath symlinks for breakpoints (PR #102223)
https://github.com/royitaqi edited https://github.com/llvm/llvm-project/pull/102223 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [lldb] [clang] Reland: Instantiate alias templates with sugar (PR #101858)
gribozavr wrote:
## On resugaring bodies of function template instantiations
> I just don't think that can be something we can always enable, as that would
> make some examples of source code take exponentially more time and memory to
> compile.
We keep track of propagated and inferred nullability annotations in a [side
data
structure](https://github.com/google/crubit/blob/main/nullability/type_nullability.h#L185).
Our approach allows us to save on memory by not actually re-instantiating
everything with new sugar.
But I do have a question - how is the built-in resugarer going to do this
without maintaining a side data structure like we do? Clang currently does not
have a concept of non-canonical instantiations. `std::vector`,
`std::vector>`, `std::vector>` are the
same canonical type, so they must share one instantiation. Are you also
proposing to change that?
> Right now the priority is to implement the stuff that won't take a big hit on
> perf. But that is just priority.
But wouldn't it be better if out-of-tree users like us were not blocked on the
priorities of your work? With the `Subst*` nodes in the AST the information is
available for out-of-tree users to use in any way they like. Are you sure you
can predict all use cases? Do you really want to be in the business of
satisfying them?
---
## On (in)completeness of the out-of-tree resugarer
> I don't think, from looking at the resugarer implemented in
> type_nullability.cc, it's anywhere close to being able to resugar through a
> complex type like an STL vector, as shown in the example.
Actually, resugaring in nullability analysis already works for
`std::vector::push_back()`:
```c++
#include
#include "absl/base/nullability.h"
void Test() {
int *p = nullptr;
std::vector> xs;
xs.push_back(p);
}
```
```
$ clang-tidy example.cc
example.cc:7:16: warning: expected parameter '__x' of 'std::vector::push_back' to be nonnull, but a nullable argument was used
[google-runtime-pointer-nullability]
7 | xs.push_back(p);
|^
```
> as for example both libc++ and libstdc++ vector implementations derive the
> element type not through the first template argument, but through the
> allocator instead.
As far as I see, libc++ does not actually use the allocator to define
`const_reference`, `value_type` and so on:
```c++
template */>
class _LIBCPP_TEMPLATE_VIS vector {
typedef _Tp value_type;
typedef value_type& reference;
typedef const value_type& const_reference;
/* ... */ void push_back(const_reference __x);
```
(see https://github.com/llvm/llvm-project/blob/main/libcxx/include/vector)
> I see the implementation of the resugarer in `type_nullability.cc`, and it's
> much more incomplete that the one I am proposing to upstream. So I think if
> you get it to work through upstream transforms, you will get there faster.
I'm not sure what you mean by "get there faster" - our implementation already
works, is already deployed internally, and covers a lot of cases that are
relevant in practice. We are still tweaking things of course, and we are
working on inference tooling (that infers nullability contracts based on the
implementation and suggests edits to the source code accordingly).
That is, the difficult part and the focus at the moment is not resugaring in
the analysis. I do admit that our implementation of resugaring is probably not
as comprehensive as the one that you're working on, but it works for the vast
majority of real-world code that people write in our monorepo - we tweaked it
based on the cases that we actually observed.
---
## On upstreaming the work
> As far as I can see, you don't need to keep the Subst* nodes after resugaring
> if you implement:
We are in the process of adding Clang's `_Nullable` and `_Nonnull` attributes
to the implementation of `absl::Nullable` and `absl::Nonnull`. This work is
taking time because we need to adjust our internal codebase to conform to
Clang's existing expectations, behaviors, and warnings that get triggered by
these attributes. These attributes will get picked up by the in-tree resugaring
implementation because the source of the annotation is physically spelled in
the code. Good.
However, a part of the nullability language extension is the [pragma that
changes the default meaning of
pointers](https://github.com/google/crubit/blob/main/nullability/test/pragma_nonnull.h#L6)
from "unknown" to "nonnull". That is, we interpret the following two files in
an identical way:
```c++
#include "absl/base/nullability.h"
#pragma nullability file_default nonnull
void TakePointer(int *p);
```
```c++
#include "absl/base/nullability.h"
void TakePointer(absl::Nonnull p);
```
Unfortunately we expect this pragma to be a controversial upstream because
Clang already has a pragma for this exact purpose (`#pragma clang
assume_nonnull begin/end`), but it was introduced by Apple in context of
Objective-C and some of its behavior is
[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)
oontvoo wrote: > I've made one pass over the PR. I think this would be easier to review if it > was split into multiple patches: > > * core llvm infrastructure > * core lldb infrastructure > * ~one patch per event type > > I think the biggest hurdle will be finding someone to approve the addition of > the new llvm library (I doubt it's going to be someone from the current > reviewer set). I wouldn't be surprised if this ends up needing its own RFC. Good point - so here's the plane: - I'll create a separate PR for the first bullet point (llvm core infra). - keep this PR as is - since branching is such a pain (and controversial) in llvm, you can "pretend" to not see the llvm/lib/ changes in this PR . hopefully the diff will disappear once the first PR goes in. https://github.com/llvm/llvm-project/pull/98528 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 12fa4b1 - [lldb] Make sure that a `Progress` "completed" update is always reported at destruction (#102097)
Author: royitaqi
Date: 2024-08-07T07:58:34-07:00
New Revision: 12fa4b17dcededbd14abfc3ae72f1b798349e847
URL:
https://github.com/llvm/llvm-project/commit/12fa4b17dcededbd14abfc3ae72f1b798349e847
DIFF:
https://github.com/llvm/llvm-project/commit/12fa4b17dcededbd14abfc3ae72f1b798349e847.diff
LOG: [lldb] Make sure that a `Progress` "completed" update is always reported
at destruction (#102097)
Make all `Progress` destructions to cause `progressEnd` events,
regardless of the value of `m_completed` before the destruction.
Currently, a `Progress` instance with `m_completed != 0 && m_complete !=
m_total` will cause a `progressUpdate` event (not `progressEnd`) at
destruction and. This contradicts with the classdoc: "a progress completed
update is reported even if the user doesn't explicitly cause one to be sent."
Added:
Modified:
lldb/source/Core/Progress.cpp
lldb/unittests/Core/ProgressReportTest.cpp
Removed:
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index 1a779e2ddf924d..e0ba1a63c508e5 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -45,8 +45,7 @@ Progress::~Progress() {
// Make sure to always report progress completed when this object is
// destructed so it indicates the progress dialog/activity should go away.
std::lock_guard guard(m_mutex);
- if (!m_completed)
-m_completed = m_total;
+ m_completed = m_total;
ReportProgress();
// Report to the ProgressManager if that subsystem is enabled.
diff --git a/lldb/unittests/Core/ProgressReportTest.cpp
b/lldb/unittests/Core/ProgressReportTest.cpp
index 141244feb1f08a..0149b1de77add7 100644
--- a/lldb/unittests/Core/ProgressReportTest.cpp
+++ b/lldb/unittests/Core/ProgressReportTest.cpp
@@ -133,6 +133,81 @@ TEST_F(ProgressReportTest, TestReportCreation) {
EXPECT_EQ(data->GetMessage(), "Progress report 1: Starting report 1");
}
+TEST_F(ProgressReportTest, TestReportDestructionWithPartialProgress) {
+ ListenerSP listener_sp = CreateListenerFor(lldb::eBroadcastBitProgress);
+ EventSP event_sp;
+ const ProgressEventData *data;
+
+ // Create a finite progress report and only increment to a non-completed
+ // state before destruction.
+ {
+Progress progress("Finite progress", "Report 1", 100);
+progress.Increment(3);
+ }
+
+ // Verify that the progress in the events are:
+ // 1. At construction: 0 out of 100
+ // 2. At increment: 3 out of 100
+ // 3. At destruction: 100 out of 100
+ ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+ EXPECT_EQ(data->GetDetails(), "Report 1");
+ EXPECT_TRUE(data->IsFinite());
+ EXPECT_EQ(data->GetCompleted(), (uint64_t)0);
+ EXPECT_EQ(data->GetTotal(), (uint64_t)100);
+ EXPECT_EQ(data->GetMessage(), "Finite progress: Report 1");
+
+ ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+ EXPECT_EQ(data->GetDetails(), "Report 1");
+ EXPECT_TRUE(data->IsFinite());
+ EXPECT_EQ(data->GetCompleted(), (uint64_t)3);
+ EXPECT_EQ(data->GetTotal(), (uint64_t)100);
+ EXPECT_EQ(data->GetMessage(), "Finite progress: Report 1");
+
+ ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+ EXPECT_EQ(data->GetDetails(), "Report 1");
+ EXPECT_TRUE(data->IsFinite());
+ EXPECT_EQ(data->GetCompleted(), (uint64_t)100);
+ EXPECT_EQ(data->GetTotal(), (uint64_t)100);
+ EXPECT_EQ(data->GetMessage(), "Finite progress: Report 1");
+
+ // Create an infinite progress report and increment by some amount.
+ {
+Progress progress("Infinite progress", "Report 2");
+progress.Increment(3);
+ }
+
+ // Verify that the progress in the events are:
+ // 1. At construction: 0
+ // 2. At increment: 3
+ // 3. At destruction: Progress::kNonDeterministicTotal
+ ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+ EXPECT_EQ(data->GetDetails(), "Report 2");
+ EXPECT_FALSE(data->IsFinite());
+ EXPECT_EQ(data->GetCompleted(), (uint64_t)0);
+ EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+ EXPECT_EQ(data->GetMessage(), "Infinite progress: Report 2");
+
+ ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+ EXPECT_EQ(data->GetDetails(), "Report 2");
+ EXPECT_FALSE(data->IsFinite());
+ EXPECT_EQ(data->GetCompleted(), (uint64_t)3);
+ EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+ EXPECT_EQ(data->GetMessage(), "Infinite progress: Report 2");
+
+ ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+ data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+ EXPECT_EQ(data->GetDetails(), "Report 2");
+ EXPECT_FALSE(data->IsFinite(
[Lldb-commits] [lldb] [lldb] Make sure that a `Progress` "completed" update is always reported at destruction (PR #102097)
https://github.com/royitaqi closed https://github.com/llvm/llvm-project/pull/102097 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb]Implement LLDB Telemetry (PR #98528)
https://github.com/oontvoo edited https://github.com/llvm/llvm-project/pull/98528 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb]Implement LLDB Telemetry (PR #98528)
https://github.com/oontvoo edited https://github.com/llvm/llvm-project/pull/98528 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #102309)
https://github.com/adrian-prantl approved this pull request. Thanks, this LGTM, next step should be to replace the use of `include_directories` with `DW_AT_APPLE_sdk`. https://github.com/llvm/llvm-project/pull/102309 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #102309)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/102309
>From 3dfbfc759f61827f010f4a65241c8464c5815484 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 7 Aug 2024 12:46:24 +0100
Subject: [PATCH 1/2] [lldb][ClangExpressionParser] Set
BuiltinHeadersInSystemModules depending on SDK version
---
lldb/include/lldb/Utility/XcodeSDK.h | 14 +
.../Clang/ClangExpressionParser.cpp | 53 +--
lldb/source/Utility/XcodeSDK.cpp | 21
3 files changed, 85 insertions(+), 3 deletions(-)
diff --git a/lldb/include/lldb/Utility/XcodeSDK.h
b/lldb/include/lldb/Utility/XcodeSDK.h
index 673ea578ffce8..69c7e23d3d0f6 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -85,6 +85,20 @@ class XcodeSDK {
/// Whether LLDB feels confident importing Clang modules from this SDK.
static bool SDKSupportsModules(Type type, llvm::VersionTuple version);
static bool SDKSupportsModules(Type desired_type, const FileSpec &sdk_path);
+
+ /// Returns true if the SDK for the specified triple supports
+ /// builtin modules in system headers.
+ ///
+ /// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
+ /// Toolchains/Darwin.cpp
+ ///
+ /// FIXME: this function will be removed once LLDB's ClangExpressionParser
+ /// constructs the compiler instance through the driver/toolchain. See \ref
+ /// SetupImportStdModuleLangOpts
+ ///
+ static bool SDKSupportsBuiltinModules(const llvm::Triple &triple,
+llvm::VersionTuple sdk_version);
+
/// Return the canonical SDK name, such as "macosx" for the macOS SDK.
static std::string GetCanonicalName(Info info);
/// Return the best-matching SDK type for a specific triple.
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index f41323d32ac86..4a4063996fe18 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ExternalASTSource.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/Basic/Builtins.h"
+#include "clang/Basic/DarwinSDKInfo.h"
#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/TargetInfo.h"
@@ -39,6 +40,7 @@
#include "llvm/ExecutionEngine/ExecutionEngine.h"
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/Debug.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/TargetSelect.h"
@@ -91,6 +93,8 @@
#include "lldb/Utility/StringList.h"
#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
+#include "Plugins/Platform/MacOSX/PlatformDarwin.h"
+#include "lldb/Utility/XcodeSDK.h"
#include
#include
@@ -279,6 +283,48 @@ class ClangDiagnosticManagerAdapter : public
clang::DiagnosticConsumer {
std::string m_output;
};
+/// Returns true if the SDK for the specified triple supports
+/// builtin modules in system headers. This is used to decide
+/// whether to pass -fbuiltin-headers-in-system-modules to
+/// the compiler instance when compiling the `std` module.
+///
+/// \param[in] target The target triple.
+///
+static llvm::Expected
+sdkSupportsBuiltinModules(lldb_private::Target &target) {
+ auto arch_spec = target.GetArchitecture();
+ auto const &triple = arch_spec.GetTriple();
+ auto module_sp = target.GetExecutableModule();
+ if (!module_sp)
+return llvm::createStringError("Executable module not found.");
+
+ // Get SDK path that the target was compiled against.
+ auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module_sp);
+ if (!sdk_or_err)
+return sdk_or_err.takeError();
+
+ // Use the SDK path from debug-info to find a local matching SDK directory.
+ auto sdk_path_or_err =
+ HostInfo::GetSDKRoot(HostInfo::SDKOptions{std::move(sdk_or_err->first)});
+ if (!sdk_path_or_err)
+return sdk_path_or_err.takeError();
+
+ auto VFS = FileSystem::Instance().GetVirtualFileSystem();
+ if (!VFS)
+return llvm::createStringError("No virtual filesystem available.");
+
+ // Extract SDK version from the /path/to/some.sdk/SDKSettings.json
+ auto parsed_or_err = clang::parseDarwinSDKInfo(*VFS, *sdk_path_or_err);
+ if (!parsed_or_err)
+return parsed_or_err.takeError();
+
+ auto maybe_sdk = *parsed_or_err;
+ if (!maybe_sdk)
+return llvm::createStringError("Couldn't find Darwin SDK info.");
+
+ return XcodeSDK::SDKSupportsBuiltinModules(triple, maybe_sdk->getVersion());
+}
+
static void SetupModuleHeaderPaths(CompilerInstance *compiler,
std::vector
include_directories,
lldb::TargetSP target_sp) {
@@ -561,7 +607,8 @@ static void SetupLangOpts(CompilerInstance &compiler,
lang_opts.NoBuiltin
[Lldb-commits] [lldb] [llvm] [lldb]Implement LLDB Telemetry (PR #98528)
https://github.com/oontvoo edited https://github.com/llvm/llvm-project/pull/98528 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
https://github.com/slydiman updated
https://github.com/llvm/llvm-project/pull/101283
>From 6b2a41ba3d71270e81e24a42d3b4f5dc2f96b939 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev
Date: Wed, 31 Jul 2024 05:41:21 +0400
Subject: [PATCH 1/3] [lldb] Updated lldb-server to spawn the child process and
share socket on Windows
`lldb-server platform --server` works on Windows now w/o multithreading. The
rest functionality remains unchanged.
Depends on #101383.
Fixes #90923, fixes #56346.
This is the part 1 of the replacement of #100670.
In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen
a common gdb port for all gdbserver connections. Then we can remove gdb port
mapping to fix #97537.
---
lldb/tools/lldb-server/lldb-platform.cpp | 347 ---
1 file changed, 306 insertions(+), 41 deletions(-)
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp
b/lldb/tools/lldb-server/lldb-platform.cpp
index 7148a1d2a30941..e23237ef574c30 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,6 +22,7 @@
#include
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/WithColor.h"
#include "llvm/Support/raw_ostream.h"
@@ -32,8 +33,10 @@
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/HostGetOpt.h"
#include "lldb/Host/OptionParser.h"
+#include "lldb/Host/Socket.h"
#include "lldb/Host/common/TCPSocket.h"
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Status.h"
using namespace lldb;
@@ -60,6 +63,9 @@ static struct option g_long_options[] = {
{"max-gdbserver-port", required_argument, nullptr, 'M'},
{"socket-file", required_argument, nullptr, 'f'},
{"server", no_argument, &g_server, 1},
+#if defined(_WIN32)
+{"accept", required_argument, nullptr, 'a'},
+#endif
{nullptr, 0, nullptr, 0}};
#if defined(__APPLE__)
@@ -114,6 +120,249 @@ static Status save_socket_id_to_file(const std::string
&socket_id,
return status;
}
+static void client_handle(GDBRemoteCommunicationServerPlatform &platform,
+ const lldb_private::Args &args) {
+ if (!platform.IsConnected())
+return;
+
+ if (args.GetArgumentCount() > 0) {
+lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
+std::optional port;
+std::string socket_name;
+Status error = platform.LaunchGDBServer(args,
+"", // hostname
+pid, port, socket_name);
+if (error.Success())
+ platform.SetPendingGdbServer(pid, *port, socket_name);
+else
+ fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString());
+ }
+
+ bool interrupt = false;
+ bool done = false;
+ Status error;
+ while (!interrupt && !done) {
+if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt,
+ done) !=
+GDBRemoteCommunication::PacketResult::Success)
+ break;
+ }
+
+ if (error.Fail())
+WithColor::error() << error.AsCString() << '\n';
+}
+
+static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
+static std::mutex gdbserver_portmap_mutex;
+
+#if defined(_WIN32)
+static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
+ std::lock_guard guard(gdbserver_portmap_mutex);
+ gdbserver_portmap.FreePortForProcess(pid);
+}
+
+static bool spawn_process_parent(const char *progname, Connection *conn,
+ uint16_t gdb_port, uint16_t port_offset,
+ const lldb_private::Args &args,
+ const std::string &log_file,
+ const StringRef log_channels) {
+ Log *log = GetLog(LLDBLog::Platform);
+ Pipe socket_pipe;
+ Status error = socket_pipe.CreateNew(true);
+ if (error.Fail()) {
+LLDB_LOGF(log,
+ "lldb-platform parent: "
+ "cannot create pipe: %s",
+ error.AsCString());
+return false;
+ }
+
+ ProcessLaunchInfo launch_info;
+ FileSpec self_spec(progname, FileSpec::Style::native);
+ launch_info.SetExecutableFile(self_spec, true);
+ Args &self_args = launch_info.GetArguments();
+ self_args.AppendArgument(llvm::StringRef("platform"));
+ self_args.AppendArgument(llvm::StringRef("--accept"));
+ self_args.AppendArgument(llvm::to_string(socket_pipe.GetReadPipe()));
+ if (gdb_port) {
+self_args.AppendArgument(llvm::StringRef("--gdbserver-port"));
+self_args.AppendArgument(llvm::to_string(gdb_port));
+ }
+ if (port_offset > 0) {
+self_args.AppendArgument(llvm::StringRef("--port-offset"));
+self_args.AppendArgument(llvm::to_string(port_offset));
+ }
+ if (!log_file.empty()) {
+self_args.AppendArgument(llvm::StringRef("--log-file"));
+self_args.AppendArgument(log_file);
+ }
+ if (!log_channels.empty()) {
+self_args.AppendA
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
slydiman wrote: @labath I have added the class SharedSocket. I have implemented all the recommendations. Note we have the buildbot for cross tests (Windows x64/Linux x64 host and Linux Aarch64 target). So everything is tested. Hope 1191 API tests run with this lldb-server are enough. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #102309)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/102309
>From 3dfbfc759f61827f010f4a65241c8464c5815484 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 7 Aug 2024 12:46:24 +0100
Subject: [PATCH 1/3] [lldb][ClangExpressionParser] Set
BuiltinHeadersInSystemModules depending on SDK version
---
lldb/include/lldb/Utility/XcodeSDK.h | 14 +
.../Clang/ClangExpressionParser.cpp | 53 +--
lldb/source/Utility/XcodeSDK.cpp | 21
3 files changed, 85 insertions(+), 3 deletions(-)
diff --git a/lldb/include/lldb/Utility/XcodeSDK.h
b/lldb/include/lldb/Utility/XcodeSDK.h
index 673ea578ffce8..69c7e23d3d0f6 100644
--- a/lldb/include/lldb/Utility/XcodeSDK.h
+++ b/lldb/include/lldb/Utility/XcodeSDK.h
@@ -85,6 +85,20 @@ class XcodeSDK {
/// Whether LLDB feels confident importing Clang modules from this SDK.
static bool SDKSupportsModules(Type type, llvm::VersionTuple version);
static bool SDKSupportsModules(Type desired_type, const FileSpec &sdk_path);
+
+ /// Returns true if the SDK for the specified triple supports
+ /// builtin modules in system headers.
+ ///
+ /// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
+ /// Toolchains/Darwin.cpp
+ ///
+ /// FIXME: this function will be removed once LLDB's ClangExpressionParser
+ /// constructs the compiler instance through the driver/toolchain. See \ref
+ /// SetupImportStdModuleLangOpts
+ ///
+ static bool SDKSupportsBuiltinModules(const llvm::Triple &triple,
+llvm::VersionTuple sdk_version);
+
/// Return the canonical SDK name, such as "macosx" for the macOS SDK.
static std::string GetCanonicalName(Info info);
/// Return the best-matching SDK type for a specific triple.
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index f41323d32ac86..4a4063996fe18 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ExternalASTSource.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/Basic/Builtins.h"
+#include "clang/Basic/DarwinSDKInfo.h"
#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/TargetInfo.h"
@@ -39,6 +40,7 @@
#include "llvm/ExecutionEngine/ExecutionEngine.h"
#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/Debug.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/TargetSelect.h"
@@ -91,6 +93,8 @@
#include "lldb/Utility/StringList.h"
#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
+#include "Plugins/Platform/MacOSX/PlatformDarwin.h"
+#include "lldb/Utility/XcodeSDK.h"
#include
#include
@@ -279,6 +283,48 @@ class ClangDiagnosticManagerAdapter : public
clang::DiagnosticConsumer {
std::string m_output;
};
+/// Returns true if the SDK for the specified triple supports
+/// builtin modules in system headers. This is used to decide
+/// whether to pass -fbuiltin-headers-in-system-modules to
+/// the compiler instance when compiling the `std` module.
+///
+/// \param[in] target The target triple.
+///
+static llvm::Expected
+sdkSupportsBuiltinModules(lldb_private::Target &target) {
+ auto arch_spec = target.GetArchitecture();
+ auto const &triple = arch_spec.GetTriple();
+ auto module_sp = target.GetExecutableModule();
+ if (!module_sp)
+return llvm::createStringError("Executable module not found.");
+
+ // Get SDK path that the target was compiled against.
+ auto sdk_or_err = PlatformDarwin::GetSDKPathFromDebugInfo(*module_sp);
+ if (!sdk_or_err)
+return sdk_or_err.takeError();
+
+ // Use the SDK path from debug-info to find a local matching SDK directory.
+ auto sdk_path_or_err =
+ HostInfo::GetSDKRoot(HostInfo::SDKOptions{std::move(sdk_or_err->first)});
+ if (!sdk_path_or_err)
+return sdk_path_or_err.takeError();
+
+ auto VFS = FileSystem::Instance().GetVirtualFileSystem();
+ if (!VFS)
+return llvm::createStringError("No virtual filesystem available.");
+
+ // Extract SDK version from the /path/to/some.sdk/SDKSettings.json
+ auto parsed_or_err = clang::parseDarwinSDKInfo(*VFS, *sdk_path_or_err);
+ if (!parsed_or_err)
+return parsed_or_err.takeError();
+
+ auto maybe_sdk = *parsed_or_err;
+ if (!maybe_sdk)
+return llvm::createStringError("Couldn't find Darwin SDK info.");
+
+ return XcodeSDK::SDKSupportsBuiltinModules(triple, maybe_sdk->getVersion());
+}
+
static void SetupModuleHeaderPaths(CompilerInstance *compiler,
std::vector
include_directories,
lldb::TargetSP target_sp) {
@@ -561,7 +607,8 @@ static void SetupLangOpts(CompilerInstance &compiler,
lang_opts.NoBuiltin
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket (PR #101283)
https://github.com/slydiman updated
https://github.com/llvm/llvm-project/pull/101283
>From 6b2a41ba3d71270e81e24a42d3b4f5dc2f96b939 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev
Date: Wed, 31 Jul 2024 05:41:21 +0400
Subject: [PATCH 1/3] [lldb] Updated lldb-server to spawn the child process and
share socket on Windows
`lldb-server platform --server` works on Windows now w/o multithreading. The
rest functionality remains unchanged.
Depends on #101383.
Fixes #90923, fixes #56346.
This is the part 1 of the replacement of #100670.
In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen
a common gdb port for all gdbserver connections. Then we can remove gdb port
mapping to fix #97537.
---
lldb/tools/lldb-server/lldb-platform.cpp | 347 ---
1 file changed, 306 insertions(+), 41 deletions(-)
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp
b/lldb/tools/lldb-server/lldb-platform.cpp
index 7148a1d2a30941..e23237ef574c30 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -22,6 +22,7 @@
#include
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/WithColor.h"
#include "llvm/Support/raw_ostream.h"
@@ -32,8 +33,10 @@
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/HostGetOpt.h"
#include "lldb/Host/OptionParser.h"
+#include "lldb/Host/Socket.h"
#include "lldb/Host/common/TCPSocket.h"
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Status.h"
using namespace lldb;
@@ -60,6 +63,9 @@ static struct option g_long_options[] = {
{"max-gdbserver-port", required_argument, nullptr, 'M'},
{"socket-file", required_argument, nullptr, 'f'},
{"server", no_argument, &g_server, 1},
+#if defined(_WIN32)
+{"accept", required_argument, nullptr, 'a'},
+#endif
{nullptr, 0, nullptr, 0}};
#if defined(__APPLE__)
@@ -114,6 +120,249 @@ static Status save_socket_id_to_file(const std::string
&socket_id,
return status;
}
+static void client_handle(GDBRemoteCommunicationServerPlatform &platform,
+ const lldb_private::Args &args) {
+ if (!platform.IsConnected())
+return;
+
+ if (args.GetArgumentCount() > 0) {
+lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
+std::optional port;
+std::string socket_name;
+Status error = platform.LaunchGDBServer(args,
+"", // hostname
+pid, port, socket_name);
+if (error.Success())
+ platform.SetPendingGdbServer(pid, *port, socket_name);
+else
+ fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString());
+ }
+
+ bool interrupt = false;
+ bool done = false;
+ Status error;
+ while (!interrupt && !done) {
+if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt,
+ done) !=
+GDBRemoteCommunication::PacketResult::Success)
+ break;
+ }
+
+ if (error.Fail())
+WithColor::error() << error.AsCString() << '\n';
+}
+
+static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
+static std::mutex gdbserver_portmap_mutex;
+
+#if defined(_WIN32)
+static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
+ std::lock_guard guard(gdbserver_portmap_mutex);
+ gdbserver_portmap.FreePortForProcess(pid);
+}
+
+static bool spawn_process_parent(const char *progname, Connection *conn,
+ uint16_t gdb_port, uint16_t port_offset,
+ const lldb_private::Args &args,
+ const std::string &log_file,
+ const StringRef log_channels) {
+ Log *log = GetLog(LLDBLog::Platform);
+ Pipe socket_pipe;
+ Status error = socket_pipe.CreateNew(true);
+ if (error.Fail()) {
+LLDB_LOGF(log,
+ "lldb-platform parent: "
+ "cannot create pipe: %s",
+ error.AsCString());
+return false;
+ }
+
+ ProcessLaunchInfo launch_info;
+ FileSpec self_spec(progname, FileSpec::Style::native);
+ launch_info.SetExecutableFile(self_spec, true);
+ Args &self_args = launch_info.GetArguments();
+ self_args.AppendArgument(llvm::StringRef("platform"));
+ self_args.AppendArgument(llvm::StringRef("--accept"));
+ self_args.AppendArgument(llvm::to_string(socket_pipe.GetReadPipe()));
+ if (gdb_port) {
+self_args.AppendArgument(llvm::StringRef("--gdbserver-port"));
+self_args.AppendArgument(llvm::to_string(gdb_port));
+ }
+ if (port_offset > 0) {
+self_args.AppendArgument(llvm::StringRef("--port-offset"));
+self_args.AppendArgument(llvm::to_string(port_offset));
+ }
+ if (!log_file.empty()) {
+self_args.AppendArgument(llvm::StringRef("--log-file"));
+self_args.AppendArgument(log_file);
+ }
+ if (!log_channels.empty()) {
+self_args.AppendA
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
@@ -134,6 +134,20 @@ bool TypeQuery::ContextMatches(
if (ctx == ctx_end)
return false; // Pattern too long.
+if (ctx->kind == CompilerContextKind::Namespace && ctx->name.IsEmpty()) {
clayborg wrote:
Do we need to check for `"(anonymous namespace)"` here? Or do we detect this
elsewhere and make sure to clear the name? Demangling names will show
`"(anonymous namespace)"`
https://github.com/llvm/llvm-project/pull/102111
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
@@ -440,12 +440,6 @@ static void GetTypeLookupContextImpl(DWARFDIE die, continue; } -// If there is no name, then there is no need to look anything up for this -// DIE. -const char *name = die.GetName(); -if (!name || !name[0]) - return; - clayborg wrote: Do we want to only pull out this match if this is a `DW_TAG_namespace`? Or is it useful to have unnamed structs/unions/classes in the contenxt? https://github.com/llvm/llvm-project/pull/102111 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
https://github.com/clayborg commented:
This patch relies on "(anonymous namespace)" being removed from the compiler
contexts to work. What is the user types any of:
```
(lldb) type lookup "(anonymous namespace)::A"
(lldb) script lldb.target.FindFirstType("(anonymous namespace)::A");
(lldb) script lldb.target.FindTypes("(anonymous namespace)::A");
```
Do we correctly set this "(anonymous namespace)" to empty? Or are we expecting
these type lookups will always use the looser matching strategy? If not we need
to check for both.
We might want to stop passing around `std::vector` and pass
around a class that contains the language:
```
struct CompilerContexts {
lldb::LanguageType language;
std::vector;
};
```
Then we could add a method to check for anonymous namespaces where C++ can
check for "(anonymous namespace)".
https://github.com/llvm/llvm-project/pull/102111
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Better matching of types in anonymous namespaces (PR #102111)
@@ -788,7 +808,13 @@ Type::GetTypeScopeAndBasename(llvm::StringRef name) {
switch (pos.value()) {
case ':':
if (prev_is_colon && template_depth == 0) {
-result.scope.push_back(name.slice(name_begin, pos.index() - 1));
+llvm::StringRef scope_name = name.slice(name_begin, pos.index() - 1);
+// The itanium demangler uses this string to represent anonymous
+// namespaces. Convert it to a more language-agnostic form (which is
+// also used in DWARF).
+if (scope_name == "(anonymous namespace)")
+ scope_name = "";
clayborg wrote:
So we can catch this here and NULL out the name, but what is the user wants to
find something in an anonymous namespace with `type lookup "(anonymous
namespace)::A"`. Do we clear the name there as well?
https://github.com/llvm/llvm-project/pull/102111
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,95 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ /// Class the provides an iterator over the memory64 memory ranges. Only the
+ /// the first descriptor is validated as readable beforehand.
+ class Memory64Iterator {
+ public:
+static Memory64Iterator
+begin(ArrayRef Storage,
+ ArrayRef Descriptors,
+ uint64_t BaseRVA) {
+ return Memory64Iterator(Storage, Descriptors, BaseRVA);
+}
+
+static Memory64Iterator end() { return Memory64Iterator(); }
+
+std::pair> Current;
+
+bool operator==(const Memory64Iterator &R) const {
+ return IsEnd == R.IsEnd;
+}
+
+bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
+
+const std::pair> &
+operator*() {
+ return Current;
+}
+
+const std::pair> *
+operator&() {
+ return &Current;
+}
+
+Error inc() {
+ if (Storage.size() == 0 || Descriptors.size() == 0)
+return make_error("No Memory64List Stream",
+ object_error::parse_failed);
+
+ if (Index >= Descriptors.size())
+return createError("Can't read past of Memory64List Stream");
+
+ const minidump::MemoryDescriptor_64 &Descriptor = Descriptors[Index];
+ if (RVA + Descriptor.DataSize > Storage.size())
+return make_error(
+"Memory64 Descriptor exceeds end of file.",
+object_error::unexpected_eof);
+
+ ArrayRef Content = Storage.slice(RVA, Descriptor.DataSize);
+ Current = std::make_pair(Descriptor, Content);
+ Index++;
+ RVA += Descriptor.DataSize;
+ if (Index >= Descriptors.size())
+IsEnd = true;
+ return Error::success();
+}
+
+ private:
+// This constructor will only happen after a validation check to see
+// if the first descriptor is readable.
+Memory64Iterator(ArrayRef Storage,
+ ArrayRef Descriptors,
+ uint64_t BaseRVA)
+: RVA(BaseRVA), Storage(Storage), Descriptors(Descriptors),
+ IsEnd(false) {
+ assert(Descriptors.size() > 0);
+ assert(Storage.size() >= BaseRVA + Descriptors.front().DataSize);
Jlalond wrote:
I simplified this assertion, but I'm checking before we construct. I think the
assertion is simpler to understand with the comment
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -132,6 +140,95 @@ class MinidumpFile : public Binary {
size_t Stride;
};
+ /// Class the provides an iterator over the memory64 memory ranges. Only the
+ /// the first descriptor is validated as readable beforehand.
+ class Memory64Iterator {
+ public:
+static Memory64Iterator
+begin(ArrayRef Storage,
+ ArrayRef Descriptors,
+ uint64_t BaseRVA) {
+ return Memory64Iterator(Storage, Descriptors, BaseRVA);
+}
+
+static Memory64Iterator end() { return Memory64Iterator(); }
+
+std::pair> Current;
+
+bool operator==(const Memory64Iterator &R) const {
+ return IsEnd == R.IsEnd;
+}
+
+bool operator!=(const Memory64Iterator &R) const { return !(*this == R); }
+
+const std::pair> &
+operator*() {
+ return Current;
+}
+
+const std::pair> *
+operator&() {
+ return &Current;
+}
+
+Error inc() {
+ if (Storage.size() == 0 || Descriptors.size() == 0)
+return make_error("No Memory64List Stream",
+ object_error::parse_failed);
+
+ if (Index >= Descriptors.size())
+return createError("Can't read past of Memory64List Stream");
Jlalond wrote:
Dropped entirely now that we're using `drop_front` on ArrayRef.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -336,3 +336,89 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
0xab, 0xad, 0xca, 0xfe}),
*ExpectedContext);
}
+
+TEST(MinidumpYAML, MemoryRegion_64bit) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile &File = **ExpectedFile;
+
+ ASSERT_THAT(1u, File.streams().size());
+
+ Error Err = Error::success();
+ // Explicit Err check
+ ASSERT_FALSE(Err);
+ Expected>
+ ExpectedMemoryList = File.getMemory64List(Err);
+
+ ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
+
+ iterator_range MemoryList =
+ *ExpectedMemoryList;
+ auto Iterator = MemoryList.begin();
+
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
+ auto DescOnePair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescOne = DescOnePair.first;
+ ASSERT_THAT(0x7FCF0818283u, DescOne.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescOne.DataSize);
+
+ ++Iterator;
+ ASSERT_FALSE(Err);
+
+ auto DescTwoPair = *Iterator;
+ const minidump::MemoryDescriptor_64 &DescTwo = DescTwoPair.first;
+ ASSERT_THAT(0x7FFF0818283u, DescTwo.StartOfMemoryRange);
+ ASSERT_THAT(5u, DescTwo.DataSize);
+ const std::optional> ExpectedContent =
+ File.getRawStream(StreamType::Memory64List);
+ ASSERT_TRUE(ExpectedContent);
+ const size_t ExpectedStreamSize =
+ sizeof(Memory64ListHeader) + (sizeof(MemoryDescriptor_64) * 2);
+ ASSERT_THAT(ExpectedStreamSize, ExpectedContent->size());
+
+ Expected ExpectedHeader =
+ File.getMemoryList64Header();
+ ASSERT_THAT_EXPECTED(ExpectedHeader, Succeeded());
+ ASSERT_THAT(ExpectedHeader->BaseRVA, 92u);
+
+ Expected> DescOneExpectedContentSlice = DescOnePair.second;
+ ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
+ ASSERT_THAT(5u, DescOneExpectedContentSlice->size());
+ ASSERT_THAT(arrayRefFromStringRef("hello"), *DescOneExpectedContentSlice);
+
+ Expected> DescTwoExpectedContentSlice = DescTwoPair.second;
+ ASSERT_THAT_EXPECTED(DescTwoExpectedContentSlice, Succeeded());
+ ASSERT_THAT(arrayRefFromStringRef("world"), *DescTwoExpectedContentSlice);
+
+ ASSERT_TRUE(Iterator == MemoryList.end());
+}
+
+TEST(MinidumpYAML, MemoryRegion_DataSize_TooSmall) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Data Size: 4 1
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Failed());
Jlalond wrote:
We added a check that we would fail if datasize was less than the content size,
so I added a small unit test to verify we can't create a yaml with a inferior
datasize.
With that in mind, do you still want me to move this?
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
https://github.com/Jlalond edited https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
Jlalond wrote: @labath Implemented almost all of your feedback, once again I appreciate the in depth review. The only functional thing I did not change was the assertion in the iterator constructor that the first element is readable. https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData support any JSON type (PR #101929)
https://github.com/medismailben updated
https://github.com/llvm/llvm-project/pull/101929
>From 424d331c65b35992475e4b5fda6b72e12a2e2bbd Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani
Date: Wed, 7 Aug 2024 11:11:27 -0700
Subject: [PATCH] [lldb/API] Fix SBStructuredData support any JSON type
This patch loosen the parsing requirement to allow parsing not only
JSON dictionaries but also valid JSON type (integer, float, string,
bool, array, null).
Signed-off-by: Med Ismail Bennani
---
lldb/source/API/SBStructuredData.cpp | 7 -
.../sbstructureddata/TestStructuredDataAPI.py | 31 +++
2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/lldb/source/API/SBStructuredData.cpp
b/lldb/source/API/SBStructuredData.cpp
index b18fc5655fc81..78afdc69fe0d2 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -86,7 +86,12 @@ lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream
&stream) {
StructuredData::ParseJSON(stream.GetData());
m_impl_up->SetObjectSP(json_obj);
- if (!json_obj || json_obj->GetType() != eStructuredDataTypeDictionary)
+ static constexpr StructuredDataType unsupported_type[] = {
+ eStructuredDataTypeInvalid,
+ eStructuredDataTypeGeneric,
+ };
+
+ if (!json_obj || llvm::is_contained(unsupported_type, json_obj->GetType()))
error.SetErrorString("Invalid Syntax");
return error;
}
diff --git a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
index b3db3bc61e4dc..21256d6c6e743 100644
--- a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -110,6 +110,37 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example = lldb.SBStructuredData()
+self.assertSuccess(example.SetFromJSON("1"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeInteger)
+self.assertEqual(example.GetIntegerValue(), 1)
+
+self.assertSuccess(example.SetFromJSON("4.19"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeFloat)
+self.assertEqual(example.GetFloatValue(), 4.19)
+
+self.assertSuccess(example.SetFromJSON('"Bonjour, 123!"'))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeString)
+self.assertEqual(example.GetStringValue(42), "Bonjour, 123!")
+
+self.assertSuccess(example.SetFromJSON("true"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeBoolean)
+self.assertTrue(example.GetBooleanValue())
+
+self.assertSuccess(example.SetFromJSON("null"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeNull)
+
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+self.assertSuccess(example.SetFromJSON(s))
+
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+sb_data = json.loads(s.GetData())
+self.assertEqual(sb_data, example_arr)
+
def invalid_struct_test(self, example):
invalid_struct = lldb.SBStructuredData()
invalid_struct = example.GetValueForKey("invalid_key")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #102309)
ian-twilightcoder wrote: This really isn't the right approach. `BuiltinHeadersInSystemModules` isn't the only important default that the driver sets up. Can we change the invocation setup to use clang::createInvocation or clang::CompilerInvocation::CreateFromArgs or clang::BuildCompilation? Or is that planned for another change? https://github.com/llvm/llvm-project/pull/102309 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData support any JSON type (PR #101929)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/101929 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Set BuiltinHeadersInSystemModules depending on SDK version (PR #102309)
@@ -259,6 +259,27 @@ bool XcodeSDK::SupportsSwift() const {
}
}
+bool XcodeSDK::SDKSupportsBuiltinModules(const llvm::Triple &target_triple,
ian-twilightcoder wrote:
This function got changed in https://github.com/llvm/llvm-project/pull/102239
https://github.com/llvm/llvm-project/pull/102309
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 5855237 - [lldb/API] Fix SBStructuredData support any JSON type (#101929)
Author: Med Ismail Bennani
Date: 2024-08-07T11:21:27-07:00
New Revision: 585523750e2bbe374d1cb3bf4ff9d53de29b9593
URL:
https://github.com/llvm/llvm-project/commit/585523750e2bbe374d1cb3bf4ff9d53de29b9593
DIFF:
https://github.com/llvm/llvm-project/commit/585523750e2bbe374d1cb3bf4ff9d53de29b9593.diff
LOG: [lldb/API] Fix SBStructuredData support any JSON type (#101929)
This patch loosen the parsing requirement to allow parsing not only
JSON dictionaries but also valid JSON type (integer, float, string,
bool, array, null).
Signed-off-by: Med Ismail Bennani
Added:
Modified:
lldb/source/API/SBStructuredData.cpp
lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
Removed:
diff --git a/lldb/source/API/SBStructuredData.cpp
b/lldb/source/API/SBStructuredData.cpp
index b18fc5655fc818..78afdc69fe0d23 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -86,7 +86,12 @@ lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream
&stream) {
StructuredData::ParseJSON(stream.GetData());
m_impl_up->SetObjectSP(json_obj);
- if (!json_obj || json_obj->GetType() != eStructuredDataTypeDictionary)
+ static constexpr StructuredDataType unsupported_type[] = {
+ eStructuredDataTypeInvalid,
+ eStructuredDataTypeGeneric,
+ };
+
+ if (!json_obj || llvm::is_contained(unsupported_type, json_obj->GetType()))
error.SetErrorString("Invalid Syntax");
return error;
}
diff --git
a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
index b3db3bc61e4dc3..21256d6c6e743b 100644
--- a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -110,6 +110,37 @@ class MyRandomClass:
self.assertTrue(my_random_class)
self.assertEqual(my_random_class.payload, MyRandomClass.payload)
+example = lldb.SBStructuredData()
+self.assertSuccess(example.SetFromJSON("1"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeInteger)
+self.assertEqual(example.GetIntegerValue(), 1)
+
+self.assertSuccess(example.SetFromJSON("4.19"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeFloat)
+self.assertEqual(example.GetFloatValue(), 4.19)
+
+self.assertSuccess(example.SetFromJSON('"Bonjour, 123!"'))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeString)
+self.assertEqual(example.GetStringValue(42), "Bonjour, 123!")
+
+self.assertSuccess(example.SetFromJSON("true"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeBoolean)
+self.assertTrue(example.GetBooleanValue())
+
+self.assertSuccess(example.SetFromJSON("null"))
+self.assertEqual(example.GetType(), lldb.eStructuredDataTypeNull)
+
+example_arr = [1, 2.3, "4", {"5": False}]
+arr_str = json.dumps(example_arr)
+s.Clear()
+s.Print(arr_str)
+self.assertSuccess(example.SetFromJSON(s))
+
+s.Clear()
+self.assertSuccess(example.GetAsJSON(s))
+sb_data = json.loads(s.GetData())
+self.assertEqual(sb_data, example_arr)
+
def invalid_struct_test(self, example):
invalid_struct = lldb.SBStructuredData()
invalid_struct = example.GetValueForKey("invalid_key")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/API] Fix SBStructuredData support any JSON type (PR #101929)
https://github.com/medismailben closed https://github.com/llvm/llvm-project/pull/101929 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
