PostfixExpression: move DWARF generator out of NativePDB internals

[labath]

Summary

The new dwarf generator is pretty much a verbatim copy of the one in
PDB.

In order to write a pdb-independent test for it, I needed to write a
dummy "symbol resolver", which (together with the fact that I'll need
one more for breakpad-specific resolution logic) prompted me to create a
more simple interface for algorithms which replace or "resolve"
SymbolNodes. The resolving algorithms in NativePDB have been updated to
make use of that too.

I have removed a couple of NativePDB tests which weren't testing
anything pdb-specific and where the tested functionality was covered by
the new format-agnostic tests I have added.

[amccarth]

Can the test deletions be a separate patch, or will they fail because of the other changes in this patch? They feel like a good but separable step.

include/lldb/Symbol/PostfixExpression.h

210

You could leave ref unnamed here to avoid any "unused parameter" warnings.

216

I'm confused why this takes symbol by (non-const) ref. Is symbol modified in the process of figuring out what should replace it?

Oh, peeking ahead at the implementation, I see it can return the address of symbol. I'm left wondering whether there's a less confusing API.

source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp

46

If I understand correctly, we don't care about the return value of Dispatch because all that matters is whether result points to a valid Node or is just nullptr. Right?

If so, then should SymbolResolver derive from Visitor<void> rather than Visitor<bool>?

[labath]

In D61056#1477914, @amccarth wrote:

Can the test deletions be a separate patch, or will they fail because of the other changes in this patch? They feel like a good but separable step.

Nope, the tests still pass, they just felt superfluous. I'll remove them in a separate patch.

include/lldb/Symbol/PostfixExpression.h

216

Yeah, I too had the feeling that there ought to be a simpler way to do this, but I couldn't figure out how.... until now...

Check out the new version of the patch. Now, instead of having to subclass something one can implement a trivial search-and-replace operation by just passing a callback to the function. (There is subclassing going on under the hood, but the user is unaware of that).

In fact, it made things so simple, I just decided to merge the two-pass algorithm in NativePDB into a single function.

source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp

46

Not exactly. The fact whether is the replacement function returned nullptr is then propagated up the Dispatch function, and eventually makes its way out as the result of the entire replacement operation (so, yes, the bool is needed here).

[labath]

• simplify symbol resolution even more
• drop the NativePDB unittest changes

[amccarth]

Clever (hopefully not too clever)! Not having to derive a special class from Visitor is really nice.

LGTM.

include/lldb/Symbol/PostfixExpression.h

216

Well, it is simpler, but we still have that non-const ref to symbol, which is what originally threw me. I don't see a way to get around that, and I like the new solution better than having to derive your own visitor.

source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp

46

Oh yes. I was looking at the Dispatch calls in the DWARF CodeGen class by mistake. Those are Visitor<void>. Perfect.

include/lldb/Symbol/PostfixExpression.h

 #ifndef LLDB_SYMBOL_POSTFIXEXPRESSION_H
 #define LLDB_SYMBOL_POSTFIXEXPRESSION_H
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 
 namespace lldb_private {
+
+class Stream;
+
 namespace postfix {
 
 /// The base class for all nodes in the parsed postfix tree.
 class Node {
 public:
   enum Kind {
     BinaryOp,
     Integer,
case Node::Symbol:
       return Visit(llvm::cast<SymbolNode>(*node), node);
     case Node::UnaryOp:
       return Visit(llvm::cast<UnaryOpNode>(*node), node);
     }
     llvm_unreachable("Fully covered switch!");
   }
 };
 
+/// A specialized visitor class for clients that want to resolve (replace)
+/// SymbolNodes in the AST. It implements reasonable defaults for all Visit
+/// functions, and presents a simpler replacement interface.
+class SymbolResolver : public Visitor<bool> {
+public:
+  /// Perform SymbolNode replacement using the given Resolver class. Returns
+  /// true if all SymbolNodes have been sucessfully replaced. (Successfulness
+  /// is determined by the result of the Replace function.)
+  template <typename ResolverT, typename... Args>
+  static bool Resolve(Node *&node, Args &&... args) {
+    static_assert(std::is_base_of<SymbolResolver, ResolverT>::value, "");
+    return ResolverT(std::forward<Args>(args)...).Dispatch(node);
+  }
+
+protected:
+  bool Visit(BinaryOpNode &binary, Node *&ref) override {
+    return Dispatch(binary.Left()) && Dispatch(binary.Right());
+  }
+
+  bool Visit(IntegerNode &integer, Node *&) override { return true; }
+  bool Visit(RegisterNode &reg, Node *&) override { return true; }
+
+  bool Visit(SymbolNode &symbol, Node *&ref) override {
+    if (Node *replacement = Replace(symbol)) {
+      ref = replacement;
+      return true;
+    }
+    return false;
+  }
+
+  bool Visit(UnaryOpNode &unary, Node *&ref) override {
+    return Dispatch(unary.Operand());
+  }
+
+  /// Return the node which should be used to replace the given SymbolNode, or
+  /// nullptr, if unable to replace/resolve.
+  virtual Node *Replace(SymbolNode &symbol) = 0;
+};
+
 template <typename T, typename... Args>
 inline T *MakeNode(llvm::BumpPtrAllocator &alloc, Args &&... args) {
   static_assert(std::is_trivially_destructible<T>::value,
                 "This object will not be destroyed!");
   return new (alloc.Allocate<T>()) T(std::forward<Args>(args)...);
 }
 
 /// Parse the given postfix expression. The parsed nodes are placed into the
 /// provided allocator.
 Node *Parse(llvm::StringRef expr, llvm::BumpPtrAllocator &alloc);
 
+/// Serialize the given expression tree as DWARF. The result is written into the
+/// given stream. The AST should not contain any SymbolNodes.
+void ToDWARF(Node &node, Stream &stream);
+
 } // namespace postfix
 } // namespace lldb_private
 
 #endif // LLDB_SYMBOL_POSTFIXEXPRESSION_H

