On Thu, 2021-02-18 at 15:09 +0100, Jakub Jelinek wrote:
> On Thu, Feb 18, 2021 at 02:40:36PM +0100, Mark Wielaard wrote:
> > On Sat, 2021-02-13 at 23:46 +0100, Mark Wielaard wrote:
> > > Since DWARF version 4 blocks just contain bytes, trying to interpret
> > > them as exprlocs will most likely fail.
> > > 
> > >      * dwz.c (add_locexpr_dummy_dies): Only handle block as exprloc
> > >      for cu_version < 4.
> > >      (checksum_die): Likewise.
> > >      (write_die): Likewise.
> > > 
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=26987
> > 
> > Ping. Any comments?
> 
> Doing some GCC archeology if it is safe, I think it principially ok, but I'd
> like slightly different patch, see below.
> [...]
> > > +++ b/dwz.c
> > > @@ -2913,43 +2913,44 @@ add_locexpr_dummy_dies (DSO *dso,
> > > dw_cu_ref
> > > cu, dw_die_ref die,
> > >    if (form == DW_FORM_block1)
> > >      {
> > >        /* Old DWARF uses blocks instead of exprlocs.  */
> 
> Instead of reindenting everything, can't you simply change
> -  if (form == DW_FORM_block1)
> +  if (form == DW_FORM_block1 && cu->cu_version < 4)

Yes, I was under the impression that the return 0; in the block was
special, but it isn't, the rest of the function does explicitly check
the form (isn't DW_FORM_block1) and so if it simply falls-through it
will also end up at return 0; at the end.

> > >        if (form == DW_FORM_block1)
> 
> And likewise here:
> -      if (form == DW_FORM_block1)
> +      if (form == DW_FORM_block1 && cu->cu_version < 4)

But here we do need to handle the DW_FORM_block && cu->cu_version >=4
version separately. But that can be done by not indention the large
block and adding an small else if block.

Does the attached variant look better?

Thanks,

Mark