[PATCH] D74309: [DebugInfo] Add check for zero debug line opcode_base

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:358-367
+  if (OpcodeBase == 0) {
+    // If the opcode base is 0, we cannot read the standard opcode lengths (of
+    // which there are supposed to be one fewer than the opcode base). Assume
+    // there are no standard opcodes and continue parsing.
+    RecoverableErrorCallback(createStringError(
+        errc::invalid_argument,
+        "parsing line table prologue at offset 0x%8.8" PRIx64
----------------
dblaikie wrote:
> probinson wrote:
> > dblaikie wrote:
> > > I don't /think/ this merits a warning. It seems to be unexceptionally DWARF conformant to have an opcode base of 0:
> > > 
> > > "Opcode base is typically one greater than the highest-numbered standard opcode defined for the specified version of the line number information (12 in DWARF Versions 3, 4 and 5, and 9 in Version 2). If opcode_base is less than the typical value, then standard opcode numbers greater than or equal to the opcode base are not used in the line number table of this unit (and the codes are treated as special opcodes)."
> > > 
> > > 
> > That non-normative comment seems to have a reasonable interpretation for an opcode base of 0, but then the normative definition of `standard_opcode_lengths` in the next item isn't really written in a way that supports an opcode base of less than 2.
> > 
> > Opcode base is really "the first special opcode" and zero can't be a special opcode because zero introduces an extended opcode.  So I agree with James, it is reasonable to have this warning.
> Ah, sorry - I think I meant to delete this comment when I read further.
> 
> I think it supports the opcode base of 1, maybe? (at least there's no interpretation I can find that would differentiate the behavior of opcode base 1 or 0, at least? Which to me, means it makes sense that the reasonable thing to do is to use 1 and never use zero from a normalization perspective - and that zero is weird enough that warning seems OK to me)
That's reasonable, to accept 1 (which implies the standard_opcode_lengths array has zero elements) but warn on 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74309