Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Commit

Permalink
gdb: fix regression in copy_type_recursive
Browse files Browse the repository at this point in the history
Commit 5b7d941 ("gdb: add owner-related methods to struct type")
introduced a regression when running gdb.base/jit-reader-simple.exp and
others.  A NULL pointer dereference happens here:

    #3  0x0000557b7e9e8650 in gdbarch_obstack (arch=0x0) at /home/simark/src/binutils-gdb/gdb/gdbarch.c:484
    #4  0x0000557b7ea5b138 in copy_type_recursive (objfile=0x614000006640, type=0x62100018da80, copied_types=0x62100018e280) at /home/simark/src/binutils-gdb/gdb/gdbtypes.c:5537
    #5  0x0000557b7ea5dcbb in copy_type_recursive (objfile=0x614000006640, type=0x62100018e200, copied_types=0x62100018e280) at /home/simark/src/binutils-gdb/gdb/gdbtypes.c:5598
    #6  0x0000557b802cef51 in preserve_one_value (value=0x6110000b3640, objfile=0x614000006640, copied_types=0x62100018e280) at /home/simark/src/binutils-gdb/gdb/value.c:2518
    #7  0x0000557b802cf787 in preserve_values (objfile=0x614000006640) at /home/simark/src/binutils-gdb/gdb/value.c:2562
    #8  0x0000557b7fbaf19b in reread_symbols () at /home/simark/src/binutils-gdb/gdb/symfile.c:2489
    #9  0x0000557b7ec65d1d in run_command_1 (args=0x0, from_tty=1, run_how=RUN_NORMAL) at /home/simark/src/binutils-gdb/gdb/infcmd.c:439
    #10 0x0000557b7ec67a97 in run_command (args=0x0, from_tty=1) at /home/simark/src/binutils-gdb/gdb/infcmd.c:546

This is inside a TYPE_ALLOC macro.  The fact that gdbarch_obstack is
called means that the type is flagged as being arch-owned, but arch=0x0
means that type::arch returned NULL, probably meaning that the m_owner
field contains NULL.

If we look at the code before the problematic patch, in the
copy_type_recursive function, we see:

    if (! TYPE_OBJFILE_OWNED (type))
      return type;

    ...

    TYPE_OBJFILE_OWNED (new_type) = 0;
    TYPE_OWNER (new_type).gdbarch = get_type_arch (type);

The last two lines were replaced with:

    new_type->set_owner (type->arch ());

get_type_arch and type->arch isn't the same thing: get_type_arch gets
the type's arch owner if it is arch-owned, and gets the objfile's arch
if the type is objfile owned.  So it always returns non-NULL.
type->arch returns the type's arch if the type is arch-owned, else NULL.
So since the original type is objfile owned, it effectively made the new
type arch-owned (that is good) but set the owner to NULL (that is bad).

Fix this by using get_type_arch again there.

I spotted one other similar change in lookup_array_range_type, in the
original patch.  But that one appears to be correct, as it is executed
only if the type is arch-owned.

Add some asserts in type::set_owner to ensure we never set a NULL owner.
That would have helped catch the issue a little bit earlier, so it could
help in the future.

gdb/ChangeLog:

	* gdbtypes.c (copy_type_recursive): Use get_type_arch.
	* gdbtypes.h (struct type) <set_owner>: Add asserts.

Change-Id: I5d8bc7bfc83b3abc579be0b5aadeae4241179a00
  • Loading branch information
simark committed Jan 23, 2021
1 parent d3ee35d commit dd5ca05
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 1 deletion.
5 changes: 5 additions & 0 deletions gdb/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
2021-01-23 Simon Marchi <[email protected]>

* gdbtypes.c (copy_type_recursive): Use get_type_arch.
* gdbtypes.h (struct type) <set_owner>: Add asserts.

2021-01-23 Lancelot SIX <[email protected]>

* Makefile.in (SELFTESTS_SRCS): Add
Expand Down
2 changes: 1 addition & 1 deletion gdb/gdbtypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -5518,7 +5518,7 @@ copy_type_recursive (struct objfile *objfile,
copy the entire thing and then update specific fields as needed. */
*TYPE_MAIN_TYPE (new_type) = *TYPE_MAIN_TYPE (type);

new_type->set_owner (type->arch ());
new_type->set_owner (get_type_arch (type));

if (type->name ())
new_type->set_name (xstrdup (type->name ()));
Expand Down
4 changes: 4 additions & 0 deletions gdb/gdbtypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1245,13 +1245,17 @@ struct type
/* Set the owner of the type to be OBJFILE. */
void set_owner (objfile *objfile)
{
gdb_assert (objfile != nullptr);

this->main_type->m_owner.objfile = objfile;
this->main_type->m_flag_objfile_owned = true;
}

/* Set the owner of the type to be ARCH. */
void set_owner (gdbarch *arch)
{
gdb_assert (arch != nullptr);

this->main_type->m_owner.gdbarch = arch;
this->main_type->m_flag_objfile_owned = false;
}
Expand Down

0 comments on commit dd5ca05

Please sign in to comment.