Fix parsing, remove dereferencing, add unit tests

Author: kuilpd

lldb/docs/dil-expr-lang.ebnf

@@ -5,7 +5,10 @@
 
 expression = unary_expression ;
 
-unary_expression = unary_operator primary_expression ;
+unary_expression = unary_operator expression
+                 | primary_expression ;
+
+unary_operator = "*" | "&" ;
 
 primary_expression = id_expression
                    | "(" expression ")";

lldb/include/lldb/API/SBFrame.h

@@ -182,6 +182,9 @@ class LLDB_API SBFrame {
   // variable value.
   lldb::SBValue GetValueForVariablePath(const char *var_expr_cstr,
                                         DynamicValueType use_dynamic);
+  lldb::SBValue TestGetValueForVariablePath(const char *var_expr_cstr,
+                                            DynamicValueType use_dynamic,
+                                            bool use_DIL = false);
 
   /// The version that doesn't supply a 'use_dynamic' value will use the
   /// target's default.

lldb/include/lldb/Target/StackFrame.h

@@ -308,6 +308,9 @@ class StackFrame : public ExecutionContextScope,
   lldb::ValueObjectSP GetValueForVariableExpressionPath(
       llvm::StringRef var_expr, lldb::DynamicValueType use_dynamic,
       uint32_t options, lldb::VariableSP &var_sp, Status &error);
+  lldb::ValueObjectSP GetValueForVariableExpressionPath(
+      llvm::StringRef var_expr, lldb::DynamicValueType use_dynamic,
+      uint32_t options, lldb::VariableSP &var_sp, Status &error, bool use_DIL);
 
   /// Determine whether this StackFrame has debug information available or not.
   ///

lldb/source/API/SBFrame.cpp

@@ -492,6 +492,41 @@ lldb::SBValue SBFrame::GetValueForVariablePath(const char *var_path,
   return sb_value;
 }
 
+lldb::SBValue SBFrame::TestGetValueForVariablePath(const char *var_path,
+                                                   DynamicValueType use_dynamic,
+                                                   bool use_DIL) {
+  LLDB_INSTRUMENT_VA(this, var_path, use_dynamic);
+
+  SBValue sb_value;
+  if (var_path == nullptr || var_path[0] == '\0') {
+    return sb_value;
+  }
+
+  std::unique_lock<std::recursive_mutex> lock;
+  ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
+
+  StackFrame *frame = nullptr;
+  Target *target = exe_ctx.GetTargetPtr();
+  Process *process = exe_ctx.GetProcessPtr();
+  if (target && process) {
+    Process::StopLocker stop_locker;
+    if (stop_locker.TryLock(&process->GetRunLock())) {
+      frame = exe_ctx.GetFramePtr();
+      if (frame) {
+        VariableSP var_sp;
+        Status error;
+        ValueObjectSP value_sp(frame->GetValueForVariableExpressionPath(
+            var_path, eNoDynamicValues,
+            StackFrame::eExpressionPathOptionCheckPtrVsMember |
+                StackFrame::eExpressionPathOptionsAllowDirectIVarAccess,
+            var_sp, error, use_DIL));
+        sb_value.SetSP(value_sp, use_dynamic);
+      }
+    }
+  }
+  return sb_value;
+}
+
 SBValue SBFrame::FindVariable(const char *name) {
   LLDB_INSTRUMENT_VA(this, name);
 

lldb/source/Target/StackFrame.cpp

@@ -523,6 +523,17 @@ ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
                                                  var_sp, error);
 }
 
