[PATCH 0/7] Remove addrmap from blockvector

[PATCH 0/7] Remove addrmap from blockvector

[Jan Vrany]

This series removes the addrmap from blockvector, a step towards
expandable blockvectors which are needed for lazy CU expansion
and for Python JIT API.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33829

Jan Vrany (7):
  gdb: implement readnow_functions::find_pc_sect_compunit_symtab
  gdb: update expanded_symbols_functions::find_pc_sect_compunit_symtab
  gdb: simplify find_compunit_symtab_for_pc_sect
  gdb: do not set blockvector address map
  gdb: update blockvector::lookup to handle non-contiguous blocks
  gdb: remove address map from struct blockvector
  gdb: add unit test for blockvector::lookup of non-contiguous blocks

 gdb/block-selftests.c | 158 +++++++++++++++++++++++++-----------------
 gdb/block.c           |  34 ++++++---
 gdb/block.h           |  17 ++---
 gdb/buildsym.c        |  55 +--------------
 gdb/dwarf2/read.c     |  39 ++++++++---
 gdb/expanded-symbol.c |  18 +++++
 gdb/expanded-symbol.h |   8 +--
 gdb/symtab.c          |  91 ------------------------
 8 files changed, 174 insertions(+), 246 deletions(-)

-- 
2.51.0

[PATCH 1/7] gdb: implement readnow_functions::find_pc_sect_compunit_symtab

[Jan Vrany]

This commit implements readnow_functions::find_pc_sect_compunit_symtab.
As comment in read.h states, index_table member dwarf2_per_bfd is null
when -readnow is used and thus this method would always return null
when using -readnow.

This issue did not manifest until now because:

 1) readnow_functions::find_pc_sect_compunit_symtab is called from
    find_compunit_symtab_for_pc_sect *after* all CUs expanded so far
    are searched and,
 2) it happens that required CUs were expanded by other "quick function"
    called prior find_pc_sect_compunit_symtab.

This is a preparation for simplifying find_compunit_symtab_for_pc_sect
by removing the code that walks all existing CUs.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33829
---
 gdb/dwarf2/read.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index efe0b046f15..702dca8e5e9 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1525,6 +1525,10 @@ struct readnow_functions : public dwarf2_base_index_functions
     return true;
   }
 
+  struct compunit_symtab *find_pc_sect_compunit_symtab (
+    struct objfile *objfile, bound_minimal_symbol msymbol, CORE_ADDR pc,
+    struct obj_section *section, int warn_if_readin) override;
+
   struct symbol *find_symbol_by_address (struct objfile *objfile,
 					 CORE_ADDR address) override
   {
@@ -2166,6 +2170,33 @@ dwarf2_base_index_functions::find_pc_sect_compunit_symtab
   return result;
 }
 
+struct compunit_symtab *
+readnow_functions::find_pc_sect_compunit_symtab
+     (struct objfile *objfile,
+      bound_minimal_symbol msymbol,
+      CORE_ADDR pc,
+      struct obj_section *section,
+      int warn_if_readin)
+{
+  dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
+  dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
+
+  /* This invariant is documented in read.h  */
+  gdb_assert (per_bfd->index_table == nullptr);
+
+  /* Since we have no index, we simply walk all units until matching CU is
+     found (of there are no more CUs).  */
+  for (int i = 0; i < per_bfd->all_units.size (); i++)
+    {
+      dwarf2_per_cu *data = per_bfd->all_units[i].get ();
+      compunit_symtab *result = find_pc_sect_compunit_symtab_includes (
+	dw2_instantiate_symtab (data, per_objfile, false), pc);
+      if (result != nullptr)
+	return result;
+    }
+  return nullptr;
+}
+
 void
 dwarf2_base_index_functions::map_symbol_filenames (objfile *objfile,
 						   symbol_filename_listener fun,
-- 
2.51.0

[PATCH 2/7] gdb: update expanded_symbols_functions::find_pc_sect_compunit_symtab

[Jan Vrany]

This commit updates expanded_symbols_functions::find_pc_sect_compunit_symtab
to search all compunits rather than just returning null.

The original implementation (just return null) was based on reasoning
that its would suffice since find_compunit_symtab_for_pc_sect walks
all CUs anyway [1]. This commit is a preparation to simplify
find_compunit_symtab_for_pc_sect by removing that code.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33829

[1] https://inbox.sourceware.org/gdb-patches/874iqoogoa.fsf@tromey.com/
---
 gdb/expanded-symbol.c | 18 ++++++++++++++++++
 gdb/expanded-symbol.h |  8 +-------
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/gdb/expanded-symbol.c b/gdb/expanded-symbol.c
index 050608795f9..3c7f9019beb 100644
--- a/gdb/expanded-symbol.c
+++ b/gdb/expanded-symbol.c
@@ -18,6 +18,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#include "block.h"
 #include "objfiles.h"
 #include "symtab.h"
 #include "source.h"
@@ -113,3 +114,20 @@ expanded_symbols_functions::find_symbol_by_address (objfile *objfile,
 
   return nullptr;
 }
+
+/* See expanded-symbol.h.  */
+
+compunit_symtab *
+expanded_symbols_functions::find_pc_sect_compunit_symtab
+    (objfile *objfile, bound_minimal_symbol msymbol, CORE_ADDR pc,
+     obj_section *section, int warn_if_readin)
+{
+  for (compunit_symtab &symtab : objfile->compunits ())
+    {
+      const blockvector *bv = symtab.blockvector ();
+      if (bv != nullptr && bv->contains (pc))
+	return &symtab;
+    }
+
+  return nullptr;
+}
diff --git a/gdb/expanded-symbol.h b/gdb/expanded-symbol.h
index c088d74f7e5..dc2e9814b3c 100644
--- a/gdb/expanded-symbol.h
+++ b/gdb/expanded-symbol.h
@@ -71,13 +71,7 @@ struct expanded_symbols_functions : public quick_symbol_functions
 
   compunit_symtab *find_pc_sect_compunit_symtab
     (objfile *objfile, bound_minimal_symbol msymbol, CORE_ADDR pc,
-     obj_section *section, int warn_if_readin) override
-  {
-    /* Simply returning NULL here is okay since the (only) caller
-       find_compunit_symtab_for_pc_sect iterates over existing CUs
-       anyway.  */
-    return nullptr;
-  }
+     obj_section *section, int warn_if_readin) override;
 
   symbol *find_symbol_by_address (objfile *objfile, CORE_ADDR address)
     override;
-- 
2.51.0

[PATCH 3/7] gdb: simplify find_compunit_symtab_for_pc_sect

[Jan Vrany]

