[PATCH] D74204: [DebugInfo] Reject line tables of version > 5

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.

Might be worth a different strategy (either add to the end, and/or use stable names rather than numbers). But otherwise seems fine.

Other thoughts: Probably print the unknown version number in decimal rather than hex? Probably doesn't need both an assembly+dumper test and a unit test. One or the other (which ever is most convenient - the unit test looks simpler/more self contained & targeted) is fine by me - assuming the general codepath for error handling is already tested for the dumper, for instance. (ie: the changed code is exercised in the new test, doesn't mean all paths that reach that changed code need to be retested).



================
Comment at: llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test:158
+
+## Case 14: V5 invalid directory content description has unsupported form.
+# NONFATAL:      debug_line[0x000002f2]
----------------
Perhaps numbering cases isn't a good idea if new cases are going to be added in the middle & shift all the numbers? Perhaps a short textual name would be better, then? (like test cases in a GUnit test, for instance)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74204