+ValueObjectSP StackFrame::GetValueForVariableExpressionPath(
+    llvm::StringRef var_expr, DynamicValueType use_dynamic, uint32_t options,
+    VariableSP &var_sp, Status &error, bool use_DIL) {
+  if (use_DIL)
+    return DILGetValueForVariableExpressionPath(var_expr, use_dynamic, options,
+                                                var_sp, error);
+
+  return LegacyGetValueForVariableExpressionPath(var_expr, use_dynamic, options,
+                                                 var_sp, error);
+}
+
 ValueObjectSP StackFrame::DILGetValueForVariableExpressionPath(
     llvm::StringRef var_expr, lldb::DynamicValueType use_dynamic,
     uint32_t options, lldb::VariableSP &var_sp, Status &error) {
@@ -538,7 +549,7 @@ ValueObjectSP StackFrame::DILGetValueForVariableExpressionPath(
   auto lex_or_err = dil::DILLexer::Create(var_expr);
   if (!lex_or_err) {
     error = Status::FromError(lex_or_err.takeError());
-    return ValueObjectSP();
+    return ValueObjectConstResult::Create(nullptr, std::move(error));
   }
 
   // Parse the expression.
@@ -547,7 +558,7 @@ ValueObjectSP StackFrame::DILGetValueForVariableExpressionPath(
       !no_synth_child, !no_fragile_ivar, check_ptr_vs_member);
   if (!tree_or_error) {
     error = Status::FromError(tree_or_error.takeError());
-    return ValueObjectSP();
+    return ValueObjectConstResult::Create(nullptr, std::move(error));
   }
 
   // Evaluate the parsed expression.
@@ -558,7 +569,7 @@ ValueObjectSP StackFrame::DILGetValueForVariableExpressionPath(
   auto valobj_or_error = interpreter.Evaluate((*tree_or_error).get());
   if (!valobj_or_error) {
     error = Status::FromError(valobj_or_error.takeError());
-    return ValueObjectSP();
+    return ValueObjectConstResult::Create(nullptr, std::move(error));
   }
 
   return *valobj_or_error;

lldb/source/ValueObject/DILEval.cpp

@@ -275,13 +275,6 @@ Interpreter::Visit(const UnaryOpNode *node) {
   }
   lldb::ValueObjectSP rhs = *rhs_or_err;
 
-  if (rhs->GetCompilerType().IsReferenceType()) {
-    rhs = rhs->Dereference(error);
-    if (error.Fail()) {
-      return llvm::make_error<DILDiagnosticError>(m_expr, error.AsCString(),
-                                                  node->GetLocation());
-    }
-  }
   CompilerType rhs_type = rhs->GetCompilerType();
   switch (node->kind()) {
   case UnaryOpKind::Deref: {
@@ -292,8 +285,14 @@ Interpreter::Visit(const UnaryOpNode *node) {
     if (dynamic_rhs)
       rhs = dynamic_rhs;
 
-    if (rhs->GetCompilerType().IsPointerType())
+    if (rhs->GetCompilerType().IsPointerType()) {
+      if (rhs->GetCompilerType().IsPointerToVoid()) {
+        return llvm::make_error<DILDiagnosticError>(
+            m_expr, "indirection not permitted on operand of type 'void *'",
+            node->GetLocation(), 1);
+      }
       return EvaluateDereference(rhs);
+    }
     lldb::ValueObjectSP child_sp = rhs->Dereference(error);
     if (error.Success())
       rhs = child_sp;

lldb/source/ValueObject/DILParser.cpp

@@ -87,7 +87,8 @@ ASTNodeUP DILParser::ParseExpression() { return ParseUnaryExpression(); }
 // Parse an unary_expression.
 //
 //  unary_expression:
-//    unary_operator primary_expression
+//    unary_operator expression
+//    primary_expression
 //
 //  unary_operator:
 //    "&"
@@ -98,7 +99,7 @@ ASTNodeUP DILParser::ParseUnaryExpression() {
     Token token = CurToken();
     uint32_t loc = token.GetLocation();
     m_dil_lexer.Advance();
-    auto rhs = ParsePrimaryExpression();
+    auto rhs = ParseExpression();
     switch (token.GetKind()) {
     case Token::star:
       return std::make_unique<UnaryOpNode>(loc, UnaryOpKind::Deref,

lldb/unittests/CMakeLists.txt

@@ -54,6 +54,7 @@ endif()
 add_subdirectory(Breakpoint)
 add_subdirectory(Callback)
 add_subdirectory(Core)
+add_subdirectory(DIL)
 add_subdirectory(DataFormatter)
 add_subdirectory(Disassembler)
 add_subdirectory(Editline)

lldb/unittests/DIL/CMakeLists.txt

@@ -0,0 +1,15 @@
+add_lldb_unittest(DILTests
+  DILTests.cpp
+  DILLexerTests.cpp
+  Runner.cpp
+
+  LINK_LIBS
+    liblldb
+    lldbUtilityHelpers
+    lldbValueObject
+    LLVMTestingSupport
+  )
+add_subdirectory(Inputs)
+add_dependencies(DILTests test_binary)
+
+add_unittest_inputs(DILTests "test_binary.cpp")