Re: DWARF5 line number program header

[Martin Cermak]

Hi Jose,

thanks for your review and comments.  Please check the attached
patch. Does it look good to you?

Specifically, I've combined the two testcases into one as you
suggested.  Two separate testcases look better to me though. If
that's fine by you, I'm happy to re-iterate, and separate them
again.  But if you really prefer one big testcase, that's fine
with me too :)

Please review.

Thanks,
Martin

On  Wed  2023-11-08  17:47 , Jose E. Marchesi wrote:
>
> Hi Martin.
>
> Thanks for the new version of the patch.
>
> >> > +dw_cfg.add
> >> > +  :class "dw-lnct"
> >> > +  :tag DW_LNCT_hi_user :name "DW_LNCT_hi_user";
> >>
> >> Does this ready belong to dwarf-abbrev.pk?  Why not having these defined
> >> in dwarf-line.pk instead?
> >
> > It's not in dwarf-abbrev.pk, but in dwarf-info.pk, see above.  My
> > point was to keep all the constants in one place.  But I'm happy
> > to move it around if prefer that.  Do you?
>
> Hmm, dwarf-info.pk is about stuff that is in .debug_info.  I think it
> would be better to have the definitions corresponding to data in
> .dwarf_line sections in dwarf-line.pk.
>
> >> > diff --git a/dwarf-line.pk b/dwarf-line.pk
> >> > new file mode 100644
> >> > index 0000000..56866c0
> >> > --- /dev/null
> >> > +++ b/dwarf-line.pk
> >> > @@ -0,0 +1,159 @@
>
> dwarf-line.pk needs a header with copyright notice, license note etc.
>
> >> > +    uint<8> maximum_operations_per_instruction;
> >> > +    uint<8> default_is_stmt;
> >> > +    int<8> line_base;
> >> > +    uint<8> line_range;
> >> > +    uint<8> opcode_base;
> >>
> >> What is this?  Does it require a constraint?
> >
> > Not sure.  The standard says that for dwarf version 5 it
> > typically is 13 (and it is in my test case) but it can be both
> > more and less and that all has its meaning.  So, not sure about
> > the constraint.
>
> The standard says opcode_base is "The number assigned to the first
> special opcode.".
>
> Typically one more than the number of "standard opcodes".  The number of
> standard opcodes is `opcode_base - 1'
>
> Apparently there are 12 standard opcodes for DWARF5.  These are defined
> later in the spec in `6.2.5.2 Standard Opcodes' with the DW_LNS_*
> definitions.  I think we should add definitions for these and base the
> constraint for opcode_base on them.
>
> >
> >>
> >> > +    uint<8>[opcode_base - 1] standard_opcode_lengths;
> >>
> >> Offset?
> >
> > Here the standard says that it essentially is operand count,
> > so IIUIC - array of dimensionless numbers.
>
> Yes then integral makes more sense.
>
> > diff --git a/test-dwarf-line.pk.in b/test-dwarf-line.pk.in
> > new file mode 100644
>
> Is it worth it to introduce a new file test-dwarf-line.pk?  What about
> having all the tests in the existing test-dwarf.pk.in?

my.patch.1699480785
Description: Text document