[PATCH] D74413: [DebugInfo] Add checks for v2 directory and file name table terminators

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:165-170
   while (*OffsetPtr < EndPrologueOffset) {
     StringRef S = DebugLineData.getCStrRef(OffsetPtr);
-    if (S.empty())
+    if (S.empty()) {
+      IncludeDirsTerminated = true;
       break;
+    }
----------------
Perhaps this'd be a little more terse (but not necessarily better) written as:

```
Terminated = S.empty();
if (Terminated)
  break;
```
or maybe
```
if ((Terminated = S.empty()))
  break;
```

Not necessary if you prefer the current version.

I'd probably more strongly suggest making the variable names a bit simpler - probably just using "Terminated" for both.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:178
+    return createStringError(errc::invalid_argument,
+                             "include directories table was not terminated "
+                             "before the end of the prologue");
----------------
maybe make it a bit more explicit by saying "null terminated"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74413