From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id A71EF3938C38; Tue, 23 Feb 2021 23:32:07 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A71EF3938C38 From: "cvs-commit at gcc dot gnu.org" To: gdb-prs@sourceware.org Subject: [Bug gdb/26828] SIGSEGV in follow_die_offset dwarf2/read.c:22950 Date: Tue, 23 Feb 2021 23:32:07 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gdb X-Bugzilla-Component: gdb X-Bugzilla-Version: 10.1 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: cvs-commit at gcc dot gnu.org X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P2 X-Bugzilla-Assigned-To: unassigned at sourceware dot org X-Bugzilla-Target-Milestone: 10.2 X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://sourceware.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: gdb-prs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-prs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Feb 2021 23:32:07 -0000 https://sourceware.org/bugzilla/show_bug.cgi?id=3D26828 --- Comment #29 from cvs-commit at gcc dot gnu.org --- The gdb-10-branch branch has been updated by Simon Marchi : https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D1c8a25b683c7= ea9621b7fccf6c07630bd67f071f commit 1c8a25b683c7ea9621b7fccf6c07630bd67f071f Author: Simon Marchi Date: Tue Feb 23 12:07:10 2021 -0500 gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded The previous commit log described how items could be left lingering in the dwarf2_per_bfd::queue and how that could cause trouble. This patch fixes the issue by changing maybe_queue_comp_unit so that it doesn't put a CU in the to-expand queue if that CU is already expanded. This will make it so that when dwarf2_fetch_die_type_sect_off calls follow_die_offset and maybe_queue_comp_unit, it won't enqueue the target CU, because it will see the CU is already expanded. This assumes that if a CU is dwarf2_fetch_die_type_sect_off's target CU, it will have previously been expanded. I think it is the case, but I can't be 100% sure. If that's not true, the assertions added in the following patch will catch it, and it means we'll have to re-think a bit more how things work (it wouldn't be well handled at all today anyway). This fixes something else in maybe_queue_comp_unit that looks wrong. Imagine the DIEs of a CU are loaded in memory, but that CU is not expanded. In that case, maybe_queue_comp_unit will use this early return: /* If the compilation unit is already loaded, just mark it as used. */ dwarf2_cu *cu =3D per_objfile->get_cu (per_cu); if (cu !=3D nullptr) { cu->last_used =3D 0; return 0; } ... so the CU won't be queued for expansion. Whether the DIEs of a CU are loaded in memory and whether that CU is expanded are two orthogonal things, but that function appears to mix them. So, move the queuing above that check / early return, so that if the CU's DIEs are loaded in memory but the CU is not expanded yet, it gets enqueued. I tried to improve maybe_queue_comp_unit's documentation to clarify what the return value means. By clarifying this, I noticed that two callers (follow_die_offset and follow_die_sig_1) access the CU's DIEs after calling maybe_queue_comp_unit, only relying on maybe_queue_comp_unit's return value to tell whether DIEs need to be loaded first or not. As explained in the new comment, this is problematic: maybe_queue_comp_unit's return value doesn't tell whether DIEs are currently loaded, it means whether maybe_queue_comp_unit requires the caller to load them. If the CU is already expanded but the DIEs to have been freed, maybe_queue_comp_unit returns 0, meaning "I don't need you to load the DIEs". So if these two functions (follow_die_offset and follow_die_sig_1) need to access the DIEs in any case, for their own usage, they should make sure to load them if they are not loaded already. I therefore added an extra check to the condition they use, making it so they will always load the DIEs if they aren't already. From what I found, other callers don't care for the CU's DIEs, they call maybe_queue_comp_unit to ensure the CU gets expanded eventually, but don't care for it after that. gdb/ChangeLog: PR gdb/26828 * dwarf2/read.c (maybe_queue_comp_unit): Check if CU is expanded to decide whether or not to enqueue it for expansion. (follow_die_offset, follow_die_sig_1): Ensure we load the DIEs after calling maybe_queue_comp_unit. Change-Id: Id98c6b60669f4b4b21b9be16d0518fc62bdf686a --=20 You are receiving this mail because: You are on the CC list for the bug.=