Skip to content

Commit 0e17d3f

Browse files
committed
gdb: fix owner passed to remove_target_sections in clear_solib
Commit 8971d27 ("gdb: link so_list using intrusive_list") introduced a bug in clear_solib. Instead of passing an `so_list *` to remove_target_sections, it passed an `so_list **`. This was not caught by the compiler, because remove_target_sections takes a `void *` as the "owner", so you can pass it any pointer and it won't complain. This happened because I previously had a patch to change the type of the disposer parameter to be a reference rather than a pointer, so had to change `so` to `&so`. When dropping that patch, I forgot to revert this bit and / or it got re-introduced when handling subsequent merge conflicts. And I didn't properly retest. Fix that, but try to make things less error prone. Add a union to represent the possible owner kinds for a target_section. Trying to pass a pointer to another type than those will not compile. Change-Id: I600cab5ea0408ccc5638467b760768161ca3036c
1 parent 4a6daab commit 0e17d3f

File tree

6 files changed

+51
-18
lines changed

6 files changed

+51
-18
lines changed

gdb/exec.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -492,8 +492,8 @@ exec_file_attach (const char *filename, int from_tty)
492492
/* Add the executable's sections to the current address spaces'
493493
list of sections. This possibly pushes the exec_ops
494494
target. */
495-
current_program_space->add_target_sections (&current_program_space->ebfd,
496-
sections);
495+
current_program_space->add_target_sections
496+
(current_program_space->ebfd.get (), sections);
497497

498498
/* Tell display code (if any) about the changed file name. */
499499
if (deprecated_exec_file_display_hook)
@@ -599,8 +599,8 @@ build_section_table (struct bfd *some_bfd)
599599
current set of target sections. */
600600

601601
void
602-
program_space::add_target_sections (const void *owner,
603-
const std::vector<target_section> &sections)
602+
program_space::add_target_sections
603+
(target_section_owner owner, const std::vector<target_section> &sections)
604604
{
605605
if (!sections.empty ())
606606
{
@@ -643,23 +643,23 @@ program_space::add_target_sections (struct objfile *objfile)
643643
continue;
644644

645645
m_target_sections.emplace_back (osect->addr (), osect->endaddr (),
646-
osect->the_bfd_section, (void *) objfile);
646+
osect->the_bfd_section, objfile);
647647
}
648648
}
649649

650650
/* Remove all target sections owned by OWNER.
651651
OWNER must be the same value passed to add_target_sections. */
652652

653653
void
654-
program_space::remove_target_sections (const void *owner)
654+
program_space::remove_target_sections (target_section_owner owner)
655655
{
656-
gdb_assert (owner != NULL);
656+
gdb_assert (owner.v () != nullptr);
657657

658658
auto it = std::remove_if (m_target_sections.begin (),
659659
m_target_sections.end (),
660660
[&] (target_section &sect)
661661
{
662-
return sect.owner == owner;
662+
return sect.owner.v () == owner.v ();
663663
});
664664
m_target_sections.erase (it, m_target_sections.end ());
665665

gdb/maint.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ maintenance_info_target_sections (const char *arg, int from_tty)
509509
(8 + digits), "",
510510
hex_string_custom (sec.addr, addr_size),
511511
hex_string_custom (sec.endaddr, addr_size),
512-
sec.owner);
512+
sec.owner.v ());
513513
}
514514
}
515515

gdb/progspace.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,11 @@ program_space::exec_close ()
207207
{
208208
/* Removing target sections may close the exec_ops target.
209209
Clear ebfd before doing so to prevent recursion. */
210+
bfd *saved_ebfd = ebfd.get ();
210211
ebfd.reset (nullptr);
211212
ebfd_mtime = 0;
212213

213-
remove_target_sections (&ebfd);
214+
remove_target_sections (saved_ebfd);
214215

215216
exec_filename.reset (nullptr);
216217
}

gdb/progspace.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,11 +284,11 @@ struct program_space
284284
bool empty ();
285285

286286
/* Remove all target sections owned by OWNER. */
287-
void remove_target_sections (const void *owner);
287+
void remove_target_sections (target_section_owner owner);
288288

289289
/* Add the sections array defined by SECTIONS to the
290290
current set of target sections. */
291-
void add_target_sections (const void *owner,
291+
void add_target_sections (target_section_owner owner,
292292
const std::vector<target_section> &sections);
293293

294294
/* Add the sections of OBJFILE to the current set of target

gdb/solib.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1195,7 +1195,7 @@ clear_solib (void)
11951195
current_program_space->so_list.clear_and_dispose ([] (shobj *so)
11961196
{
11971197
notify_solib_unloaded (current_program_space, *so);
1198-
current_program_space->remove_target_sections (&so);
1198+
current_program_space->remove_target_sections (so);
11991199
delete so;
12001200
});
12011201

gdb/target-section.h

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,46 @@
2020
#ifndef GDB_TARGET_SECTION_H
2121
#define GDB_TARGET_SECTION_H
2222

23+
struct bfd;
24+
struct objfile;
25+
struct shobj;
26+
27+
/* A union representing the possible owner types of a target_section. */
28+
29+
union target_section_owner
30+
{
31+
target_section_owner () : m_v (nullptr) {}
32+
target_section_owner (const bfd *bfd) : bfd (bfd) {}
33+
target_section_owner (const objfile *objfile) : objfile (objfile) {}
34+
target_section_owner (const shobj *shobj) : shobj (shobj) {}
35+
36+
/* Use this to access the type-erased version of the owner, for
37+
comparisons, printing, etc. We don't access the M_V member
38+
directly, because pedantically it is not valid to access a
39+
non-active union member. */
40+
const void *v () const
41+
{
42+
void *tmp;
43+
memcpy (&tmp, this, sizeof (*this));
44+
return tmp;
45+
}
46+
47+
const struct bfd *bfd;
48+
const struct objfile *objfile;
49+
const struct shobj *shobj;
50+
51+
private:
52+
const void *m_v;
53+
};
54+
2355
/* Struct target_section maps address ranges to file sections. It is
2456
mostly used with BFD files, but can be used without (e.g. for handling
2557
raw disks, or files not in formats handled by BFD). */
2658

2759
struct target_section
2860
{
2961
target_section (CORE_ADDR addr_, CORE_ADDR end_, struct bfd_section *sect_,
30-
void *owner_ = nullptr)
62+
target_section_owner owner_ = {})
3163
: addr (addr_),
3264
endaddr (end_),
3365
the_bfd_section (sect_),
@@ -44,11 +76,11 @@ struct target_section
4476
struct bfd_section *the_bfd_section;
4577

4678
/* The "owner" of the section.
47-
It can be any unique value. It is set by add_target_sections
48-
and used by remove_target_sections.
79+
80+
It is set by add_target_sections and used by remove_target_sections.
4981
For example, for executables it is a pointer to exec_bfd and
50-
for shlibs it is the so_list pointer. */
51-
const void *owner;
82+
for shlibs it is the shobj pointer. */
83+
target_section_owner owner;
5284
};
5385

5486
#endif /* GDB_TARGET_SECTION_H */

0 commit comments

Comments
 (0)