This commit simplifies find_compunit_symtab_for_pc_sect by removing the
code that walks over all (currently expanded) CUs and delegating to
quick_symbol_functions::find_pc_sect_compunit_symtab instead.

With this commit on Linux x86_64 I see no regression. With -readnow
there are some regressions, mainly caused by slightly different order
of expanding CUs. Since there's a proposal to remove -readnow support,
I have not fixed nor investigated failing tests in depth.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33829
---
 gdb/dwarf2/read.c |  8 -----
 gdb/symtab.c      | 91 -----------------------------------------------
 2 files changed, 99 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 702dca8e5e9..35046439235 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2156,17 +2156,9 @@ dwarf2_base_index_functions::find_pc_sect_compunit_symtab
   if (data == nullptr)
     return nullptr;
 
-  if (warn_if_readin && per_objfile->symtab_set_p (data))
-    warning (_("(Internal error: pc %s in read in CU, but not in symtab.)"),
-	     paddress (objfile->arch (), pc));
-
   compunit_symtab *result = find_pc_sect_compunit_symtab_includes
     (dw2_instantiate_symtab (data, per_objfile, false), pc);
 
-  if (warn_if_readin && result == nullptr)
-    warning (_("(Error: pc %s in address map, but not in symtab.)"),
-	     paddress (objfile->arch (), pc));
-
   return result;
 }
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index cd3bf876551..4eea86319f8 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2742,9 +2742,6 @@ iterate_over_symbols (const struct block *block,
 struct compunit_symtab *
 find_compunit_symtab_for_pc_sect (CORE_ADDR pc, struct obj_section *section)
 {
-  struct compunit_symtab *best_cust = NULL;
-  CORE_ADDR best_cust_range = 0;
-
   /* If we know that this is not a text address, return failure.  This is
      necessary because we loop based on the block's high and low code
      addresses, which do not include the data ranges, and because
@@ -2755,94 +2752,6 @@ find_compunit_symtab_for_pc_sect (CORE_ADDR pc, struct obj_section *section)
   if (msymbol.minsym && msymbol.minsym->data_p ())
     return NULL;
 
-  /* Search all symtabs for the one whose file contains our address, and which
-     is the smallest of all the ones containing the address.  This is designed
-     to deal with a case like symtab a is at 0x1000-0x2000 and 0x3000-0x4000
-     and symtab b is at 0x2000-0x3000.  So the GLOBAL_BLOCK for a is from
-     0x1000-0x4000, but for address 0x2345 we want to return symtab b.
-
-     This happens for native ecoff format, where code from included files
-     gets its own symtab.  The symtab for the included file should have
-     been read in already via the dependency mechanism.
-     It might be swifter to create several symtabs with the same name
-     like xcoff does (I'm not sure).
-
-     It also happens for objfiles that have their functions reordered.
-     For these, the symtab we are looking for is not necessarily read in.  */
-
-  for (objfile &obj_file : current_program_space->objfiles ())
-    {
-      for (compunit_symtab &cust : obj_file.compunits ())
-	{
-	  const struct blockvector *bv = cust.blockvector ();
-	  const struct block *global_block = bv->global_block ();
-	  CORE_ADDR start = global_block->start ();
-	  CORE_ADDR end = global_block->end ();
-	  bool in_range_p = start <= pc && pc < end;
-	  if (!in_range_p)
-	    continue;
-
-	  if (bv->map () != nullptr)
-	    {
-	      if (bv->map ()->find (pc) == nullptr)
-		continue;
-
-	      return &cust;
-	    }
-
-	  CORE_ADDR range = end - start;
-	  if (best_cust != nullptr
-	      && range >= best_cust_range)
-	    /* Cust doesn't have a smaller range than best_cust, skip it.  */
-	    continue;
-
-	  /* For an objfile that has its functions reordered,
-	     find_pc_psymtab will find the proper partial symbol table
-	     and we simply return its corresponding symtab.  */
-	  /* In order to better support objfiles that contain both
-	     stabs and coff debugging info, we continue on if a psymtab
-	     can't be found.  */
-	  struct compunit_symtab *result
-	    = obj_file.find_pc_sect_compunit_symtab (msymbol, pc,
-						     section, 0);
-	  if (result != nullptr)
-	    return result;
-
-	  if (section != 0)
-	    {
-	      struct symbol *found_sym = nullptr;
-
-	      for (int b_index = GLOBAL_BLOCK;
-		   b_index <= STATIC_BLOCK && found_sym == nullptr;
-		   ++b_index)
-		{
-		  const struct block *b = bv->block (b_index);
-		  for (struct symbol *sym : block_iterator_range (b))
-		    {
-		      if (matching_obj_sections (sym->obj_section (&obj_file),
-						 section))
-			{
-			  found_sym = sym;
-			  break;
-			}
-		    }
-		}
-	      if (found_sym == nullptr)
-		continue;		/* No symbol in this symtab matches
-					   section.  */
-	    }
-
-	  /* Cust is best found so far, save it.  */
-	  best_cust = &cust;
-	  best_cust_range = range;
-	}
-    }
-
-  if (best_cust != NULL)
-    return best_cust;
-
-  /* Not found in symtabs, search the "quick" symtabs (e.g. psymtabs).  */
-
   for (objfile &objf : current_program_space->objfiles ())
     {
       struct compunit_symtab *result
-- 
2.51.0

[PATCH 4/7] gdb: do not set blockvector address map

[Jan Vrany]

This commit removes the code that sets blockvector's addrmap in case one
or more blocks are non-contiguous. Following commit will fix GDB to
handle such blocks without use of the addrmap.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33829
---
 gdb/buildsym.c | 55 ++------------------------------------------------
 1 file changed, 2 insertions(+), 53 deletions(-)

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index aa95889424b..5f47d0d5835 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -316,18 +316,10 @@ buildsym_compunit::make_blockvector ()
   std::unique_ptr<struct blockvector> blockvector;
   int i;
 
-  /* Count the length of the list of blocks.  Also, if any blocks are
-     non-contiguous then we need to make use of the addrmap for mapping
-     addresses to blocks (PENDING_ADDRMAP_INTERESTING is set to true).  If
-     all the blocks are contiguous then we can avoid creating the addrmap,
-     and perform block look up using the blockvector.  */
+  /* Count the length of the list of blocks.  */
 
-  bool pending_addrmap_interesting = false;
   for (next = m_pending_blocks, i = 0; next; next = next->next, i++)
-    {
-      if (!next->block->is_contiguous ())
-	pending_addrmap_interesting = true;
-    }
+    ;
 
   blockvector = std::make_unique<struct blockvector> (i);
 
@@ -345,49 +337,6 @@ buildsym_compunit::make_blockvector ()
   m_pending_block_obstack.clear ();
   m_pending_blocks = nullptr;
 
-  /* If we needed an address map for this symtab, record it in the
-     blockvector.  */
-  if (pending_addrmap_interesting)
-    {
-      struct addrmap_mutable pending_addrmap;
-      int num_blocks = blockvector->num_blocks ();
-
-      /* If PENDING_ADDRMAP_INTERESTING is true then we must have seen
-	 an interesting block.  If we see one block, then we should at a
-	 minimum have a global block, and a static block.  */
-      gdb_assert (num_blocks > 1);
-
-      /* Assert our understanding of how the blocks are laid out.  */
-      gdb_assert (blockvector->block (0)->is_global_block ());
-      gdb_assert (blockvector->block (1)->is_static_block ());
-
-      /* The 'J > 1' here is so that we don't place the global block into
-	 the map.  For CU with gaps, the static block will reflect the
-	 gaps, while the global block will just reflect the full extent of
-	 the range.  */
-      for (int j = num_blocks; j > 1; )
-	{
-	  --j;
-	  struct block *b = blockvector->block (j);
-
-	  gdb_assert (!b->is_global_block ());
-
-	  if (b->is_contiguous ())
-	    pending_addrmap.set_empty (b->start (), (b->end () - 1), b);
-	  else
-	    {
-	      for (const auto &br : b->ranges ())
-		pending_addrmap.set_empty (br.start (), (br.end () - 1), b);
-	    }
-	}
-
-      blockvector->set_map
-	(new (&m_objfile->objfile_obstack) addrmap_fixed
-	 (&m_objfile->objfile_obstack, &pending_addrmap));
-    }
-  else
-    blockvector->set_map (nullptr);
-
   /* Some compilers output blocks in the wrong order, but we depend on
      their being in the right order so we can binary search.  Check the
      order and moan about it.
-- 
2.51.0

[PATCH 5/7] gdb: update blockvector::lookup to handle non-contiguous blocks

[Jan Vrany]

This commit updates blockvector::lookup to handle non-contiguous without
help of addrmap. It introduces a new method, block::contains(CORE_ADDR),
to check whether given block contains given address. This new method is
then used in blockvector::lookup instead of simply using block's
start and end addresses.

A unit test for non-contiguous blocks will come later in this series.

On Debian x86_64 I see no regressions except

    FAIL: gdb.dwarf2/debug-names.exp: print _start

This is caused by discrepancy between the debug info and the debug names
and will be eventually fixed in a separate patch.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33829
---
 gdb/block.c | 24 +++++++++++++++++++++++-
 gdb/block.h |  4 ++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/gdb/block.c b/gdb/block.c
index 730e4580a5b..b964674865b 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -85,6 +85,25 @@ block::contains (const struct block *a, bool allow_nested) const
 
 /* See block.h.  */
 
+bool
+block::contains (const CORE_ADDR addr) const
+{
+  if (addr >= start () && addr < end ())
+    {
+      if (is_contiguous ())
+	return true;
+
+      for (auto range : ranges ())
+	{
+	  if (range.start () <= addr && addr < range.end ())
+	    return true;
+	}
+    }
+  return false;
+}
+
+/* See block.h.  */
+
 struct symbol *
 block::linkage_function () const
 {
@@ -857,7 +876,10 @@ blockvector::lookup (CORE_ADDR addr) const
       if (b->start () > addr)
 	return nullptr;
       if (b->end () > addr)
-	return b;
+	{
+	  if (b->contains (addr))
+	    return b;
+	}
       bot--;
     }
 
diff --git a/gdb/block.h b/gdb/block.h
index b84ca12c35a..f65546792ca 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -315,6 +315,10 @@ struct block : public allocate_on_obstack<block>
 
   bool contains (const struct block *a, bool allow_nested = false) const;
 
+  /* FIXME!!! */
+
+  bool contains (const CORE_ADDR addr) const;
+
   /* Relocate this block and all contained blocks.  OBJFILE is the
      objfile holding this block, and OFFSETS is the relocation offsets
      to use.  */
-- 
2.51.0

[PATCH 6/7] gdb: remove address map from struct blockvector

[Jan Vrany]

This commit removes m_map member and its accessors from struct blockvector
since it is no longer used. It also updates unit test accordingly.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33829
---
 gdb/block-selftests.c | 90 ++++++++++++-------------------------------
 gdb/block.c           | 10 -----
 gdb/block.h           | 13 -------
 3 files changed, 24 insertions(+), 89 deletions(-)

diff --git a/gdb/block-selftests.c b/gdb/block-selftests.c
index d23783d6bdc..77b8d67ded8 100644
--- a/gdb/block-selftests.c
+++ b/gdb/block-selftests.c
@@ -40,35 +40,19 @@ make_block (struct blockvector &bv, struct obstack &ob, CORE_ADDR start,
   return b;
 }
 
-/* A helper to create and set address map given a blockvector.  */
 static void
-make_map (struct blockvector &bv, struct obstack &ob)
+test_blockvector_lookup_contains ()
 {
-  struct addrmap_mutable map;
-
-  for (int i = bv.num_blocks () - 1; i > STATIC_BLOCK; i--)
-    {
-      auto b = bv.block (i);
-      map.set_empty (b->start (), b->end () - 1, b);
-    }
-
-  bv.set_map (new (&ob) addrmap_fixed (&ob, &map));
-}
-
-/* Create and return blockvector with following blocks:
+  /* Create blockvector with following blocks:
 
 	B0    0x1000 - 0x4000   (global block)
 	B1    0x1000 - 0x4000   (static block)
-	 B2   0x1000 - 0x2000
+	  B2  0x1000 - 0x2000
 				(hole)
-	 B3   0x3000 - 0x4000
-
-  If USE_MAP is true, then also set blockvector's address map.
-*/
-static blockvector_up
-make_blockvector (struct obstack &ob, bool use_map)
-{
-  auto bv = std::make_unique<struct blockvector> (0);
+	  B3  0x3000 - 0x4000
+  */
+  auto_obstack ob;
+  blockvector_up bv = std::make_unique<struct blockvector> (0);
 
   auto global_block = make_block (*bv.get (), ob, 0x1000, 0x4000);
   auto static_block = make_block (*bv.get (), ob, 0x1000, 0x4000,
@@ -76,51 +60,25 @@ make_blockvector (struct obstack &ob, bool use_map)
   make_block (*bv.get (), ob, 0x1000, 0x2000, static_block);
   make_block (*bv.get (), ob, 0x3000, 0x4000, static_block);
 
-  if (use_map)
-    make_map (*bv.get (), ob);
+  /* Test address outside global block's range.  */
+  SELF_CHECK (bv->lookup (0x0500) == nullptr);
+  SELF_CHECK (bv->contains (0x0500) == false);
 
-  return bv;
-}
+  /* Test address falling into a block.  */
+  SELF_CHECK (bv->lookup (0x1500) == bv->block (2));
+  SELF_CHECK (bv->contains (0x1500) == true);
 
-static void
-test_blockvector_lookup_contains ()
-{
-  for (bool with_map : { false, true })
-    {
-      /* Test blockvector without an address map.  */
-      auto_obstack ob;
-      blockvector_up bv = make_blockvector (ob, with_map);
-
-      /* Test address outside global block's range.  */
-      SELF_CHECK (bv->lookup (0x0500) == nullptr);
-      SELF_CHECK (bv->contains (0x0500) == false);
-
-      /* Test address falling into a block.  */
-      SELF_CHECK (bv->lookup (0x1500) == bv->block (2));
-      SELF_CHECK (bv->contains (0x1500) == true);
-
-      /* Test address falling into a "hole".  If BV has an address map,
-	 lookup () returns nullptr and contains (). returns false.  If not,
-	 lookup () return static block and contains() returns true.  */
-      if (with_map)
-	{
-	  SELF_CHECK (bv->lookup (0x2500) == nullptr);
-	  SELF_CHECK (bv->contains (0x2500) == false);
-	}
-      else
-	{
-	  SELF_CHECK (bv->lookup (0x2500) == bv->block (STATIC_BLOCK));
-	  SELF_CHECK (bv->contains (0x2500) == true);
-	}
-
-      /* Test address falling into a block above the "hole".  */
-      SELF_CHECK (bv->lookup (0x3500) == bv->block (3));
-      SELF_CHECK (bv->contains (0x3500) == true);
-
-      /* Test address outside global block's range.  */
-      SELF_CHECK (bv->lookup (0x4000) == nullptr);
-      SELF_CHECK (bv->contains (0x4000) == false);
-    }
+  /* Test address falling into a "hole".  */
+  SELF_CHECK (bv->lookup (0x2500) == bv->block (STATIC_BLOCK));
+  SELF_CHECK (bv->contains (0x2500) == true);
+
+  /* Test address falling into a block above the "hole".  */
+  SELF_CHECK (bv->lookup (0x3500) == bv->block (3));
+  SELF_CHECK (bv->contains (0x3500) == true);
+
+  /* Test address outside global block's range.  */
+  SELF_CHECK (bv->lookup (0x4000) == nullptr);
+  SELF_CHECK (bv->contains (0x4000) == false);
 }
 
 } /* namespace selftests */
diff --git a/gdb/block.c b/gdb/block.c
index b964674865b..cd0f60ef3cb 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -842,11 +842,6 @@ blockvector::lookup (CORE_ADDR addr) const
   if (addr < start || end <= addr)
     return nullptr;
 
-  /* If we have an addrmap mapping code addresses to blocks, then use
-     that.  */
-  if (map () != nullptr)
-    return (const struct block *) map ()->find (addr);
-
   /* Otherwise, use binary search to find the last block that starts
      before PC.
      Note: GLOBAL_BLOCK is block 0, STATIC_BLOCK is block 1.
@@ -925,11 +920,6 @@ void
 blockvector::relocate (struct objfile *objfile,
 		       gdb::array_view<const CORE_ADDR> offsets)
 {
-  int block_line_section = SECT_OFF_TEXT (objfile);
-
-  if (m_map != nullptr)
-    m_map->relocate (offsets[block_line_section]);
-
   for (struct block *b : m_blocks)
     b->relocate (objfile, offsets);
 }
diff --git a/gdb/block.h b/gdb/block.h
index f65546792ca..9134cd43d0f 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -492,14 +492,6 @@ struct blockvector
   const struct block *static_block () const
   { return this->block (STATIC_BLOCK); }
 
-  /* Const version of the above.  */
-  const addrmap_fixed *map () const
-  { return m_map; }
-
-  /* Set this blockvector's address -> block map.  */
-  void set_map (addrmap_fixed *map)
-  { m_map = map; }
-
   /* Block comparison function.  Returns true if B1 must be ordered before
      B2 in a blockvector, false otherwise.  */
   static bool block_less_than (const struct block *b1, const struct block *b2);
@@ -526,11 +518,6 @@ struct blockvector
 		 gdb::array_view<const CORE_ADDR> offsets);
 
 private:
-  /* An address map mapping addresses to blocks in this blockvector.
-     This pointer is zero if the blocks' start and end addresses are
-     enough.  */
-  addrmap_fixed *m_map = nullptr;
-
   /* The blocks themselves.  */
   std::vector<struct block *> m_blocks;
 };
-- 
2.51.0

[PATCH 7/7] gdb: add unit test for blockvector::lookup of non-contiguous blocks

[Jan Vrany]

This commit add a new unit test that tests block lookup when blocks are
non-contiguous.
---
 gdb/block-selftests.c | 74 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/gdb/block-selftests.c b/gdb/block-selftests.c
index 77b8d67ded8..ca4289fbb6f 100644
--- a/gdb/block-selftests.c
+++ b/gdb/block-selftests.c
@@ -20,6 +20,7 @@
 #include "gdbsupport/selftest.h"
 #include "block.h"
 #include "addrmap.h"
+#include "obstack.h"
 
 
 namespace selftests {
@@ -81,6 +82,76 @@ test_blockvector_lookup_contains ()
   SELF_CHECK (bv->contains (0x4000) == false);
 }
 
+/* Create and return struct blockranges* from 2 ranges.  */
+static struct blockranges *
+make_blockranges_2 (struct obstack &ob, CORE_ADDR start0,
+	    CORE_ADDR end0, CORE_ADDR start1,
+	    CORE_ADDR end1)
+{
+  struct blockranges *ranges = (struct blockranges *) obstack_alloc
+    (&ob, sizeof (struct blockranges) + (2 - 1) * sizeof (struct blockrange));
+
+  ranges->nranges = 2;
+  ranges->range[0].set_start (start0);
+  ranges->range[0].set_end (end0);
+  ranges->range[1].set_start (start1);
+  ranges->range[1].set_end (end1);
+
+  return ranges;
+}
+
+static void
+test_blockvector_lookup_non_continuguous ()
+{
+  /* Create blockvector with following blocks:
+
+	B0      0x1000 - 0x8000   (global block)
+	B1      0x1000 - 0x8000   (static block)
+	  B2    0x1000 - 0x2000   (B2's range 1)
+	  B3    0x2000 - 0x3000   (B3's range 1)
+				  (hole 1)
+	    B4  0x5000 - 0x5500
+				  (hole 2)
+	 (B2)   0x6000 - 0x7000   (B2's range 2)
+	 (B3)   0x7000 - 0x8000   (B3's range 2)
+
+    Blocks B2 and B3 are non-continguous (consist of two
+    disjoint ranges) and interleaved.
+  */
+  auto_obstack ob;
+  blockvector_up bv = std::make_unique<struct blockvector> (0);
+
+  auto global_block = make_block (*bv.get (), ob, 0x1000, 0x8000);
+  auto static_block = make_block (*bv.get (), ob, 0x1000, 0x8000,
+				  global_block);
+  auto b2 = make_block (*bv.get (), ob, 0x1000, 0x7000, static_block);
+  b2->set_ranges (make_blockranges_2 (ob, 0x1000, 0x2000, 0x6000, 0x7000));
+  auto b3 = make_block (*bv.get (), ob, 0x2000, 0x8000, static_block);
+  b3->set_ranges (make_blockranges_2 (ob, 0x2000, 0x3000, 0x7000, 0x8000));
+  auto b4 = make_block (*bv.get (), ob, 0x5000, 0x5500, static_block);
+
+  /* Test address falling into range 1 of B2.  */
+  SELF_CHECK (bv->lookup (0x1500) == b2);
+
+  /* Test address falling into range 2 of B2.  */
+  SELF_CHECK (bv->lookup (0x6000) == b2);
+
+  /* Test address falling into range 1 of B3.  */
+  SELF_CHECK (bv->lookup (0x2500) == b3);
+
+  /* Test address falling into range 2 of B3.  */
+  SELF_CHECK (bv->lookup (0x7999) == b3);
+
+  /* Test address falling into B4.  */
+  SELF_CHECK (bv->lookup (0x5250) == b4);
+
+  /* Test address falling into hole 1.  */
+  SELF_CHECK (bv->lookup (0x4000) == static_block);
+
+  /* Test address falling into hole 2.  */
+  SELF_CHECK (bv->lookup (0x5750) == static_block);
+}
+
 } /* namespace selftests */
 
 
