* [PATCH] DWZ aborted "write_types: Assertion `ref && ref->die_dup == NULL'"
@ 2021-02-05 6:09 Sharma, Alok Kumar
2021-02-05 13:09 ` Mark Wielaard
0 siblings, 1 reply; 4+ messages in thread
From: Sharma, Alok Kumar @ 2021-02-05 6:09 UTC (permalink / raw)
To: dwz; +Cc: George, Jini Susan, E, Nagajyothi, Achra, Nitika
[-- Attachment #1: Type: text/plain, Size: 831 bytes --]
Hi all,
Would you please review the attached patch containing modification of assert condition.
It was required to fix an abort which seem to be incorrect.
There is an assert condition as "ref && ref->die_dup == NULL".
As per definition of "struct dw_die", the structure fields starting
from 'die_dup' are present only if 'die_toplevel' is 1. In line with
this at multiple places in code, full/paritial memory is allocated for
pointer of type dw_die.
Ex. die = pool_alloc (dw_die, offsetof (struct dw_die, die_dup));
Due to this, since memory is not allocated for field die_dup onwards,
it may contain junk values. Macro 'die_safe_dupe' must be used in place
of directly accessing 'die_dup' field whenever required.
* dwz.c (write_types): Use 'die_safe_dup' to access field 'die_dup'.
Regards,
Alok
[-- Attachment #2: 0001-DWZ-aborted-write_types-Assertion-ref-ref-die_dup-NU.patch --]
[-- Type: application/octet-stream, Size: 1454 bytes --]
From a3274a19e5b73b7e41e74f6ddfcdd2073f83ab63 Mon Sep 17 00:00:00 2001
From: Alok Kumar Sharma <AlokKumar.Sharma@amd.com>
Date: Mon, 1 Feb 2021 23:57:51 +0530
Subject: [PATCH] DWZ aborted "write_types: Assertion `ref && ref->die_dup ==
NULL'"
There is an assert condition as "ref && ref->die_dup == NULL".
As per definition of "struct dw_die", the structure fields starting
from 'die_dup' are present only if 'die_toplevel' is 1. In line with
this at multiple places in code, full/paritial memory is allocated for
pointer of type dw_die.
Ex. die = pool_alloc (dw_die, offsetof (struct dw_die, die_dup));
Due to this, since memory is not allocated for field die_dup onwards,
it may contain junk values. Macro 'die_safe_dupe' must be used in place
of directly accessing 'die_dup' in assert condition here.
* dwz.c (write_types): Use 'die_safe_dup' to access field 'die_dup'.
---
dwz.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dwz.c b/dwz.c
index 1db93dd..920a9b8 100644
--- a/dwz.c
+++ b/dwz.c
@@ -12868,7 +12868,7 @@ write_types (void)
memcpy (ptr, inptr - 8, 8);
ptr += 8;
ref = off_htab_lookup (cu, cu->cu_offset + read_32 (inptr));
- assert (ref && ref->die_dup == NULL);
+ assert (ref && die_safe_dup(ref) == NULL);
write_32 (ptr, ref->u.p2.die_new_offset);
ptr = write_die (ptr, cu, cu->cu_die, NULL, NULL, NULL);
assert (types + next_off == ptr);
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] DWZ aborted "write_types: Assertion `ref && ref->die_dup == NULL'"
2021-02-05 6:09 [PATCH] DWZ aborted "write_types: Assertion `ref && ref->die_dup == NULL'" Sharma, Alok Kumar
@ 2021-02-05 13:09 ` Mark Wielaard
2021-02-05 13:21 ` George, Jini Susan
0 siblings, 1 reply; 4+ messages in thread
From: Mark Wielaard @ 2021-02-05 13:09 UTC (permalink / raw)
To: Sharma, Alok Kumar, dwz; +Cc: George, Jini Susan, E, Nagajyothi, Achra, Nitika
Hi Alok,
On Fri, 2021-02-05 at 06:09 +0000, Sharma, Alok Kumar via Dwz wrote:
> Would you please review the attached patch containing modification of
> assert condition.
> It was required to fix an abort which seem to be incorrect.
>
> There is an assert condition as "ref && ref->die_dup == NULL".
> As per definition of "struct dw_die", the structure fields starting
> from 'die_dup' are present only if 'die_toplevel' is 1. In line with
> this at multiple places in code, full/paritial memory is allocated for
> pointer of type dw_die.
> Ex. die = pool_alloc (dw_die, offsetof (struct dw_die, die_dup));
> Due to this, since memory is not allocated for field die_dup onwards,
> it may contain junk values. Macro 'die_safe_dupe' must be used in place
> of directly accessing 'die_dup' field whenever required.
>
> * dwz.c (write_types): Use 'die_safe_dup' to access field 'die_dup'.
I think you analysis and the code fix are correct. Thanks.
This was probably missed before because -fdebug-types-section isn't the
default, so we are not seeing many DWARF files with debug types.
Do you have an example where this triggers?
Thanks,
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] DWZ aborted "write_types: Assertion `ref && ref->die_dup == NULL'"
2021-02-05 13:09 ` Mark Wielaard
@ 2021-02-05 13:21 ` George, Jini Susan
2021-02-06 3:01 ` Mark Wielaard
0 siblings, 1 reply; 4+ messages in thread
From: George, Jini Susan @ 2021-02-05 13:21 UTC (permalink / raw)
To: Mark Wielaard, Sharma, Alok Kumar, dwz; +Cc: E, Nagajyothi, Achra, Nitika
[AMD Public Use]
Hi Mark,
This was triggering with the gdb test case gdb.dwarf2/dw4-sig-types.exp when compiled with clang.
Thanks,
Jini.
-----Original Message-----
From: Mark Wielaard <mark@klomp.org>
Sent: Friday, February 5, 2021 6:39 PM
To: Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>; dwz@sourceware.org
Cc: George, Jini Susan <JiniSusan.George@amd.com>; E, Nagajyothi <Nagajyothi.E@amd.com>; Achra, Nitika <Nitika.Achra@amd.com>
Subject: Re: [PATCH] DWZ aborted "write_types: Assertion `ref && ref->die_dup == NULL'"
[CAUTION: External Email]
Hi Alok,
On Fri, 2021-02-05 at 06:09 +0000, Sharma, Alok Kumar via Dwz wrote:
> Would you please review the attached patch containing modification of
> assert condition.
> It was required to fix an abort which seem to be incorrect.
>
> There is an assert condition as "ref && ref->die_dup == NULL".
> As per definition of "struct dw_die", the structure fields starting
> from 'die_dup' are present only if 'die_toplevel' is 1. In line with
> this at multiple places in code, full/paritial memory is allocated for
> pointer of type dw_die.
> Ex. die = pool_alloc (dw_die, offsetof (struct dw_die, die_dup)); Due
> to this, since memory is not allocated for field die_dup onwards, it
> may contain junk values. Macro 'die_safe_dupe' must be used in place
> of directly accessing 'die_dup' field whenever required.
>
> * dwz.c (write_types): Use 'die_safe_dup' to access field 'die_dup'.
I think you analysis and the code fix are correct. Thanks.
This was probably missed before because -fdebug-types-section isn't the default, so we are not seeing many DWARF files with debug types.
Do you have an example where this triggers?
Thanks,
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] DWZ aborted "write_types: Assertion `ref && ref->die_dup == NULL'"
2021-02-05 13:21 ` George, Jini Susan
@ 2021-02-06 3:01 ` Mark Wielaard
0 siblings, 0 replies; 4+ messages in thread
From: Mark Wielaard @ 2021-02-06 3:01 UTC (permalink / raw)
To: George, Jini Susan; +Cc: Sharma, Alok Kumar, dwz, E, Nagajyothi, Achra, Nitika
Hi Jini,
On Fri, Feb 05, 2021 at 01:21:16PM +0000, George, Jini Susan via Dwz wrote:
> This was triggering with the gdb test case
> gdb.dwarf2/dw4-sig-types.exp when compiled with clang.
Thanks, I could replicate it with that. The difference between g++
and clang++ building that testcase with -gdwarf-4
-fdebug-types-section is that g++ always creates the actual type DIE
as a top-level DIE in the type unit, while clang++ can create a deeper
embedded type DIE. If g++ needs the type to be nested (for example
inside a namespace, it will still create a top-level DIE with a
DW_AT_specification pointing to the nested type. Which is why we never
saw this assert accidentially trigger in the g++ case.
I pushed the patch.
Thanks,
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-06 3:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-05 6:09 [PATCH] DWZ aborted "write_types: Assertion `ref && ref->die_dup == NULL'" Sharma, Alok Kumar
2021-02-05 13:09 ` Mark Wielaard
2021-02-05 13:21 ` George, Jini Susan
2021-02-06 3:01 ` Mark Wielaard
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).