Re: [PATCH] Don't handle blocks as exprlocs for DWARF version 4 or higher.

[Jakub Jelinek <jakub@redhat.com>]

On Thu, Feb 18, 2021 at 05:18:10PM +0100, Mark Wielaard wrote:
> > > >        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.

Ah, I got confused by DW_FORM_block{2,4,} cases changing form to
DW_FORM_block1, indeed, for all of DW_FORM_{block{1,2,4,},exprloc} we need
to do ptr += len;
But perhaps we could do instead do:
-      if (form == DW_FORM_block1)
+      if (form == DW_FORM_block1 && cu->cu_version < 4)
...
-	  ptr += len;
...
-	  ptr += len;
 	}
+      ptr += len;
?
len is only set to non-0 for:
        case DW_FORM_block1:
          len = *ptr++;
          break;
        case DW_FORM_block2:
          len = read_16 (ptr);
          form = DW_FORM_block1;
          break;
        case DW_FORM_block4:
          len = read_32 (ptr);
          form = DW_FORM_block1;
          break;
        case DW_FORM_block:
          len = read_uleb128 (ptr);
          form = DW_FORM_block1;
          break;
        case DW_FORM_exprloc:
          len = read_uleb128 (ptr);
          break;
i.e. exactly the cases we want to move.

Anyway, looking around some more,
              if (unlikely (low_mem_phase1)
                  && add_locexpr_dummy_dies (dso, cu, die, ptr, form,
                                             t->attr[i].attr, len))
                  goto fail;
looks incorrect to me, form in that case will be DW_FORM_block{2,4,}
and won't be canonicalized to DW_FORM_block1.  And furthermore
len will be always 0.  It is preceded only by
              size_t len = 0;
and a loop handling DW_FORM_indirect.  So, ptr will always be
the pointer to the block count too.
This has been added for PR dwz/24204 by Tom, Tom, can you please comment on
that?
That function handles the DW_FORM_block1 (it wants canonicalization of
DW_FORM_block{2,4,} to DW_FORM_block1) and DW_FORM_exprloc but wants
ptr to be the start of those blocks and len to be the block length, or
it handles DW_FORM_data{4,8} and DW_FORM_sec_offset for which it wants
ptr to stay before the bump.

So, I bet we need something like:
	      switch (form)
		{
		case DW_FORM_block1:
		  len = *ptr++;
		  break;
		case DW_FORM_block2:
		  len = read_16 (ptr);
		  form = DW_FORM_block1;
		  break;
		case DW_FORM_block4:
		  len = read_32 (ptr);
		  form = DW_FORM_block1;
		  break;
		case DW_FORM_block:
		  len = read_uleb128 (ptr);
		  form = DW_FORM_block1;
		  break;
		case DW_FORM_exprloc:
		  len = read_uleb128 (ptr);
		  break;
		default:
		  break;
		}
added before the
              if (unlikely (low_mem_phase1)
                  && add_locexpr_dummy_dies (dso, cu, die, ptr, form,
                                             t->attr[i].attr, len))
                  goto fail;
and then the DW_FORM_{block*,exprloc} handling later on be changed to:
                case DW_FORM_block1:
		  break;
		case DW_FORM_exprloc:
		  form = DW_FORM_block1;
		  break;
and remove the DW_FORM_block{2,4,} cases.

	Jakub