public inbox for dwz@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [multi-file] Handle DW_FORM_ref_addr reference that points to containing unit
@ 2019-01-01  0:00 Tom de Vries
  2019-01-01  0:00 ` [committed][multi-file] Handle intra-CU DW_FORM_ref_addr reference Tom de Vries
  0 siblings, 1 reply; 2+ messages in thread
From: Tom de Vries @ 2019-01-01  0:00 UTC (permalink / raw)
  To: dwz, jakub

[ Submitted earlier off-list ]

Hi,

For test-cases with DW_FORM_ref_addr attributes that point to within the CU
containing the attribute, we run into the following assert in multifile mode:
...
dwz: dwz.c:1984: checksum_die: Assertion `
  ((!op_multifile && !rd_multifile && !fi_multifile) || cu != die_cu (ref))
  && (!op_multifile || cu->cu_chunk == die_cu (ref)->cu_chunk)
  ' failed.
...

This conservative fix detects the situation, and instead of asserting stops
further processing of the DIE.

OK for trunk?

Thanks,
- Tom

[multi-file] Handle DW_FORM_ref_addr reference that points to containing unit

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

	PR dwz/24170
	* dwz.c (checksum_die): Handle DW_FORM_ref_addr references that points
	to containing unit conservatively.

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

diff --git a/dwz.c b/dwz.c
index 4ef8657..0415630 100644
--- a/dwz.c
+++ b/dwz.c
@@ -1979,10 +1979,13 @@ checksum_die (DSO *dso, dw_cu_ref cu, dw_die_ref top_die, dw_die_ref die)
 		}
 	      if (unlikely (op_multifile) && ref->die_collapsed_child)
 		ref = ref->die_parent;
-	      assert (((!op_multifile && !rd_multifile && !fi_multifile)
-		       || cu != die_cu (ref))
-		      && (!op_multifile
-			  || cu->cu_chunk == die_cu (ref)->cu_chunk));
+	      if (cu == die_cu (ref))
+		{
+		  die->die_ck_state = CK_BAD;
+		  return 0;
+		}
+	      assert (!op_multifile
+		      || cu->cu_chunk == die_cu (ref)->cu_chunk);
 	      handled = true;
 	      break;
 	    }

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

* [committed][multi-file] Handle intra-CU DW_FORM_ref_addr reference
  2019-01-01  0:00 [PATCH] [multi-file] Handle DW_FORM_ref_addr reference that points to containing unit Tom de Vries
@ 2019-01-01  0:00 ` Tom de Vries
  0 siblings, 0 replies; 2+ messages in thread
From: Tom de Vries @ 2019-01-01  0:00 UTC (permalink / raw)
  To: dwz, jakub, Mark Wielaard

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

[ was: [PATCH] [multi-file] Handle DW_FORM_ref_addr reference that
points to containing unit ]

On 07-03-19 08:36, Tom de Vries wrote:
> [ Submitted earlier off-list ]
> 
> Hi,
> 
> For test-cases with DW_FORM_ref_addr attributes that point to within the CU
> containing the attribute, we run into the following assert in multifile mode:
> ...
> dwz: dwz.c:1984: checksum_die: Assertion `
>   ((!op_multifile && !rd_multifile && !fi_multifile) || cu != die_cu (ref))
>   && (!op_multifile || cu->cu_chunk == die_cu (ref)->cu_chunk)
>   ' failed.
> ...
> 
> This conservative fix detects the situation, and instead of asserting stops
> further processing of the DIE.
> 

I've committed this version, which:
- does essentially the same fix
- has a more elaborate rationale
- adds comments
- is more minimal (remaining assert still checks op_multifile and
  rd_multifile).
- includes a test-case (which happens to break the buildbot for
  dwz-debian-armhf and dwz-debian-i386), I'll push a fix shortly.

Thanks,
- Tom

