[Lldb-commits] [lldb] [LLDB] Fix crash in TypeSystemClang::GetIndexofChildMemberWithName. (PR #117808)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Dec 4 10:29:34 PST 2024
https://github.com/cmtice updated https://github.com/llvm/llvm-project/pull/117808
>From b8c64e227b8f9f82b420cc5c2f24fbd3f75f67f5 Mon Sep 17 00:00:00 2001
From: Caroline Tice <cmtice at google.com>
Date: Tue, 26 Nov 2024 15:08:32 -0800
Subject: [PATCH 1/9] [lLDB] Fix crash in
TypeSystemClang::GetIndexofChildMemberWithName.
LLDB can crash in TypeSystemClang::GetIndexOfChildMemberWithName, at a
point where it pushes an index onto the child_indexes vector, tries to call
itself recursively, then tries to pop the entry from child_indexes.
The problem is that the recursive call can clear child_indexes, so that
this code ends up trying to pop an already empty vector. This change saves
the old vector before the push, then restores the saved vector rather than
trying to pop.
---
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 1a77c7cf9161a0..16eca7700d9fff 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6754,12 +6754,12 @@ size_t TypeSystemClang::GetIndexOfChildMemberWithName(
llvm::StringRef field_name = field->getName();
if (field_name.empty()) {
CompilerType field_type = GetType(field->getType());
+ std::vector<uint32_t> save_indices = child_indexes;
child_indexes.push_back(child_idx);
if (field_type.GetIndexOfChildMemberWithName(
name, omit_empty_base_classes, child_indexes))
return child_indexes.size();
- child_indexes.pop_back();
-
+ child_indexes = save_indices;
} else if (field_name == name) {
// We have to add on the number of base classes to this index!
child_indexes.push_back(
>From 94e40c83dbeb2ef5384fe3177372dfd7e208552d Mon Sep 17 00:00:00 2001
From: Caroline Tice <cmtice at google.com>
Date: Sun, 1 Dec 2024 12:29:27 -0800
Subject: [PATCH 2/9] Add test case.
---
.../commands/frame/var/anon-struct/Makefile | 3 +
.../var/anon-struct/TestFrameVarAnonStruct.py | 63 +++++++++++++++++++
.../commands/frame/var/anon-struct/main.cpp | 20 ++++++
3 files changed, 86 insertions(+)
create mode 100644 lldb/test/API/commands/frame/var/anon-struct/Makefile
create mode 100644 lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
create mode 100644 lldb/test/API/commands/frame/var/anon-struct/main.cpp
diff --git a/lldb/test/API/commands/frame/var/anon-struct/Makefile b/lldb/test/API/commands/frame/var/anon-struct/Makefile
new file mode 100644
index 00000000000000..99998b20bcb050
--- /dev/null
+++ b/lldb/test/API/commands/frame/var/anon-struct/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py b/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
new file mode 100644
index 00000000000000..8ff26f137df972
--- /dev/null
+++ b/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
@@ -0,0 +1,63 @@
+"""
+Make sure the frame variable -g, -a, and -l flags work.
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import os
+import shutil
+import time
+
+
+class TestFrameVarAnonStruct(TestBase):
+ # If your test case doesn't stress debug info, then
+ # set this to true. That way it won't be run once for
+ # each debug info format.
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def test_frame_var(self):
+ self.build()
+ self.do_test()
+
+ def do_test(self):
+ target = self.createTestTarget()
+
+ # Now create a breakpoint in main.c at the source matching
+ # "Set a breakpoint here"
+ breakpoint = target.BreakpointCreateBySourceRegex(
+ "Set a breakpoint here", lldb.SBFileSpec("main.cpp")
+ )
+ self.assertTrue(
+ breakpoint and breakpoint.GetNumLocations() >= 1, VALID_BREAKPOINT
+ )
+
+ error = lldb.SBError()
+ # This is the launch info. If you want to launch with arguments or
+ # environment variables, add them using SetArguments or
+ # SetEnvironmentEntries
+
+ launch_info = target.GetLaunchInfo()
+ process = target.Launch(launch_info, error)
+ self.assertTrue(process, PROCESS_IS_VALID)
+
+ # Did we hit our breakpoint?
+ from lldbsuite.test.lldbutil import get_threads_stopped_at_breakpoint
+
+ threads = get_threads_stopped_at_breakpoint(process, breakpoint)
+ self.assertEqual(
+ len(threads), 1, "There should be a thread stopped at our breakpoint"
+ )
+
+ # The hit count for the breakpoint should be 1.
+ self.assertEqual(breakpoint.GetHitCount(), 1)
+
+ frame = threads[0].GetFrameAtIndex(0)
+ command_result = lldb.SBCommandReturnObject()
+ interp = self.dbg.GetCommandInterpreter()
+
+ # Verify that we don't crash in this case.
+ self.expect("frame variable 'b.x'", error=True,
+ substrs=['"x" is not a member of "(B) b"'])
diff --git a/lldb/test/API/commands/frame/var/anon-struct/main.cpp b/lldb/test/API/commands/frame/var/anon-struct/main.cpp
new file mode 100644
index 00000000000000..1deda70376dad5
--- /dev/null
+++ b/lldb/test/API/commands/frame/var/anon-struct/main.cpp
@@ -0,0 +1,20 @@
+int main(int argc, char** argv)
+{
+ struct A {
+ struct {
+ int x = 1;
+ };
+ int y = 2;
+ } a;
+
+ struct B {
+ // Anonymous struct inherits another struct.
+ struct : public A {
+ int z = 3;
+ };
+ int w = 4;
+ A a;
+ } b;
+
+ return 0; // Set a breakpoint here
+}
>From 59c71a9ea44f34ac1d4d074d0c91bc8291701dd3 Mon Sep 17 00:00:00 2001
From: Caroline Tice <cmtice at google.com>
Date: Sun, 1 Dec 2024 12:31:19 -0800
Subject: [PATCH 3/9] Fix comment.
---
.../commands/frame/var/anon-struct/TestFrameVarAnonStruct.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py b/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
index 8ff26f137df972..90479a1ea45d69 100644
--- a/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
+++ b/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
@@ -1,5 +1,5 @@
"""
-Make sure the frame variable -g, -a, and -l flags work.
+Test handling of Anonymous Structs, especially that they don't crash lldb.
"""
>From b7b7b54218ce6a9d4805138bc03cd0fc3b051a86 Mon Sep 17 00:00:00 2001
From: Caroline Tice <cmtice at google.com>
Date: Sun, 1 Dec 2024 15:43:08 -0800
Subject: [PATCH 4/9] Fix clang format issues.
---
.../frame/var/anon-struct/TestFrameVarAnonStruct.py | 7 +++++--
lldb/test/API/commands/frame/var/anon-struct/main.cpp | 3 +--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py b/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
index 90479a1ea45d69..25f4275d0ac7b2 100644
--- a/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
+++ b/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
@@ -59,5 +59,8 @@ def do_test(self):
interp = self.dbg.GetCommandInterpreter()
# Verify that we don't crash in this case.
- self.expect("frame variable 'b.x'", error=True,
- substrs=['"x" is not a member of "(B) b"'])
+ self.expect(
+ "frame variable 'b.x'",
+ error=True,
+ substrs=['"x" is not a member of "(B) b"'],
+ )
diff --git a/lldb/test/API/commands/frame/var/anon-struct/main.cpp b/lldb/test/API/commands/frame/var/anon-struct/main.cpp
index 1deda70376dad5..0cbd8a9c632f02 100644
--- a/lldb/test/API/commands/frame/var/anon-struct/main.cpp
+++ b/lldb/test/API/commands/frame/var/anon-struct/main.cpp
@@ -1,5 +1,4 @@
-int main(int argc, char** argv)
-{
+int main(int argc, char** argv) {
struct A {
struct {
int x = 1;
>From 6ad2f0a319c9401d0dd3e9e3b38e5b6632012854 Mon Sep 17 00:00:00 2001
From: Caroline Tice <cmtice at google.com>
Date: Tue, 3 Dec 2024 13:51:52 -0800
Subject: [PATCH 5/9] Update test to use 'target var' instead of 'frame var'.
Move it to the appropriate location.
---
.../var/anon-struct/TestFrameVarAnonStruct.py | 66 -------------------
.../commands/frame/var/anon-struct/main.cpp | 19 ------
.../var => target}/anon-struct/Makefile | 0
.../anon-struct/TestTargetVarAnonStruct.py | 33 ++++++++++
.../API/commands/target/anon-struct/main.cpp | 19 ++++++
5 files changed, 52 insertions(+), 85 deletions(-)
delete mode 100644 lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
delete mode 100644 lldb/test/API/commands/frame/var/anon-struct/main.cpp
rename lldb/test/API/commands/{frame/var => target}/anon-struct/Makefile (100%)
create mode 100644 lldb/test/API/commands/target/anon-struct/TestTargetVarAnonStruct.py
create mode 100644 lldb/test/API/commands/target/anon-struct/main.cpp
diff --git a/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py b/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
deleted file mode 100644
index 25f4275d0ac7b2..00000000000000
--- a/lldb/test/API/commands/frame/var/anon-struct/TestFrameVarAnonStruct.py
+++ /dev/null
@@ -1,66 +0,0 @@
-"""
-Test handling of Anonymous Structs, especially that they don't crash lldb.
-"""
-
-
-import lldb
-import lldbsuite.test.lldbutil as lldbutil
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-import os
-import shutil
-import time
-
-
-class TestFrameVarAnonStruct(TestBase):
- # If your test case doesn't stress debug info, then
- # set this to true. That way it won't be run once for
- # each debug info format.
- NO_DEBUG_INFO_TESTCASE = True
-
- def test_frame_var(self):
- self.build()
- self.do_test()
-
- def do_test(self):
- target = self.createTestTarget()
-
- # Now create a breakpoint in main.c at the source matching
- # "Set a breakpoint here"
- breakpoint = target.BreakpointCreateBySourceRegex(
- "Set a breakpoint here", lldb.SBFileSpec("main.cpp")
- )
- self.assertTrue(
- breakpoint and breakpoint.GetNumLocations() >= 1, VALID_BREAKPOINT
- )
-
- error = lldb.SBError()
- # This is the launch info. If you want to launch with arguments or
- # environment variables, add them using SetArguments or
- # SetEnvironmentEntries
-
- launch_info = target.GetLaunchInfo()
- process = target.Launch(launch_info, error)
- self.assertTrue(process, PROCESS_IS_VALID)
-
- # Did we hit our breakpoint?
- from lldbsuite.test.lldbutil import get_threads_stopped_at_breakpoint
-
- threads = get_threads_stopped_at_breakpoint(process, breakpoint)
- self.assertEqual(
- len(threads), 1, "There should be a thread stopped at our breakpoint"
- )
-
- # The hit count for the breakpoint should be 1.
- self.assertEqual(breakpoint.GetHitCount(), 1)
-
- frame = threads[0].GetFrameAtIndex(0)
- command_result = lldb.SBCommandReturnObject()
- interp = self.dbg.GetCommandInterpreter()
-
- # Verify that we don't crash in this case.
- self.expect(
- "frame variable 'b.x'",
- error=True,
- substrs=['"x" is not a member of "(B) b"'],
- )
diff --git a/lldb/test/API/commands/frame/var/anon-struct/main.cpp b/lldb/test/API/commands/frame/var/anon-struct/main.cpp
deleted file mode 100644
index 0cbd8a9c632f02..00000000000000
--- a/lldb/test/API/commands/frame/var/anon-struct/main.cpp
+++ /dev/null
@@ -1,19 +0,0 @@
-int main(int argc, char** argv) {
- struct A {
- struct {
- int x = 1;
- };
- int y = 2;
- } a;
-
- struct B {
- // Anonymous struct inherits another struct.
- struct : public A {
- int z = 3;
- };
- int w = 4;
- A a;
- } b;
-
- return 0; // Set a breakpoint here
-}
diff --git a/lldb/test/API/commands/frame/var/anon-struct/Makefile b/lldb/test/API/commands/target/anon-struct/Makefile
similarity index 100%
rename from lldb/test/API/commands/frame/var/anon-struct/Makefile
rename to lldb/test/API/commands/target/anon-struct/Makefile
diff --git a/lldb/test/API/commands/target/anon-struct/TestTargetVarAnonStruct.py b/lldb/test/API/commands/target/anon-struct/TestTargetVarAnonStruct.py
new file mode 100644
index 00000000000000..869081dc6e5e4c
--- /dev/null
+++ b/lldb/test/API/commands/target/anon-struct/TestTargetVarAnonStruct.py
@@ -0,0 +1,33 @@
+"""
+Test handling of Anonymous Structs, especially that they don't crash lldb.
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import os
+import shutil
+import time
+
+
+class TestFrameVarAnonStruct(TestBase):
+ # If your test case doesn't stress debug info, then
+ # set this to true. That way it won't be run once for
+ # each debug info format.
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def test_frame_var(self):
+ self.build()
+ self.do_test()
+
+ def do_test(self):
+ target = self.createTestTarget()
+
+ # Verify that we don't crash in this case.
+ self.expect(
+ "target variable 'b.x'",
+ error=True,
+ substrs=["can't find global variable 'b.x'"],
+ )
diff --git a/lldb/test/API/commands/target/anon-struct/main.cpp b/lldb/test/API/commands/target/anon-struct/main.cpp
new file mode 100644
index 00000000000000..5a782206a1c073
--- /dev/null
+++ b/lldb/test/API/commands/target/anon-struct/main.cpp
@@ -0,0 +1,19 @@
+struct A {
+ struct {
+ int x = 1;
+ };
+ int y = 2;
+} a;
+
+struct B {
+ // Anonymous struct inherits another struct.
+ struct : public A {
+ int z = 3;
+ };
+ int w = 4;
+ A a;
+} b;
+
+int main(int argc, char **argv) {
+ return 0;
+}
>From c3009fdb7dac801fd9fa6625d8e2acc0c944789f Mon Sep 17 00:00:00 2001
From: Caroline Tice <cmtice at google.com>
Date: Wed, 4 Dec 2024 08:55:41 -0800
Subject: [PATCH 6/9] Remove unnecessary fields from test.
---
lldb/test/API/commands/target/anon-struct/main.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/lldb/test/API/commands/target/anon-struct/main.cpp b/lldb/test/API/commands/target/anon-struct/main.cpp
index 5a782206a1c073..072c06227abfae 100644
--- a/lldb/test/API/commands/target/anon-struct/main.cpp
+++ b/lldb/test/API/commands/target/anon-struct/main.cpp
@@ -2,7 +2,6 @@ struct A {
struct {
int x = 1;
};
- int y = 2;
} a;
struct B {
@@ -10,7 +9,6 @@ struct B {
struct : public A {
int z = 3;
};
- int w = 4;
A a;
} b;
>From a8ec2d69ae731a61e63b4e27d4735cb309deaea4 Mon Sep 17 00:00:00 2001
From: Caroline Tice <cmtice at google.com>
Date: Wed, 4 Dec 2024 08:58:51 -0800
Subject: [PATCH 7/9] Remove another unnecessary field from test.
---
lldb/test/API/commands/target/anon-struct/main.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/lldb/test/API/commands/target/anon-struct/main.cpp b/lldb/test/API/commands/target/anon-struct/main.cpp
index 072c06227abfae..415d8d7382b525 100644
--- a/lldb/test/API/commands/target/anon-struct/main.cpp
+++ b/lldb/test/API/commands/target/anon-struct/main.cpp
@@ -9,7 +9,6 @@ struct B {
struct : public A {
int z = 3;
};
- A a;
} b;
int main(int argc, char **argv) {
>From a3bfbb93824bfeeb3d49318a896a16c049a3b6ad Mon Sep 17 00:00:00 2001
From: Caroline Tice <cmtice at google.com>
Date: Wed, 4 Dec 2024 09:44:29 -0800
Subject: [PATCH 8/9] Fix clang-format issues.
---
lldb/test/API/commands/target/anon-struct/main.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/lldb/test/API/commands/target/anon-struct/main.cpp b/lldb/test/API/commands/target/anon-struct/main.cpp
index 415d8d7382b525..fbe26ea0ab8752 100644
--- a/lldb/test/API/commands/target/anon-struct/main.cpp
+++ b/lldb/test/API/commands/target/anon-struct/main.cpp
@@ -11,6 +11,4 @@ struct B {
};
} b;
-int main(int argc, char **argv) {
- return 0;
-}
+int main(int argc, char **argv) { return 0; }
>From f324c3418d799f6687f5a932a66e63923219f749 Mon Sep 17 00:00:00 2001
From: Caroline Tice <cmtice at google.com>
Date: Wed, 4 Dec 2024 10:28:39 -0800
Subject: [PATCH 9/9] Use std::move when restoring the saved vector.
---
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 16eca7700d9fff..46c2002fe139d0 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6759,7 +6759,7 @@ size_t TypeSystemClang::GetIndexOfChildMemberWithName(
if (field_type.GetIndexOfChildMemberWithName(
name, omit_empty_base_classes, child_indexes))
return child_indexes.size();
- child_indexes = save_indices;
+ child_indexes = std::move(save_indices);
} else if (field_name == name) {
// We have to add on the number of base classes to this index!
child_indexes.push_back(
More information about the lldb-commits
mailing list