Summary: | [gdb] out of bounds access in objfile::section_offset | ||
---|---|---|---|
Product: | gdb | Reporter: | Tom de Vries <vries> |
Component: | gdb | Assignee: | Not yet assigned to anyone <unassigned> |
Status: | NEW --- | ||
Severity: | normal | ||
Priority: | P2 | ||
Version: | HEAD | ||
Target Milestone: | --- | ||
Host: | Target: | ||
Build: | Last reconfirmed: | ||
Attachments: |
WIP patch
tentative patch |
Description
Tom de Vries
2022-06-28 06:45:16 UTC
Say we set a breakpoint in gdb_bfd_section_index at the return here: ... else if (section == bfd_ind_section_ptr) return bfd_count_sections (abfd) + 3; ... When stopped there we see: ... (gdb) p abfd.section_count $4 = 6 (gdb) p bfd_count_sections (abfd) $1 = 6 (gdb) p gdb_bfd_count_sections (abfd) $5 = 10 ... so we use index 9 to get the ind section, which falls in the 0..9 range of the 10 sections counted by gdb. Now set a watchpoint on the bfd section count: ... (gdb) watch -l abfd.section_count Hardware watchpoint 2: -location abfd.section_count ... and we see: ... Old value = 6 New value = 7 bfd_section_init (abfd=0x2cc7100, newsect=0x361f758) at /home/vries/gdb_versions/devel/src/bfd/section.c:834 834 bfd_section_list_append (abfd, newsect); ... and: ... (gdb) bt #0 bfd_section_init (abfd=0x2cc7100, newsect=0x361f758) at /home/vries/gdb_versions/devel/src/bfd/section.c:834 #1 0x0000000000ebccb7 in bfd_make_section_old_way (abfd=0x2cc7100, name=0x2147c87 "COMMON") at /home/vries/gdb_versions/devel/src/bfd/section.c:1122 #2 0x0000000001356804 in _bfd_generic_link_add_one_symbol (info=0x7fffffffc770, abfd=0x2cc7100, name=0x3629cd9 "p_struct", flags=65536, section=0x28dd940 <_bfd_std_section>, value=8, string=0x3629cd9 "p_struct", copy=false, collect=false, hashp=0x7fffffffc5a8) at /home/vries/gdb_versions/devel/src/bfd/linker.c:1606 #3 0x0000000001355fe3 in generic_link_add_symbol_list (abfd=0x2cc7100, info=0x7fffffffc770, symbol_count=14, symbols=0x3629ce8) at /home/vries/gdb_versions/devel/src/bfd/linker.c:1195 #4 0x00000000013557e3 in generic_link_add_object_symbols (abfd=0x2cc7100, info=0x7fffffffc770) at /home/vries/gdb_versions/devel/src/bfd/linker.c:888 #5 0x000000000135574c in _bfd_generic_link_add_symbols (abfd=0x2cc7100, info=0x7fffffffc770) at /home/vries/gdb_versions/devel/src/bfd/linker.c:861 #6 0x0000000000ebd9f9 in bfd_simple_get_relocated_section_contents (abfd=0x2cc7100, sec=0x361f3c8, outbuf=0x3628020 "``\021\006\212r\003\305\031\030\030\030\341J\205\031a\202̼\f\026\\ ]@\245*\f\334\334vhJ\221\365\003", symbol_table=0x0) at /home/vries/gdb_versions/devel/src/bfd/simple.c:289 #7 0x0000000000b67300 in default_symfile_relocate (objfile=0x2cc4bf0, sectp=0x361f3c8, buf=0x3628020 "``\021\006\212r\003\305\031\030\030\030\341J\205\031a\202̼\f\026\\ ]@\245*\f\334\334vhJ\221\365\003") at /home/vries/gdb_versions/devel/src/gdb/symfile.c:3583 #8 0x0000000000b67376 in symfile_relocate_debug_section (objfile=0x2cc4bf0, sectp=0x361f3c8, buf=0x3628020 "``\021\006\212r\003\305\031\030\030\030\341J\205\031a\202̼\f\026\\ ]@\245*\f\334\334vhJ\221\365\003") at /home/vries/gdb_versions/devel/src/gdb/symfile.c:3606 #9 0x0000000000749cb2 in dwarf2_section_info::read (this=0x362ab60, objfile=0x2cc4bf0) at /home/vries/gdb_versions/devel/src/gdb/dwarf2/section.c:174 #10 0x00000000006d3f67 in dwarf2_per_bfd::map_info_sections (this=0x362ab00, objfile=0x2cc4bf0) at /home/vries/gdb_versions/devel/src/gdb/dwarf2/read.c:1769 #11 0x00000000006e0ce6 in dwarf2_build_psymtabs_hard (per_objfile=0x2b2b2e0) at /home/vries/gdb_versions/devel/src/gdb/dwarf2/read.c:7081 #12 0x00000000006dcdcd in dwarf2_build_psymtabs (objfile=0x2cc4bf0) at /home/vries/gdb_versions/devel/src/gdb/dwarf2/read.c:5374 #13 0x00000000007186cb in cooked_index_functions::read_partial_symbols (this=0x362af30, objfile=0x2cc4bf0) at /home/vries/gdb_versions/devel/src/gdb/dwarf2/read.c:18522 #14 0x0000000000b5cfce in objfile::require_partial_symbols (this=0x2cc4bf0, verbose=false) at /home/vries/gdb_versions/devel/src/gdb/symfile-debug.c:541 #15 0x0000000000b606bb in read_symbols (objfile=0x2cc4bf0, add_flags=...) at /home/vries/gdb_versions/devel/src/gdb/symfile.c:795 #16 0x0000000000b60bcf in syms_from_objfile_1 (objfile=0x2cc4bf0, addrs=0x7fffffffcd50, add_flags=...) at /home/vries/gdb_versions/devel/src/gdb/symfile.c:968 #17 0x0000000000b60c97 in syms_from_objfile (objfile=0x2cc4bf0, addrs=0x0, add_flags=...) at /home/vries/gdb_versions/devel/src/gdb/symfile.c:985 #18 0x0000000000b6116a in symbol_file_add_with_addrs (abfd=0x2cc7100, name=0x2c5e140 "/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-icc-opaque/dw2-icc-opaque", add_flags=..., addrs=0x0, flags=..., parent=0x0) at /home/vries/gdb_versions/devel/src/gdb/symfile.c:1088 #19 0x0000000000b614e4 in symbol_file_add_from_bfd (abfd=0x2cc7100, name=0x2c5e140 "/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-icc-opaque/dw2-icc-opaque", add_flags=..., addrs=0x0, flags=..., parent=0x0) at /home/vries/gdb_versions/devel/src/gdb/symfile.c:1169 #20 0x0000000000b6153e in symbol_file_add ( name=0x2c5e140 "/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-icc-opaque/dw2-icc-opaque", add_flags=..., addrs=0x0, flags=...) at /home/vries/gdb_versions/devel/src/gdb/symfile.c:1182 #21 0x0000000000b61603 in symbol_file_add_main_1 ( args=0x2c5e140 "/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-icc-opaque/dw2-icc-opaque", add_flags=..., flags=..., reloff=0) at /home/vries/gdb_versions/devel/src/gdb/symfile.c:1205 #22 0x0000000000b628c6 in symbol_file_command ( args=0x2c9d6e5 "/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-icc-opaque/dw2-icc-opaque", from_tty=0) at /home/vries/gdb_versions/devel/src/gdb/symfile.c:1653 #23 0x000000000075d65e in file_command ( arg=0x2c9d6e5 "/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-icc-opaque/dw2-icc-opaque", from_tty=0) at /home/vries/gdb_versions/devel/src/gdb/exec.c:555 #24 0x00000000005d5854 in do_simple_func ( args=0x2c9d6e5 "/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-icc-opaque/dw2-icc-opaque", from_tty=0, c=0x2b1bb90) at /home/vries/gdb_versions/devel/src/gdb/cli/cli-decode.c:95 #25 0x00000000005da440 in cmd_func (cmd=0x2b1bb90, args=0x2c9d6e5 "/home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-icc-opaque/dw2-icc-opaque", from_tty=0) at /home/vries/gdb_versions/devel/src/gdb/cli/cli-decode.c:2514 #26 0x0000000000bd5081 in execute_command (p=0x2c9d747 "e", from_tty=0) at /home/vries/gdb_versions/devel/src/gdb/top.c:699 #27 0x000000000075a5b1 in command_handler ( command=0x2c9d6e0 "file /home/vries/gdb_versions/devel/build/gdb/testsuite/outputs/gdb.dwarf2/dw2-icc-opaque/dw2-icc-opaque") at /home/vries/gdb_versions/devel/src/gdb/event-top.c:598 #28 0x0000000000bd48bb in read_command_file (stream=0x293e2a0) at /home/vries/gdb_versions/devel/src/gdb/top.c:468 #29 0x00000000005ee481 in script_from_file (stream=0x293e2a0, file=0x7fffffffe144 "outputs/gdb.dwarf2/dw2-icc-opaque/gdb.in.1") at /home/vries/gdb_versions/devel/src/gdb/cli/cli-script.c:1625 #30 0x00000000005cd063 in source_script_from_stream (stream=0x293e2a0, file=0x7fffffffe144 "outputs/gdb.dwarf2/dw2-icc-opaque/gdb.in.1", file_to_open=0x2c19470 "outputs/gdb.dwarf2/dw2-icc-opaque/gdb.in.1") at /home/vries/gdb_versions/devel/src/gdb/cli/cli-cmds.c:715 #31 0x00000000005cd1b8 in source_script_with_search ( file=0x7fffffffe144 "outputs/gdb.dwarf2/dw2-icc-opaque/gdb.in.1", from_tty=0, search_path=0) at /home/vries/gdb_versions/devel/src/gdb/cli/cli-cmds.c:760 #32 0x00000000005cd234 in source_script ( file=0x7fffffffe144 "outputs/gdb.dwarf2/dw2-icc-opaque/gdb.in.1", from_tty=0) at /home/vries/gdb_versions/devel/src/gdb/cli/cli-cmds.c:769 #33 0x00000000008be20e in catch_command_errors ( command=0x5cd20f <source_script(char const*, int)>, arg=0x7fffffffe144 "outputs/gdb.dwarf2/dw2-icc-opaque/gdb.in.1", from_tty=0, do_bp_actions=false) at /home/vries/gdb_versions/devel/src/gdb/main.c:513 #34 0x00000000008be3a9 in execute_cmdargs (cmdarg_vec=0x7fffffffd780, file_type=CMDARG_FILE, cmd_type=CMDARG_COMMAND, ret=0x7fffffffd75c) at /home/vries/gdb_versions/devel/src/gdb/main.c:605 #35 0x00000000008bf767 in captured_main_1 (context=0x7fffffffd9c0) --Type <RET> for more, q to quit, c to continue without paging-- at /home/vries/gdb_versions/devel/src/gdb/main.c:1298 #36 0x00000000008bf96a in captured_main (data=0x7fffffffd9c0) at /home/vries/gdb_versions/devel/src/gdb/main.c:1319 #37 0x00000000008bf9d5 in gdb_main (args=0x7fffffffd9c0) at /home/vries/gdb_versions/devel/src/gdb/main.c:1344 #38 0x0000000000418b3e in main (argc=14, argv=0x7fffffffdad8) at /home/vries/gdb_versions/devel/src/gdb/gdb.c:32 ... Continuing, we run into the original breakpoint: ... Thread 1 "gdb" hit Breakpoint 1, gdb_bfd_section_index (abfd=0x2cc7100, section=0x28ddc88 <_bfd_std_section+840>) at /home/vries/gdb_versions/devel/src/gdb/gdb_bfd.c:1013 1013 return bfd_count_sections (abfd) + 3; ... which now returns index 10. But the section_offsets vector did not get an entry added for the new common section: ... (gdb) p section_offsets.size () $7 = 10 ... so we're accessing out of bounds. This fixes the assert: ... diff --git a/gdb/symfile.c b/gdb/symfile.c index 6f546f5b059..04b00a0ec9b 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -3579,8 +3579,12 @@ default_symfile_relocate (struct objfile *objfile, asection *se ctp, sect->output_section = sect; sect->output_offset = 0; } - - return bfd_simple_get_relocated_section_contents (abfd, sectp, buf, NULL); + int old = bfd_count_sections (abfd); + bfd_byte *res = bfd_simple_get_relocated_section_contents (abfd, sectp, buf, NULL ); + for (int i = old; i < bfd_count_sections (abfd); ++i) + objfile->section_offsets.insert (objfile->section_offsets.begin () + i, + (CORE_ADDR)0); + return res; } /* Relocate the contents of a debug section SECTP in ABFD. The ... I'm not sure if (CORE_ADDR)0 is the right value, but I'm not sure where to find a better value. Also, I suppose more data need updating than just section_offsets. Finally, I'm not sure if this is a good place to fix it. Maybe we should define a _new_section_hook and use that instead? It'll be worthwhile to compile with the patch adding the assert, run the test-suite and see which test-cases are affected. (In reply to Tom de Vries from comment #3) > It'll be worthwhile to compile with the patch adding the assert, run the > test-suite and see which test-cases are affected. $ grep ^FAIL: gdb.sum FAIL: gdb.base/readline-ask.exp: bell for more message (GDB internal error) FAIL: gdb.base/symbol-without-target_section.exp: print symbol_without_target_section (GDB internal error) FAIL: gdb.dwarf2/dw2-icc-opaque.exp: ptype p_struct (GDB internal error) *** Bug 25724 has been marked as a duplicate of this bug. *** *** Bug 25723 has been marked as a duplicate of this bug. *** Created attachment 14203 [details]
WIP patch
I'm now trying a less ambitious approach.
Rather than trying to completely accommodate the newly created section, just try to get the indexing right for the old sections.
So, if we start out with 6 bfd sections, meaning 10 gdb sections, then the ind section is at 9.
Then a new bfd section is added, and now it's ambiguous: does 9 refer to the ind section (6+3), or the abs section (7+2)?
We try to resolve this ambiguity by making gdb_bfd_section_index independent from bfd_count_sections:
...
else if (section == bfd_com_section_ptr)
- return bfd_count_sections (abfd);
+ return INT_MAX - 3;
else if (section == bfd_und_section_ptr)
- return bfd_count_sections (abfd) + 1;
+ return INT_MAX - 2;
else if (section == bfd_abs_section_ptr)
- return bfd_count_sections (abfd) + 2;
+ return INT_MAX - 1;
else if (section == bfd_ind_section_ptr)
- return bfd_count_sections (abfd) + 3;
+ return INT_MAX;
...
(In reply to Tom de Vries from comment #7) > Created attachment 14203 [details] > WIP patch > Well, it passes testing. It just that it's hard to make sure that the patch is complete. I tried a bit with making gdb_bfd_section_index return a gdb_bfd_section_index_t which is defined as: ... enum class gdb_bfd_section_index_t : int {}; ... such that we always known whether we have a bfd section or a gdb_bfd section index, but that looks like a lot of changes. Created attachment 14204 [details]
tentative patch
Another approach, using a side table.
Roughly the idea is to save bfd_section_count the first time is used, and then reuse it afterwards. If bfd_section_count is increased, it doesn't affect gdb_bfd_section_count.
(In reply to Tom de Vries from comment #9) > Created attachment 14204 [details] > tentative patch > > Another approach, using a side table. > > Roughly the idea is to save bfd_section_count the first time is used, and > then reuse it afterwards. If bfd_section_count is increased, it doesn't > affect gdb_bfd_section_count. https://sourceware.org/pipermail/gdb-patches/2022-July/190658.html Asked question at https://lists.gnu.org/archive/html/bug-binutils/2022-07/msg00103.html |