[llvm] [llvm][mustache] Fix UB in ASTNode::render() (PR #142249)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-support

Author: Paul Kirth (ilovepi)

<details>
<summary>Changes</summary>

The current implementation set a reference to a nullptr, leading to all
kinds of problems. Instead, we can check the various uses to ensure we
don't deref invalid memory, and improve the logic for how contexts are
passed to children, since that was also subtly wrong in some cases.

---
Full diff: https://github.com/llvm/llvm-project/pull/142249.diff


1 Files Affected:

- (modified) llvm/lib/Support/Mustache.cpp (+33-22) 


``````````diff
diff --git a/llvm/lib/Support/Mustache.cpp b/llvm/lib/Support/Mustache.cpp
index 89900ed4d03a5..187e90e101a13 100644
--- a/llvm/lib/Support/Mustache.cpp
+++ b/llvm/lib/Support/Mustache.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 #include "llvm/Support/Mustache.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/IR/Value.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_ostream.h"
 #include <sstream>
@@ -23,6 +24,13 @@ static bool isFalsey(const json::Value &V) {
          (V.getAsArray() && V.getAsArray()->empty());
 }
 
+static bool isContextFalsey(const json::Value *V) {
+  // A missing context (represented by a nullptr) is defined as falsey.
+  if (!V)
+    return true;
+  return isFalsey(*V);
+}
+
 static Accessor splitMustacheString(StringRef Str) {
   // We split the mustache string into an accessor.
   // For example:
@@ -560,14 +568,15 @@ void toMustacheString(const json::Value &Data, raw_ostream &OS) {
   }
 }
 
-void ASTNode::render(const json::Value &Data, raw_ostream &OS) {
-  ParentContext = &Data;
+void ASTNode::render(const json::Value &CurrentCtx, raw_ostream &OS) {
+  // Set the parent context to the incoming context so that we
+  // can walk up the context tree correctly in findContext().
+  ParentContext = &CurrentCtx;
   const json::Value *ContextPtr = Ty == Root ? ParentContext : findContext();
-  const json::Value &Context = ContextPtr ? *ContextPtr : nullptr;
 
   switch (Ty) {
   case Root:
-    renderChild(Data, OS);
+    renderChild(CurrentCtx, OS);
     return;
   case Text:
     OS << Body;
@@ -575,53 +584,55 @@ void ASTNode::render(const json::Value &Data, raw_ostream &OS) {
   case Partial: {
     auto Partial = Partials.find(AccessorValue[0]);
     if (Partial != Partials.end())
-      renderPartial(Data, OS, Partial->getValue().get());
+      renderPartial(CurrentCtx, OS, Partial->getValue().get());
     return;
   }
   case Variable: {
     auto Lambda = Lambdas.find(AccessorValue[0]);
-    if (Lambda != Lambdas.end())
-      renderLambdas(Data, OS, Lambda->getValue());
-    else {
+    if (Lambda != Lambdas.end()) {
+      renderLambdas(CurrentCtx, OS, Lambda->getValue());
+    } else if (ContextPtr) {
       EscapeStringStream ES(OS, Escapes);
-      toMustacheString(Context, ES);
+      toMustacheString(*ContextPtr, ES);
     }
     return;
   }
   case UnescapeVariable: {
     auto Lambda = Lambdas.find(AccessorValue[0]);
-    if (Lambda != Lambdas.end())
-      renderLambdas(Data, OS, Lambda->getValue());
-    else
-      toMustacheString(Context, OS);
+    if (Lambda != Lambdas.end()) {
+      renderLambdas(CurrentCtx, OS, Lambda->getValue());
+    } else if (ContextPtr) {
+      toMustacheString(*ContextPtr, OS);
+    }
     return;
   }
   case Section: {
-    // Sections are not rendered if the context is falsey.
     auto SectionLambda = SectionLambdas.find(AccessorValue[0]);
     bool IsLambda = SectionLambda != SectionLambdas.end();
-    if (isFalsey(Context) && !IsLambda)
+
+    if (isContextFalsey(ContextPtr) && !IsLambda)
       return;
 
     if (IsLambda) {
-      renderSectionLambdas(Data, OS, SectionLambda->getValue());
+      renderSectionLambdas(CurrentCtx, OS, SectionLambda->getValue());
       return;
     }
 
-    if (Context.getAsArray()) {
-      const json::Array *Arr = Context.getAsArray();
+    if (const json::Array *Arr = ContextPtr->getAsArray()) {
       for (const json::Value &V : *Arr)
         renderChild(V, OS);
       return;
     }
-    renderChild(Context, OS);
+    renderChild(*ContextPtr, OS);
     return;
   }
   case InvertSection: {
     bool IsLambda = SectionLambdas.contains(AccessorValue[0]);
-    if (!isFalsey(Context) || IsLambda)
-      return;
-    renderChild(Context, OS);
+    if (isContextFalsey(ContextPtr) && !IsLambda) {
+      // The context for the children remains UNCHANGED from the parent's.
+      // We pass 'Data', which is this node's original incoming context.
+      renderChild(CurrentCtx, OS);
+    }
     return;
   }
   }

``````````

</details>


https://github.com/llvm/llvm-project/pull/142249