@@ -88,4 +159,7 @@ INIT_GDB_FILE (block_selftest)
 {
   selftests::register_test ("blockvector-lookup-contains",
 			    selftests::test_blockvector_lookup_contains);
+  selftests::register_test
+    ("blockvector-lookup-non-contiguous",
+    selftests::test_blockvector_lookup_non_continuguous);
 }
-- 
2.51.0

Re: [PATCH 1/7] gdb: implement readnow_functions::find_pc_sect_compunit_symtab

[Tom Tromey]

>>>>> "Jan" == Jan Vrany <jan.vrany@labware.com> writes:

Jan> +struct compunit_symtab *
Jan> +readnow_functions::find_pc_sect_compunit_symtab
Jan> +     (struct objfile *objfile,
Jan> +      bound_minimal_symbol msymbol,
Jan> +      CORE_ADDR pc,
Jan> +      struct obj_section *section,
Jan> +      int warn_if_readin)
Jan> +{
Jan> +  dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
Jan> +  dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
Jan> +
Jan> +  /* This invariant is documented in read.h  */
Jan> +  gdb_assert (per_bfd->index_table == nullptr);
Jan> +
Jan> +  /* Since we have no index, we simply walk all units until matching CU is
Jan> +     found (of there are no more CUs).  */
Jan> +  for (int i = 0; i < per_bfd->all_units.size (); i++)
Jan> +    {
Jan> +      dwarf2_per_cu *data = per_bfd->all_units[i].get ();
Jan> +      compunit_symtab *result = find_pc_sect_compunit_symtab_includes (
Jan> +	dw2_instantiate_symtab (data, per_objfile, false), pc);

There shouldn't be any need to instantiate a symtab here, since for
readnow they are all already expanded.

Tom

Re: [PATCH 2/7] gdb: update expanded_symbols_functions::find_pc_sect_compunit_symtab

[Tom Tromey]

>>>>> "Jan" == Jan Vrany <jan.vrany@labware.com> writes:

Jan> This commit updates expanded_symbols_functions::find_pc_sect_compunit_symtab
Jan> to search all compunits rather than just returning null.

Jan> The original implementation (just return null) was based on reasoning
Jan> that its would suffice since find_compunit_symtab_for_pc_sect walks
Jan> all CUs anyway [1]. This commit is a preparation to simplify
Jan> find_compunit_symtab_for_pc_sect by removing that code.

This is ok, thanks.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 3/7] gdb: simplify find_compunit_symtab_for_pc_sect

[Tom Tromey]

>>>>> "Jan" == Jan Vrany <jan.vrany@labware.com> writes:

Jan> This commit simplifies find_compunit_symtab_for_pc_sect by removing the
Jan> code that walks over all (currently expanded) CUs and delegating to
Jan> quick_symbol_functions::find_pc_sect_compunit_symtab instead.

Jan> With this commit on Linux x86_64 I see no regression. With -readnow
Jan> there are some regressions, mainly caused by slightly different order
Jan> of expanding CUs. Since there's a proposal to remove -readnow support,
Jan> I have not fixed nor investigated failing tests in depth.

I think that proposal was rejected so I'm afraid you'll have to revisit
that.

Tom

Re: [PATCH 4/7] gdb: do not set blockvector address map

[Tom Tromey]

>>>>> "Jan" == Jan Vrany <jan.vrany@labware.com> writes:

Jan> This commit removes the code that sets blockvector's addrmap in case one
Jan> or more blocks are non-contiguous. Following commit will fix GDB to
Jan> handle such blocks without use of the addrmap.

These patches probably should be combined since I think this patch by
itself would cause regressions; and normally when possible we try to
keep the tree bisectable.

Tom

Re: [PATCH 5/7] gdb: update blockvector::lookup to handle non-contiguous blocks

[Tom Tromey]

>>>>> "Jan" == Jan Vrany <jan.vrany@labware.com> writes:

Jan> +  /* FIXME!!! */
Jan> +
Jan> +  bool contains (const CORE_ADDR addr) const;

Oops

Tom

Re: [PATCH 7/7] gdb: add unit test for blockvector::lookup of non-contiguous blocks

[Tom Tromey]

>>>>> "Jan" == Jan Vrany <jan.vrany@labware.com> writes:

Jan> +#include "obstack.h"
 
This shouldn't be needed.  The code uses auto_obstack and the header
that declares that already includes obstack.h.

Jan> +/* Create and return struct blockranges* from 2 ranges.  */
Jan> +static struct blockranges *
Jan> +make_blockranges_2 (struct obstack &ob, CORE_ADDR start0,
Jan> +	    CORE_ADDR end0, CORE_ADDR start1,
Jan> +	    CORE_ADDR end1)
Jan> +{
Jan> +  struct blockranges *ranges = (struct blockranges *) obstack_alloc
Jan> +    (&ob, sizeof (struct blockranges) + (2 - 1) * sizeof (struct blockrange));

Indentation of the function header and this first code line both look off.

Tom

Re: [PATCH 5/7] gdb: update blockvector::lookup to handle non-contiguous blocks

[Tom Tromey]

>>>>> "Jan" == Jan Vrany <jan.vrany@labware.com> writes:

Jan> This commit updates blockvector::lookup to handle non-contiguous without
Jan> help of addrmap. It introduces a new method, block::contains(CORE_ADDR),
Jan> to check whether given block contains given address. This new method is
Jan> then used in blockvector::lookup instead of simply using block's
Jan> start and end addresses.

Jan> A unit test for non-contiguous blocks will come later in this series.

Jan> On Debian x86_64 I see no regressions except

Jan>     FAIL: gdb.dwarf2/debug-names.exp: print _start

Jan> This is caused by discrepancy between the debug info and the debug names
Jan> and will be eventually fixed in a separate patch.

I glossed over this before but is this patch in this series?

Jan> +bool
Jan> +block::contains (const CORE_ADDR addr) const
Jan> +{
Jan> +  if (addr >= start () && addr < end ())
Jan> +    {
Jan> +      if (is_contiguous ())
Jan> +	return true;
Jan> +
Jan> +      for (auto range : ranges ())
Jan> +	{
Jan> +	  if (range.start () <= addr && addr < range.end ())
Jan> +	    return true;

It took me a long time to understand that this patch was doing the real
work in this series...

For this method I guess there is an invariant that the block's start is
the minimum of the range minima and the block's end is the maximum of
the range maxima.

It seems to me that since this is an important feature, it ought to be
enforced by an assert, if necessary adding checks in the reader to
ensure that bad DWARF doesn't cause an assertion failure.

Jan> @@ -857,7 +876,10 @@ blockvector::lookup (CORE_ADDR addr) const
Jan>        if (b->start () > addr)
Jan>  	return nullptr;
Jan>        if (b->end () > addr)
Jan> -	return b;
Jan> +	{
Jan> +	  if (b->contains (addr))
Jan> +	    return b;
Jan> +	}
Jan>        bot--;
Jan>      }
 
The heart of the series.

I wonder about the efficiency of this.  Like how many blocks can we
expect to see in the linear phase?

I don't know what a representative program might be here.

Tom

Re: [PATCH 1/7] gdb: implement readnow_functions::find_pc_sect_compunit_symtab

[Jan Vraný]

On Thu, 2026-02-19 at 13:01 -0700, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vrany <jan.vrany@labware.com> writes:
> 
> Jan> +struct compunit_symtab *
> Jan> +readnow_functions::find_pc_sect_compunit_symtab
> Jan> +     (struct objfile *objfile,
> Jan> +      bound_minimal_symbol msymbol,
> Jan> +      CORE_ADDR pc,
> Jan> +      struct obj_section *section,
> Jan> +      int warn_if_readin)
> Jan> +{
> Jan> +  dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
> Jan> +  dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
> Jan> +
> Jan> +  /* This invariant is documented in read.h  */
> Jan> +  gdb_assert (per_bfd->index_table == nullptr);
> Jan> +
> Jan> +  /* Since we have no index, we simply walk all units until matching CU is
> Jan> +     found (of there are no more CUs).  */
> Jan> +  for (int i = 0; i < per_bfd->all_units.size (); i++)
> Jan> +    {
> Jan> +      dwarf2_per_cu *data = per_bfd->all_units[i].get ();
> Jan> +      compunit_symtab *result = find_pc_sect_compunit_symtab_includes (
> Jan> +	dw2_instantiate_symtab (data, per_objfile, false), pc);
> 
> There shouldn't be any need to instantiate a symtab here, since for
> readnow they are all already expanded.

Not always. dwarf2_base_index_functions::expand_all_symtabs skips 
"partial symtabs", so in some cases not all symtabs are instantiated.

An example of this is dwz-symtabs.exp. If I change the code as

--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2181,8 +2181,10 @@ readnow_functions::find_pc_sect_compunit_symtab
   for (int i = 0; i < per_bfd->all_units.size (); i++)
     {
       dwarf2_per_cu *data = per_bfd->all_units[i].get ();
-      compunit_symtab *result = find_pc_sect_compunit_symtab_includes (
-       dw2_instantiate_symtab (data, per_objfile, false), pc);
+
+      gdb_assert (per_objfile->symtab_set_p (data));
+      compunit_symtab *result = find_pc_sect_compunit_symtab_includes
+       (per_objfile->get_symtab (data), pc);
       if (result != nullptr)
        return result;
     }

I get an assertion failure. 

dw2_instantiate_symtab checks if the symtab has been instantiated and if 
yes, it's an no-op so calling it should not hurt much.

Jan

> 
> Tom

Re: [PATCH 3/7] gdb: simplify find_compunit_symtab_for_pc_sect

[Jan Vraný]

On Thu, 2026-02-19 at 13:02 -0700, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vrany <jan.vrany@labware.com> writes:
> 
> Jan> This commit simplifies find_compunit_symtab_for_pc_sect by removing the
> Jan> code that walks over all (currently expanded) CUs and delegating to
> Jan> quick_symbol_functions::find_pc_sect_compunit_symtab instead.
> 
> Jan> With this commit on Linux x86_64 I see no regression. With -readnow
> Jan> there are some regressions, mainly caused by slightly different order
> Jan> of expanding CUs. Since there's a proposal to remove -readnow support,
> Jan> I have not fixed nor investigated failing tests in depth.
> 
> I think that proposal was rejected so I'm afraid you'll have to revisit
> that.

I was hoping is it still under consideration...

I'll look into that then. It may need more fixes as quite a few test do not pass 
with -readnow on current master (on my machine).

Jan

> 
> Tom

Re: [PATCH 4/7] gdb: do not set blockvector address map

[Jan Vraný]

On Thu, 2026-02-19 at 13:03 -0700, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vrany <jan.vrany@labware.com> writes:
> 
> Jan> This commit removes the code that sets blockvector's addrmap in case one
> Jan> or more blocks are non-contiguous. Following commit will fix GDB to
> Jan> handle such blocks without use of the addrmap.
> 
> These patches probably should be combined since I think this patch by
> itself would cause regressions; and normally when possible we try to
> keep the tree bisectable.

Right, I'll swap them around, this way it should not cause regressions.
Thanks! 

Jan

> 
> Tom

Re: [PATCH 5/7] gdb: update blockvector::lookup to handle non-contiguous blocks

[Jan Vraný]

On Thu, 2026-02-19 at 13:20 -0700, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vrany <jan.vrany@labware.com> writes:
> 
> Jan> This commit updates blockvector::lookup to handle non-contiguous without
> Jan> help of addrmap. It introduces a new method, block::contains(CORE_ADDR),
> Jan> to check whether given block contains given address. This new method is
> Jan> then used in blockvector::lookup instead of simply using block's
> Jan> start and end addresses.
> 
> Jan> A unit test for non-contiguous blocks will come later in this series.
> 
> Jan> On Debian x86_64 I see no regressions except
> 
> Jan>     FAIL: gdb.dwarf2/debug-names.exp: print _start
> 
> Jan> This is caused by discrepancy between the debug info and the debug names
> Jan> and will be eventually fixed in a separate patch.
> 
> I glossed over this before but is this patch in this series?

No, this is the patch "Fix debug_names function visibility" from your 
branch you said you'll check in [1]. I can include it if you prefer.

[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=33829#c12
> 
> Jan> +bool
> Jan> +block::contains (const CORE_ADDR addr) const
> Jan> +{
> Jan> +  if (addr >= start () && addr < end ())
> Jan> +    {
> Jan> +      if (is_contiguous ())
> Jan> +	return true;
> Jan> +
> Jan> +      for (auto range : ranges ())
> Jan> +	{
> Jan> +	  if (range.start () <= addr && addr < range.end ())
> Jan> +	    return true;
> 
> It took me a long time to understand that this patch was doing the real
> work in this series...
> 
> For this method I guess there is an invariant that the block's start is
> the minimum of the range minima and the block's end is the maximum of
> the range maxima.
> 
> It seems to me that since this is an important feature, it ought to be
> enforced by an assert, if necessary adding checks in the reader to
> ensure that bad DWARF doesn't cause an assertion failure.

Yes. I originally had these assertions there, thinking that blocks's start
is first range's start and end is last range's end but it turned out there's
no defined ordering (and DWARF5 spec does not say IIUC).

So I'd have to iterate over ranges to find min/max to use in asserts. I did not
do it in the end, but doing it in block::set_ranges probably won't hurt much. 
I'll do that in v2.

Other option I was thinking was to change make_blockranges to always sort then,
then the assertions in set_ranges would be trivial and in block::contains one
can binary-search block ranges too (though for up to ~4 ranges perhaps not worth).


> 
> Jan> @@ -857,7 +876,10 @@ blockvector::lookup (CORE_ADDR addr) const
> Jan>        if (b->start () > addr)
> Jan>  	return nullptr;
> Jan>        if (b->end () > addr)
> Jan> -	return b;
> Jan> +	{
> Jan> +	  if (b->contains (addr))
> Jan> +	    return b;
> Jan> +	}
> Jan>        bot--;
> Jan>      }
>  
> The heart of the series.
> 
> I wonder about the efficiency of this.  Like how many blocks can we
> expect to see in the linear phase?
> 
> I don't know what a representative program might be here.

TBH I have no idea. How common is this, how many ranges blocks have, nor
how to force compiler to produce such binary. Maybe torturing the compiler
with -O3 and PGO?

Jan

> 
> Tom

Re: [PATCH 1/7] gdb: implement readnow_functions::find_pc_sect_compunit_symtab

[Tom Tromey]

>>>>> "Jan" == Jan Vraný <Jan.Vrany@labware.com> writes:

Jan> Not always. dwarf2_base_index_functions::expand_all_symtabs skips 
Jan> "partial symtabs", so in some cases not all symtabs are instantiated.

A partial symtab should be expanded as a side effect of expanding any CU
that includes it.

Now, an orphan partial symtab will never be expanded -- but I think
those are just an error and can be ignored.

Instead of asserting that the symtab is set, perhaps this should be
skipping partial units.  Though I also wonder why it's referencing the
DWARF data structures at all and not just working off the compunits.

Tom

Re: [PATCH 3/7] gdb: simplify find_compunit_symtab_for_pc_sect

[Tom Tromey]

>>>>> "Jan" == Jan Vraný <Jan.Vrany@labware.com> writes:

Jan> I'll look into that then. It may need more fixes as quite a few test do not pass 
Jan> with -readnow on current master (on my machine).

FWIW regression-free is enough, you don't have to fix any pre-existing bugs.

Tom

Re: [PATCH 1/7] gdb: implement readnow_functions::find_pc_sect_compunit_symtab

[Jan Vraný]

On Fri, 2026-02-20 at 07:25 -0700, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vraný <Jan.Vrany@labware.com> writes:
> 
> Jan> Not always. dwarf2_base_index_functions::expand_all_symtabs skips 
> Jan> "partial symtabs", so in some cases not all symtabs are instantiated.
> 
> A partial symtab should be expanded as a side effect of expanding any CU
> that includes it.
> 
> Now, an orphan partial symtab will never be expanded -- but I think
> those are just an error and can be ignored.
> 
> Instead of asserting that the symtab is set, perhaps this should be
> skipping partial units.  

Okay, I'll do that. 

> Though I also wonder why it's referencing the
> DWARF data structures at all and not just working off the compunits.

Other readnow_functions use DWARF structures too so I simply took the same
approach. 

Alternatively, we can base readnow_functions on expanded_symbol_functions and
just override expand_all_symtabs (taking the implementation from dwarf2_base_index_functions).

Thanks!

Jan 

Re: [PATCH 5/7] gdb: update blockvector::lookup to handle non-contiguous blocks

[Tom Tromey]

>> I glossed over this before but is this patch in this series?

Jan> No, this is the patch "Fix debug_names function visibility" from your 
Jan> branch you said you'll check in [1]. I can include it if you prefer.

Jan> [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=33829#c12

Oops, I forgot about this.  I'll send it shortly.

Jan> Other option I was thinking was to change make_blockranges to always sort then,
Jan> then the assertions in set_ranges would be trivial and in block::contains one
Jan> can binary-search block ranges too (though for up to ~4 ranges perhaps not worth).

Lately I've been thinking that weirdness should be hidden in the readers
and not exposed to the core.  But putting stuff like this in "builder"
code also seems fine.  The main point is that core/generic code
shouldn't have to cope with weirdness in the data structures, that
should be ironed out when creating them.

Jan> TBH I have no idea. How common is this, how many ranges blocks have, nor
Jan> how to force compiler to produce such binary. Maybe torturing the compiler
Jan> with -O3 and PGO?

Yeah, maybe just an optimized LTO build of gdb would show it.

Tom

Re: [PATCH 3/7] gdb: simplify find_compunit_symtab_for_pc_sect

[Jan Vraný]

On Thu, 2026-02-19 at 13:02 -0700, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vrany <jan.vrany@labware.com> writes:
> 
> Jan> This commit simplifies find_compunit_symtab_for_pc_sect by removing the
> Jan> code that walks over all (currently expanded) CUs and delegating to
> Jan> quick_symbol_functions::find_pc_sect_compunit_symtab instead.
> 
> Jan> With this commit on Linux x86_64 I see no regression. With -readnow
> Jan> there are some regressions, mainly caused by slightly different order
> Jan> of expanding CUs. Since there's a proposal to remove -readnow support,
> Jan> I have not fixed nor investigated failing tests in depth.
> 
> I think that proposal was rejected so I'm afraid you'll have to revisit
> that.

So I did that. The only regression I spotted is dw2-entry-points.exp in
which the hand-written DWARF creates to CUs, first one with "hole". 
And since blockvector::lookup() returns static block if no better block 
is found and because the CU with hole happens to be tried first, 
the test fails. If one changes the order of CUs in the test, it passed. 

There's an easy "fix" for that - look for compunit that contains shortest 
block containing the PC, somewhat along the lines of "big chunk": in current 
find_compunit_symtab_for_pc_sect (see patch below). 

But: I'm not sure we want to do that. This brings us back to the idea that 
blockvector::lookup() should never return static block. Or accept that
regression. 

Jan

-- >8 --
From a4b9d584634f4aff9fc0d2f4b3ce2a2a5019b3ad Mon Sep 17 00:00:00 2001
From: Jan Vrany <jan.vrany@labware.com>
Date: Wed, 25 Feb 2026 22:34:40 +0000
Subject: [PATCH] FIXUP: gdb: implement
 readnow_functions::find_pc_sect_compunit_symtab

---
 gdb/dwarf2/read.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 2ed8b78d596..824f5081105 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2176,21 +2176,41 @@ readnow_functions::find_pc_sect_compunit_symtab
   /* This invariant is documented in read.h  */
   gdb_assert (per_bfd->index_table == nullptr);
 
-  /* Since we have no index, we simply walk all units until matching CU is
-     found (of there are no more CUs).  */
+  /* Since we have no index, we have to walk all CUs and find one with
+     best-matching block that contains PC.  For the purpose of this method,
+     a block B1 is considerent better matching than block B2 if B1 is not null
+     and B2 is either null or has larger total span than B1.  In other words,
+     smaller blocks are "better".  */
+
+  auto better = [&] (const block *b1, const block *b2)
+    {
+      if (b1 == nullptr)
+	return false;
+      if (b2 == nullptr)
+	return true;
+      return (b1->end () - b1->start ()) < (b2->end () - b2->start ());
+    };
+
+  compunit_symtab *best_cu = nullptr;
+  const block *best_bl = nullptr;
+
   for (int i = 0; i < per_bfd->all_units.size (); i++)
     {
       dwarf2_per_cu *data = per_bfd->all_units[i].get ();
 
       if (per_objfile->symtab_set_p (data))
 	{
-	  compunit_symtab *result = find_pc_sect_compunit_symtab_includes (
-	    per_objfile->get_symtab (data), pc);
-	  if (result != nullptr)
-	    return result;
+	  compunit_symtab *this_cu = per_objfile->get_symtab (data);
+	  const block *this_bl = this_cu->blockvector ()->lookup (pc);
+
+	  if (better (this_bl, best_bl))
+	    {
+	      best_cu = this_cu;
+	      best_bl = this_bl;
+	    }
 	}
     }
-  return nullptr;
+  return best_cu;
 }
 
 void
-- 
2.51.0

Re: [PATCH 5/7] gdb: update blockvector::lookup to handle non-contiguous blocks

[Jan Vraný]

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

On Fri, 2026-02-20 at 09:38 -0700, Tom Tromey wrote:
> > 
> 
> Jan> TBH I have no idea. How common is this, how many ranges blocks have, nor
> Jan> how to force compiler to produce such binary. Maybe torturing the compiler
> Jan> with -O3 and PGO?
> 
> Yeah, maybe just an optimized LTO build of gdb would show it.

I tried to build with PGO but no success so I resorted to LTO build
and wrote a script to get some data:

# CUs                             1263
# CUs with addr map               1075
# blocks                          654629
# blocks with multiple ranges:    270730

Max # blocks in linear phase:     42
Avg # blocks in linear phase:     2.372987707844722
Med # blocks in linear phase:     1

Here "blocks in linear phase" means number of blocks that start
at the same address, which is the worst case.

Assuming my thinking and script is correct, in little more than 50%
cases there's only one block in linear phase, in 17% cases there are
2 blocks, in 23% cases there are 3 to 5 blocks (histogram attached).

Jan

[-- Attachment #2: blocks_in_linear_phase_gdb_lto.png --]
[-- Type: image/png, Size: 23123 bytes --]

Re: [PATCH 5/7] gdb: update blockvector::lookup to handle non-contiguous blocks

[Jan Vraný]

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

I just noticed that this message somehow did not make it to gdb-patches,
not sure why.


On Fri, 2026-02-20 at 09:38 -0700, Tom Tromey wrote:
> > 
> 
> Jan> TBH I have no idea. How common is this, how many ranges blocks have, nor
> Jan> how to force compiler to produce such binary. Maybe torturing the compiler
> Jan> with -O3 and PGO?
> 
> Yeah, maybe just an optimized LTO build of gdb would show it.

I tried to build with PGO but no success so I resorted to LTO build
and wrote a script to get some data:

# CUs                             1263
# CUs with addr map               1075
# blocks                          654629
# blocks with multiple ranges:    270730

Max # blocks in linear phase:     42
Avg # blocks in linear phase:     2.372987707844722
Med # blocks in linear phase:     1

Here "blocks in linear phase" means number of blocks that start
at the same address, which is the worst case.

Assuming my thinking and script is correct, in little more than 50%
cases there's only one block in linear phase, in 17% cases there are
2 blocks, in 23% cases there are 3 to 5 blocks (histogram attached).

Jan

[-- Attachment #2: blocks_in_linear_phase_gdb_lto.png --]
[-- Type: image/png, Size: 23123 bytes --]