* [pushed] gdb: update blockvector::lookup
@ 2025-11-28 13:49 Jan Vrany
2025-11-28 13:49 ` [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes" Jan Vrany
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Jan Vrany @ 2025-11-28 13:49 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Vrany, Simon Marchi
This commit changes blockvector::lookup() to improve style and
readability. It also adds quick range check to see if given address
falls into global block.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
---
gdb/block.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/gdb/block.c b/gdb/block.c
index 98088aa3807..e21580bcf63 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -810,12 +810,17 @@ blockvector::append_block (struct block *block)
const struct block *
blockvector::lookup (CORE_ADDR addr) const
{
- const struct block *b;
- int bot, top, half;
+ const CORE_ADDR start = global_block ()->start ();
+ const CORE_ADDR end = global_block ()->end ();
+
+ /* Check if the given address falls into the global block. If not, this
+ blockvector definitely does not contain any block at ADDR. */
+ if (addr < start || end <= addr)
+ return nullptr;
/* If we have an addrmap mapping code addresses to blocks, then use
that. */
- if (map ())
+ if (map () != nullptr)
return (const struct block *) map ()->find (addr);
/* Otherwise, use binary search to find the last block that starts
@@ -824,14 +829,15 @@ blockvector::lookup (CORE_ADDR addr) const
They both have the same START,END values.
Historically this code would choose STATIC_BLOCK over GLOBAL_BLOCK but the
fact that this choice was made was subtle, now we make it explicit. */
- gdb_assert (blocks ().size () >= 2);
- bot = STATIC_BLOCK;
- top = blocks ().size ();
+ gdb_assert (num_blocks () >= 2);
+
+ int bot = STATIC_BLOCK;
+ int top = num_blocks ();
while (top - bot > 1)
{
- half = (top - bot + 1) >> 1;
- b = block (bot + half);
+ auto half = (top - bot + 1) >> 1;
+ auto b = block (bot + half);
if (b->start () <= addr)
bot += half;
else
@@ -842,15 +848,15 @@ blockvector::lookup (CORE_ADDR addr) const
while (bot >= STATIC_BLOCK)
{
- b = block (bot);
- if (!(b->start () <= addr))
- return NULL;
+ auto b = block (bot);
+ if (b->start () > addr)
+ return nullptr;
if (b->end () > addr)
return b;
bot--;
}
- return NULL;
+ return nullptr;
}
/* See block.h. */
--
2.51.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes"
2025-11-28 13:49 [pushed] gdb: update blockvector::lookup Jan Vrany
@ 2025-11-28 13:49 ` Jan Vrany
2025-12-02 17:41 ` Tom Tromey
2025-11-28 13:49 ` [pushed] gdb: add unittests for blockvector lookup functions Jan Vrany
2025-11-28 13:49 ` [pushed] gdb: update is_addr_in_objfile to support "dynamic" objfiles Jan Vrany
2 siblings, 1 reply; 10+ messages in thread
From: Jan Vrany @ 2025-11-28 13:49 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Vrany, Simon Marchi
This commit slightly changes the logic in blockvector::contains()
to handle a case where the blockvector contains blocks with disjoint
regions (see the comment in blockvector::contains for details).
This change may potentially change GDB's behavior. It is not clear to me
if there was a reason for blockvector_contains_pc() behaving differently
depending whether or not given blockvector contain a map. With this
change, blockvector::contains() return the same value regardless. The
reason for it is to make it work as expected also for "dynamic" code
created by JIT reader (and perhaps by Python in the future).
Note that for CUs created from DWARF, blockvectors have always a map set
so this change in behavior should not affect them. Running testsuite
on Linux x86_64 shown no regressions.
Finally, I was considering of making this change up in lookup method
but in the end decided to be bit more conservative because comment in
original find_block_in_blockvector() suggested that returning a static
block from there is an expected situation.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
---
gdb/block.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/gdb/block.c b/gdb/block.c
index e21580bcf63..3d2c51cc554 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -864,7 +864,34 @@ blockvector::lookup (CORE_ADDR addr) const
bool
blockvector::contains (CORE_ADDR addr) const
{
- return lookup (addr) != nullptr;
+ auto b = lookup (addr);
+ if (b == nullptr)
+ return false;
+
+ /* Handle the case that the blockvector has no address map but still has
+ "holes". For example, consider the following blockvector:
+
+ B0 0x1000 - 0x4000 (global block)
+ B1 0x1000 - 0x4000 (static block)
+ B3 0x1000 - 0x2000
+ (hole)
+ B4 0x3000 - 0x4000
+
+ In this case, the above blockvector does not contain address 0x2500 but
+ lookup (0x2500) would return the blockvector's static block.
+
+ So here we check if the returned block is a static block and if yes, still
+ return false. However, if the blockvector contains no blocks other than
+ the global and static blocks and ADDR falls into the static block,
+ conservatively return true.
+
+ See comment in find_compunit_symtab_for_pc_sect, symtab.c.
+
+ Also, note that if the blockvector in the above example would contain
+ an address map, then lookup (0x2500) would return NULL instead of
+ the static block.
+ */
+ return b != static_block () || num_blocks () == 2;
}
/* See block.h. */
--
2.51.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pushed] gdb: add unittests for blockvector lookup functions
2025-11-28 13:49 [pushed] gdb: update blockvector::lookup Jan Vrany
2025-11-28 13:49 ` [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes" Jan Vrany
@ 2025-11-28 13:49 ` Jan Vrany
2025-11-28 13:49 ` [pushed] gdb: update is_addr_in_objfile to support "dynamic" objfiles Jan Vrany
2 siblings, 0 replies; 10+ messages in thread
From: Jan Vrany @ 2025-11-28 13:49 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Vrany, Simon Marchi
This commit adds new unittest - blockvector-lookup - that tests
blockvector::lookup () and blockvector::contains ().
Approved-By: Simon Marchi <simon.marchi@efficios.com>
---
gdb/Makefile.in | 1 +
gdb/block-selftests.c | 128 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 129 insertions(+)
create mode 100644 gdb/block-selftests.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 538d10a824d..ed29f1703cd 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -451,6 +451,7 @@ SUBDIR_PYTHON_LDFLAGS =
SUBDIR_PYTHON_CFLAGS =
SELFTESTS_SRCS = \
+ block-selftests.c \
disasm-selftests.c \
gdbarch-selftests.c \
selftest-arch.c \
diff --git a/gdb/block-selftests.c b/gdb/block-selftests.c
new file mode 100644
index 00000000000..f5883f18660
--- /dev/null
+++ b/gdb/block-selftests.c
@@ -0,0 +1,128 @@
+/* Self tests for blockvectors
+
+ Copyright (C) 2025 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ 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 "gdbsupport/selftest.h"
+#include "block.h"
+#include "addrmap.h"
+
+
+namespace selftests {
+
+/* Create and add new block to given blockvector. */
+static struct block *
+make_block (struct blockvector &bv, struct obstack &ob, CORE_ADDR start,
+ CORE_ADDR end, struct block *superblock = nullptr)
+{
+ auto b = new (&ob) struct block;
+ b->set_start (start);
+ b->set_end (end);
+ b->set_superblock (superblock);
+ b->set_multidict (mdict_create_linear_expandable (language_unknown));
+
+ bv.append_block (b);
+
+ return b;
+}
+
+/* A helper to create and set address map given a blockvector. */
+static void
+make_map (struct blockvector &bv, struct obstack &ob)
+{
+ 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:
+
+ B0 0x1000 - 0x4000 (global block)
+ B1 0x1000 - 0x4000 (static block)
+ 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);
+
+ auto global_block = make_block (*bv.get (), ob, 0x1000, 0x4000);
+ auto static_block = make_block (*bv.get (), ob, 0x1000, 0x4000,
+ global_block);
+ 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);
+
+ return bv;
+}
+
+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. If not, lookup () return static block.
+ contains() returns false in both cases. */
+ if (with_map)
+ SELF_CHECK (bv->lookup (0x2500) == nullptr);
+ else
+ SELF_CHECK (bv->lookup (0x2500) == bv->block (STATIC_BLOCK));
+ SELF_CHECK (bv->contains (0x2500) == false);
+
+ /* 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 */
+
+
+INIT_GDB_FILE (block_selftest)
+{
+ selftests::register_test ("blockvector-lookup-contains",
+ selftests::test_blockvector_lookup_contains);
+}
--
2.51.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [pushed] gdb: update is_addr_in_objfile to support "dynamic" objfiles
2025-11-28 13:49 [pushed] gdb: update blockvector::lookup Jan Vrany
2025-11-28 13:49 ` [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes" Jan Vrany
2025-11-28 13:49 ` [pushed] gdb: add unittests for blockvector lookup functions Jan Vrany
@ 2025-11-28 13:49 ` Jan Vrany
2 siblings, 0 replies; 10+ messages in thread
From: Jan Vrany @ 2025-11-28 13:49 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Vrany
While working with objfiles in Python I noticed that
gdb.Progspace.objfile_for_address () does not return "dynamic" objfiles
created by (for example) GDB's JIT reader API.
This is because is_addr_in_objfile() checks if a given address falls into
any (mappped) section of that objfile. However objfiles created by JIT
reader API do not have sections.
To solve this issue, this commit updates is_addr_in_objfile() to also
check if the address fall into any compunit in that objfile. It does so
only if the objfile has no sections.
---
gdb/objfiles.c | 24 +++++++++++++++++++-----
gdb/objfiles.h | 2 +-
gdb/symtab.c | 8 ++++++++
gdb/symtab.h | 3 +++
gdb/testsuite/gdb.base/jit-reader.exp | 9 +++++++++
5 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 0d166ceec4c..5c5b04c6458 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -1084,14 +1084,28 @@ is_addr_in_objfile (CORE_ADDR addr, const struct objfile *objfile)
if (objfile == NULL)
return false;
- for (obj_section &osect : objfile->sections ())
+ if (objfile->sections_start == nullptr)
{
- if (section_is_overlay (&osect) && !section_is_mapped (&osect))
- continue;
+ /* Objfiles created dynamically by the JIT reader API (and possibly by
+ other means too) do not have sections. For such "dynamic" objfiles
+ walk over all compunits and check if any of them contains given
+ ADDR. */
+ for (const compunit_symtab &cu : objfile->compunits ())
+ if (cu.contains (addr))
+ return true;
+ }
+ else
+ {
+ for (obj_section &osect : objfile->sections ())
+ {
+ if (section_is_overlay (&osect) && !section_is_mapped (&osect))
+ continue;
- if (osect.contains (addr))
- return true;
+ if (osect.contains (addr))
+ return true;
+ }
}
+
return false;
}
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 0dd3fafac03..f65f3bde930 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -476,7 +476,7 @@ struct objfile : intrusive_list_node<objfile>
/* A range adapter that makes it possible to iterate over all
compunits in one objfile. */
- compunit_symtab_range compunits ()
+ compunit_symtab_range compunits () const
{
auto begin = compunit_symtab_iterator (compunit_symtabs.begin ());
auto end = compunit_symtab_iterator (compunit_symtabs.end ());
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 701aa5d0007..5754d944b6c 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -513,6 +513,14 @@ compunit_symtab::symbol_at_address (CORE_ADDR addr) const
/* See symtab.h. */
+bool
+compunit_symtab::contains (CORE_ADDR addr) const
+{
+ return blockvector ()->contains (addr);
+}
+
+/* See symtab.h. */
+
compunit_symtab::compunit_symtab (struct objfile *objfile,
const char *name_)
: m_objfile (objfile),
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 33cf0a6c229..45aca07bc36 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1964,6 +1964,9 @@ struct compunit_symtab : intrusive_list_node<compunit_symtab>
for ADDR are considered. */
struct symbol *symbol_at_address (CORE_ADDR addr) const;
+ /* True if ADDR is in this compunit_symtab, false otherwise. */
+ bool contains (CORE_ADDR addr) const;
+
/* Object file from which this symtab information was read. */
struct objfile *m_objfile;
diff --git a/gdb/testsuite/gdb.base/jit-reader.exp b/gdb/testsuite/gdb.base/jit-reader.exp
index cd844ca75d2..df2dd74a6ab 100644
--- a/gdb/testsuite/gdb.base/jit-reader.exp
+++ b/gdb/testsuite/gdb.base/jit-reader.exp
@@ -234,6 +234,15 @@ proc jit_reader_test {} {
gdb_test "python print( \[o for o in gdb.objfiles() if o.filename.startswith('<< JIT compiled code')\]\[0\].build_id )" \
"None" \
"python gdb.Objfile.build_id"
+
+ # Check that Progspace.objfile_for_address () finds "jitted"
+ # objfile
+ gdb_test "frame 0" \
+ "#0 $hex in jit_function_stack_mangle ()$any" \
+ "select frame 0"
+ gdb_test "python print( gdb.current_progspace().objfile_for_address(gdb.parse_and_eval('\$pc')) )" \
+ "<gdb.Objfile filename=<< JIT compiled code at $hex >>>" \
+ "python gdb.Progspace.objfile_for_address"
}
}
}
--
2.51.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes"
2025-11-28 13:49 ` [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes" Jan Vrany
@ 2025-12-02 17:41 ` Tom Tromey
2025-12-02 21:58 ` Jan Vraný
0 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2025-12-02 17:41 UTC (permalink / raw)
To: Jan Vrany; +Cc: gdb-patches, Simon Marchi
>>>>> "Jan" == Jan Vrany <jan.vrany@labware.com> writes:
Jan> Finally, I was considering of making this change up in lookup method
Jan> but in the end decided to be bit more conservative because comment in
Jan> original find_block_in_blockvector() suggested that returning a static
Jan> block from there is an expected situation.
FWIW it seems to me that the blockvector should just have a single
lookup function, and it should be used to find precisely the code block
containing the given address. That is, it should never return the
static or global block, since those aren't really "code" but instead
just containing scopes. This is the direction I was trying to head by
removing calls to map(); the one remaining call is one of these weird
ones...
Anyway I consider this important for supporting expandable blockvectors,
which in turn is important for lazy CU expansion.
Tom
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes"
2025-12-02 17:41 ` Tom Tromey
@ 2025-12-02 21:58 ` Jan Vraný
2025-12-03 19:36 ` Tom Tromey
0 siblings, 1 reply; 10+ messages in thread
From: Jan Vraný @ 2025-12-02 21:58 UTC (permalink / raw)
To: tom; +Cc: gdb-patches, simon.marchi
On Tue, 2025-12-02 at 10:41 -0700, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vrany <jan.vrany@labware.com> writes:
>
> Jan> Finally, I was considering of making this change up in lookup method
> Jan> but in the end decided to be bit more conservative because comment in
> Jan> original find_block_in_blockvector() suggested that returning a static
> Jan> block from there is an expected situation.
>
> FWIW it seems to me that the blockvector should just have a single
> lookup function, and it should be used to find precisely the code block
> containing the given address. That is, it should never return the
> static or global block, since those aren't really "code" but instead
> just containing scopes. This is the direction I was trying to head by
> removing calls to map(); the one remaining call is one of these weird
> ones...
I agree, I just do not really understand why there was the difference in first place.
In fact, it seems that it matters - I've got a report that this commit caused
regression on arm (still investigating, I do not have armhf system at hand, so
need to set it up first).
Jan
>
> Anyway I consider this important for supporting expandable blockvectors,
> which in turn is important for lazy CU expansion.
>
> Tom
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes"
2025-12-02 21:58 ` Jan Vraný
@ 2025-12-03 19:36 ` Tom Tromey
2025-12-03 21:31 ` Jan Vraný
0 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2025-12-03 19:36 UTC (permalink / raw)
To: Jan Vraný; +Cc: tom, gdb-patches, simon.marchi
>>>>> "Jan" == Jan Vraný <Jan.Vrany@labware.com> writes:
>> FWIW it seems to me that the blockvector should just have a single
>> lookup function, and it should be used to find precisely the code block
>> containing the given address. That is, it should never return the
>> static or global block, since those aren't really "code" but instead
>> just containing scopes. This is the direction I was trying to head by
>> removing calls to map(); the one remaining call is one of these weird
>> ones...
Jan> I agree, I just do not really understand why there was the
Jan> difference in first place.
I don't really, either. That code is pretty old, though, and some of
the older code is pretty questionable. Like sometimes problems with
debug readers were worked around in core code rather than being solved
in the reader, abstractions were very leaky (or in this case
nonexistent), etc.
Jan> In fact, it seems that it matters - I've got a report that this
Jan> commit caused regression on arm (still investigating, I do not have
Jan> armhf system at hand, so need to set it up first).
My current attempt at cleanups here also run into some regressions that
I wasn't really expecting :(
Anyway I'd find it interesting to learn what the ARM regression is
about.
Tom
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes"
2025-12-03 19:36 ` Tom Tromey
@ 2025-12-03 21:31 ` Jan Vraný
2025-12-04 11:42 ` Tom de Vries
0 siblings, 1 reply; 10+ messages in thread
From: Jan Vraný @ 2025-12-03 21:31 UTC (permalink / raw)
To: tom; +Cc: gdb-patches, simon.marchi
On Wed, 2025-12-03 at 12:36 -0700, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vraný <Jan.Vrany@labware.com> writes:
>
> > > FWIW it seems to me that the blockvector should just have a single
> > > lookup function, and it should be used to find precisely the code block
> > > containing the given address. That is, it should never return the
> > > static or global block, since those aren't really "code" but instead
> > > just containing scopes. This is the direction I was trying to head by
> > > removing calls to map(); the one remaining call is one of these weird
> > > ones...
>
> Jan> I agree, I just do not really understand why there was the
> Jan> difference in first place.
>
> I don't really, either. That code is pretty old, though, and some of
> the older code is pretty questionable. Like sometimes problems with
> debug readers were worked around in core code rather than being solved
> in the reader, abstractions were very leaky (or in this case
> nonexistent), etc.
>
> Jan> In fact, it seems that it matters - I've got a report that this
> Jan> commit caused regression on arm (still investigating, I do not have
> Jan> armhf system at hand, so need to set it up first).
>
> My current attempt at cleanups here also run into some regressions that
> I wasn't really expecting :(
>
> Anyway I'd find it interesting to learn what the ARM regression is
> about.
Yeah, it's weird. The report is here:
https://linaro.atlassian.net/browse/GNU-1767
I did not yet managed to reproduce it. It does not seem to manifest Debian
armhf running on QEMU. I'm not familiar with ARM ecosystem myself and
I do not know yet how to arrive at working "armv8l" system. I'll let you
know when I learn more but just now I'm running out of ideas on how to
debug this :(
Jan
>
> Tom
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes"
2025-12-03 21:31 ` Jan Vraný
@ 2025-12-04 11:42 ` Tom de Vries
2025-12-04 15:03 ` Jan Vraný
0 siblings, 1 reply; 10+ messages in thread
From: Tom de Vries @ 2025-12-04 11:42 UTC (permalink / raw)
To: Jan Vraný, tom; +Cc: gdb-patches, simon.marchi
On 12/3/25 10:31 PM, Jan Vraný wrote:
> On Wed, 2025-12-03 at 12:36 -0700, Tom Tromey wrote:
>>>>>>> "Jan" == Jan Vraný <Jan.Vrany@labware.com> writes:
>>
>>>> FWIW it seems to me that the blockvector should just have a single
>>>> lookup function, and it should be used to find precisely the code block
>>>> containing the given address. That is, it should never return the
>>>> static or global block, since those aren't really "code" but instead
>>>> just containing scopes. This is the direction I was trying to head by
>>>> removing calls to map(); the one remaining call is one of these weird
>>>> ones...
>>
>> Jan> I agree, I just do not really understand why there was the
>> Jan> difference in first place.
>>
>> I don't really, either. That code is pretty old, though, and some of
>> the older code is pretty questionable. Like sometimes problems with
>> debug readers were worked around in core code rather than being solved
>> in the reader, abstractions were very leaky (or in this case
>> nonexistent), etc.
>>
>> Jan> In fact, it seems that it matters - I've got a report that this
>> Jan> commit caused regression on arm (still investigating, I do not have
>> Jan> armhf system at hand, so need to set it up first).
>>
>> My current attempt at cleanups here also run into some regressions that
>> I wasn't really expecting :(
>>
>> Anyway I'd find it interesting to learn what the ARM regression is
>> about.
>
> Yeah, it's weird. The report is here:
>
> https://linaro.atlassian.net/browse/GNU-1767
>
> I did not yet managed to reproduce it. It does not seem to manifest Debian
> armhf running on QEMU. I'm not familiar with ARM ecosystem myself and
> I do not know yet how to arrive at working "armv8l" system. I'll let you
> know when I learn more but just now I'm running out of ideas on how to
> debug this :(
>
I bisected this PR (
https://sourceware.org/bugzilla/show_bug.cgi?id=33679 ) to this commit.
Are the regressions you mention the same issue?
Thanks,
- Tom
> Jan
>
>>
>> Tom
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes"
2025-12-04 11:42 ` Tom de Vries
@ 2025-12-04 15:03 ` Jan Vraný
0 siblings, 0 replies; 10+ messages in thread
From: Jan Vraný @ 2025-12-04 15:03 UTC (permalink / raw)
To: tom, tdevries; +Cc: gdb-patches, simon.marchi
On Thu, 2025-12-04 at 12:42 +0100, Tom de Vries wrote:
> On 12/3/25 10:31 PM, Jan Vraný wrote:
> > On Wed, 2025-12-03 at 12:36 -0700, Tom Tromey wrote:
> > > > > > > > "Jan" == Jan Vraný <Jan.Vrany@labware.com> writes:
> > >
> > > > > FWIW it seems to me that the blockvector should just have a single
> > > > > lookup function, and it should be used to find precisely the code block
> > > > > containing the given address. That is, it should never return the
> > > > > static or global block, since those aren't really "code" but instead
> > > > > just containing scopes. This is the direction I was trying to head by
> > > > > removing calls to map(); the one remaining call is one of these weird
> > > > > ones...
> > >
> > > Jan> I agree, I just do not really understand why there was the
> > > Jan> difference in first place.
> > >
> > > I don't really, either. That code is pretty old, though, and some of
> > > the older code is pretty questionable. Like sometimes problems with
> > > debug readers were worked around in core code rather than being solved
> > > in the reader, abstractions were very leaky (or in this case
> > > nonexistent), etc.
> > >
> > > Jan> In fact, it seems that it matters - I've got a report that this
> > > Jan> commit caused regression on arm (still investigating, I do not have
> > > Jan> armhf system at hand, so need to set it up first).
> > >
> > > My current attempt at cleanups here also run into some regressions that
> > > I wasn't really expecting :(
> > >
> > > Anyway I'd find it interesting to learn what the ARM regression is
> > > about.
> >
> > Yeah, it's weird. The report is here:
> >
> > https://linaro.atlassian.net/browse/GNU-1767
> >
> > I did not yet managed to reproduce it. It does not seem to manifest Debian
> > armhf running on QEMU. I'm not familiar with ARM ecosystem myself and
> > I do not know yet how to arrive at working "armv8l" system. I'll let you
> > know when I learn more but just now I'm running out of ideas on how to
> > debug this :(
> >
>
> I bisected this PR (
> https://sourceware.org/bugzilla/show_bug.cgi?id=33679 ) to this commit.
>
> Are the regressions you mention the same issue?
Yes! "gdb.base/annota1.exp: send SIGUSR1 (timeout)" is one of the failing tests.
Thanks a lot!
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-12-04 15:04 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-28 13:49 [pushed] gdb: update blockvector::lookup Jan Vrany
2025-11-28 13:49 ` [pushed] gdb: change blockvector::contains() to handle blockvectors with "holes" Jan Vrany
2025-12-02 17:41 ` Tom Tromey
2025-12-02 21:58 ` Jan Vraný
2025-12-03 19:36 ` Tom Tromey
2025-12-03 21:31 ` Jan Vraný
2025-12-04 11:42 ` Tom de Vries
2025-12-04 15:03 ` Jan Vraný
2025-11-28 13:49 ` [pushed] gdb: add unittests for blockvector lookup functions Jan Vrany
2025-11-28 13:49 ` [pushed] gdb: update is_addr_in_objfile to support "dynamic" objfiles Jan Vrany
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox