[PATCH] D143513: [DebugInfo] Allow parsing line tables aligned to 4 or 8-byte boundaries

Thu Mar 9 01:06:06 PST 2023

jhenderson added a comment.

In D143513#4179083 <https://reviews.llvm.org/D143513#4179083>, @benmxwl-arm wrote:

> It seems like they are removed by the test runner before passing the file to llvm-mc (but it only removes lines that start with a directive it recognises), though I can't point to precisely where this happens.

I am 99% certain that this is incorrect. The %s that is used for the llvm-mc test input is simply a lit substitution which expands to the current file's path. In other words, llvm-mc sees the entire test file verbatim, with no modification.

> I'll just change the test to use `//` for the checks too for consistency.

This is fine from my point of view.

================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:1537-1538
+
+  // Heuristic: If the version is valid, then this is probably a line table,
+  // otherwise the offset might need alignment.
+  if (lineTableHeaderHasValidVersion(Offset))
----------------
jhenderson wrote:
> 
You only did half my suggestion. The "Otherwise" should be the start of a new sentence.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143513