source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp

 //===-- PDBFPOProgramToDWARFExpression.cpp ----------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "PdbFPOProgramToDWARFExpression.h"
 #include "CodeViewRegisterMapping.h"
 
 #include "lldb/Core/StreamBuffer.h"
-#include "lldb/Core/dwarf.h"
 #include "lldb/Symbol/PostfixExpression.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Stream.h"
 #include "llvm/ADT/DenseMap.h"
 
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/DebugInfo/CodeView/CodeView.h"
 #include "llvm/DebugInfo/CodeView/EnumTables.h"
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::postfix;
 
 namespace {
 
-class FPOProgramASTVisitorMergeDependent : public Visitor<> {
+class FPOProgramASTVisitorMergeDependent : public SymbolResolver {
 public:
-  void Visit(BinaryOpNode &binary, Node *&) override {
-    Dispatch(binary.Left());
-    Dispatch(binary.Right());
-  }
-
-  void Visit(UnaryOpNode &unary, Node *&) override {
-    Dispatch(unary.Operand());
-  }
-
-  void Visit(RegisterNode &, Node *&) override {}
-  void Visit(IntegerNode &, Node *&) override {}
-  void Visit(SymbolNode &symbol, Node *&ref) override;
-
-  static void
-  Merge(const llvm::DenseMap<llvm::StringRef, Node *> &dependent_programs,
-        Node *&ast) {
-    FPOProgramASTVisitorMergeDependent(dependent_programs).Dispatch(ast);
-  }
-
-private:
   FPOProgramASTVisitorMergeDependent(
       const llvm::DenseMap<llvm::StringRef, Node *> &dependent_programs)
       : m_dependent_programs(dependent_programs) {}
 
+private:
+  Node *Replace(SymbolNode &symbol) override;
+
   const llvm::DenseMap<llvm::StringRef, Node *> &m_dependent_programs;
 };
 
-void FPOProgramASTVisitorMergeDependent::Visit(SymbolNode &symbol, Node *&ref) {
+Node *FPOProgramASTVisitorMergeDependent::Replace(SymbolNode &symbol) {
   auto it = m_dependent_programs.find(symbol.GetName());
   if (it == m_dependent_programs.end())
-    return;
+    return &symbol;
 
-  ref = it->second;
-  Dispatch(ref);
+  Node *result = it->second;
+  Dispatch(result); // Recursively process the result.
+  return result;    // And return it.
 }
 
-class FPOProgramASTVisitorResolveRegisterRefs : public Visitor<bool> {
+class FPOProgramASTVisitorResolveRegisterRefs : public SymbolResolver {
 public:
-  static bool
-  Resolve(const llvm::DenseMap<llvm::StringRef, Node *> &dependent_programs,
-          llvm::Triple::ArchType arch_type, llvm::BumpPtrAllocator &alloc,
-          Node *&ast) {
-    return FPOProgramASTVisitorResolveRegisterRefs(dependent_programs,
-                                                   arch_type, alloc)
-        .Dispatch(ast);
-  }
-
-  bool Visit(BinaryOpNode &binary, Node *&) override {
-    return Dispatch(binary.Left()) && Dispatch(binary.Right());
-  }
-
-  bool Visit(UnaryOpNode &unary, Node *&) override {
-    return Dispatch(unary.Operand());
-  }
-
-  bool Visit(RegisterNode &, Node *&) override { return true; }
-
-  bool Visit(IntegerNode &, Node *&) override { return true; }
-
-  bool Visit(SymbolNode &symbol, Node *&ref) override;
-
-private:
   FPOProgramASTVisitorResolveRegisterRefs(
       const llvm::DenseMap<llvm::StringRef, Node *> &dependent_programs,
       llvm::Triple::ArchType arch_type, llvm::BumpPtrAllocator &alloc)
-      : m_dependent_programs(dependent_programs), m_arch_type(arch_type),
-        m_alloc(alloc) {}
+      : m_alloc(alloc), m_dependent_programs(dependent_programs),
+        m_arch_type(arch_type) {}
+
+private:
+  Node *Replace(SymbolNode &symbol) override;
 
+  llvm::BumpPtrAllocator &m_alloc;
   const llvm::DenseMap<llvm::StringRef, Node *> &m_dependent_programs;
   llvm::Triple::ArchType m_arch_type;
-  llvm::BumpPtrAllocator &m_alloc;
 };
 
 static uint32_t ResolveLLDBRegisterNum(llvm::StringRef reg_name, llvm::Triple::ArchType arch_type) {
   // lookup register name to get lldb register number
   llvm::ArrayRef<llvm::EnumEntry<uint16_t>> register_names =
       llvm::codeview::getRegisterNames();
   auto it = llvm::find_if(
       register_names,
       [&reg_name](const llvm::EnumEntry<uint16_t> &register_entry) {
         return reg_name.compare_lower(register_entry.Name) == 0;
       });
 
   if (it == register_names.end())
     return LLDB_INVALID_REGNUM;
 
   auto reg_id = static_cast<llvm::codeview::RegisterId>(it->Value);
   return npdb::GetLLDBRegisterNumber(arch_type, reg_id);
 }
 
-bool FPOProgramASTVisitorResolveRegisterRefs::Visit(SymbolNode &symbol,
-                                                    Node *&ref) {
+Node *FPOProgramASTVisitorResolveRegisterRefs::Replace(SymbolNode &symbol) {
   // Look up register reference as lvalue in preceding assignments.
   auto it = m_dependent_programs.find(symbol.GetName());
   if (it != m_dependent_programs.end()) {
     // Dependent programs are handled elsewhere.
-    return true;
+    return &symbol;
   }
 
   uint32_t reg_num =
       ResolveLLDBRegisterNum(symbol.GetName().drop_front(1), m_arch_type);
 
   if (reg_num == LLDB_INVALID_REGNUM)
-    return false;
-
-  ref = MakeNode<RegisterNode>(m_alloc, reg_num);
-  return true;
-}
-
-class FPOProgramASTVisitorDWARFCodegen : public Visitor<> {
-public:
-  static void Emit(Stream &stream, Node *&ast) {
-    FPOProgramASTVisitorDWARFCodegen(stream).Dispatch(ast);
-  }
-
-  void Visit(RegisterNode &reg, Node *&);
-  void Visit(BinaryOpNode &binary, Node *&);
-  void Visit(UnaryOpNode &unary, Node *&);
-  void Visit(SymbolNode &symbol, Node *&) {
-    llvm_unreachable("Symbols should have been resolved by now!");
-  }
-  void Visit(IntegerNode &integer, Node *&);
-
-private:
-  FPOProgramASTVisitorDWARFCodegen(Stream &stream) : m_out_stream(stream) {}
-
-  Stream &m_out_stream;
-};
-
-void FPOProgramASTVisitorDWARFCodegen::Visit(RegisterNode &reg, Node *&) {
-  uint32_t reg_num = reg.GetRegNum();
-  lldbassert(reg_num != LLDB_INVALID_REGNUM);
-
-  if (reg_num > 31) {
-    m_out_stream.PutHex8(DW_OP_bregx);
-    m_out_stream.PutULEB128(reg_num);
-  } else
-    m_out_stream.PutHex8(DW_OP_breg0 + reg_num);
-
-  m_out_stream.PutSLEB128(0);
-}
-
-void FPOProgramASTVisitorDWARFCodegen::Visit(IntegerNode &integer, Node *&) {
-  uint32_t value = integer.GetValue();
-  m_out_stream.PutHex8(DW_OP_constu);
-  m_out_stream.PutULEB128(value);
-}
-
-void FPOProgramASTVisitorDWARFCodegen::Visit(BinaryOpNode &binary, Node *&) {
-  Dispatch(binary.Left());
-  Dispatch(binary.Right());
-
-  switch (binary.GetOpType()) {
-  case BinaryOpNode::Plus:
-    m_out_stream.PutHex8(DW_OP_plus);
-    // NOTE: can be optimized by using DW_OP_plus_uconst opcpode
-    //       if right child node is constant value
-    break;
-  case BinaryOpNode::Minus:
-    m_out_stream.PutHex8(DW_OP_minus);
-    break;
-  case BinaryOpNode::Align:
-    // emit align operator a @ b as
-    // a & ~(b - 1)
-    // NOTE: implicitly assuming that b is power of 2
-    m_out_stream.PutHex8(DW_OP_lit1);
-    m_out_stream.PutHex8(DW_OP_minus);
-    m_out_stream.PutHex8(DW_OP_not);
-
-    m_out_stream.PutHex8(DW_OP_and);
-    break;
-  }
-}
-
-void FPOProgramASTVisitorDWARFCodegen::Visit(UnaryOpNode &unary, Node *&) {
-  Dispatch(unary.Operand());
+    return nullptr;
 
-  switch (unary.GetOpType()) {
-  case UnaryOpNode::Deref:
-    m_out_stream.PutHex8(DW_OP_deref);
-    break;
-  }
+  return MakeNode<RegisterNode>(m_alloc, reg_num);
 }
 
 } // namespace
 
 static bool ParseFPOSingleAssignmentProgram(llvm::StringRef program,
                                             llvm::BumpPtrAllocator &alloc,
                                             llvm::StringRef &register_name,
                                             Node *&ast) {
while (true) {
     if (!ParseFPOSingleAssignmentProgram(assignment_program, alloc, lvalue_name,
                                          rvalue_ast)) {
       return nullptr;
     }
 
     lldbassert(rvalue_ast);
 
     // check & resolve assignment program
-    if (!FPOProgramASTVisitorResolveRegisterRefs::Resolve(
-            dependent_programs, arch_type, alloc, rvalue_ast))
+    if (!SymbolResolver::Resolve<FPOProgramASTVisitorResolveRegisterRefs>(
+            rvalue_ast, dependent_programs, arch_type, alloc))
       return nullptr;
 
     if (lvalue_name == register_name) {
       // found target assignment program - no need to parse further
 
       // emplace valid dependent subtrees to make target assignment independent
       // from predecessors
-      FPOProgramASTVisitorMergeDependent::Merge(dependent_programs, rvalue_ast);
+      SymbolResolver::Resolve<FPOProgramASTVisitorMergeDependent>(
+          rvalue_ast, dependent_programs);
 
       return rvalue_ast;
     }
 
     dependent_programs[lvalue_name] = rvalue_ast;
     cur = assign_index + 1;
   }
 
   return nullptr;
 }
 
 bool lldb_private::npdb::TranslateFPOProgramToDWARFExpression(
     llvm::StringRef program, llvm::StringRef register_name,
     llvm::Triple::ArchType arch_type, Stream &stream) {
   llvm::BumpPtrAllocator node_alloc;
   Node *target_program =
       ParseFPOProgram(program, register_name, arch_type, node_alloc);
   if (target_program == nullptr) {
     return false;
   }
 
-  FPOProgramASTVisitorDWARFCodegen::Emit(stream, target_program);
+  ToDWARF(*target_program, stream);
   return true;
 }

source/Symbol/PostfixExpression.cpp

 //===-- PostfixExpression.cpp -----------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements support for postfix expressions found in several symbol
 //  file formats, and their conversion to DWARF.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Symbol/PostfixExpression.h"
+#include "lldb/Core/dwarf.h"
+#include "lldb/Utility/Stream.h"
 #include "llvm/ADT/StringExtras.h"
 
 using namespace lldb_private;
 using namespace lldb_private::postfix;
 
 static llvm::Optional<BinaryOpNode::OpType>
 GetBinaryOpType(llvm::StringRef token) {
   if (token.size() != 1)
while (std::tie(token, expr) = getToken(expr), !token.empty()) {
     stack.push_back(MakeNode<SymbolNode>(alloc, token));
   }
 
   if (stack.size() != 1)
     return nullptr;
 
   return stack.back();
 }
+
+namespace {
+class DWARFCodegen : public Visitor<> {
+public:
+  DWARFCodegen(Stream &stream) : m_out_stream(stream) {}
+
+  using Visitor<>::Dispatch;
+
+private:
+  void Visit(BinaryOpNode &binary, Node *&);
+
+  void Visit(IntegerNode &integer, Node *&) {
+    m_out_stream.PutHex8(DW_OP_constu);
+    m_out_stream.PutULEB128(integer.GetValue());
+  }
+
+  void Visit(RegisterNode &reg, Node *&);
+
+  void Visit(SymbolNode &symbol, Node *&) {
+    llvm_unreachable("Symbols should have been resolved by now!");
+  }
+
+  void Visit(UnaryOpNode &unary, Node *&);
+
+  Stream &m_out_stream;
+};
+} // namespace
+
+void DWARFCodegen::Visit(BinaryOpNode &binary, Node *&) {
+  Dispatch(binary.Left());
+  Dispatch(binary.Right());
+
+  switch (binary.GetOpType()) {
+  case BinaryOpNode::Plus:
+    m_out_stream.PutHex8(DW_OP_plus);
+    // NOTE: can be optimized by using DW_OP_plus_uconst opcpode
+    //       if right child node is constant value
+    break;
+  case BinaryOpNode::Minus:
+    m_out_stream.PutHex8(DW_OP_minus);
+    break;
+  case BinaryOpNode::Align:
+    // emit align operator a @ b as
+    // a & ~(b - 1)
+    // NOTE: implicitly assuming that b is power of 2
+    m_out_stream.PutHex8(DW_OP_lit1);
+    m_out_stream.PutHex8(DW_OP_minus);
+    m_out_stream.PutHex8(DW_OP_not);
+
+    m_out_stream.PutHex8(DW_OP_and);
+    break;
+  }
+}
+
+void DWARFCodegen::Visit(RegisterNode &reg, Node *&) {
+  uint32_t reg_num = reg.GetRegNum();
+  assert(reg_num != LLDB_INVALID_REGNUM);
+
+  if (reg_num > 31) {
+    m_out_stream.PutHex8(DW_OP_bregx);
+    m_out_stream.PutULEB128(reg_num);
+  } else
+    m_out_stream.PutHex8(DW_OP_breg0 + reg_num);
+
+  m_out_stream.PutSLEB128(0);
+}
+
+void DWARFCodegen::Visit(UnaryOpNode &unary, Node *&) {
+  Dispatch(unary.Operand());
+
+  switch (unary.GetOpType()) {
+  case UnaryOpNode::Deref:
+    m_out_stream.PutHex8(DW_OP_deref);
+    break;
+  }
+}
+
+void postfix::ToDWARF(Node &node, Stream &stream) {
+  Node *ptr = &node;
+  DWARFCodegen(stream).Dispatch(ptr);
+}

unittests/Symbol/PostfixExpressionTest.cpp

 //===-- PostfixExpressionTest.cpp -------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Symbol/PostfixExpression.h"
+#include "lldb/Expression/DWARFExpression.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/StreamString.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
 using namespace lldb_private::postfix;
 
 static std::string ToString(BinaryOpNode::OpType type) {
TEST(PostfixExpression, Parse) {
   EXPECT_EQ("nullptr", ParseAndStringify("1 2 ^"));
   EXPECT_EQ("nullptr", ParseAndStringify("1 2 3 +"));
   EXPECT_EQ("nullptr", ParseAndStringify("^ 1"));
   EXPECT_EQ("nullptr", ParseAndStringify("+ 1 2"));
   EXPECT_EQ("nullptr", ParseAndStringify("1 + 2"));
   EXPECT_EQ("nullptr", ParseAndStringify("1 2"));
   EXPECT_EQ("nullptr", ParseAndStringify(""));
 }
+
+namespace {
+class SimpleResolver : public SymbolResolver {
+public:
+  SimpleResolver(llvm::BumpPtrAllocator &alloc) : m_alloc(alloc) {}
+
+private:
+  Node *Replace(SymbolNode &symbol) override {
+    uint32_t num;
+    if (to_integer(symbol.GetName().drop_front(), num))
+      return MakeNode<RegisterNode>(m_alloc, num);
+    return nullptr;
+  }
+
+  llvm::BumpPtrAllocator &m_alloc;
+};
+} // namespace
+
+static std::string ParseAndGenerateDWARF(llvm::StringRef expr) {
+  llvm::BumpPtrAllocator alloc;
+  Node *ast = Parse(expr, alloc);
+  if (!ast)
+    return "Parse failed.";
+  if (!SymbolResolver::Resolve<SimpleResolver>(ast, alloc))
+    return "Resolution failed.";
+
+  const size_t addr_size = 4;
+  StreamString dwarf(Stream::eBinary, addr_size, lldb::eByteOrderLittle);
+  ToDWARF(*ast, dwarf);
+
+  // print dwarf expression to comparable textual representation
+  DataExtractor extractor(dwarf.GetData(), dwarf.GetSize(),
+                          lldb::eByteOrderLittle, addr_size);
+
+  StreamString result;
+  if (!DWARFExpression::PrintDWARFExpression(result, extractor, addr_size,
+                                             /*dwarf_ref_size*/ 4,
+                                             /*location_expression*/ false)) {
+    return "DWARF printing failed.";
+  }
+
+  return result.GetString();
+}
+
+TEST(PostfixExpression, ToDWARF) {
+  EXPECT_EQ("DW_OP_constu 0x0", ParseAndGenerateDWARF("0"));
+
+  EXPECT_EQ("DW_OP_breg1 +0", ParseAndGenerateDWARF("R1"));
+
+  EXPECT_EQ("DW_OP_bregx 65 0", ParseAndGenerateDWARF("R65"));
+
+  EXPECT_EQ("DW_OP_constu 0x4, DW_OP_constu 0x5, DW_OP_plus ",
+            ParseAndGenerateDWARF("4 5 +"));
+
+  EXPECT_EQ("DW_OP_constu 0x4, DW_OP_constu 0x5, DW_OP_minus ",
+            ParseAndGenerateDWARF("4 5 -"));
+
+  EXPECT_EQ("DW_OP_constu 0x4, DW_OP_deref ", ParseAndGenerateDWARF("4 ^"));
+
+  EXPECT_EQ("DW_OP_breg6 +0, DW_OP_constu 0x80, DW_OP_lit1 "
+            ", DW_OP_minus , DW_OP_not , DW_OP_and ",
+            ParseAndGenerateDWARF("R6 128 @"));
+}

unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp

CheckValidProgramTranslation(llvm::StringRef fpo_program,
   ASSERT_TRUE(DWARFExpression::PrintDWARFExpression(
       result_dwarf_expression, extractor, address_size, 4, false));
 
   // actual check
   ASSERT_STREQ(expected_dwarf_expression.data(),
                result_dwarf_expression.GetString().data());
 }
 
-TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentConst) {
-  CheckValidProgramTranslation("$T0 0 = ", "$T0", "DW_OP_constu 0x0");
-}
-
 TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentRegisterRef) {
   CheckValidProgramTranslation("$T0 $ebp = ", "$T0", "DW_OP_breg6 +0");
 }
 
-TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentExpressionPlus) {
-  CheckValidProgramTranslation("$T0 $ebp 4 + = ", "$T0",
-                               "DW_OP_breg6 +0, DW_OP_constu 0x4, DW_OP_plus ");
-}
-
-TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentExpressionDeref) {
-  CheckValidProgramTranslation("$T0 $ebp ^ = ", "$T0",
-                               "DW_OP_breg6 +0, DW_OP_deref ");
-}
-
-TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentExpressionMinus) {
-  CheckValidProgramTranslation(
-      "$T0 $ebp 4 - = ", "$T0",
-      "DW_OP_breg6 +0, DW_OP_constu 0x4, DW_OP_minus ");
-}
-
-TEST(PDBFPOProgramToDWARFExpressionTests, SingleAssignmentExpressionAlign) {
-  CheckValidProgramTranslation("$T0 $ebp 128 @ = ", "$T0",
-                               "DW_OP_breg6 +0, DW_OP_constu 0x80, DW_OP_lit1 "
-                               ", DW_OP_minus , DW_OP_not , DW_OP_and ");
-}
-
 TEST(PDBFPOProgramToDWARFExpressionTests, MultipleIndependentAssignments) {
   CheckValidProgramTranslation("$T1 1 = $T0 0 =", "$T0", "DW_OP_constu 0x0");
 }
 
 TEST(PDBFPOProgramToDWARFExpressionTests, MultipleDependentAssignments) {
   CheckValidProgramTranslation(
       "$T1 $ebp 4 + = $T0 $T1 8 - 128 @ = ", "$T0",
       "DW_OP_breg6 +0, DW_OP_constu 0x4, DW_OP_plus , DW_OP_constu 0x8, "