Re: [PATCH][low-mem] Fix DW_OP_GNU_parameter_ref handling in read_exprloc

[Tom de Vries <tdevries@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

On 14-02-19 10:38, Tom de Vries wrote:
> Hi,
> > Function read_exprloc contains a loop that marks all parents of a
> DW_OP_GNU_parameter_ref reference with CK_BAD.  The loop however has no
> private loop variable, so the ref variable, initially pointing to the
> referenced DIE, ends up after the loop pointing to the root parent of the
> reference instead.  Consequently, the code after the loop, intended to be
> executed for the referenced DIE, is instead executed for the root parent of
> the referenced DIE.
> 
> Fix this by moving the loop alap.
> 

This updated patch introduces a private loop variable instead.

Also, the rationale now contains a more elaborate analysis of the PR.

Committed as below.

Thanks,
- Tom

[-- Attachment #2: 0001-low-mem-Fix-DW_OP_GNU_parameter_ref-handling-in-read_exprloc.patch --]
[-- Type: text/x-patch, Size: 3403 bytes --]

[low-mem] Fix DW_OP_GNU_parameter_ref handling in read_exprloc

When dwz-ing a cc1 build with bootstrap-lto in low-mem mode, we run into:
...
$ dwz -l0 cc1
dwz: dwz.c:8558: adjust_exprloc: \
  Assertion `refd != NULL && !refd->die_remove' failed.
Aborted (core dumped)
...

This abort is triggered as follows.  There's a DIE 0x27a408:
...
 <2><27a408>: Abbrev Number: 6 (DW_TAG_formal_parameter)
    <27a409>   DW_AT_abstract_origin: <0x648e6c6>
    <27a40d>   DW_AT_location    : 0x540585 (location list)
    <27a411>   DW_AT_GNU_locviews: 0x540583
...
which refers to location list 0x540585:
...
    00540583 v000000000000001 v000000000000000 location view pair

    00540585 v000000000000001 v000000000000000 views at 00540583 for:
             00000000005884a2 00000000005884b7 (DW_OP_GNU_parameter_ref: \
	                                        <0x267c92>; DW_OP_stack_value)
    0054059d <End of list>
...
which contains a DWARF op referring to the DIE at 0x267c92 (listed here with
parent 0x267c89):
...
 <2><267c89>: Abbrev Number: 56 (DW_TAG_subprogram)
    <267c8a>   DW_AT_abstract_origin: <0x648e6b5>
    <267c8e>   DW_AT_sibling     : <0x267c98>
 <3><267c92>: Abbrev Number: 21 (DW_TAG_formal_parameter)
    <267c93>   DW_AT_abstract_origin: <0x648e6c6>
...

When invoking read_exprloc for DIE 0x27a408 and ref DIE 0x267c92, we execute:
...
	  if (ref->die_ck_state == CK_KNOWN)
	    {
	      ref->die_ck_state = CK_BAD;
	      while (!ref->die_root
		     && ref->die_parent->die_ck_state == CK_KNOWN)
		{
		  ref = ref->die_parent;
		  ref->die_ck_state = CK_BAD;
		}
	    }
	  else
	    ref->die_ck_state = CK_BAD;
...
and end up in the while loop, after which the ref variable has been updated to
0x267c89.

Subsequently, we execute a low-mem specific bit:
...
	  if (unlikely (low_mem))
	    {
	      ref->die_referenced = 1;
	      /* As .debug_loc adjustment is done after
		 write_info finishes, we need to keep the referenced
		 DIEs around uncollapsed.  */
	      if (need_adjust)
		ref->die_intercu_referenced = 1;
	    }
...
but fail to mark the referenced DIE 0x267c92 with die_referenced, and instead
mark its parent 0x267c89.

Consequently, since nothing else references 0x267c92, it's removed from the
off_htab during remove_unneeded, and when we try to find it in off_htab during
adjust_exprloc:
...
          refd = off_htab_lookup (refcu, refcu->cu_offset + addr);
          assert (refd != NULL && !refd->die_remove);
...
we abort because refd == NULL.

Fix this by using a private loop variable in the while loop.

2019-04-27  Tom de Vries  <tdevries@suse.de>

	PR dwz/24195
	* dwz.c (read_exprloc): Use private loop variable in while loop.

---
 dwz.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/dwz.c b/dwz.c
index 2c0e4c4..b968fef 100644
--- a/dwz.c
+++ b/dwz.c
@@ -1540,12 +1540,15 @@ read_exprloc (DSO *dso, dw_die_ref die, unsigned char *ptr, size_t len,
 	    ref->die_op_call2_referenced = 1;
 	  if (ref->die_ck_state == CK_KNOWN)
 	    {
+	      dw_die_ref d;
 	      ref->die_ck_state = CK_BAD;
-	      while (!ref->die_root
-		     && ref->die_parent->die_ck_state == CK_KNOWN)
+
+	      d = ref;
+	      while (!d->die_root
+		     && d->die_parent->die_ck_state == CK_KNOWN)
 		{
-		  ref = ref->die_parent;
-		  ref->die_ck_state = CK_BAD;
+		  d = d->die_parent;
+		  d->die_ck_state = CK_BAD;
 		}
 	    }
 	  else