public inbox for dwz@sourceware.org
 help / color / mirror / Atom feed
* [PATCH][low-mem] Fix DW_OP_GNU_parameter_ref handling in read_exprloc
@ 2019-01-01  0:00 Tom de Vries
  2019-01-01  0:00 ` [PRING][PATCH][low-mem] " Tom de Vries
  2019-01-01  0:00 ` [PATCH][low-mem] " Tom de Vries
  0 siblings, 2 replies; 3+ messages in thread
From: Tom de Vries @ 2019-01-01  0:00 UTC (permalink / raw)
  To: dwz, jakub

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.

OK for trunk?

Thanks,
- Tom

[low-mem] Fix DW_OP_GNU_parameter_ref handling in read_exprloc

2019-02-14  Tom de Vries  <tdevries@suse.de>

	PR dwz/24195
	* dwz.c (read_exprloc): Move loop marking parents with CK_BAD alap.

---
 dwz.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/dwz.c b/dwz.c
index d348418..6e6b6fb 100644
--- a/dwz.c
+++ b/dwz.c
@@ -1492,6 +1492,15 @@ read_exprloc (DSO *dso, dw_die_ref die, unsigned char *ptr, size_t len,
 	    }
 	  if (op == DW_OP_call2)
 	    ref->die_op_call2_referenced = 1;
+	  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;
+	    }
 	  if (ref->die_ck_state == CK_KNOWN)
 	    {
 	      ref->die_ck_state = CK_BAD;
@@ -1504,15 +1513,6 @@ read_exprloc (DSO *dso, dw_die_ref die, unsigned char *ptr, size_t len,
 	    }
 	  else
 	    ref->die_ck_state = CK_BAD;
-	  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;
-	    }
 	  die->die_ck_state = CK_BAD;
 	  if (need_adjust)
 	    *need_adjust = true;

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PRING][PATCH][low-mem] Fix DW_OP_GNU_parameter_ref handling in read_exprloc
  2019-01-01  0:00 [PATCH][low-mem] Fix DW_OP_GNU_parameter_ref handling in read_exprloc Tom de Vries
@ 2019-01-01  0:00 ` Tom de Vries
  2019-01-01  0:00 ` [PATCH][low-mem] " Tom de Vries
  1 sibling, 0 replies; 3+ messages in thread
From: Tom de Vries @ 2019-01-01  0:00 UTC (permalink / raw)
  To: jakub; +Cc: dwz

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.
> 
> OK for trunk?
> 

Ping.

[ This fixes an assert triggered on a cc1 executable generated by gcc
bootstrap-lto:
...
$ dwz cc1
dwz: dwz.c:8562: adjust_exprloc: Assertion `refd != NULL &&
!refd->die_remove' failed.
... ]

Thanks,
- Tom

> [low-mem] Fix DW_OP_GNU_parameter_ref handling in read_exprloc
> 
> 2019-02-14  Tom de Vries  <tdevries@suse.de>
> 
> 	PR dwz/24195
> 	* dwz.c (read_exprloc): Move loop marking parents with CK_BAD alap.
> 
> ---
>  dwz.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/dwz.c b/dwz.c
> index d348418..6e6b6fb 100644
> --- a/dwz.c
> +++ b/dwz.c
> @@ -1492,6 +1492,15 @@ read_exprloc (DSO *dso, dw_die_ref die, unsigned char *ptr, size_t len,
>  	    }
>  	  if (op == DW_OP_call2)
>  	    ref->die_op_call2_referenced = 1;
> +	  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;
> +	    }
>  	  if (ref->die_ck_state == CK_KNOWN)
>  	    {
>  	      ref->die_ck_state = CK_BAD;
> @@ -1504,15 +1513,6 @@ read_exprloc (DSO *dso, dw_die_ref die, unsigned char *ptr, size_t len,
>  	    }
>  	  else
>  	    ref->die_ck_state = CK_BAD;
> -	  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;
> -	    }
>  	  die->die_ck_state = CK_BAD;
>  	  if (need_adjust)
>  	    *need_adjust = true;
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][low-mem] Fix DW_OP_GNU_parameter_ref handling in read_exprloc
  2019-01-01  0:00 [PATCH][low-mem] Fix DW_OP_GNU_parameter_ref handling in read_exprloc Tom de Vries
  2019-01-01  0:00 ` [PRING][PATCH][low-mem] " Tom de Vries
@ 2019-01-01  0:00 ` Tom de Vries
  1 sibling, 0 replies; 3+ messages in thread
From: Tom de Vries @ 2019-01-01  0:00 UTC (permalink / raw)
  To: dwz, jakub

[-- 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-04-27 19:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-01  0:00 [PATCH][low-mem] Fix DW_OP_GNU_parameter_ref handling in read_exprloc Tom de Vries
2019-01-01  0:00 ` [PRING][PATCH][low-mem] " Tom de Vries
2019-01-01  0:00 ` [PATCH][low-mem] " Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).