[PATCH] D61608: YAML parser robustness improvements

[Don Hinton via Phabricator via llvm-commits]

hintonda requested changes to this revision.
hintonda added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Support/YAMLTraits.cpp:373
   SmallString<128> StringStorage;
-  if (ScalarNode *SN = dyn_cast<ScalarNode>(N)) {
+  if (ScalarNode *SN = dyn_cast_or_null<ScalarNode>(N)) {
     StringRef KeyStr = SN->getValue(StringStorage);
----------------
Since `createHNodes` is an internal function and only ever called 3 times within `Input::` (and nowhere else), and `N` is already checked before each of those calls, there's no need add  `_of_null` to these `dyn_cast` calls and check for null again each time.

To enforce this, I'd suggest you make `createHNodes` and `setError` private which will make the class more robust, e.g.:

```
index f365d60b3ff..fe2fe8c56da 100644
--- a/llvm/include/llvm/Support/YAMLTraits.h
+++ b/llvm/include/llvm/Support/YAMLTraits.h
@@ -1509,6 +1509,7 @@ private:
     std::vector<std::unique_ptr<HNode>> Entries;
   };

+private:
   std::unique_ptr<Input::HNode> createHNodes(Node *node);
   void setError(HNode *hnode, const Twine &message);
   void setError(Node *node, const Twine &message);
```

However, `KeyNode` on line 396 could be null, so either checking or adding `_or_null` to the call is definitely needed there.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61608/new/

https://reviews.llvm.org/D61608