* [PATCH 0/4] Fix CU expansion queue-related problems @ 2020-11-17 19:12 Simon Marchi 2020-11-17 19:12 ` [PATCH 1/4] gdb/dwarf: add some logging in dwarf2/read.c Simon Marchi ` (4 more replies) 0 siblings, 5 replies; 20+ messages in thread From: Simon Marchi @ 2020-11-17 19:12 UTC (permalink / raw) To: gdb-patches; +Cc: nilsgladitz, Simon Marchi This patch series fixes problems explained in PR 26828 [1]. The problems are related to CU DIEs loading/freeing and the CU expansion queue. All the gory details are in the commit logs. I tried to write a test case for this using the DWARF assembler but I couldn't manage to do it. I decided to submit the series anyway, because I'd still like to fix the issue. I might try again later, but I might also not have the time, so if anybody would like to give it a try, you are welcome. [1] https://sourceware.org/bugzilla/show_bug.cgi?id=26828 Simon Marchi (4): gdb/dwarf: add some logging in dwarf2/read.c gdb/dwarf: add assertion in maybe_queue_comp_unit gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded gdb/dwarf: create and destroy dwarf2_per_bfd's CUs-to-expand queue gdb/dwarf2/read.c | 112 ++++++++++++++++++++++++++++++---------------- gdb/dwarf2/read.h | 2 +- 2 files changed, 75 insertions(+), 39 deletions(-) -- 2.29.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/4] gdb/dwarf: add some logging in dwarf2/read.c 2020-11-17 19:12 [PATCH 0/4] Fix CU expansion queue-related problems Simon Marchi @ 2020-11-17 19:12 ` Simon Marchi 2020-12-09 21:08 ` Tom Tromey 2020-11-17 19:12 ` [PATCH 2/4] gdb/dwarf: add assertion in maybe_queue_comp_unit Simon Marchi ` (3 subsequent siblings) 4 siblings, 1 reply; 20+ messages in thread From: Simon Marchi @ 2020-11-17 19:12 UTC (permalink / raw) To: gdb-patches; +Cc: nilsgladitz, Simon Marchi This patch adds some logging that helped me diagnose the problems fixed later in this series. I'm thinking that if it helped me now, it could help somebody else (or myself) in the future, so I might as well add them for real. They can happen quite frequently and be noisy, so I used dwarf_read_debug_printf_v for them, which means they'll only print if `set debug dwarf-read` is >= 2. gdb/ChangeLog: * dwarf2/read.c (follow_die_offset): Add logging. (dwarf2_per_objfile::age_comp_units): Add logging. Change-Id: I7483c0b05c37bc9710b9b5d40e272935bc010863 --- gdb/dwarf2/read.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 3c598262913f..ea7a2327d4a2 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -23429,6 +23429,12 @@ follow_die_offset (sect_offset sect_off, int offset_in_dwz, target_cu = cu; + dwarf_read_debug_printf_v ("source CU offset: %s, target offset: %s, " + "source CU contains target offset: %d", + sect_offset_str (cu->per_cu->sect_off), + sect_offset_str (sect_off), + cu->header.offset_in_cu_p (sect_off)); + if (cu->per_cu->is_debug_types) { /* .debug_types CUs cannot reference anything outside their CU. @@ -23445,6 +23451,11 @@ follow_die_offset (sect_offset sect_off, int offset_in_dwz, per_cu = dwarf2_find_containing_comp_unit (sect_off, offset_in_dwz, per_objfile); + dwarf_read_debug_printf_v ("target CU offset: %s, " + "target CU DIEs loaded: %d", + sect_offset_str (per_cu->sect_off), + per_objfile->get_cu (per_cu) != nullptr); + /* If necessary, add it to the queue and load its DIEs. */ if (maybe_queue_comp_unit (cu, per_cu, per_objfile, cu->language)) load_full_comp_unit (per_cu, per_objfile, per_objfile->get_cu (per_cu), @@ -24825,6 +24836,8 @@ dwarf2_per_objfile::set_cu (dwarf2_per_cu_data *per_cu, dwarf2_cu *cu) void dwarf2_per_objfile::age_comp_units () { + dwarf_read_debug_printf_v ("running"); + /* Start by clearing all marks. */ for (auto pair : m_dwarf2_cus) pair.second->mark = false; @@ -24847,6 +24860,8 @@ dwarf2_per_objfile::age_comp_units () if (!cu->mark) { + dwarf_read_debug_printf_v ("deleting old CU %s", + sect_offset_str (cu->per_cu->sect_off)); delete cu; it = m_dwarf2_cus.erase (it); } -- 2.29.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] gdb/dwarf: add some logging in dwarf2/read.c 2020-11-17 19:12 ` [PATCH 1/4] gdb/dwarf: add some logging in dwarf2/read.c Simon Marchi @ 2020-12-09 21:08 ` Tom Tromey 0 siblings, 0 replies; 20+ messages in thread From: Tom Tromey @ 2020-12-09 21:08 UTC (permalink / raw) To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi, nilsgladitz >>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes: Simon> This patch adds some logging that helped me diagnose the problems fixed Simon> later in this series. I'm thinking that if it helped me now, it could Simon> help somebody else (or myself) in the future, so I might as well add Simon> them for real. Simon> They can happen quite frequently and be noisy, so I used Simon> dwarf_read_debug_printf_v for them, which means they'll only print if Simon> `set debug dwarf-read` is >= 2. Simon> gdb/ChangeLog: Simon> * dwarf2/read.c (follow_die_offset): Add logging. Simon> (dwarf2_per_objfile::age_comp_units): Add logging. This looks good to me. Tom ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/4] gdb/dwarf: add assertion in maybe_queue_comp_unit 2020-11-17 19:12 [PATCH 0/4] Fix CU expansion queue-related problems Simon Marchi 2020-11-17 19:12 ` [PATCH 1/4] gdb/dwarf: add some logging in dwarf2/read.c Simon Marchi @ 2020-11-17 19:12 ` Simon Marchi 2020-12-09 21:12 ` Tom Tromey 2020-11-17 19:12 ` [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded Simon Marchi ` (2 subsequent siblings) 4 siblings, 1 reply; 20+ messages in thread From: Simon Marchi @ 2020-11-17 19:12 UTC (permalink / raw) To: gdb-patches; +Cc: nilsgladitz, Simon Marchi The symptom that leads to this is the crash described in PR 26828: /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:23478:25: runtime error: member access within null pointer of type 'struct dwarf2_cu' The line of the crash is the following, in follow_die_offset: if (target_cu != cu) target_cu->ancestor = cu; <--- HERE The line that assign nullptr to `target_cu` is the `per_objfile->get_cu` call after having called maybe_queue_comp_unit: /* If necessary, add it to the queue and load its DIEs. */ if (maybe_queue_comp_unit (cu, per_cu, per_objfile, cu->language)) load_full_comp_unit (per_cu, per_objfile, per_objfile->get_cu (per_cu), false, cu->language); target_cu = per_objfile->get_cu (per_cu); <--- HERE Some background: there is an invariant, documented in maybe_queue_comp_unit's doc, that if a CU is queued for expansion (present in dwarf2_per_bfd::queue), then its DIEs are loaded in memory. "its DIEs are loaded in memory" is a synonym for saying that a dwarf2_cu object exists for this CU. Yet another way to say it is that `per_objfile->get_cu (per_cu)` returns something not nullptr for that CU. The crash documented in PR 26828 triggers some hard-to-reproduce sequence that ends up violating the invariant: - dwarf2_fetch_die_type_sect_off gets called for a DIE in CU A - The DIE in CU A requires some DIE in CU B - follow_die_offset calls maybe_queue_comp_unit. maybe_queue_comp_unit sees CU B is not queued and its DIEs are not loaded, so it enqueues it and returns 1 to its caller - meaning "the DIEs are not loaded, you should load them" - prompting follow_die_offset to load the DIEs by calling load_full_comp_unit - Note that CU B is enqueued by maybe_queue_comp_unit even if it has already been expanded. It's a bit useless (and causes trouble, see next patch), but that's how it works right now. - Since we entered the dwarf2/read code through dwarf2_fetch_die_type_sect_off, nothing processes the queue, so we exit the dwarf2/read code with CU B still lingering in the queue. - dwarf2_fetch_die_type_sect_off gets called for a DIE in CU A, again - The DIE in CU A requires some DIE in CU B, again - This time, maybe_queue_comp_unit sees that CU B is in the queue. Because of the invariant that if a CU is in the queue, its DIEs are loaded in the memory, it returns 0 to its caller, meaning "you don't need to load the DIEs!". - That happens to be true, so everything is fine for now. - Time passes, some things call dwarf2_per_objfile::age_comp_units enough so that CU B's age becomes past the dwarf_max_cache_age threshold. age_comp_units proceeds to free CU B's DIEs. Remember that CU B is still lingering in the queue (oops, the invariant just got violated). - dwarf2_fetch_die_type_sect_off gets called for a DIE in CU A, again - The DIE in CU A requires some DIE in CU B, again - maybe_queue_comp_unit sees that CU B is in the queue, so returns to its caller "you don't need to load the DIEs!". However, we know at this point this is false. - follow_die_offset doesn't load the DIEs and tries to obtain the DIEs for CU B: target_cu = per_objfile->get_cu (per_cu); But since they are not loaded, target_cu is nullptr, and we get the crash mentioned above a few lines after that. This patch adds an assertions in maybe_queue_comp_unit to verify the invariant, to make sure it doesn't return a falsehood to its caller. The current patch doesn't fix the issue (the next patch does), but it makes it so we catch the problem earlier and get this assertion failure instead of a segmentation fault: /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:9100: internal-error: int maybe_queue_comp_unit(dwarf2_cu*, dwarf2_per_cu_data*, dwarf2_per_objfile*, language): Assertion `per_objfile->get_cu (per_cu) != nullptr' failed. gdb/ChangeLog: * dwarf2/read.c (maybe_queue_comp_unit): Add assertion. Change-Id: I4e51bd7bd58773f9fadf480179cbc4bae61508fe --- gdb/dwarf2/read.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index ea7a2327d4a2..698fdd23c1a1 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -9094,7 +9094,12 @@ maybe_queue_comp_unit (struct dwarf2_cu *dependent_cu, /* If it's already on the queue, we have nothing to do. */ if (per_cu->queued) - return 0; + { + /* Verify the invariant that if a CU is queue for expansion, its DIEs are + loaded. */ + gdb_assert (per_objfile->get_cu (per_cu) != nullptr); + return 0; + } /* If the compilation unit is already loaded, just mark it as used. */ -- 2.29.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] gdb/dwarf: add assertion in maybe_queue_comp_unit 2020-11-17 19:12 ` [PATCH 2/4] gdb/dwarf: add assertion in maybe_queue_comp_unit Simon Marchi @ 2020-12-09 21:12 ` Tom Tromey 2020-12-10 14:18 ` Simon Marchi 0 siblings, 1 reply; 20+ messages in thread From: Tom Tromey @ 2020-12-09 21:12 UTC (permalink / raw) To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi, nilsgladitz >>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes: Simon> gdb/ChangeLog: Simon> * dwarf2/read.c (maybe_queue_comp_unit): Add assertion. Thank you for tracking this down. Simon> + /* Verify the invariant that if a CU is queue for expansion, its DIEs are Probably should s/queue/queued/ here. Tom ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] gdb/dwarf: add assertion in maybe_queue_comp_unit 2020-12-09 21:12 ` Tom Tromey @ 2020-12-10 14:18 ` Simon Marchi 2021-01-21 2:18 ` Simon Marchi 0 siblings, 1 reply; 20+ messages in thread From: Simon Marchi @ 2020-12-10 14:18 UTC (permalink / raw) To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: nilsgladitz On 2020-12-09 4:12 p.m., Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes: > > Simon> gdb/ChangeLog: > > Simon> * dwarf2/read.c (maybe_queue_comp_unit): Add assertion. > > Thank you for tracking this down. > > Simon> + /* Verify the invariant that if a CU is queue for expansion, its DIEs are > > Probably should s/queue/queued/ here. Yes, fixed locally. Simon ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] gdb/dwarf: add assertion in maybe_queue_comp_unit 2020-12-10 14:18 ` Simon Marchi @ 2021-01-21 2:18 ` Simon Marchi 0 siblings, 0 replies; 20+ messages in thread From: Simon Marchi @ 2021-01-21 2:18 UTC (permalink / raw) To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: nilsgladitz On 2020-12-10 9:18 a.m., Simon Marchi via Gdb-patches wrote: > > > On 2020-12-09 4:12 p.m., Tom Tromey wrote: >>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes: >> >> Simon> gdb/ChangeLog: >> >> Simon> * dwarf2/read.c (maybe_queue_comp_unit): Add assertion. >> >> Thank you for tracking this down. >> >> Simon> + /* Verify the invariant that if a CU is queue for expansion, its DIEs are >> >> Probably should s/queue/queued/ here. > > Yes, fixed locally. > > Simon > I pushed patches 1 and 2. Simon ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded 2020-11-17 19:12 [PATCH 0/4] Fix CU expansion queue-related problems Simon Marchi 2020-11-17 19:12 ` [PATCH 1/4] gdb/dwarf: add some logging in dwarf2/read.c Simon Marchi 2020-11-17 19:12 ` [PATCH 2/4] gdb/dwarf: add assertion in maybe_queue_comp_unit Simon Marchi @ 2020-11-17 19:12 ` Simon Marchi 2020-12-09 21:24 ` Tom Tromey 2020-11-17 19:12 ` [PATCH 4/4] gdb/dwarf: create and destroy dwarf2_per_bfd's CUs-to-expand queue Simon Marchi 2020-11-23 22:17 ` [PATCH 0/4] Fix CU expansion queue-related problems Simon Marchi 4 siblings, 1 reply; 20+ messages in thread From: Simon Marchi @ 2020-11-17 19:12 UTC (permalink / raw) To: gdb-patches; +Cc: nilsgladitz, Simon Marchi 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 = per_objfile->get_cu (per_cu); if (cu != nullptr) { cu->last_used = 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. gdb/ChangeLog: * dwarf2/read.c (maybe_queue_comp_unit): Check if CU is expanded to decide whether or not to enqueue it for expansion. Change-Id: Id98c6b60669f4b4b21b9be16d0518fc62bdf686a --- gdb/dwarf2/read.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 698fdd23c1a1..51cc94f6ce00 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -9101,19 +9101,19 @@ maybe_queue_comp_unit (struct dwarf2_cu *dependent_cu, return 0; } + if (!per_objfile->symtab_set_p (per_cu)) + { + /* Add it to the queue. */ + queue_comp_unit (per_cu, per_objfile, pretend_language); + } + /* If the compilation unit is already loaded, just mark it as used. */ dwarf2_cu *cu = per_objfile->get_cu (per_cu); if (cu != nullptr) - { - cu->last_used = 0; - return 0; - } + cu->last_used = 0; - /* Add it to the queue. */ - queue_comp_unit (per_cu, per_objfile, pretend_language); - - return 1; + return cu == nullptr; } /* Process the queue. */ -- 2.29.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded 2020-11-17 19:12 ` [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded Simon Marchi @ 2020-12-09 21:24 ` Tom Tromey 2020-12-10 14:52 ` Simon Marchi 0 siblings, 1 reply; 20+ messages in thread From: Tom Tromey @ 2020-12-09 21:24 UTC (permalink / raw) To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi, nilsgladitz >>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes: Simon> * dwarf2/read.c (maybe_queue_comp_unit): Check if CU is expanded Simon> to decide whether or not to enqueue it for expansion. This looks good. Re-reading this code, I realized again that the return value of this function does not really make sense to me. The intro says: The result is non-zero if PER_CU was queued, otherwise the result is zero meaning either PER_CU is already queued or it is already loaded. ... but it seems unlikely for callees to want to detect that just this call caused the enqueueing. Tom ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded 2020-12-09 21:24 ` Tom Tromey @ 2020-12-10 14:52 ` Simon Marchi 2020-12-10 17:40 ` Simon Marchi 0 siblings, 1 reply; 20+ messages in thread From: Simon Marchi @ 2020-12-10 14:52 UTC (permalink / raw) To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: nilsgladitz On 2020-12-09 4:24 p.m., Tom Tromey wrote: > Re-reading this code, I realized again that the return value of this > function does not really make sense to me. The intro says: > > The result is non-zero if PER_CU was queued, otherwise the result is zero > meaning either PER_CU is already queued or it is already loaded. > > ... but it seems unlikely for callees to want to detect that just this > call caused the enqueueing. Ok, re-reading that comment, I think I understand things a bit differently than I did previously: /* If PER_CU is not yet queued, add it to the queue. If DEPENDENT_CU is non-NULL, it has a reference to PER_CU so add a dependency. The result is non-zero if PER_CU was queued, otherwise the result is zero meaning either PER_CU is already queued or it is already loaded. N.B. There is an invariant here that if a CU is queued then it is loaded. The caller is required to load PER_CU if we return non-zero. */ The premise is: there is the invariant that if a CU is queued for expansion, its DIEs are loaded. If maybe_queue_comp_unit enqueues a CU for expansion whose DIEs are not loaded, it returns 1 to its caller to ask "please load the DIEs for that CU because I just enqueued it and if you don't the invariant will get violated and we'll get in trouble". So if a CU is already expanded and maybe_queue_comp_unit doesn't enqueue it, it makes sense that it returns 0, because it doesn't *require* the caller to load the DIEs. However, that means that the caller shouldn't rely on maybe_queue_comp_unit's return value to determine whether the CU's DIEs are currently loaded, because: 1. whether maybe_queue_comp_unit requires it to load the CU's DIEs 2. whether the CU's DIEs are currently loaded are two different things. If the caller wants to know #2, because it itself needs to ensure the DIEs are loaded, it should not rely on maybe_queue_comp_unit's return value, but instead check itself with dwarf2_per_objfile::get_cu. I'll go over my patch and think about it a little more. Simon ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded 2020-12-10 14:52 ` Simon Marchi @ 2020-12-10 17:40 ` Simon Marchi 2021-01-21 2:15 ` Simon Marchi 0 siblings, 1 reply; 20+ messages in thread From: Simon Marchi @ 2020-12-10 17:40 UTC (permalink / raw) To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: nilsgladitz On 2020-12-10 9:52 a.m., Simon Marchi via Gdb-patches wrote: > > > On 2020-12-09 4:24 p.m., Tom Tromey wrote: >> Re-reading this code, I realized again that the return value of this >> function does not really make sense to me. The intro says: >> >> The result is non-zero if PER_CU was queued, otherwise the result is zero >> meaning either PER_CU is already queued or it is already loaded. >> >> ... but it seems unlikely for callees to want to detect that just this >> call caused the enqueueing. > > Ok, re-reading that comment, I think I understand things a bit differently > than I did previously: > > /* If PER_CU is not yet queued, add it to the queue. > If DEPENDENT_CU is non-NULL, it has a reference to PER_CU so add a > dependency. > The result is non-zero if PER_CU was queued, otherwise the result is zero > meaning either PER_CU is already queued or it is already loaded. > > N.B. There is an invariant here that if a CU is queued then it is loaded. > The caller is required to load PER_CU if we return non-zero. */ > > The premise is: there is the invariant that if a CU is queued for expansion, > its DIEs are loaded. If maybe_queue_comp_unit enqueues a CU for expansion > whose DIEs are not loaded, it returns 1 to its caller to ask "please load the > DIEs for that CU because I just enqueued it and if you don't the invariant > will get violated and we'll get in trouble". So if a CU is already expanded > and maybe_queue_comp_unit doesn't enqueue it, it makes sense that it returns > 0, because it doesn't *require* the caller to load the DIEs. > > However, that means that the caller shouldn't rely on maybe_queue_comp_unit's > return value to determine whether the CU's DIEs are currently loaded, because: > > 1. whether maybe_queue_comp_unit requires it to load the CU's DIEs > 2. whether the CU's DIEs are currently loaded > > are two different things. > > If the caller wants to know #2, because it itself needs to ensure the DIEs are > loaded, it should not rely on maybe_queue_comp_unit's return value, but > instead check itself with dwarf2_per_objfile::get_cu. > > I'll go over my patch and think about it a little more. > > Simon > Hi Tom, Following my reconsideration of what the return value of maybe_queue_comp_unit means and the original intent of the function, here's an updated patch. The differences are: - New comment on maybe_queue_comp_unit, hopefully making it clearer. - Update logic in maybe_queue_comp_unit: if the CU does not get enqueued because it is already expanded and its DIEs are not loaded, return false. Because in this case, maybe_queue_comp_unit doesn't _need_ the caller to load the DIEs. - Update callers follow_die_sig_1 and follow_die_ref to not rely on maybe_queue_comp_unit to tell them whether DIEs are loaded right now. From 70ed5a2468e2776e887c3481b89945347a856089 Mon Sep 17 00:00:00 2001 From: Simon Marchi <simon.marchi@polymtl.ca> Date: Wed, 11 Nov 2020 21:30:22 -0500 Subject: [PATCH] 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 = per_objfile->get_cu (per_cu); if (cu != nullptr) { cu->last_used = 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 --- gdb/dwarf2/read.c | 70 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 53 insertions(+), 17 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index f199229985e9..62676cd91492 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -9153,14 +9153,30 @@ queue_comp_unit (dwarf2_per_cu_data *per_cu, per_cu->per_bfd->queue.emplace (per_cu, per_objfile, pretend_language); } -/* If PER_CU is not yet queued, add it to the queue. +/* If PER_CU is not yet expanded of queued for expansion, add it to the queue. + If DEPENDENT_CU is non-NULL, it has a reference to PER_CU so add a dependency. - The result is non-zero if PER_CU was queued, otherwise the result is zero - meaning either PER_CU is already queued or it is already loaded. - N.B. There is an invariant here that if a CU is queued then it is loaded. - The caller is required to load PER_CU if we return non-zero. */ + Return true if maybe_queue_comp_unit requires the caller to load the CU's + DIEs, false otherwise. + + Explanation: there is an invariant that if a CU is queued for expansion + (present in `dwarf2_per_bfd::queue`), then its DIEs are loaded + (a dwarf2_cu object exists for this CU, and `dwarf2_per_objfile::get_cu` + returns non-nullptr). If the CU gets enqueued by this function but its DIEs + are not yet loaded, the the caller must load the CU's DIEs to ensure the + invariant is respected. + + The caller is therefore not required to load the CU's DIEs (we return false) + if: + + - the CU is already expanded, and therefore does not get enqueued + - the CU gets enqueued for expansion, but its DIEs are already loaded + + Note that the caller should not use this function's return value as an + indicator of whether the CU's DIEs are loaded right now, it should check + that by calling `dwarf2_per_objfile::get_cu` instead. */ static int maybe_queue_comp_unit (struct dwarf2_cu *dependent_cu, @@ -9191,22 +9207,32 @@ maybe_queue_comp_unit (struct dwarf2_cu *dependent_cu, /* Verify the invariant that if a CU is queued for expansion, its DIEs are loaded. */ gdb_assert (per_objfile->get_cu (per_cu) != nullptr); + + /* If the CU is queued for expansion, it should not already be + expanded. */ + gdb_assert (!per_objfile->symtab_set_p (per_cu)); + + /* The DIEs are already loaded, the caller doesn't need to do it. */ return 0; } + bool queued = false; + if (!per_objfile->symtab_set_p (per_cu)) + { + /* Add it to the queue. */ + queue_comp_unit (per_cu, per_objfile, pretend_language); + queued = true; + } + /* If the compilation unit is already loaded, just mark it as used. */ dwarf2_cu *cu = per_objfile->get_cu (per_cu); if (cu != nullptr) - { - cu->last_used = 0; - return 0; - } + cu->last_used = 0; - /* Add it to the queue. */ - queue_comp_unit (per_cu, per_objfile, pretend_language); - - return 1; + /* Ask the caller to load the CU's DIEs if the CU got enqueued for expansion + and the DIEs are not already loaded. */ + return queued && cu == nullptr; } /* Process the queue. */ @@ -23568,12 +23594,18 @@ follow_die_offset (sect_offset sect_off, int offset_in_dwz, sect_offset_str (per_cu->sect_off), per_objfile->get_cu (per_cu) != nullptr); - /* If necessary, add it to the queue and load its DIEs. */ - if (maybe_queue_comp_unit (cu, per_cu, per_objfile, cu->language)) + /* If necessary, add it to the queue and load its DIEs. + + Even if maybe_queue_comp_unit doesn't require us to load the CU's DIEs, + it doesn't mean they are currently loaded. Since we require them + to be loaded, we must check for ourselves. */ + if (maybe_queue_comp_unit (cu, per_cu, per_objfile, cu->language) + || per_objfile->get_cu (per_cu) == nullptr) load_full_comp_unit (per_cu, per_objfile, per_objfile->get_cu (per_cu), false, cu->language); target_cu = per_objfile->get_cu (per_cu); + gdb_assert (target_cu != nullptr); } else if (cu->dies == NULL) { @@ -23947,10 +23979,14 @@ follow_die_sig_1 (struct die_info *src_die, struct signatured_type *sig_type, we can get here for DW_AT_imported_declaration where we need the DIE not the type. */ - /* If necessary, add it to the queue and load its DIEs. */ + /* If necessary, add it to the queue and load its DIEs. + Even if maybe_queue_comp_unit doesn't require us to load the CU's DIEs, + it doesn't mean they are currently loaded. Since we require them + to be loaded, we must check for ourselves. */ if (maybe_queue_comp_unit (*ref_cu, &sig_type->per_cu, per_objfile, - language_minimal)) + language_minimal) + || per_objfile->get_cu (&sig_type->per_cu) == nullptr) read_signatured_type (sig_type, per_objfile); sig_cu = per_objfile->get_cu (&sig_type->per_cu); -- 2.29.2 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded 2020-12-10 17:40 ` Simon Marchi @ 2021-01-21 2:15 ` Simon Marchi 2021-02-23 6:53 ` Joel Brobecker 2021-02-24 20:32 ` Tom Tromey 0 siblings, 2 replies; 20+ messages in thread From: Simon Marchi @ 2021-01-21 2:15 UTC (permalink / raw) To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: nilsgladitz Hi Tom, Do you have an opinion on this? Simon On 2020-12-10 12:40 p.m., Simon Marchi via Gdb-patches wrote: > On 2020-12-10 9:52 a.m., Simon Marchi via Gdb-patches wrote: >> >> >> On 2020-12-09 4:24 p.m., Tom Tromey wrote: >>> Re-reading this code, I realized again that the return value of this >>> function does not really make sense to me. The intro says: >>> >>> The result is non-zero if PER_CU was queued, otherwise the result is zero >>> meaning either PER_CU is already queued or it is already loaded. >>> >>> ... but it seems unlikely for callees to want to detect that just this >>> call caused the enqueueing. >> >> Ok, re-reading that comment, I think I understand things a bit differently >> than I did previously: >> >> /* If PER_CU is not yet queued, add it to the queue. >> If DEPENDENT_CU is non-NULL, it has a reference to PER_CU so add a >> dependency. >> The result is non-zero if PER_CU was queued, otherwise the result is zero >> meaning either PER_CU is already queued or it is already loaded. >> >> N.B. There is an invariant here that if a CU is queued then it is loaded. >> The caller is required to load PER_CU if we return non-zero. */ >> >> The premise is: there is the invariant that if a CU is queued for expansion, >> its DIEs are loaded. If maybe_queue_comp_unit enqueues a CU for expansion >> whose DIEs are not loaded, it returns 1 to its caller to ask "please load the >> DIEs for that CU because I just enqueued it and if you don't the invariant >> will get violated and we'll get in trouble". So if a CU is already expanded >> and maybe_queue_comp_unit doesn't enqueue it, it makes sense that it returns >> 0, because it doesn't *require* the caller to load the DIEs. >> >> However, that means that the caller shouldn't rely on maybe_queue_comp_unit's >> return value to determine whether the CU's DIEs are currently loaded, because: >> >> 1. whether maybe_queue_comp_unit requires it to load the CU's DIEs >> 2. whether the CU's DIEs are currently loaded >> >> are two different things. >> >> If the caller wants to know #2, because it itself needs to ensure the DIEs are >> loaded, it should not rely on maybe_queue_comp_unit's return value, but >> instead check itself with dwarf2_per_objfile::get_cu. >> >> I'll go over my patch and think about it a little more. >> >> Simon >> > > Hi Tom, > > Following my reconsideration of what the return value of maybe_queue_comp_unit > means and the original intent of the function, here's an updated patch. > > The differences are: > > - New comment on maybe_queue_comp_unit, hopefully making it clearer. > - Update logic in maybe_queue_comp_unit: if the CU does not get enqueued because > it is already expanded and its DIEs are not loaded, return false. Because in > this case, maybe_queue_comp_unit doesn't _need_ the caller to load the DIEs. > - Update callers follow_die_sig_1 and follow_die_ref to not rely on > maybe_queue_comp_unit to tell them whether DIEs are loaded right now. > > > From 70ed5a2468e2776e887c3481b89945347a856089 Mon Sep 17 00:00:00 2001 > From: Simon Marchi <simon.marchi@polymtl.ca> > Date: Wed, 11 Nov 2020 21:30:22 -0500 > Subject: [PATCH] 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 = per_objfile->get_cu (per_cu); > if (cu != nullptr) > { > cu->last_used = 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 > --- > gdb/dwarf2/read.c | 70 +++++++++++++++++++++++++++++++++++------------ > 1 file changed, 53 insertions(+), 17 deletions(-) > > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > index f199229985e9..62676cd91492 100644 > --- a/gdb/dwarf2/read.c > +++ b/gdb/dwarf2/read.c > @@ -9153,14 +9153,30 @@ queue_comp_unit (dwarf2_per_cu_data *per_cu, > per_cu->per_bfd->queue.emplace (per_cu, per_objfile, pretend_language); > } > > -/* If PER_CU is not yet queued, add it to the queue. > +/* If PER_CU is not yet expanded of queued for expansion, add it to the queue. > + > If DEPENDENT_CU is non-NULL, it has a reference to PER_CU so add a > dependency. > - The result is non-zero if PER_CU was queued, otherwise the result is zero > - meaning either PER_CU is already queued or it is already loaded. > > - N.B. There is an invariant here that if a CU is queued then it is loaded. > - The caller is required to load PER_CU if we return non-zero. */ > + Return true if maybe_queue_comp_unit requires the caller to load the CU's > + DIEs, false otherwise. > + > + Explanation: there is an invariant that if a CU is queued for expansion > + (present in `dwarf2_per_bfd::queue`), then its DIEs are loaded > + (a dwarf2_cu object exists for this CU, and `dwarf2_per_objfile::get_cu` > + returns non-nullptr). If the CU gets enqueued by this function but its DIEs > + are not yet loaded, the the caller must load the CU's DIEs to ensure the > + invariant is respected. > + > + The caller is therefore not required to load the CU's DIEs (we return false) > + if: > + > + - the CU is already expanded, and therefore does not get enqueued > + - the CU gets enqueued for expansion, but its DIEs are already loaded > + > + Note that the caller should not use this function's return value as an > + indicator of whether the CU's DIEs are loaded right now, it should check > + that by calling `dwarf2_per_objfile::get_cu` instead. */ > > static int > maybe_queue_comp_unit (struct dwarf2_cu *dependent_cu, > @@ -9191,22 +9207,32 @@ maybe_queue_comp_unit (struct dwarf2_cu *dependent_cu, > /* Verify the invariant that if a CU is queued for expansion, its DIEs are > loaded. */ > gdb_assert (per_objfile->get_cu (per_cu) != nullptr); > + > + /* If the CU is queued for expansion, it should not already be > + expanded. */ > + gdb_assert (!per_objfile->symtab_set_p (per_cu)); > + > + /* The DIEs are already loaded, the caller doesn't need to do it. */ > return 0; > } > > + bool queued = false; > + if (!per_objfile->symtab_set_p (per_cu)) > + { > + /* Add it to the queue. */ > + queue_comp_unit (per_cu, per_objfile, pretend_language); > + queued = true; > + } > + > /* If the compilation unit is already loaded, just mark it as > used. */ > dwarf2_cu *cu = per_objfile->get_cu (per_cu); > if (cu != nullptr) > - { > - cu->last_used = 0; > - return 0; > - } > + cu->last_used = 0; > > - /* Add it to the queue. */ > - queue_comp_unit (per_cu, per_objfile, pretend_language); > - > - return 1; > + /* Ask the caller to load the CU's DIEs if the CU got enqueued for expansion > + and the DIEs are not already loaded. */ > + return queued && cu == nullptr; > } > > /* Process the queue. */ > @@ -23568,12 +23594,18 @@ follow_die_offset (sect_offset sect_off, int offset_in_dwz, > sect_offset_str (per_cu->sect_off), > per_objfile->get_cu (per_cu) != nullptr); > > - /* If necessary, add it to the queue and load its DIEs. */ > - if (maybe_queue_comp_unit (cu, per_cu, per_objfile, cu->language)) > + /* If necessary, add it to the queue and load its DIEs. > + > + Even if maybe_queue_comp_unit doesn't require us to load the CU's DIEs, > + it doesn't mean they are currently loaded. Since we require them > + to be loaded, we must check for ourselves. */ > + if (maybe_queue_comp_unit (cu, per_cu, per_objfile, cu->language) > + || per_objfile->get_cu (per_cu) == nullptr) > load_full_comp_unit (per_cu, per_objfile, per_objfile->get_cu (per_cu), > false, cu->language); > > target_cu = per_objfile->get_cu (per_cu); > + gdb_assert (target_cu != nullptr); > } > else if (cu->dies == NULL) > { > @@ -23947,10 +23979,14 @@ follow_die_sig_1 (struct die_info *src_die, struct signatured_type *sig_type, > we can get here for DW_AT_imported_declaration where we need > the DIE not the type. */ > > - /* If necessary, add it to the queue and load its DIEs. */ > + /* If necessary, add it to the queue and load its DIEs. > > + Even if maybe_queue_comp_unit doesn't require us to load the CU's DIEs, > + it doesn't mean they are currently loaded. Since we require them > + to be loaded, we must check for ourselves. */ > if (maybe_queue_comp_unit (*ref_cu, &sig_type->per_cu, per_objfile, > - language_minimal)) > + language_minimal) > + || per_objfile->get_cu (&sig_type->per_cu) == nullptr) > read_signatured_type (sig_type, per_objfile); > > sig_cu = per_objfile->get_cu (&sig_type->per_cu); > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded 2021-01-21 2:15 ` Simon Marchi @ 2021-02-23 6:53 ` Joel Brobecker 2021-02-23 18:39 ` Simon Marchi 2021-02-24 20:32 ` Tom Tromey 1 sibling, 1 reply; 20+ messages in thread From: Joel Brobecker @ 2021-02-23 6:53 UTC (permalink / raw) To: Simon Marchi via Gdb-patches; +Cc: Tom Tromey, nilsgladitz Hi Simon, On Wed, Jan 20, 2021 at 09:15:15PM -0500, Simon Marchi via Gdb-patches wrote: > Do you have an opinion on this? It's been two months since the patch has been waiting for feedback, with a ping a month ago. At this point, I think we should just trust your judgement on this one, and go ahead, especially if the person who reported the issue confirm his issue was fixed. WDYT? > > On 2020-12-10 12:40 p.m., Simon Marchi via Gdb-patches wrote: > > On 2020-12-10 9:52 a.m., Simon Marchi via Gdb-patches wrote: > >> > >> > >> On 2020-12-09 4:24 p.m., Tom Tromey wrote: > >>> Re-reading this code, I realized again that the return value of this > >>> function does not really make sense to me. The intro says: > >>> > >>> The result is non-zero if PER_CU was queued, otherwise the result is zero > >>> meaning either PER_CU is already queued or it is already loaded. > >>> > >>> ... but it seems unlikely for callees to want to detect that just this > >>> call caused the enqueueing. > >> > >> Ok, re-reading that comment, I think I understand things a bit differently > >> than I did previously: > >> > >> /* If PER_CU is not yet queued, add it to the queue. > >> If DEPENDENT_CU is non-NULL, it has a reference to PER_CU so add a > >> dependency. > >> The result is non-zero if PER_CU was queued, otherwise the result is zero > >> meaning either PER_CU is already queued or it is already loaded. > >> > >> N.B. There is an invariant here that if a CU is queued then it is loaded. > >> The caller is required to load PER_CU if we return non-zero. */ > >> > >> The premise is: there is the invariant that if a CU is queued for expansion, > >> its DIEs are loaded. If maybe_queue_comp_unit enqueues a CU for expansion > >> whose DIEs are not loaded, it returns 1 to its caller to ask "please load the > >> DIEs for that CU because I just enqueued it and if you don't the invariant > >> will get violated and we'll get in trouble". So if a CU is already expanded > >> and maybe_queue_comp_unit doesn't enqueue it, it makes sense that it returns > >> 0, because it doesn't *require* the caller to load the DIEs. > >> > >> However, that means that the caller shouldn't rely on maybe_queue_comp_unit's > >> return value to determine whether the CU's DIEs are currently loaded, because: > >> > >> 1. whether maybe_queue_comp_unit requires it to load the CU's DIEs > >> 2. whether the CU's DIEs are currently loaded > >> > >> are two different things. > >> > >> If the caller wants to know #2, because it itself needs to ensure the DIEs are > >> loaded, it should not rely on maybe_queue_comp_unit's return value, but > >> instead check itself with dwarf2_per_objfile::get_cu. > >> > >> I'll go over my patch and think about it a little more. > >> > >> Simon > >> > > > > Hi Tom, > > > > Following my reconsideration of what the return value of maybe_queue_comp_unit > > means and the original intent of the function, here's an updated patch. > > > > The differences are: > > > > - New comment on maybe_queue_comp_unit, hopefully making it clearer. > > - Update logic in maybe_queue_comp_unit: if the CU does not get enqueued because > > it is already expanded and its DIEs are not loaded, return false. Because in > > this case, maybe_queue_comp_unit doesn't _need_ the caller to load the DIEs. > > - Update callers follow_die_sig_1 and follow_die_ref to not rely on > > maybe_queue_comp_unit to tell them whether DIEs are loaded right now. > > > > > > From 70ed5a2468e2776e887c3481b89945347a856089 Mon Sep 17 00:00:00 2001 > > From: Simon Marchi <simon.marchi@polymtl.ca> > > Date: Wed, 11 Nov 2020 21:30:22 -0500 > > Subject: [PATCH] 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 = per_objfile->get_cu (per_cu); > > if (cu != nullptr) > > { > > cu->last_used = 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 > > --- > > gdb/dwarf2/read.c | 70 +++++++++++++++++++++++++++++++++++------------ > > 1 file changed, 53 insertions(+), 17 deletions(-) > > > > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c > > index f199229985e9..62676cd91492 100644 > > --- a/gdb/dwarf2/read.c > > +++ b/gdb/dwarf2/read.c > > @@ -9153,14 +9153,30 @@ queue_comp_unit (dwarf2_per_cu_data *per_cu, > > per_cu->per_bfd->queue.emplace (per_cu, per_objfile, pretend_language); > > } > > > > -/* If PER_CU is not yet queued, add it to the queue. > > +/* If PER_CU is not yet expanded of queued for expansion, add it to the queue. > > + > > If DEPENDENT_CU is non-NULL, it has a reference to PER_CU so add a > > dependency. > > - The result is non-zero if PER_CU was queued, otherwise the result is zero > > - meaning either PER_CU is already queued or it is already loaded. > > > > - N.B. There is an invariant here that if a CU is queued then it is loaded. > > - The caller is required to load PER_CU if we return non-zero. */ > > + Return true if maybe_queue_comp_unit requires the caller to load the CU's > > + DIEs, false otherwise. > > + > > + Explanation: there is an invariant that if a CU is queued for expansion > > + (present in `dwarf2_per_bfd::queue`), then its DIEs are loaded > > + (a dwarf2_cu object exists for this CU, and `dwarf2_per_objfile::get_cu` > > + returns non-nullptr). If the CU gets enqueued by this function but its DIEs > > + are not yet loaded, the the caller must load the CU's DIEs to ensure the > > + invariant is respected. > > + > > + The caller is therefore not required to load the CU's DIEs (we return false) > > + if: > > + > > + - the CU is already expanded, and therefore does not get enqueued > > + - the CU gets enqueued for expansion, but its DIEs are already loaded > > + > > + Note that the caller should not use this function's return value as an > > + indicator of whether the CU's DIEs are loaded right now, it should check > > + that by calling `dwarf2_per_objfile::get_cu` instead. */ > > > > static int > > maybe_queue_comp_unit (struct dwarf2_cu *dependent_cu, > > @@ -9191,22 +9207,32 @@ maybe_queue_comp_unit (struct dwarf2_cu *dependent_cu, > > /* Verify the invariant that if a CU is queued for expansion, its DIEs are > > loaded. */ > > gdb_assert (per_objfile->get_cu (per_cu) != nullptr); > > + > > + /* If the CU is queued for expansion, it should not already be > > + expanded. */ > > + gdb_assert (!per_objfile->symtab_set_p (per_cu)); > > + > > + /* The DIEs are already loaded, the caller doesn't need to do it. */ > > return 0; > > } > > > > + bool queued = false; > > + if (!per_objfile->symtab_set_p (per_cu)) > > + { > > + /* Add it to the queue. */ > > + queue_comp_unit (per_cu, per_objfile, pretend_language); > > + queued = true; > > + } > > + > > /* If the compilation unit is already loaded, just mark it as > > used. */ > > dwarf2_cu *cu = per_objfile->get_cu (per_cu); > > if (cu != nullptr) > > - { > > - cu->last_used = 0; > > - return 0; > > - } > > + cu->last_used = 0; > > > > - /* Add it to the queue. */ > > - queue_comp_unit (per_cu, per_objfile, pretend_language); > > - > > - return 1; > > + /* Ask the caller to load the CU's DIEs if the CU got enqueued for expansion > > + and the DIEs are not already loaded. */ > > + return queued && cu == nullptr; > > } > > > > /* Process the queue. */ > > @@ -23568,12 +23594,18 @@ follow_die_offset (sect_offset sect_off, int offset_in_dwz, > > sect_offset_str (per_cu->sect_off), > > per_objfile->get_cu (per_cu) != nullptr); > > > > - /* If necessary, add it to the queue and load its DIEs. */ > > - if (maybe_queue_comp_unit (cu, per_cu, per_objfile, cu->language)) > > + /* If necessary, add it to the queue and load its DIEs. > > + > > + Even if maybe_queue_comp_unit doesn't require us to load the CU's DIEs, > > + it doesn't mean they are currently loaded. Since we require them > > + to be loaded, we must check for ourselves. */ > > + if (maybe_queue_comp_unit (cu, per_cu, per_objfile, cu->language) > > + || per_objfile->get_cu (per_cu) == nullptr) > > load_full_comp_unit (per_cu, per_objfile, per_objfile->get_cu (per_cu), > > false, cu->language); > > > > target_cu = per_objfile->get_cu (per_cu); > > + gdb_assert (target_cu != nullptr); > > } > > else if (cu->dies == NULL) > > { > > @@ -23947,10 +23979,14 @@ follow_die_sig_1 (struct die_info *src_die, struct signatured_type *sig_type, > > we can get here for DW_AT_imported_declaration where we need > > the DIE not the type. */ > > > > - /* If necessary, add it to the queue and load its DIEs. */ > > + /* If necessary, add it to the queue and load its DIEs. > > > > + Even if maybe_queue_comp_unit doesn't require us to load the CU's DIEs, > > + it doesn't mean they are currently loaded. Since we require them > > + to be loaded, we must check for ourselves. */ > > if (maybe_queue_comp_unit (*ref_cu, &sig_type->per_cu, per_objfile, > > - language_minimal)) > > + language_minimal) > > + || per_objfile->get_cu (&sig_type->per_cu) == nullptr) > > read_signatured_type (sig_type, per_objfile); > > > > sig_cu = per_objfile->get_cu (&sig_type->per_cu); > > -- Joel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded 2021-02-23 6:53 ` Joel Brobecker @ 2021-02-23 18:39 ` Simon Marchi 2021-02-23 23:32 ` Simon Marchi 0 siblings, 1 reply; 20+ messages in thread From: Simon Marchi @ 2021-02-23 18:39 UTC (permalink / raw) To: Joel Brobecker, Simon Marchi via Gdb-patches; +Cc: Tom Tromey, nilsgladitz On 2021-02-23 1:53 a.m., Joel Brobecker wrote: > Hi Simon, > > On Wed, Jan 20, 2021 at 09:15:15PM -0500, Simon Marchi via Gdb-patches wrote: >> Do you have an opinion on this? > > It's been two months since the patch has been waiting for feedback, > with a ping a month ago. At this point, I think we should just trust > your judgement on this one, and go ahead, especially if the person > who reported the issue confirm his issue was fixed. > > WDYT? Ok, I'll push them to master, and push them to gdb-10-branch after doing a bit of testing. Simon ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded 2021-02-23 18:39 ` Simon Marchi @ 2021-02-23 23:32 ` Simon Marchi 2021-02-24 7:30 ` Joel Brobecker 0 siblings, 1 reply; 20+ messages in thread From: Simon Marchi @ 2021-02-23 23:32 UTC (permalink / raw) To: Joel Brobecker, Simon Marchi via Gdb-patches; +Cc: Tom Tromey, nilsgladitz On 2021-02-23 1:39 p.m., Simon Marchi via Gdb-patches wrote: > On 2021-02-23 1:53 a.m., Joel Brobecker wrote: >> Hi Simon, >> >> On Wed, Jan 20, 2021 at 09:15:15PM -0500, Simon Marchi via Gdb-patches wrote: >>> Do you have an opinion on this? >> >> It's been two months since the patch has been waiting for feedback, >> with a ping a month ago. At this point, I think we should just trust >> your judgement on this one, and go ahead, especially if the person >> who reported the issue confirm his issue was fixed. >> >> WDYT? > > Ok, I'll push them to master, and push them to gdb-10-branch after doing a bit of testing. > > Simon > Ok, this is now pushed to both branches. Simon ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded 2021-02-23 23:32 ` Simon Marchi @ 2021-02-24 7:30 ` Joel Brobecker 0 siblings, 0 replies; 20+ messages in thread From: Joel Brobecker @ 2021-02-24 7:30 UTC (permalink / raw) To: Simon Marchi; +Cc: Simon Marchi via Gdb-patches, Tom Tromey, nilsgladitz > >> It's been two months since the patch has been waiting for feedback, > >> with a ping a month ago. At this point, I think we should just trust > >> your judgement on this one, and go ahead, especially if the person > >> who reported the issue confirm his issue was fixed. > >> > >> WDYT? > > > > Ok, I'll push them to master, and push them to gdb-10-branch after doing a bit of testing. > > > > Simon > > Ok, this is now pushed to both branches. Awesome. Thanks Simon. -- Joel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded 2021-01-21 2:15 ` Simon Marchi 2021-02-23 6:53 ` Joel Brobecker @ 2021-02-24 20:32 ` Tom Tromey 1 sibling, 0 replies; 20+ messages in thread From: Tom Tromey @ 2021-02-24 20:32 UTC (permalink / raw) To: Simon Marchi via Gdb-patches; +Cc: Tom Tromey, Simon Marchi, nilsgladitz >>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes: Simon> Hi Tom, Simon> Do you have an opinion on this? I'm sorry I dropped this one. It does look good to me. Tom ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/4] gdb/dwarf: create and destroy dwarf2_per_bfd's CUs-to-expand queue 2020-11-17 19:12 [PATCH 0/4] Fix CU expansion queue-related problems Simon Marchi ` (2 preceding siblings ...) 2020-11-17 19:12 ` [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded Simon Marchi @ 2020-11-17 19:12 ` Simon Marchi 2020-12-09 21:27 ` Tom Tromey 2020-11-23 22:17 ` [PATCH 0/4] Fix CU expansion queue-related problems Simon Marchi 4 siblings, 1 reply; 20+ messages in thread From: Simon Marchi @ 2020-11-17 19:12 UTC (permalink / raw) To: gdb-patches; +Cc: nilsgladitz, Simon Marchi As described in the log of patch "gdb/dwarf: add assertion in maybe_queue_comp_unit", it would happen that a call to maybe_queue_comp_unit would enqueue a CU in the to-expand queue while nothing up the stack was processing the queue. This is not desirable, as items are then left lingering in the queue when we exit the dwarf2/read code. This is an inconsistent state. The normal case of using the queue is when we go through dw2_do_instantiate_symtab and process_queue. As depended-on CUs are found, they get added to the queue. process_queue expands CUs until the queue is empty. To catch these cases where things are enqueued while nothing up the stack is processing the queue, change dwarf2_per_bfd::queue to be an optional. The optional is instantiated in dwarf2_queue_guard, just before where we call process_queue. In the dwarf2_queue_guard destructor, the optional gets reset. Therefore, the queue object is instantiated only when something up the stack is handling it. If another entry point tries to enqueue a CU for expansion, an assertion will fail and we know we have something to fix. dwarf2_queue_guard sounds like the good place for this, as it's currently responsible for making sure the queue gets cleared if we exit due to an error. This also allows asserting that when age_comp_units or remove_all_cus run, the queue is not instantiated, and gives us one more level of assurance that we won't free the DIEs of a CU that is in the CUs-to-expand queue. gdb/ChangeLog: * dwarf2/read.c (dwarf2_queue_guard) <dwarf2_queue_guard>: Instantiate queue. (~dwarf2_queue_guard): Clear queue. (queue_comp_unit): Assert that queue is instantiated. (process_queue): Adjust. * dwarf2/read.h (struct dwarf2_per_bfd) <queue>: Make optional. Change-Id: I8fe3d77845bb4ad3d309eac906acebe79d9f0a9d --- gdb/dwarf2/read.c | 74 ++++++++++++++++++++++++++++------------------- gdb/dwarf2/read.h | 2 +- 2 files changed, 46 insertions(+), 30 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 51cc94f6ce00..bf3429c8d147 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -1667,15 +1667,18 @@ class dwarf2_queue_guard explicit dwarf2_queue_guard (dwarf2_per_objfile *per_objfile) : m_per_objfile (per_objfile) { + gdb_assert (!m_per_objfile->per_bfd->queue.has_value ()); + + m_per_objfile->per_bfd->queue.emplace (); } /* Free any entries remaining on the queue. There should only be entries left if we hit an error while processing the dwarf. */ ~dwarf2_queue_guard () { - /* Ensure that no memory is allocated by the queue. */ - std::queue<dwarf2_queue_item> empty; - std::swap (m_per_objfile->per_bfd->queue, empty); + gdb_assert (m_per_objfile->per_bfd->queue.has_value ()); + + m_per_objfile->per_bfd->queue.reset (); } DISABLE_COPY_AND_ASSIGN (dwarf2_queue_guard); @@ -1844,6 +1847,8 @@ dwarf2_per_bfd::~dwarf2_per_bfd () void dwarf2_per_objfile::remove_all_cus () { + gdb_assert (!this->per_bfd->queue.has_value ()); + for (auto pair : m_dwarf2_cus) delete pair.second; @@ -2432,30 +2437,32 @@ dw2_do_instantiate_symtab (dwarf2_per_cu_data *per_cu, if (per_cu->type_unit_group_p ()) return; - /* The destructor of dwarf2_queue_guard frees any entries left on - the queue. After this point we're guaranteed to leave this function - with the dwarf queue empty. */ - dwarf2_queue_guard q_guard (per_objfile); - - if (!per_objfile->symtab_set_p (per_cu)) - { - queue_comp_unit (per_cu, per_objfile, language_minimal); - dwarf2_cu *cu = load_cu (per_cu, per_objfile, skip_partial); + { + /* The destructor of dwarf2_queue_guard frees any entries left on + the queue. After this point we're guaranteed to leave this function + with the dwarf queue empty. */ + dwarf2_queue_guard q_guard (per_objfile); - /* If we just loaded a CU from a DWO, and we're working with an index - that may badly handle TUs, load all the TUs in that DWO as well. - http://sourceware.org/bugzilla/show_bug.cgi?id=15021 */ - if (!per_cu->is_debug_types - && cu != NULL - && cu->dwo_unit != NULL - && per_objfile->per_bfd->index_table != NULL - && per_objfile->per_bfd->index_table->version <= 7 - /* DWP files aren't supported yet. */ - && get_dwp_file (per_objfile) == NULL) - queue_and_load_all_dwo_tus (cu); - } + if (!per_objfile->symtab_set_p (per_cu)) + { + queue_comp_unit (per_cu, per_objfile, language_minimal); + dwarf2_cu *cu = load_cu (per_cu, per_objfile, skip_partial); + + /* If we just loaded a CU from a DWO, and we're working with an index + that may badly handle TUs, load all the TUs in that DWO as well. + http://sourceware.org/bugzilla/show_bug.cgi?id=15021 */ + if (!per_cu->is_debug_types + && cu != NULL + && cu->dwo_unit != NULL + && per_objfile->per_bfd->index_table != NULL + && per_objfile->per_bfd->index_table->version <= 7 + /* DWP files aren't supported yet. */ + && get_dwp_file (per_objfile) == NULL) + queue_and_load_all_dwo_tus (cu); + } - process_queue (per_objfile); + process_queue (per_objfile); + } /* Age the cache, releasing compilation units that have not been used recently. */ @@ -9057,7 +9064,9 @@ queue_comp_unit (dwarf2_per_cu_data *per_cu, enum language pretend_language) { per_cu->queued = 1; - per_cu->per_bfd->queue.emplace (per_cu, per_objfile, pretend_language); + + gdb_assert (per_objfile->per_bfd->queue.has_value ()); + per_cu->per_bfd->queue->emplace (per_cu, per_objfile, pretend_language); } /* If PER_CU is not yet queued, add it to the queue. @@ -9126,9 +9135,9 @@ process_queue (dwarf2_per_objfile *per_objfile) /* The queue starts out with one item, but following a DIE reference may load a new CU, adding it to the end of the queue. */ - while (!per_objfile->per_bfd->queue.empty ()) + while (!per_objfile->per_bfd->queue->empty ()) { - dwarf2_queue_item &item = per_objfile->per_bfd->queue.front (); + dwarf2_queue_item &item = per_objfile->per_bfd->queue->front (); dwarf2_per_cu_data *per_cu = item.per_cu; if (!per_objfile->symtab_set_p (per_cu)) @@ -9174,7 +9183,7 @@ process_queue (dwarf2_per_objfile *per_objfile) } per_cu->queued = 0; - per_objfile->per_bfd->queue.pop (); + per_objfile->per_bfd->queue->pop (); } dwarf_read_debug_printf ("Done expanding symtabs of %s.", @@ -24843,6 +24852,13 @@ dwarf2_per_objfile::age_comp_units () { dwarf_read_debug_printf_v ("running"); + /* This is not expected to be called in the middle of CU expansion. There is + an invariant that if a CU is in the CUs-to-expand queue, its DIEs are + loaded in memory. Calling age_comp_units while the queue is in use could + make us free the DIEs for a CU that is in the queue and therefore break + that invariant. */ + gdb_assert (!this->per_bfd->queue.has_value ()); + /* Start by clearing all marks. */ for (auto pair : m_dwarf2_cus) pair.second->mark = false; diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h index a0d76f349e82..290d577ee38b 100644 --- a/gdb/dwarf2/read.h +++ b/gdb/dwarf2/read.h @@ -250,7 +250,7 @@ struct dwarf2_per_bfd abstract_to_concrete; /* CUs that are queued to be read. */ - std::queue<dwarf2_queue_item> queue; + gdb::optional<std::queue<dwarf2_queue_item>> queue; /* We keep a separate reference to the partial symtabs, in case we are sharing them between objfiles. This is only set after -- 2.29.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] gdb/dwarf: create and destroy dwarf2_per_bfd's CUs-to-expand queue 2020-11-17 19:12 ` [PATCH 4/4] gdb/dwarf: create and destroy dwarf2_per_bfd's CUs-to-expand queue Simon Marchi @ 2020-12-09 21:27 ` Tom Tromey 0 siblings, 0 replies; 20+ messages in thread From: Tom Tromey @ 2020-12-09 21:27 UTC (permalink / raw) To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi, nilsgladitz >>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes: Simon> gdb/ChangeLog: Simon> * dwarf2/read.c (dwarf2_queue_guard) <dwarf2_queue_guard>: Simon> Instantiate queue. Simon> (~dwarf2_queue_guard): Clear queue. Simon> (queue_comp_unit): Assert that queue is Simon> instantiated. Simon> (process_queue): Adjust. Simon> * dwarf2/read.h (struct dwarf2_per_bfd) <queue>: Make optional. Thanks for doing this. It seems like a good idea to me. Tom ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/4] Fix CU expansion queue-related problems 2020-11-17 19:12 [PATCH 0/4] Fix CU expansion queue-related problems Simon Marchi ` (3 preceding siblings ...) 2020-11-17 19:12 ` [PATCH 4/4] gdb/dwarf: create and destroy dwarf2_per_bfd's CUs-to-expand queue Simon Marchi @ 2020-11-23 22:17 ` Simon Marchi 4 siblings, 0 replies; 20+ messages in thread From: Simon Marchi @ 2020-11-23 22:17 UTC (permalink / raw) To: gdb-patches; +Cc: nilsgladitz On 2020-11-17 2:12 p.m., Simon Marchi wrote: > This patch series fixes problems explained in PR 26828 [1]. The > problems are related to CU DIEs loading/freeing and the CU expansion > queue. All the gory details are in the commit logs. > > I tried to write a test case for this using the DWARF assembler but I > couldn't manage to do it. I decided to submit the series anyway, > because I'd still like to fix the issue. I might try again later, but I > might also not have the time, so if anybody would like to give it a try, > you are welcome. > > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=26828 I forgot to mention: this fixes a regression of GDB 10.1, so I would like to consider this for the GDB 10 stable branch. Simon ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-02-24 20:32 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-17 19:12 [PATCH 0/4] Fix CU expansion queue-related problems Simon Marchi 2020-11-17 19:12 ` [PATCH 1/4] gdb/dwarf: add some logging in dwarf2/read.c Simon Marchi 2020-12-09 21:08 ` Tom Tromey 2020-11-17 19:12 ` [PATCH 2/4] gdb/dwarf: add assertion in maybe_queue_comp_unit Simon Marchi 2020-12-09 21:12 ` Tom Tromey 2020-12-10 14:18 ` Simon Marchi 2021-01-21 2:18 ` Simon Marchi 2020-11-17 19:12 ` [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded Simon Marchi 2020-12-09 21:24 ` Tom Tromey 2020-12-10 14:52 ` Simon Marchi 2020-12-10 17:40 ` Simon Marchi 2021-01-21 2:15 ` Simon Marchi 2021-02-23 6:53 ` Joel Brobecker 2021-02-23 18:39 ` Simon Marchi 2021-02-23 23:32 ` Simon Marchi 2021-02-24 7:30 ` Joel Brobecker 2021-02-24 20:32 ` Tom Tromey 2020-11-17 19:12 ` [PATCH 4/4] gdb/dwarf: create and destroy dwarf2_per_bfd's CUs-to-expand queue Simon Marchi 2020-12-09 21:27 ` Tom Tromey 2020-11-23 22:17 ` [PATCH 0/4] Fix CU expansion queue-related problems Simon Marchi
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).