[Support] [lldb] Fix thread jump #45326 #135778
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-binary-utilities @llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) ChangesFixes #45326 When you thread jump by calling Full diff: https://github.com/llvm/llvm-project/pull/135778.diff 3 Files Affected:
diff --git a/lldb/test/API/functionalities/thread/jump/TestThreadJump.py b/lldb/test/API/functionalities/thread/jump/TestThreadJump.py
index 3c13a969bc3fd..d603580ac6f36 100644
--- a/lldb/test/API/functionalities/thread/jump/TestThreadJump.py
+++ b/lldb/test/API/functionalities/thread/jump/TestThreadJump.py
@@ -10,9 +10,12 @@
class ThreadJumpTestCase(TestBase):
+ def setUp(self):
+ TestBase.setUp(self)
+ self.build()
+
def test(self):
"""Test thread jump handling."""
- self.build()
exe = self.getBuildArtifact("a.out")
self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
@@ -62,6 +65,71 @@ def test(self):
substrs=["error"],
)
+ def test_jump_offset(self):
+ """Test Thread Jump by negative or positive offset"""
+ exe = self.getBuildArtifact("a.out")
+ file_name = "main.cpp"
+ self.runCmd(f"target create {exe}", CURRENT_EXECUTABLE_SET)
+
+ pos_jump = line_number(file_name, "// jump_offset 1")
+ neg_jump = line_number(file_name, "// jump_offset 2")
+ pos_breakpoint = line_number(file_name, "// breakpoint 1")
+ neg_breakpoint = line_number(file_name, "// breakpoint 2")
+ pos_jump_offset = pos_jump - pos_breakpoint
+ neg_jump_offset = neg_jump - neg_breakpoint
+
+ var_1, var_1_value = ("var_1", "10")
+ var_2, var_2_value = ("var_2", "40")
+ var_3, var_3_value = ("var_3", "10")
+
+ # create pos_breakpoint and neg_breakpoint
+ lldbutil.run_break_set_by_file_and_line(
+ self, file_name, pos_breakpoint, num_expected_locations=1
+ )
+ lldbutil.run_break_set_by_file_and_line(
+ self, file_name, neg_breakpoint, num_expected_locations=1
+ )
+
+ self.runCmd("run", RUN_SUCCEEDED)
+
+ # test positive jump
+ # The stop reason of the thread should be breakpoint 1.
+ self.expect(
+ "thread list",
+ STOPPED_DUE_TO_BREAKPOINT + " 1",
+ substrs=[
+ "stopped",
+ f"{file_name}:{pos_breakpoint}",
+ "stop reason = breakpoint 1",
+ ],
+ )
+
+ self.runCmd(f"thread jump --by +{pos_jump_offset}")
+ self.expect("process status", substrs=[f"at {file_name}:{pos_jump}"])
+ self.expect(f"print {var_1}", substrs=[var_1_value])
+
+ self.runCmd("thread step-over")
+ self.expect(f"print {var_2}", substrs=[var_2_value])
+
+ self.runCmd("continue")
+
+ # test negative jump
+ # The stop reason of the thread should be breakpoint 1.
+ self.expect(
+ "thread list",
+ STOPPED_DUE_TO_BREAKPOINT + " 2",
+ substrs=[
+ "stopped",
+ f"{file_name}:{neg_breakpoint}",
+ "stop reason = breakpoint 2",
+ ],
+ )
+
+ self.runCmd(f"thread jump --by {neg_jump_offset}")
+ self.expect("process status", substrs=[f"at {file_name}:{neg_jump}"])
+ self.runCmd("thread step-over")
+ self.expect(f"print {var_3}", substrs=[var_3_value])
+
def do_min_test(self, start, jump, var, value):
# jump to the start marker
self.runCmd("j %i" % start)
diff --git a/llvm/lib/Support/StringRef.cpp b/llvm/lib/Support/StringRef.cpp
index 4f5fcb4857e80..bdf7a9aa5c7e0 100644
--- a/llvm/lib/Support/StringRef.cpp
+++ b/llvm/lib/Support/StringRef.cpp
@@ -409,6 +409,9 @@ static unsigned GetAutoSenseRadix(StringRef &Str) {
bool llvm::consumeUnsignedInteger(StringRef &Str, unsigned Radix,
unsigned long long &Result) {
+ // Consume the + value
+ Str.consume_front("+");
+
// Autosense radix if not specified.
if (Radix == 0)
Radix = GetAutoSenseRadix(Str);
diff --git a/llvm/unittests/ADT/StringRefTest.cpp b/llvm/unittests/ADT/StringRefTest.cpp
index ec9cdc197597d..55d222a915ab5 100644
--- a/llvm/unittests/ADT/StringRefTest.cpp
+++ b/llvm/unittests/ADT/StringRefTest.cpp
@@ -622,19 +622,21 @@ TEST(StringRefTest, Hashing) {
struct UnsignedPair {
const char *Str;
uint64_t Expected;
-} Unsigned[] =
- { {"0", 0}
- , {"255", 255}
- , {"256", 256}
- , {"65535", 65535}
- , {"65536", 65536}
- , {"4294967295", 4294967295ULL}
- , {"4294967296", 4294967296ULL}
- , {"18446744073709551615", 18446744073709551615ULL}
- , {"042", 34}
- , {"0x42", 66}
- , {"0b101010", 42}
- };
+} Unsigned[] = {{"0", 0},
+ {"255", 255},
+ {"256", 256},
+ {"65535", 65535},
+ {"65536", 65536},
+ {"4294967295", 4294967295ULL},
+ {"4294967296", 4294967296ULL},
+ {"18446744073709551615", 18446744073709551615ULL},
+ {"042", 34},
+ {"0x42", 66},
+ {"0b101010", 42},
+ {"+42", 42},
+ {"+042", 34},
+ {"+0x42", 66},
+ {"+0b101010", 42}};
struct SignedPair {
const char *Str;
|
|
@llvm/pr-subscribers-llvm-support Author: Ebuka Ezike (da-viper) ChangesFixes #45326 When you thread jump by calling Full diff: https://github.com/llvm/llvm-project/pull/135778.diff 3 Files Affected:
diff --git a/lldb/test/API/functionalities/thread/jump/TestThreadJump.py b/lldb/test/API/functionalities/thread/jump/TestThreadJump.py
index 3c13a969bc3fd..d603580ac6f36 100644
--- a/lldb/test/API/functionalities/thread/jump/TestThreadJump.py
+++ b/lldb/test/API/functionalities/thread/jump/TestThreadJump.py
@@ -10,9 +10,12 @@
class ThreadJumpTestCase(TestBase):
+ def setUp(self):
+ TestBase.setUp(self)
+ self.build()
+
def test(self):
"""Test thread jump handling."""
- self.build()
exe = self.getBuildArtifact("a.out")
self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
@@ -62,6 +65,71 @@ def test(self):
substrs=["error"],
)
+ def test_jump_offset(self):
+ """Test Thread Jump by negative or positive offset"""
+ exe = self.getBuildArtifact("a.out")
+ file_name = "main.cpp"
+ self.runCmd(f"target create {exe}", CURRENT_EXECUTABLE_SET)
+
+ pos_jump = line_number(file_name, "// jump_offset 1")
+ neg_jump = line_number(file_name, "// jump_offset 2")
+ pos_breakpoint = line_number(file_name, "// breakpoint 1")
+ neg_breakpoint = line_number(file_name, "// breakpoint 2")
+ pos_jump_offset = pos_jump - pos_breakpoint
+ neg_jump_offset = neg_jump - neg_breakpoint
+
+ var_1, var_1_value = ("var_1", "10")
+ var_2, var_2_value = ("var_2", "40")
+ var_3, var_3_value = ("var_3", "10")
+
+ # create pos_breakpoint and neg_breakpoint
+ lldbutil.run_break_set_by_file_and_line(
+ self, file_name, pos_breakpoint, num_expected_locations=1
+ )
+ lldbutil.run_break_set_by_file_and_line(
+ self, file_name, neg_breakpoint, num_expected_locations=1
+ )
+
+ self.runCmd("run", RUN_SUCCEEDED)
+
+ # test positive jump
+ # The stop reason of the thread should be breakpoint 1.
+ self.expect(
+ "thread list",
+ STOPPED_DUE_TO_BREAKPOINT + " 1",
+ substrs=[
+ "stopped",
+ f"{file_name}:{pos_breakpoint}",
+ "stop reason = breakpoint 1",
+ ],
+ )
+
+ self.runCmd(f"thread jump --by +{pos_jump_offset}")
+ self.expect("process status", substrs=[f"at {file_name}:{pos_jump}"])
+ self.expect(f"print {var_1}", substrs=[var_1_value])
+
+ self.runCmd("thread step-over")
+ self.expect(f"print {var_2}", substrs=[var_2_value])
+
+ self.runCmd("continue")
+
+ # test negative jump
+ # The stop reason of the thread should be breakpoint 1.
+ self.expect(
+ "thread list",
+ STOPPED_DUE_TO_BREAKPOINT + " 2",
+ substrs=[
+ "stopped",
+ f"{file_name}:{neg_breakpoint}",
+ "stop reason = breakpoint 2",
+ ],
+ )
+
+ self.runCmd(f"thread jump --by {neg_jump_offset}")
+ self.expect("process status", substrs=[f"at {file_name}:{neg_jump}"])
+ self.runCmd("thread step-over")
+ self.expect(f"print {var_3}", substrs=[var_3_value])
+
def do_min_test(self, start, jump, var, value):
# jump to the start marker
self.runCmd("j %i" % start)
diff --git a/llvm/lib/Support/StringRef.cpp b/llvm/lib/Support/StringRef.cpp
index 4f5fcb4857e80..bdf7a9aa5c7e0 100644
--- a/llvm/lib/Support/StringRef.cpp
+++ b/llvm/lib/Support/StringRef.cpp
@@ -409,6 +409,9 @@ static unsigned GetAutoSenseRadix(StringRef &Str) {
bool llvm::consumeUnsignedInteger(StringRef &Str, unsigned Radix,
unsigned long long &Result) {
+ // Consume the + value
+ Str.consume_front("+");
+
// Autosense radix if not specified.
if (Radix == 0)
Radix = GetAutoSenseRadix(Str);
diff --git a/llvm/unittests/ADT/StringRefTest.cpp b/llvm/unittests/ADT/StringRefTest.cpp
index ec9cdc197597d..55d222a915ab5 100644
--- a/llvm/unittests/ADT/StringRefTest.cpp
+++ b/llvm/unittests/ADT/StringRefTest.cpp
@@ -622,19 +622,21 @@ TEST(StringRefTest, Hashing) {
struct UnsignedPair {
const char *Str;
uint64_t Expected;
-} Unsigned[] =
- { {"0", 0}
- , {"255", 255}
- , {"256", 256}
- , {"65535", 65535}
- , {"65536", 65536}
- , {"4294967295", 4294967295ULL}
- , {"4294967296", 4294967296ULL}
- , {"18446744073709551615", 18446744073709551615ULL}
- , {"042", 34}
- , {"0x42", 66}
- , {"0b101010", 42}
- };
+} Unsigned[] = {{"0", 0},
+ {"255", 255},
+ {"256", 256},
+ {"65535", 65535},
+ {"65536", 65536},
+ {"4294967295", 4294967295ULL},
+ {"4294967296", 4294967296ULL},
+ {"18446744073709551615", 18446744073709551615ULL},
+ {"042", 34},
+ {"0x42", 66},
+ {"0b101010", 42},
+ {"+42", 42},
+ {"+042", 34},
+ {"+0x42", 66},
+ {"+0b101010", 42}};
struct SignedPair {
const char *Str;
|
|
@llvm/pr-subscribers-llvm-adt Author: Ebuka Ezike (da-viper) ChangesFixes #45326 When you thread jump by calling Full diff: https://github.com/llvm/llvm-project/pull/135778.diff 3 Files Affected:
diff --git a/lldb/test/API/functionalities/thread/jump/TestThreadJump.py b/lldb/test/API/functionalities/thread/jump/TestThreadJump.py
index 3c13a969bc3fd..d603580ac6f36 100644
--- a/lldb/test/API/functionalities/thread/jump/TestThreadJump.py
+++ b/lldb/test/API/functionalities/thread/jump/TestThreadJump.py
@@ -10,9 +10,12 @@
class ThreadJumpTestCase(TestBase):
+ def setUp(self):
+ TestBase.setUp(self)
+ self.build()
+
def test(self):
"""Test thread jump handling."""
- self.build()
exe = self.getBuildArtifact("a.out")
self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
@@ -62,6 +65,71 @@ def test(self):
substrs=["error"],
)
+ def test_jump_offset(self):
+ """Test Thread Jump by negative or positive offset"""
+ exe = self.getBuildArtifact("a.out")
+ file_name = "main.cpp"
+ self.runCmd(f"target create {exe}", CURRENT_EXECUTABLE_SET)
+
+ pos_jump = line_number(file_name, "// jump_offset 1")
+ neg_jump = line_number(file_name, "// jump_offset 2")
+ pos_breakpoint = line_number(file_name, "// breakpoint 1")
+ neg_breakpoint = line_number(file_name, "// breakpoint 2")
+ pos_jump_offset = pos_jump - pos_breakpoint
+ neg_jump_offset = neg_jump - neg_breakpoint
+
+ var_1, var_1_value = ("var_1", "10")
+ var_2, var_2_value = ("var_2", "40")
+ var_3, var_3_value = ("var_3", "10")
+
+ # create pos_breakpoint and neg_breakpoint
+ lldbutil.run_break_set_by_file_and_line(
+ self, file_name, pos_breakpoint, num_expected_locations=1
+ )
+ lldbutil.run_break_set_by_file_and_line(
+ self, file_name, neg_breakpoint, num_expected_locations=1
+ )
+
+ self.runCmd("run", RUN_SUCCEEDED)
+
+ # test positive jump
+ # The stop reason of the thread should be breakpoint 1.
+ self.expect(
+ "thread list",
+ STOPPED_DUE_TO_BREAKPOINT + " 1",
+ substrs=[
+ "stopped",
+ f"{file_name}:{pos_breakpoint}",
+ "stop reason = breakpoint 1",
+ ],
+ )
+
+ self.runCmd(f"thread jump --by +{pos_jump_offset}")
+ self.expect("process status", substrs=[f"at {file_name}:{pos_jump}"])
+ self.expect(f"print {var_1}", substrs=[var_1_value])
+
+ self.runCmd("thread step-over")
+ self.expect(f"print {var_2}", substrs=[var_2_value])
+
+ self.runCmd("continue")
+
+ # test negative jump
+ # The stop reason of the thread should be breakpoint 1.
+ self.expect(
+ "thread list",
+ STOPPED_DUE_TO_BREAKPOINT + " 2",
+ substrs=[
+ "stopped",
+ f"{file_name}:{neg_breakpoint}",
+ "stop reason = breakpoint 2",
+ ],
+ )
+
+ self.runCmd(f"thread jump --by {neg_jump_offset}")
+ self.expect("process status", substrs=[f"at {file_name}:{neg_jump}"])
+ self.runCmd("thread step-over")
+ self.expect(f"print {var_3}", substrs=[var_3_value])
+
def do_min_test(self, start, jump, var, value):
# jump to the start marker
self.runCmd("j %i" % start)
diff --git a/llvm/lib/Support/StringRef.cpp b/llvm/lib/Support/StringRef.cpp
index 4f5fcb4857e80..bdf7a9aa5c7e0 100644
--- a/llvm/lib/Support/StringRef.cpp
+++ b/llvm/lib/Support/StringRef.cpp
@@ -409,6 +409,9 @@ static unsigned GetAutoSenseRadix(StringRef &Str) {
bool llvm::consumeUnsignedInteger(StringRef &Str, unsigned Radix,
unsigned long long &Result) {
+ // Consume the + value
+ Str.consume_front("+");
+
// Autosense radix if not specified.
if (Radix == 0)
Radix = GetAutoSenseRadix(Str);
diff --git a/llvm/unittests/ADT/StringRefTest.cpp b/llvm/unittests/ADT/StringRefTest.cpp
index ec9cdc197597d..55d222a915ab5 100644
--- a/llvm/unittests/ADT/StringRefTest.cpp
+++ b/llvm/unittests/ADT/StringRefTest.cpp
@@ -622,19 +622,21 @@ TEST(StringRefTest, Hashing) {
struct UnsignedPair {
const char *Str;
uint64_t Expected;
-} Unsigned[] =
- { {"0", 0}
- , {"255", 255}
- , {"256", 256}
- , {"65535", 65535}
- , {"65536", 65536}
- , {"4294967295", 4294967295ULL}
- , {"4294967296", 4294967296ULL}
- , {"18446744073709551615", 18446744073709551615ULL}
- , {"042", 34}
- , {"0x42", 66}
- , {"0b101010", 42}
- };
+} Unsigned[] = {{"0", 0},
+ {"255", 255},
+ {"256", 256},
+ {"65535", 65535},
+ {"65536", 65536},
+ {"4294967295", 4294967295ULL},
+ {"4294967296", 4294967296ULL},
+ {"18446744073709551615", 18446744073709551615ULL},
+ {"042", 34},
+ {"0x42", 66},
+ {"0b101010", 42},
+ {"+42", 42},
+ {"+042", 34},
+ {"+0x42", 66},
+ {"+0b101010", 42}};
struct SignedPair {
const char *Str;
|
da-viper
force-pushed
the
fix-thread-jump-#45326
branch
from
becf2af to
90ec3ff
Compare
|
having a $ projects/test_lldb → addr2line --exe=/home/test/Inputs/symbols.so 0x1138
/tmp/dbginfo/symbols.part1.cpp:12
$ projects/test_lldb → addr2line --exe=/home/test/Inputs/symbols.so +0x1138
/tmp/dbginfo/symbols.part1.cpp:12
$ projects/test_lldb → addr2line --exe=/home/test/Inputs/symbols.so -a +0x1138
0x0000000000001138
/tmp/dbginfo/symbols.part1.cpp:12 |
|
Looks like this could/should be 2-3 comits. The ADT change is one, the llvm-symbolizer could be another (with or without the lldb test coverage), then possibly the lldb test coverage as a separate third step. (the contents of the patch I don't have much opinion on, I haven't even thought about whether the ADT change is good/bad, etc) |
|
I wasn't sure if I should split it as they are quite small on their own. Will create a two new PR for the |
da-viper
force-pushed
the
fix-thread-jump-#45326
branch
from
f5c3ac9 to
6704733
Compare
Signed-off-by: Ebuka Ezike <[email protected]>
Signed-off-by: Ebuka Ezike <[email protected]>
da-viper
force-pushed
the
fix-thread-jump-#45326
branch
from
6704733 to
a84c26a
Compare
checking if it starts_with prefix is redundant since consume front also check it
ashgti
reviewed
May 13, 2025
| @@ -1649,11 +1649,14 @@ class CommandObjectThreadJump : public CommandObjectParsed { | |||
| return Status::FromErrorStringWithFormat("invalid line number: '%s'.", | |||
| option_arg.str().c_str()); | |||
| break; | |||
| case 'b': | |||
| case 'b': { | |||
| option_arg.consume_front("+"); | |||
There was a problem hiding this comment.
Should this be documented that an optional + can be specified?
There was a problem hiding this comment.
It is for _regexp-jump, as for the full thread jump command users are most like to just write the number without the plus.
There was a problem hiding this comment.
I was just think that
llvm-project/lldb/source/Commands/Options.td
Line 1139 in 88c4ef2
+ is supported or that the value can be negative. Also I can't tell, at least from the help text, if this includes whitespace or not.
Like if I had:
int a = 3;
a = 1;
a = 2; //<pc=here> thread jump -b -1 does this go to the blank line?
There was a problem hiding this comment.
added documentation clarifying it can be negative or positive.
ashgti
approved these changes
May 27, 2025
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
Fixes llvm#45326 When you thread jump by calling `j +2` or `thread jump --by +2` the offset is not recognised. This commit fixes that. --------- Signed-off-by: Ebuka Ezike <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #45326
When you thread jump by calling
j +2orthread jump --by +2the offset is not recognised. This commit fixes that.