[PATCH] D62231: [DebugInfoMetadata] return early on nullptr arg

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers created this revision.
nickdesaulniers added a reviewer: echristo.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

This was flagged in https://www.viva64.com/en/b/0629/ under "Snippet No.
40".

It seems there's a nullptr check, but then Expr is later dereferenced
outside of the check. Since we can return an llvm::Optional, assume the
caller can handle llvm::None.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62231

Files:
  llvm/lib/IR/DebugInfoMetadata.cpp


Index: llvm/lib/IR/DebugInfoMetadata.cpp
===================================================================
--- llvm/lib/IR/DebugInfoMetadata.cpp
+++ llvm/lib/IR/DebugInfoMetadata.cpp
@@ -1076,33 +1076,33 @@
 
 Optional<DIExpression *> DIExpression::createFragmentExpression(
     const DIExpression *Expr, unsigned OffsetInBits, unsigned SizeInBits) {
+  if (!Expr)
+    return None;
   SmallVector<uint64_t, 8> Ops;
   // Copy over the expression, but leave off any trailing DW_OP_LLVM_fragment.
-  if (Expr) {
-    for (auto Op : Expr->expr_ops()) {
-      switch (Op.getOp()) {
-      default: break;
-      case dwarf::DW_OP_plus:
-      case dwarf::DW_OP_minus:
-        // We can't safely split arithmetic into multiple fragments because we
-        // can't express carry-over between fragments.
-        //
-        // FIXME: We *could* preserve the lowest fragment of a constant offset
-        // operation if the offset fits into SizeInBits.
-        return None;
-      case dwarf::DW_OP_LLVM_fragment: {
-        // Make the new offset point into the existing fragment.
-        uint64_t FragmentOffsetInBits = Op.getArg(0);
-        uint64_t FragmentSizeInBits = Op.getArg(1);
-        (void)FragmentSizeInBits;
-        assert((OffsetInBits + SizeInBits <= FragmentSizeInBits) &&
-               "new fragment outside of original fragment");
-        OffsetInBits += FragmentOffsetInBits;
-        continue;
-      }
-      }
-      Op.appendToVector(Ops);
+  for (auto Op : Expr->expr_ops()) {
+    switch (Op.getOp()) {
+    default: break;
+    case dwarf::DW_OP_plus:
+    case dwarf::DW_OP_minus:
+      // We can't safely split arithmetic into multiple fragments because we
+      // can't express carry-over between fragments.
+      //
+      // FIXME: We *could* preserve the lowest fragment of a constant offset
+      // operation if the offset fits into SizeInBits.
+      return None;
+    case dwarf::DW_OP_LLVM_fragment: {
+      // Make the new offset point into the existing fragment.
+      uint64_t FragmentOffsetInBits = Op.getArg(0);
+      uint64_t FragmentSizeInBits = Op.getArg(1);
+      (void)FragmentSizeInBits;
+      assert((OffsetInBits + SizeInBits <= FragmentSizeInBits) &&
+             "new fragment outside of original fragment");
+      OffsetInBits += FragmentOffsetInBits;
+      continue;
     }
+    }
+    Op.appendToVector(Ops);
   }
   Ops.push_back(dwarf::DW_OP_LLVM_fragment);
   Ops.push_back(OffsetInBits);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D62231.200633.patch
Type: text/x-patch
Size: 2471 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190522/d22cf1e3/attachment.bin>