[-- Attachment #2: 0001-multi-file-Handle-intra-CU-DW_FORM_ref_addr-reference.patch --]
[-- Type: text/x-patch, Size: 13651 bytes --]

[multi-file] Handle intra-CU DW_FORM_ref_addr reference

When self-dwz-m-ing the implptr-64bit-d2o4a8r8t0 test-case, we run into the
following assert in multifile mode:
...
dwz: dwz.c:2363: checksum_die: Assertion `
  ((!op_multifile && !rd_multifile && !fi_multifile) || cu != die_cu (ref))
  && (!op_multifile || cu->cu_chunk == die_cu (ref)->cu_chunk)
  ' failed.
...

The test-case contains a DIE:
...
 <1><110>: Abbrev Number: 8 (DW_TAG_subprogram)
    <111>   DW_AT_name        : main
    <116>   DW_AT_low_pc      : 0x400497
    <11e>   DW_AT_high_pc     : 0x400597
    <126>   DW_AT_type        : <0xe4>
    <12e>   DW_AT_external    : 1
...
with DW_AT_type referring to a DIE at 0xe4:
...
<1><e4>: Abbrev Number: 3 (DW_TAG_base_type)
    <e5>   DW_AT_byte_size   : 4
    <e6>   DW_AT_encoding    : 5        (signed)
    <e7>   DW_AT_name        : int
...
using a section-relative encoding DW_FORM_ref_addr:
...
   8      DW_TAG_subprogram    [has children]
    DW_AT_name         DW_FORM_string
    DW_AT_low_pc       DW_FORM_addr
    DW_AT_high_pc      DW_FORM_addr
    DW_AT_type         DW_FORM_ref_addr
    DW_AT_external     DW_FORM_flag
    DW_AT value: 0     DW_FORM value: 0
...
[ This is not incorrect, merely unoptimal.  The DW_FORM_ref_addr reference uses
4 bytes, while it could have been encoded with DW_FORM_ref1 using only 1
byte. ]

This part of the assert triggers during finalize_multifile:
...
((!op_multifile && !rd_multifile && !fi_multifile) || cu != die_cu (ref))
...

Since the offending reference is part of the input DWARF, and there's nothing
to guarantee that it will be removed during single-file optimization, we can
expect it to occur during finalize_multifile.

This conservative fix detects the situation, and instead of asserting stops
further processing of the DIE.

2019-06-25  Tom de Vries  <tdevries@suse.de>

	PR dwz/24170
	* dwz.c (checksum_die): Handle intra-CU DW_FORM_ref_addr references
	conservatively.
	* testsuite/dwz.tests/implptr-64bit-d2o4a8r8t0.S: New test source.
	* testsuite/dwz.tests/main.c: Same.
	* testsuite/dwz.tests/pr24170.sh: New test.

---
 Makefile                                       |   5 +-
 dwz.c                                          |  31 ++++-
 testsuite/dwz.tests/implptr-64bit-d2o4a8r8t0.S | 156 +++++++++++++++++++++++++
 testsuite/dwz.tests/main.c                     |   5 +
 testsuite/dwz.tests/pr24170.sh                 |  16 +++
 5 files changed, 208 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index f039b4d..45ca3d2 100644
--- a/Makefile
+++ b/Makefile
@@ -24,7 +24,7 @@ PWD:=$(shell pwd -P)
 
 TEST_SRC = $(srcdir)/testsuite/dwz.tests
 TEST_EXECS = hello dw2-restrict py-section-script dwz-for-test min two-typedef \
-	dw2-skip-prologue start
+	dw2-skip-prologue start implptr-64bit-d2o4a8r8t0
 
 hello:
 	$(CC) $(TEST_SRC)/hello.c -o $@ -g
@@ -58,6 +58,9 @@ two-typedef:
 start:
 	$(CC) $(TEST_SRC)/start.c -o $@ -g -nostdlib
 
+implptr-64bit-d2o4a8r8t0:
+	$(CC) $(TEST_SRC)/implptr-64bit-d2o4a8r8t0.S $(TEST_SRC)/main.c -o $@ -g
+
 # On some systems we need to set and export DEJAGNU to suppress
 # WARNING: Couldn't find the global config file.
 DEJAGNU ?= /dev/null
diff --git a/dwz.c b/dwz.c
index 6f34a0c..9e39824 100644
--- a/dwz.c
+++ b/dwz.c
@@ -2357,10 +2357,33 @@ checksum_die (DSO *dso, dw_cu_ref cu, dw_die_ref top_die, dw_die_ref die)
 		}
 	      if (unlikely (op_multifile) && ref->die_collapsed_child)
 		ref = ref->die_parent;
-	      assert (((!op_multifile && !rd_multifile && !fi_multifile)
-		       || cu != die_cu (ref))
-		      && (!op_multifile
-			  || cu->cu_chunk == die_cu (ref)->cu_chunk));
+	      if (cu == die_cu (ref))
+		{
+		  /* The reference was encoded using a section-relative
+		     encoding, while if it could have been encoded using
+		     CU-relative encoding.  Typically, the latter is used,
+		     because:
+		     - it's potentially smaller, and
+		     - it doesn't require a link-time relocation.  */
+
+		  /* Assert that the multifile only contains section-relative
+		     encoding when necessary.  */
+		  assert (!op_multifile && !rd_multifile);
+
+		  if (fi_multifile)
+		    {
+		      /* It's possible that the input DWARF contains this
+			 sub-optimal reference.  We currently don't optimize
+			 this during single-file optimization, so it will still
+			 be there during finalize_multifile.  Bail out to handle
+			 this conservatively.  */
+		      die->die_ck_state = CK_BAD;
+		      return 0;
+		    }
+		}
+	      /* Assert that during op_multifile, die belongs to the same object
+		 as ref.  */
+	      assert (!op_multifile || cu->cu_chunk == die_cu (ref)->cu_chunk);
 	      handled = true;
 	      break;
 	    }
diff --git a/testsuite/dwz.tests/implptr-64bit-d2o4a8r8t0.S b/testsuite/dwz.tests/implptr-64bit-d2o4a8r8t0.S
new file mode 100644
index 0000000..7c41f5b
--- /dev/null
+++ b/testsuite/dwz.tests/implptr-64bit-d2o4a8r8t0.S
@@ -0,0 +1,156 @@
+/* This was generated using git://sourceware.org/git/binutils-gdb.git
+   repository, file gdb/testsuite/gdb.dwarf2/implptr-64bit.exp.  */
+        .section .debug_info
+.Lcu1_begin:
+        .4byte        .Lcu1_end - .Lcu1_start
+.Lcu1_start:
+        .2byte        2                 /* Version */
+        .4byte        .Labbrev1_begin   /* Abbrevs */
+        .byte        8                  /* Pointer size */
+        .uleb128        2               /* Abbrev (DW_TAG_compile_unit) */
+        .ascii        "GNU C 4.4.3\0"
+        .sleb128        0x0001
+        .ascii        "1.c\0"
+.Llabel3:
+        .uleb128        3               /* Abbrev (DW_TAG_base_type) */
+        .sleb128        4
+        .sleb128        0x5
+        .ascii        "int\0"
+.Llabel1:
+        .uleb128        4               /* Abbrev (DW_TAG_structure_type) */
+        .ascii        "s\0"
+        .sleb128        4
+        .uleb128        5               /* Abbrev (DW_TAG_member) */
+        .ascii        "f\0"
+        .4byte        .Llabel3 - .Lcu1_begin
+        .byte        0
+        .byte        0x0                /* Terminate children */
+.Llabel4:
+        .uleb128        6               /* Abbrev (DW_TAG_pointer_type) */
+        .sleb128        8
+        .4byte        .Llabel1 - .Lcu1_begin
+.Llabel2:
+        .uleb128        7               /* Abbrev (DW_TAG_variable) */
+        .ascii        "v\0"
+        .uleb128        .Lexpr_end6 - .Lexpr_start5/* expression */
+.Lexpr_start5:
+        .byte        0x9e               /* DW_OP_implicit_value */
+        .uleb128        .Lvalue_end8 - .Lvalue_start7
+.Lvalue_start7:
+        .byte        0x1
+        .byte        0x1
+        .byte        0x1
+        .byte        0x1
+.Lvalue_end8:
+.Lexpr_end6:
+        .8byte        .Llabel1 - .Lcu1_begin
+        .uleb128        8               /* Abbrev (DW_TAG_subprogram) */
+        .ascii        "main\0"
+        .8byte        main
+        .8byte        main+0x100
+        .8byte        .Llabel3
+        .byte        1
+        .uleb128        9               /* Abbrev (DW_TAG_variable) */
+        .ascii        "p\0"
+        .uleb128        .Lexpr_end10 - .Lexpr_start9/* expression */
+.Lexpr_start9:
+        .byte        0xf2               /* DW_OP_GNU_implicit_pointer */
+        .8byte        .Llabel2
+        .sleb128        0
+.Lexpr_end10:
+        .8byte        .Llabel4 - .Lcu1_begin
+        .byte        0x0                /* Terminate children */
+        .byte        0x0                /* Terminate children */
+.Lcu1_end:
+        .section .debug_abbrev
+.Labbrev1_begin:
+        .uleb128        2               /* Abbrev start */
+        .uleb128        0x11            /* DW_TAG_compile_unit */
+        .byte        1                  /* has_children */
+        .uleb128        0x25            /* DW_AT_producer */
+        .uleb128        0x08            /* DW_FORM_string */
+        .uleb128        0x13            /* DW_AT_language */
+        .uleb128        0x0d            /* DW_FORM_sdata */
+        .uleb128        0x03            /* DW_AT_name */
+        .uleb128        0x08            /* DW_FORM_string */
+        .byte        0x0                /* Terminator */
+        .byte        0x0                /* Terminator */
+        .uleb128        3               /* Abbrev start */
+        .uleb128        0x24            /* DW_TAG_base_type */
+        .byte        0                  /* has_children */
+        .uleb128        0x0b            /* DW_AT_byte_size */
+        .uleb128        0x0d            /* DW_FORM_sdata */
+        .uleb128        0x3e            /* DW_AT_encoding */
+        .uleb128        0x0d            /* DW_FORM_sdata */
+        .uleb128        0x03            /* DW_AT_name */
+        .uleb128        0x08            /* DW_FORM_string */
+        .byte        0x0                /* Terminator */
+        .byte        0x0                /* Terminator */
+        .uleb128        4               /* Abbrev start */
+        .uleb128        0x13            /* DW_TAG_structure_type */
+        .byte        1                  /* has_children */
+        .uleb128        0x03            /* DW_AT_name */
+        .uleb128        0x08            /* DW_FORM_string */
+        .uleb128        0x0b            /* DW_AT_byte_size */
+        .uleb128        0x0d            /* DW_FORM_sdata */
+        .byte        0x0                /* Terminator */
+        .byte        0x0                /* Terminator */
+        .uleb128        5               /* Abbrev start */
+        .uleb128        0x0d            /* DW_TAG_member */
+        .byte        0                  /* has_children */
+        .uleb128        0x03            /* DW_AT_name */
+        .uleb128        0x08            /* DW_FORM_string */
+        .uleb128        0x49            /* DW_AT_type */
+        .uleb128        0x13            /* DW_FORM_ref4 */
+        .uleb128        0x38            /* DW_AT_data_member_location */
+        .uleb128        0x0b            /* DW_FORM_data1 */
+        .byte        0x0                /* Terminator */
+        .byte        0x0                /* Terminator */
+        .uleb128        6               /* Abbrev start */
+        .uleb128        0x0f            /* DW_TAG_pointer_type */
+        .byte        0                  /* has_children */
+        .uleb128        0x0b            /* DW_AT_byte_size */
+        .uleb128        0x0d            /* DW_FORM_sdata */
+        .uleb128        0x49            /* DW_AT_type */
+        .uleb128        0x13            /* DW_FORM_ref4 */
+        .byte        0x0                /* Terminator */
+        .byte        0x0                /* Terminator */
+        .uleb128        7               /* Abbrev start */
+        .uleb128        0x34            /* DW_TAG_variable */
+        .byte        0                  /* has_children */
+        .uleb128        0x03            /* DW_AT_name */
+        .uleb128        0x08            /* DW_FORM_string */
+        .uleb128        0x02            /* DW_AT_location */
+        .uleb128        0x09            /* SPECIAL_expr */
+        .uleb128        0x49            /* DW_AT_type */
+        .uleb128        0x14            /* DW_FORM_ref8 */
+        .byte        0x0                /* Terminator */
+        .byte        0x0                /* Terminator */
+        .uleb128        8               /* Abbrev start */
+        .uleb128        0x2e            /* DW_TAG_subprogram */
+        .byte        1                  /* has_children */
+        .uleb128        0x03            /* DW_AT_name */
+        .uleb128        0x08            /* DW_FORM_string */
+        .uleb128        0x11            /* DW_AT_low_pc */
+        .uleb128        0x01            /* DW_FORM_addr */
+        .uleb128        0x12            /* DW_AT_high_pc */
+        .uleb128        0x01            /* DW_FORM_addr */
+        .uleb128        0x49            /* DW_AT_type */
+        .uleb128        0x10            /* DW_FORM_ref_addr */
+        .uleb128        0x3f            /* DW_AT_external */
+        .uleb128        0x0c            /* DW_FORM_flag */
+        .byte        0x0                /* Terminator */
+        .byte        0x0                /* Terminator */
+        .uleb128        9               /* Abbrev start */
+        .uleb128        0x34            /* DW_TAG_variable */
+        .byte        0                  /* has_children */
+        .uleb128        0x03            /* DW_AT_name */
+        .uleb128        0x08            /* DW_FORM_string */
+        .uleb128        0x02            /* DW_AT_location */
+        .uleb128        0x09            /* SPECIAL_expr */
+        .uleb128        0x49            /* DW_AT_type */
+        .uleb128        0x14            /* DW_FORM_ref8 */
+        .byte        0x0                /* Terminator */
+        .byte        0x0                /* Terminator */
+        .byte        0x0                /* Terminator */
+        .byte        0x0                /* Terminator */
diff --git a/testsuite/dwz.tests/main.c b/testsuite/dwz.tests/main.c
new file mode 100644
index 0000000..398ec67
--- /dev/null
+++ b/testsuite/dwz.tests/main.c
@@ -0,0 +1,5 @@
+int
+main (void)
+{
+  return 0;
+}
diff --git a/testsuite/dwz.tests/pr24170.sh b/testsuite/dwz.tests/pr24170.sh
new file mode 100644
index 0000000..20fe339
--- /dev/null
+++ b/testsuite/dwz.tests/pr24170.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+exec=$execs/implptr-64bit-d2o4a8r8t0
+
+cp $exec 1
+cp 1 2
+
+dwz -m 3 1 2
+
+smaller-than.sh 1 $exec
+smaller-than.sh 2 $exec
+
+[ "$(gnu-debugaltlink-name.sh 1)" = "3" ]
+[ "$(gnu-debugaltlink-name.sh 2)" = "3" ]
+
+rm -f 1 2 3

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

end of thread, other threads:[~2019-06-25 10:17 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-01  0:00 [PATCH] [multi-file] Handle DW_FORM_ref_addr reference that points to containing unit Tom de Vries
2019-01-01  0:00 ` [committed][multi-file] Handle intra-CU DW_FORM_ref_addr reference 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).