[LLDB] Add bit extraction to DIL #141422
Open
[LLDB] Add bit extraction to DIL #141422
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-lldb Author: Ilia Kuklin (kuilpd) ChangesFull diff: https://github.com/llvm/llvm-project/pull/141422.diff 11 Files Affected:
diff --git a/lldb/include/lldb/ValueObject/DILAST.h b/lldb/include/lldb/ValueObject/DILAST.h
index 6c7838e05c93c..709f0639135f1 100644
--- a/lldb/include/lldb/ValueObject/DILAST.h
+++ b/lldb/include/lldb/ValueObject/DILAST.h
@@ -19,6 +19,7 @@ namespace lldb_private::dil {
/// The various types DIL AST nodes (used by the DIL parser).
enum class NodeKind {
eArraySubscriptNode,
+ eBitExtractionNode,
eErrorNode,
eIdentifierNode,
eMemberOfNode,
@@ -153,6 +154,30 @@ class ArraySubscriptNode : public ASTNode {
int64_t m_index;
};
+class BitFieldExtractionNode : public ASTNode {
+public:
+ BitFieldExtractionNode(uint32_t location, ASTNodeUP base, int64_t first_index,
+ int64_t last_index)
+ : ASTNode(location, NodeKind::eBitExtractionNode),
+ m_base(std::move(base)), m_first_index(first_index),
+ m_last_index(last_index) {}
+
+ llvm::Expected<lldb::ValueObjectSP> Accept(Visitor *v) const override;
+
+ ASTNode *GetBase() const { return m_base.get(); }
+ int64_t GetFirstIndex() const { return m_first_index; }
+ int64_t GetLastIndex() const { return m_last_index; }
+
+ static bool classof(const ASTNode *node) {
+ return node->GetKind() == NodeKind::eBitExtractionNode;
+ }
+
+private:
+ ASTNodeUP m_base;
+ int64_t m_first_index;
+ int64_t m_last_index;
+};
+
/// This class contains one Visit method for each specialized type of
/// DIL AST node. The Visit methods are used to dispatch a DIL AST node to
/// the correct function in the DIL expression evaluator for evaluating that
@@ -168,6 +193,8 @@ class Visitor {
Visit(const UnaryOpNode *node) = 0;
virtual llvm::Expected<lldb::ValueObjectSP>
Visit(const ArraySubscriptNode *node) = 0;
+ virtual llvm::Expected<lldb::ValueObjectSP>
+ Visit(const BitFieldExtractionNode *node) = 0;
};
} // namespace lldb_private::dil
diff --git a/lldb/include/lldb/ValueObject/DILEval.h b/lldb/include/lldb/ValueObject/DILEval.h
index 9d0fa53c6622a..2a0cb548a810f 100644
--- a/lldb/include/lldb/ValueObject/DILEval.h
+++ b/lldb/include/lldb/ValueObject/DILEval.h
@@ -54,6 +54,8 @@ class Interpreter : Visitor {
llvm::Expected<lldb::ValueObjectSP> Visit(const UnaryOpNode *node) override;
llvm::Expected<lldb::ValueObjectSP>
Visit(const ArraySubscriptNode *node) override;
+ llvm::Expected<lldb::ValueObjectSP>
+ Visit(const BitFieldExtractionNode *node) override;
// Used by the interpreter to create objects, perform casts, etc.
lldb::TargetSP m_target;
diff --git a/lldb/include/lldb/ValueObject/DILLexer.h b/lldb/include/lldb/ValueObject/DILLexer.h
index 7d70f88f9a718..9c1ba97680253 100644
--- a/lldb/include/lldb/ValueObject/DILLexer.h
+++ b/lldb/include/lldb/ValueObject/DILLexer.h
@@ -31,6 +31,7 @@ class Token {
identifier,
l_paren,
l_square,
+ minus,
numeric_constant,
period,
r_paren,
diff --git a/lldb/source/ValueObject/DILAST.cpp b/lldb/source/ValueObject/DILAST.cpp
index 8b5e64ad462cc..b1cd824c2299e 100644
--- a/lldb/source/ValueObject/DILAST.cpp
+++ b/lldb/source/ValueObject/DILAST.cpp
@@ -32,4 +32,9 @@ ArraySubscriptNode::Accept(Visitor *v) const {
return v->Visit(this);
}
+llvm::Expected<lldb::ValueObjectSP>
+BitFieldExtractionNode::Accept(Visitor *v) const {
+ return v->Visit(this);
+}
+
} // namespace lldb_private::dil
diff --git a/lldb/source/ValueObject/DILEval.cpp b/lldb/source/ValueObject/DILEval.cpp
index c8cb54aa18a93..b2bb4e20ddc24 100644
--- a/lldb/source/ValueObject/DILEval.cpp
+++ b/lldb/source/ValueObject/DILEval.cpp
@@ -430,4 +430,36 @@ Interpreter::Visit(const ArraySubscriptNode *node) {
return base->GetSyntheticArrayMember(signed_child_idx, true);
}
+llvm::Expected<lldb::ValueObjectSP>
+Interpreter::Visit(const BitFieldExtractionNode *node) {
+ auto lhs_or_err = Evaluate(node->GetBase());
+ if (!lhs_or_err)
+ return lhs_or_err;
+ lldb::ValueObjectSP base = *lhs_or_err;
+ int64_t first_index = node->GetFirstIndex();
+ int64_t last_index = node->GetLastIndex();
+
+ // if the format given is [high-low], swap range
+ if (first_index > last_index)
+ std::swap(first_index, last_index);
+
+ Status error;
+ if (base->GetCompilerType().IsReferenceType()) {
+ base = base->Dereference(error);
+ if (error.Fail())
+ return error.ToError();
+ }
+ lldb::ValueObjectSP child_valobj_sp =
+ base->GetSyntheticBitFieldChild(first_index, last_index, true);
+ if (!child_valobj_sp) {
+ std::string message = llvm::formatv(
+ "bitfield range {0}-{1} is not valid for \"({2}) {3}\"", first_index,
+ last_index, base->GetTypeName().AsCString("<invalid type>"),
+ base->GetName().AsCString());
+ return llvm::make_error<DILDiagnosticError>(m_expr, message,
+ node->GetLocation());
+ }
+ return child_valobj_sp;
+}
+
} // namespace lldb_private::dil
diff --git a/lldb/source/ValueObject/DILLexer.cpp b/lldb/source/ValueObject/DILLexer.cpp
index 99182d2da1131..eaefaf484bc18 100644
--- a/lldb/source/ValueObject/DILLexer.cpp
+++ b/lldb/source/ValueObject/DILLexer.cpp
@@ -34,6 +34,8 @@ llvm::StringRef Token::GetTokenName(Kind kind) {
return "l_paren";
case Kind::l_square:
return "l_square";
+ case Kind::minus:
+ return "minus";
case Kind::numeric_constant:
return "numeric_constant";
case Kind::period:
@@ -113,8 +115,9 @@ llvm::Expected<Token> DILLexer::Lex(llvm::StringRef expr,
constexpr std::pair<Token::Kind, const char *> operators[] = {
{Token::amp, "&"}, {Token::arrow, "->"}, {Token::coloncolon, "::"},
- {Token::l_paren, "("}, {Token::l_square, "["}, {Token::period, "."},
- {Token::r_paren, ")"}, {Token::r_square, "]"}, {Token::star, "*"},
+ {Token::l_paren, "("}, {Token::l_square, "["}, {Token::minus, "-"},
+ {Token::period, "."}, {Token::r_paren, ")"}, {Token::r_square, "]"},
+ {Token::star, "*"},
};
for (auto [kind, str] : operators) {
if (remainder.consume_front(str))
diff --git a/lldb/source/ValueObject/DILParser.cpp b/lldb/source/ValueObject/DILParser.cpp
index 9667885734f21..32af0820acb98 100644
--- a/lldb/source/ValueObject/DILParser.cpp
+++ b/lldb/source/ValueObject/DILParser.cpp
@@ -120,6 +120,7 @@ ASTNodeUP DILParser::ParseUnaryExpression() {
// postfix_expression:
// primary_expression
// postfix_expression "[" integer_literal "]"
+// postfix_expression "[" integer_literal "-" integer_literal "]"
// postfix_expression "." id_expression
// postfix_expression "->" id_expression
//
@@ -131,17 +132,30 @@ ASTNodeUP DILParser::ParsePostfixExpression() {
switch (token.GetKind()) {
case Token::l_square: {
m_dil_lexer.Advance();
- std::optional<int64_t> rhs = ParseIntegerConstant();
- if (!rhs) {
+ std::optional<int64_t> index = ParseIntegerConstant();
+ if (!index) {
BailOut(
llvm::formatv("failed to parse integer constant: {0}", CurToken()),
CurToken().GetLocation(), CurToken().GetSpelling().length());
return std::make_unique<ErrorNode>();
}
+ if (CurToken().GetKind() == Token::minus) {
+ m_dil_lexer.Advance();
+ std::optional<int64_t> last_index = ParseIntegerConstant();
+ if (!last_index) {
+ BailOut(llvm::formatv("failed to parse integer constant: {0}",
+ CurToken()),
+ CurToken().GetLocation(), CurToken().GetSpelling().length());
+ return std::make_unique<ErrorNode>();
+ }
+ lhs = std::make_unique<BitFieldExtractionNode>(
+ loc, std::move(lhs), std::move(*index), std::move(*last_index));
+ } else {
+ lhs = std::make_unique<ArraySubscriptNode>(loc, std::move(lhs),
+ std::move(*index));
+ }
Expect(Token::r_square);
m_dil_lexer.Advance();
- lhs = std::make_unique<ArraySubscriptNode>(loc, std::move(lhs),
- std::move(*rhs));
break;
}
case Token::period:
diff --git a/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py b/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py
index 740a64f1477ff..c28ec6bda799e 100644
--- a/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py
+++ b/lldb/test/API/commands/frame/var-dil/basics/ArraySubscript/TestFrameVarDILArraySubscript.py
@@ -62,7 +62,7 @@ def test_subscript(self):
self.expect(
"frame var 'int_arr[-1]'",
error=True,
- substrs=["unrecognized token"],
+ substrs=["failed to parse integer constant"],
)
# Test for floating point index
diff --git a/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/Makefile b/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/TestFrameVarDILBitFieldExtraction.py b/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/TestFrameVarDILBitFieldExtraction.py
new file mode 100644
index 0000000000000..7b5ef0650b6e1
--- /dev/null
+++ b/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/TestFrameVarDILBitFieldExtraction.py
@@ -0,0 +1,56 @@
+"""
+Test DIL BifField extraction.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test import lldbutil
+
+
+class TestFrameVarDILBitFieldExtraction(TestBase):
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def expect_var_path(self, expr, compare_to_framevar=False, value=None, type=None):
+ value_dil = super().expect_var_path(expr, value=value, type=type)
+ if compare_to_framevar:
+ self.runCmd("settings set target.experimental.use-DIL false")
+ value_frv = super().expect_var_path(expr, value=value, type=type)
+ self.runCmd("settings set target.experimental.use-DIL true")
+ self.assertEqual(value_dil.GetValue(), value_frv.GetValue())
+
+ def test_bitfield_extraction(self):
+ self.build()
+ lldbutil.run_to_source_breakpoint(
+ self, "Set a breakpoint here", lldb.SBFileSpec("main.cpp")
+ )
+
+ self.runCmd("settings set target.experimental.use-DIL true")
+
+ # Test ranges and type
+ self.expect_var_path("value[0-1]", True, value="3", type="int:2")
+ self.expect_var_path("value[4-7]", True, value="7", type="int:4")
+ self.expect_var_path("value[7-0]", True, value="115", type="int:8")
+
+ # Test reference and dereferenced pointer
+ self.expect_var_path("value_ref[0-1]", value="3", type="int:2")
+ self.expect_var_path("(*value_ptr)[0-1]", value="3", type="int:2")
+
+ # Test array and pointer
+ self.expect(
+ "frame var 'int_arr[0-2]'",
+ error=True,
+ substrs=["bitfield range 0-2 is not valid"],
+ )
+ self.expect(
+ "frame var 'value_ptr[0-1]'",
+ error=True,
+ substrs=["bitfield range 0-1 is not valid"],
+ )
+
+ # Test invalid input
+ self.expect(
+ "frame var 'value[1-]'",
+ error=True,
+ substrs=["failed to parse integer constant"],
+ )
diff --git a/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/main.cpp b/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/main.cpp
new file mode 100644
index 0000000000000..a35f68a9e30a8
--- /dev/null
+++ b/lldb/test/API/commands/frame/var-dil/basics/BitFieldExtraction/main.cpp
@@ -0,0 +1,9 @@
+int main(int argc, char **argv) {
+ int value = 0b01110011;
+ int &value_ref = value;
+ int *value_ptr = &value;
+
+ int int_arr[] = {7, 3, 1};
+
+ return 0; // Set a breakpoint here
+}
|
labath
approved these changes
May 29, 2025
There was a problem hiding this comment.
A straight-forward patch. I have one comment, but that's really a problem with the previous PR, so it'd be better to handle that separately.
The interesting (and very tricky) question is what to do about this feature in the long run, as it conflicts with the usage of - for subtraction (which, I assume, you're still interested in implementing).
Options I see are:
- treat
-inside[]specially. I'm not particularly fond of that as one may legitimately want to use subtraction in an indexing expression. We could work around that by forcing you to writefoo[(a-b)], but I think people would forget that most of the time. - replace
-with another character -- probably:which is the "standard" syntax for array slicing and should not conflict with other language features - drop it altogether as this could be implemented via bitwise operations (
(foo>>a)&((1<<b)-1)) -- once they're implemented
For the last two, we would probably need some sort of a transitionary period where we accept the existing syntax, depending how much use does this feature get (personally, this is the first time I've seen it).
I'm sure @jimingham will have something to say about this. :)
| @@ -62,7 +62,7 @@ def test_subscript(self): | |||
| self.expect( | |||
| "frame var 'int_arr[-1]'", | |||
| error=True, | |||
| substrs=["unrecognized token"], | |||
| substrs=["failed to parse integer constant"], | |||
There was a problem hiding this comment.
This is more of the previous PR, but I think this should actually work. Current "frame var" will happily access a pointer/array with negative indexes.
|
I agree with Pavel that using If we are worried about supporting older uses of path expressions while we're quite dramatically expanding what you can express in this syntax, we probably should make the parser have a mode where it emulates the old path expression, turning off the new operators.
|
My 2 cents: I prefer using ':' |
FTR, I am not worried about supporting older uses. I was just anticipating that someone might be. As far as I am concerned we can pretty much any aspect of the path expression (but let's do that after flipping the flag).
Ah, I didn't realise this was so diverse. |
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.
No description provided.