[PATCH] D84050: YAML: Don't assume an arbitrary StringRef is null terminated

[Scott Linder via Phabricator via llvm-commits]

scott.linder marked an inline comment as done.
scott.linder added a comment.

I tried to address the issues brought up, but I'm not particularly confident I found every case in the parser where it assumes `End` is OK to dereference. I think the presence of a null-terminator just made these bugs harder to detect, but as far as I can tell they are in fact bugs (i.e. `End` is documented as being one-past-the-end, and a lot of other places do guard against `Current == End`).

I didn't do a stellar job with unit testing either, I essentially just tried to produce at least enough test cases to stress the changes I made. I considered updating all of the existing tests to not use null-terminated buffers, or to check with both, but wanted to see what people thought first.

As a note, the tests will only deterministically fail with ASan enabled, but I don't know a reliable way to avoid that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84050