Fix heap-use-after-free in typename_concat

Message ID 20190503093124.GA27838@delia
State New, archived
Headers

Commit Message

Tom de Vries May 3, 2019, 9:31 a.m. UTC
  Hi,

When running gdb using AddressSanitizer, and loading a cc1plus binary built
with profiledbootstrap and -flto, we run into a heap-use-after-free error:
...
$ LD_PRELOAD=/usr/lib64/libasan.so.3 ./gdb -batch cc1plus
==26855==ERROR: AddressSanitizer: heap-use-after-free on address \
  0x62100ad8a8b0 at pc 0x7f13803cc9e3 bp 0x7ffe55b0d090 sp 0x7ffe55b0c840
READ of size 47 at 0x62100ad8a8b0 thread T0
    #0 0x7f13803cc9e2  (/usr/lib64/libasan.so.3+0x3e9e2)
    #1 0x5e7a0d in typename_concat gdb/dwarf2read.c:22661
    #2 0x5c6437 in partial_die_full_name gdb/dwarf2read.c:8876
    #3 0x5c6555 in add_partial_symbol gdb/dwarf2read.c:8893
    #4 0x5c6ecf in add_partial_subprogram gdb/dwarf2read.c:9156
    #5 0x5c5e90 in scan_partial_symbols gdb/dwarf2read.c:8668
    #6 0x5c6c0a in add_partial_namespace gdb/dwarf2read.c:9081
    #7 0x5c5f99 in scan_partial_symbols gdb/dwarf2read.c:8702
    #8 0x5c48b6 in process_psymtab_comp_unit_reader gdb/dwarf2read.c:8056
    #9 0x5c3c1f in init_cutu_and_read_dies gdb/dwarf2read.c:7689
    #10 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
    #11 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
    #12 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
    #13 0x612359 in read_psyms gdb/elfread.c:1311
    #14 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
    #15 0x867d7b in read_symbols gdb/symfile.c:821
    #16 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
    #17 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
    #18 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
    #19 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
                    enum_flags<symfile_add_flag>, std::vector<other_sections, \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
    #20 0x868b64 in symbol_file_add(char const*, \
                    enum_flags<symfile_add_flag>, \
		    std::vector<other_sections, \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>) gdb/symfile.c:1217
    #21 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
    #22 0x868bd0 in symbol_file_add_main(char const*, \
                    enum_flags<symfile_add_flag>) gdb/symfile.c:1231
    #23 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
    #24 0x71f10e in catch_command_errors gdb/main.c:372
    #25 0x71ff5f in captured_main_1 gdb/main.c:1043
    #26 0x72045d in captured_main gdb/main.c:1163
    #27 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
    #28 0x40fd7d in main gdb/gdb.c:32
    #29 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)
    #30 0x40fc89 in _start (/data/gdb_versions/devel/build/gdb/gdb+0x40fc89)

0x62100ad8a8b0 is located 944 bytes inside of 4064-byte region \
  [0x62100ad8a500,0x62100ad8b4e0)
freed by thread T0 here:
    #0 0x7f13804523a0 in __interceptor_free (/usr/lib64/libasan.so.3+0xc43a0)
    #1 0x435e44 in xfree<void> gdb/common/common-utils.h:60
    #2 0xa82c25 in call_freefun libiberty/obstack.c:103
    #3 0xa83098 in _obstack_free libiberty/obstack.c:280
    #4 0x4367da in auto_obstack::~auto_obstack() gdb/gdb_obstack.h:101
    #5 0x5ed72c in dwarf2_cu::~dwarf2_cu() gdb/dwarf2read.c:25341
    #6 0x5fb5bb in std::default_delete<dwarf2_cu>::operator()(dwarf2_cu*) const \
                   /usr/include/c++/7/bits/unique_ptr.h:78
    #7 0x5f7334 in std::unique_ptr<dwarf2_cu, \
                   std::default_delete<dwarf2_cu> >::~unique_ptr() \
		   /usr/include/c++/7/bits/unique_ptr.h:268
    #8 0x5c3ce5 in init_cutu_and_read_dies gdb/dwarf2read.c:7624
    #9 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
    #10 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
    #11 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
    #12 0x612359 in read_psyms gdb/elfread.c:1311
    #13 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
    #14 0x867d7b in read_symbols gdb/symfile.c:821
    #15 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
    #16 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
    #17 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
    #18 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
                    enum_flags<symfile_add_flag>, std::vector<other_sections,  \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
    #19 0x868b64 in symbol_file_add(char const*, \
                    enum_flags<symfile_add_flag>, std::vector<other_sections, \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>) gdb/symfile.c:1217
    #20 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
    #21 0x868bd0 in symbol_file_add_main(char const*, \
                    enum_flags<symfile_add_flag>) gdb/symfile.c:1231
    #22 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
    #23 0x71f10e in catch_command_errors gdb/main.c:372
    #24 0x71ff5f in captured_main_1 gdb/main.c:1043
    #25 0x72045d in captured_main gdb/main.c:1163
    #26 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
    #27 0x40fd7d in main gdb/gdb.c:32
    #28 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)

previously allocated by thread T0 here:
    #0 0x7f13804526b8 in __interceptor_malloc (/usr/lib64/libasan.so.3+0xc46b8)
    #1 0x5114b5 in xmalloc gdb/common/common-utils.c:44
    #2 0xa82bd5 in call_chunkfun libiberty/obstack.c:94
    #3 0xa82eda in _obstack_newchunk libiberty/obstack.c:206
    #4 0x477310 in allocate_on_obstack::operator new(unsigned long, obstack*) \
                   gdb/gdb_obstack.h:117
    #5 0x5dea8c in load_partial_dies gdb/dwarf2read.c:18571
    #6 0x5c487f in process_psymtab_comp_unit_reader gdb/dwarf2read.c:8054
    #7 0x5c3c1f in init_cutu_and_read_dies gdb/dwarf2read.c:7689
    #8 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
    #9 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
    #10 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
    #11 0x612359 in read_psyms gdb/elfread.c:1311
    #12 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
    #13 0x867d7b in read_symbols gdb/symfile.c:821
    #14 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
    #15 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
    #16 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
    #17 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
                    enum_flags<symfile_add_flag>, \
		    std::vector<other_sections, \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
    #18 0x868b64 in symbol_file_add(char const*, enum_flags<symfile_add_flag>, \
                    std::vector<other_sections, \
		    std::allocator<other_sections> >*, \
		    enum_flags<objfile_flag>) gdb/symfile.c:1217
    #19 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
    #20 0x868bd0 in symbol_file_add_main(char const*, \
                    enum_flags<symfile_add_flag>) gdb/symfile.c:1231
    #21 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
    #22 0x71f10e in catch_command_errors gdb/main.c:372
    #23 0x71ff5f in captured_main_1 gdb/main.c:1043
    #24 0x72045d in captured_main gdb/main.c:1163
    #25 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
    #26 0x40fd7d in main gdb/gdb.c:32
    #27 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)
...

This error happens as follows.

The function find_partial_die has a cu argument, but returns a pdi which may
or may not be from that cu:
...
/* Find a partial DIE at OFFSET, which may or may not be in CU,
   except in the case of .debug_types DIEs which do not reference
   outside their CU (they do however referencing other types via
   DW_FORM_ref_sig8).  */

static struct partial_die_info *
find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
...

So the pdi returned by find_partial_die here in partial_die_parent_scope may
be from another cu:
...
partial_die_parent_scope (struct partial_die_info *pdi,
                          struct dwarf2_cu *cu)
{
  const char *grandparent_scope;
  struct partial_die_info *parent, *real_pdi;

  /* We need to look at our parent DIE; if we have a DW_AT_specification,
     then this means the parent of the specification DIE.  */

  real_pdi = pdi;
  while (real_pdi->has_specification)
    real_pdi = find_partial_die (real_pdi->spec_offset,
                                 real_pdi->spec_is_dwz, cu);

  parent = real_pdi->die_parent;
...
in which case both real_pdi and parent will be not from cu, but from another
one, say cu2.

Subsequently, cu's comp_unit_obstack is used to set parent->scope:
...
        parent->scope = typename_concat (&cu->comp_unit_obstack,
                                         grandparent_scope,
                                         parent->name, 0, cu);
...

So, we use cu->comp_unit_obstack to assign a value to the scope field of
a pdi belonging to cu2, and when cu is deleted, the scope field points to a
freed value.

Fix this by making find_partial_die return the cu corresponding to the
returned pdi, and handling this at the call sites.

Tested on x86_64-linux.

OK for trunk?

Thanks,
- Tom

[gdb] Fix heap-use-after-free in typename_concat

gdb/ChangeLog:

2019-05-03  Tom de Vries  <tdevries@suse.de>

	PR gdb/24094
	* dwarf2read.c (struct cu_partial_die_info): New struct.
	(find_partial_die): Return cu_partial_die_info.
	(partial_die_parent_scope, guess_partial_die_structure_name)
	(partial_die_info::fixup): Handle new return type of find_partial_die.

---
 gdb/dwarf2read.c | 49 +++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 39 insertions(+), 10 deletions(-)
  

Comments

Tom de Vries May 16, 2019, 8:17 a.m. UTC | #1
On 03-05-19 11:31, Tom de Vries wrote:
> Hi,
> 
> When running gdb using AddressSanitizer, and loading a cc1plus binary built
> with profiledbootstrap and -flto, we run into a heap-use-after-free error:
> ...
> $ LD_PRELOAD=/usr/lib64/libasan.so.3 ./gdb -batch cc1plus
> ==26855==ERROR: AddressSanitizer: heap-use-after-free on address \
>   0x62100ad8a8b0 at pc 0x7f13803cc9e3 bp 0x7ffe55b0d090 sp 0x7ffe55b0c840
> READ of size 47 at 0x62100ad8a8b0 thread T0
>     #0 0x7f13803cc9e2  (/usr/lib64/libasan.so.3+0x3e9e2)
>     #1 0x5e7a0d in typename_concat gdb/dwarf2read.c:22661
>     #2 0x5c6437 in partial_die_full_name gdb/dwarf2read.c:8876
>     #3 0x5c6555 in add_partial_symbol gdb/dwarf2read.c:8893
>     #4 0x5c6ecf in add_partial_subprogram gdb/dwarf2read.c:9156
>     #5 0x5c5e90 in scan_partial_symbols gdb/dwarf2read.c:8668
>     #6 0x5c6c0a in add_partial_namespace gdb/dwarf2read.c:9081
>     #7 0x5c5f99 in scan_partial_symbols gdb/dwarf2read.c:8702
>     #8 0x5c48b6 in process_psymtab_comp_unit_reader gdb/dwarf2read.c:8056
>     #9 0x5c3c1f in init_cutu_and_read_dies gdb/dwarf2read.c:7689
>     #10 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
>     #11 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
>     #12 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
>     #13 0x612359 in read_psyms gdb/elfread.c:1311
>     #14 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
>     #15 0x867d7b in read_symbols gdb/symfile.c:821
>     #16 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
>     #17 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
>     #18 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
>     #19 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
>                     enum_flags<symfile_add_flag>, std::vector<other_sections, \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
>     #20 0x868b64 in symbol_file_add(char const*, \
>                     enum_flags<symfile_add_flag>, \
> 		    std::vector<other_sections, \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>) gdb/symfile.c:1217
>     #21 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
>     #22 0x868bd0 in symbol_file_add_main(char const*, \
>                     enum_flags<symfile_add_flag>) gdb/symfile.c:1231
>     #23 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
>     #24 0x71f10e in catch_command_errors gdb/main.c:372
>     #25 0x71ff5f in captured_main_1 gdb/main.c:1043
>     #26 0x72045d in captured_main gdb/main.c:1163
>     #27 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
>     #28 0x40fd7d in main gdb/gdb.c:32
>     #29 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)
>     #30 0x40fc89 in _start (/data/gdb_versions/devel/build/gdb/gdb+0x40fc89)
> 
> 0x62100ad8a8b0 is located 944 bytes inside of 4064-byte region \
>   [0x62100ad8a500,0x62100ad8b4e0)
> freed by thread T0 here:
>     #0 0x7f13804523a0 in __interceptor_free (/usr/lib64/libasan.so.3+0xc43a0)
>     #1 0x435e44 in xfree<void> gdb/common/common-utils.h:60
>     #2 0xa82c25 in call_freefun libiberty/obstack.c:103
>     #3 0xa83098 in _obstack_free libiberty/obstack.c:280
>     #4 0x4367da in auto_obstack::~auto_obstack() gdb/gdb_obstack.h:101
>     #5 0x5ed72c in dwarf2_cu::~dwarf2_cu() gdb/dwarf2read.c:25341
>     #6 0x5fb5bb in std::default_delete<dwarf2_cu>::operator()(dwarf2_cu*) const \
>                    /usr/include/c++/7/bits/unique_ptr.h:78
>     #7 0x5f7334 in std::unique_ptr<dwarf2_cu, \
>                    std::default_delete<dwarf2_cu> >::~unique_ptr() \
> 		   /usr/include/c++/7/bits/unique_ptr.h:268
>     #8 0x5c3ce5 in init_cutu_and_read_dies gdb/dwarf2read.c:7624
>     #9 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
>     #10 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
>     #11 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
>     #12 0x612359 in read_psyms gdb/elfread.c:1311
>     #13 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
>     #14 0x867d7b in read_symbols gdb/symfile.c:821
>     #15 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
>     #16 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
>     #17 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
>     #18 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
>                     enum_flags<symfile_add_flag>, std::vector<other_sections,  \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
>     #19 0x868b64 in symbol_file_add(char const*, \
>                     enum_flags<symfile_add_flag>, std::vector<other_sections, \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>) gdb/symfile.c:1217
>     #20 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
>     #21 0x868bd0 in symbol_file_add_main(char const*, \
>                     enum_flags<symfile_add_flag>) gdb/symfile.c:1231
>     #22 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
>     #23 0x71f10e in catch_command_errors gdb/main.c:372
>     #24 0x71ff5f in captured_main_1 gdb/main.c:1043
>     #25 0x72045d in captured_main gdb/main.c:1163
>     #26 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
>     #27 0x40fd7d in main gdb/gdb.c:32
>     #28 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)
> 
> previously allocated by thread T0 here:
>     #0 0x7f13804526b8 in __interceptor_malloc (/usr/lib64/libasan.so.3+0xc46b8)
>     #1 0x5114b5 in xmalloc gdb/common/common-utils.c:44
>     #2 0xa82bd5 in call_chunkfun libiberty/obstack.c:94
>     #3 0xa82eda in _obstack_newchunk libiberty/obstack.c:206
>     #4 0x477310 in allocate_on_obstack::operator new(unsigned long, obstack*) \
>                    gdb/gdb_obstack.h:117
>     #5 0x5dea8c in load_partial_dies gdb/dwarf2read.c:18571
>     #6 0x5c487f in process_psymtab_comp_unit_reader gdb/dwarf2read.c:8054
>     #7 0x5c3c1f in init_cutu_and_read_dies gdb/dwarf2read.c:7689
>     #8 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
>     #9 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
>     #10 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
>     #11 0x612359 in read_psyms gdb/elfread.c:1311
>     #12 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
>     #13 0x867d7b in read_symbols gdb/symfile.c:821
>     #14 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
>     #15 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
>     #16 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
>     #17 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
>                     enum_flags<symfile_add_flag>, \
> 		    std::vector<other_sections, \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
>     #18 0x868b64 in symbol_file_add(char const*, enum_flags<symfile_add_flag>, \
>                     std::vector<other_sections, \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>) gdb/symfile.c:1217
>     #19 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
>     #20 0x868bd0 in symbol_file_add_main(char const*, \
>                     enum_flags<symfile_add_flag>) gdb/symfile.c:1231
>     #21 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
>     #22 0x71f10e in catch_command_errors gdb/main.c:372
>     #23 0x71ff5f in captured_main_1 gdb/main.c:1043
>     #24 0x72045d in captured_main gdb/main.c:1163
>     #25 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
>     #26 0x40fd7d in main gdb/gdb.c:32
>     #27 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)
> ...
> 
> This error happens as follows.
> 
> The function find_partial_die has a cu argument, but returns a pdi which may
> or may not be from that cu:
> ...
> /* Find a partial DIE at OFFSET, which may or may not be in CU,
>    except in the case of .debug_types DIEs which do not reference
>    outside their CU (they do however referencing other types via
>    DW_FORM_ref_sig8).  */
> 
> static struct partial_die_info *
> find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
> ...
> 
> So the pdi returned by find_partial_die here in partial_die_parent_scope may
> be from another cu:
> ...
> partial_die_parent_scope (struct partial_die_info *pdi,
>                           struct dwarf2_cu *cu)
> {
>   const char *grandparent_scope;
>   struct partial_die_info *parent, *real_pdi;
> 
>   /* We need to look at our parent DIE; if we have a DW_AT_specification,
>      then this means the parent of the specification DIE.  */
> 
>   real_pdi = pdi;
>   while (real_pdi->has_specification)
>     real_pdi = find_partial_die (real_pdi->spec_offset,
>                                  real_pdi->spec_is_dwz, cu);
> 
>   parent = real_pdi->die_parent;
> ...
> in which case both real_pdi and parent will be not from cu, but from another
> one, say cu2.
> 
> Subsequently, cu's comp_unit_obstack is used to set parent->scope:
> ...
>         parent->scope = typename_concat (&cu->comp_unit_obstack,
>                                          grandparent_scope,
>                                          parent->name, 0, cu);
> ...
> 
> So, we use cu->comp_unit_obstack to assign a value to the scope field of
> a pdi belonging to cu2, and when cu is deleted, the scope field points to a
> freed value.
> 
> Fix this by making find_partial_die return the cu corresponding to the
> returned pdi, and handling this at the call sites.
> 
> Tested on x86_64-linux.
> 
> OK for trunk?
> 

Ping.

Thanks,
- Tom

> [gdb] Fix heap-use-after-free in typename_concat
> 
> gdb/ChangeLog:
> 
> 2019-05-03  Tom de Vries  <tdevries@suse.de>
> 
> 	PR gdb/24094
> 	* dwarf2read.c (struct cu_partial_die_info): New struct.
> 	(find_partial_die): Return cu_partial_die_info.
> 	(partial_die_parent_scope, guess_partial_die_structure_name)
> 	(partial_die_info::fixup): Handle new return type of find_partial_die.
> 
> ---
>  gdb/dwarf2read.c | 49 +++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index b0bdecf96f..442b618f6e 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -1518,8 +1518,14 @@ static unsigned int peek_abbrev_code (bfd *, const gdb_byte *);
>  static struct partial_die_info *load_partial_dies
>    (const struct die_reader_specs *, const gdb_byte *, int);
>  
> -static struct partial_die_info *find_partial_die (sect_offset, int,
> -						  struct dwarf2_cu *);
> +struct cu_partial_die_info
> +{
> +  struct dwarf2_cu *cu;
> +  struct partial_die_info *pdi;
> +};
> +
> +static struct cu_partial_die_info find_partial_die (sect_offset, int,
> +						    struct dwarf2_cu *);
>  
>  static const gdb_byte *read_attribute (const struct die_reader_specs *,
>  				       struct attribute *, struct attr_abbrev *,
> @@ -8771,14 +8777,19 @@ partial_die_parent_scope (struct partial_die_info *pdi,
>  {
>    const char *grandparent_scope;
>    struct partial_die_info *parent, *real_pdi;
> +  struct cu_partial_die_info res;
>  
>    /* We need to look at our parent DIE; if we have a DW_AT_specification,
>       then this means the parent of the specification DIE.  */
>  
>    real_pdi = pdi;
>    while (real_pdi->has_specification)
> -    real_pdi = find_partial_die (real_pdi->spec_offset,
> -				 real_pdi->spec_is_dwz, cu);
> +    {
> +      res = find_partial_die (real_pdi->spec_offset,
> +			      real_pdi->spec_is_dwz, cu);
> +      real_pdi = res.pdi;
> +      cu = res.cu;
> +    }
>  
>    parent = real_pdi->die_parent;
>    if (parent == NULL)
> @@ -18922,7 +18933,7 @@ dwarf2_cu::find_partial_die (sect_offset sect_off)
>     outside their CU (they do however referencing other types via
>     DW_FORM_ref_sig8).  */
>  
> -static struct partial_die_info *
> +static struct cu_partial_die_info
>  find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
>  {
>    struct dwarf2_per_objfile *dwarf2_per_objfile
> @@ -18936,7 +18947,12 @@ find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
>      {
>        pd = cu->find_partial_die (sect_off);
>        if (pd != NULL)
> -	return pd;
> +	{
> +	  struct cu_partial_die_info res;
> +	  res.pdi = pd;
> +	  res.cu = cu;
> +	  return res;
> +	}
>        /* We missed recording what we needed.
>  	 Load all dies and try again.  */
>        per_cu = cu->per_cu;
> @@ -18984,7 +19000,12 @@ find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
>  		    _("could not find partial DIE %s "
>  		      "in cache [from module %s]\n"),
>  		    sect_offset_str (sect_off), bfd_get_filename (objfile->obfd));
> -  return pd;
> +  {
> +    struct cu_partial_die_info res;
> +    res.pdi = pd;
> +    res.cu = per_cu->cu;
> +    return res;
> +  }
>  }
>  
>  /* See if we can figure out if the class lives in a namespace.  We do
> @@ -19003,6 +19024,7 @@ guess_partial_die_structure_name (struct partial_die_info *struct_pdi,
>  
>    struct partial_die_info *real_pdi;
>    struct partial_die_info *child_pdi;
> +  struct cu_partial_die_info res;
>  
>    /* If this DIE (this DIE's specification, if any) has a parent, then
>       we should not do this.  We'll prepend the parent's fully qualified
> @@ -19010,8 +19032,12 @@ guess_partial_die_structure_name (struct partial_die_info *struct_pdi,
>  
>    real_pdi = struct_pdi;
>    while (real_pdi->has_specification)
> -    real_pdi = find_partial_die (real_pdi->spec_offset,
> -				 real_pdi->spec_is_dwz, cu);
> +    {
> +      res = find_partial_die (real_pdi->spec_offset,
> +			      real_pdi->spec_is_dwz, cu);
> +      real_pdi = res.pdi;
> +      cu = res.cu;
> +    }
>  
>    if (real_pdi->die_parent != NULL)
>      return;
> @@ -19056,8 +19082,11 @@ partial_die_info::fixup (struct dwarf2_cu *cu)
>    if (name == NULL && has_specification)
>      {
>        struct partial_die_info *spec_die;
> +      struct cu_partial_die_info res;
>  
> -      spec_die = find_partial_die (spec_offset, spec_is_dwz, cu);
> +      res = find_partial_die (spec_offset, spec_is_dwz, cu);
> +      spec_die = res.pdi;
> +      cu = res.cu;
>  
>        spec_die->fixup (cu);
>  
>
  
Andrew Burgess May 16, 2019, 3:37 p.m. UTC | #2
* Tom de Vries <tdevries@suse.de> [2019-05-03 11:31:26 +0200]:

> Hi,
> 
> When running gdb using AddressSanitizer, and loading a cc1plus binary built
> with profiledbootstrap and -flto, we run into a heap-use-after-free error:
> ...
> $ LD_PRELOAD=/usr/lib64/libasan.so.3 ./gdb -batch cc1plus
> ==26855==ERROR: AddressSanitizer: heap-use-after-free on address \
>   0x62100ad8a8b0 at pc 0x7f13803cc9e3 bp 0x7ffe55b0d090 sp 0x7ffe55b0c840
> READ of size 47 at 0x62100ad8a8b0 thread T0
>     #0 0x7f13803cc9e2  (/usr/lib64/libasan.so.3+0x3e9e2)
>     #1 0x5e7a0d in typename_concat gdb/dwarf2read.c:22661
>     #2 0x5c6437 in partial_die_full_name gdb/dwarf2read.c:8876
>     #3 0x5c6555 in add_partial_symbol gdb/dwarf2read.c:8893
>     #4 0x5c6ecf in add_partial_subprogram gdb/dwarf2read.c:9156
>     #5 0x5c5e90 in scan_partial_symbols gdb/dwarf2read.c:8668
>     #6 0x5c6c0a in add_partial_namespace gdb/dwarf2read.c:9081
>     #7 0x5c5f99 in scan_partial_symbols gdb/dwarf2read.c:8702
>     #8 0x5c48b6 in process_psymtab_comp_unit_reader gdb/dwarf2read.c:8056
>     #9 0x5c3c1f in init_cutu_and_read_dies gdb/dwarf2read.c:7689
>     #10 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
>     #11 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
>     #12 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
>     #13 0x612359 in read_psyms gdb/elfread.c:1311
>     #14 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
>     #15 0x867d7b in read_symbols gdb/symfile.c:821
>     #16 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
>     #17 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
>     #18 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
>     #19 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
>                     enum_flags<symfile_add_flag>, std::vector<other_sections, \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
>     #20 0x868b64 in symbol_file_add(char const*, \
>                     enum_flags<symfile_add_flag>, \
> 		    std::vector<other_sections, \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>) gdb/symfile.c:1217
>     #21 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
>     #22 0x868bd0 in symbol_file_add_main(char const*, \
>                     enum_flags<symfile_add_flag>) gdb/symfile.c:1231
>     #23 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
>     #24 0x71f10e in catch_command_errors gdb/main.c:372
>     #25 0x71ff5f in captured_main_1 gdb/main.c:1043
>     #26 0x72045d in captured_main gdb/main.c:1163
>     #27 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
>     #28 0x40fd7d in main gdb/gdb.c:32
>     #29 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)
>     #30 0x40fc89 in _start (/data/gdb_versions/devel/build/gdb/gdb+0x40fc89)
> 
> 0x62100ad8a8b0 is located 944 bytes inside of 4064-byte region \
>   [0x62100ad8a500,0x62100ad8b4e0)
> freed by thread T0 here:
>     #0 0x7f13804523a0 in __interceptor_free (/usr/lib64/libasan.so.3+0xc43a0)
>     #1 0x435e44 in xfree<void> gdb/common/common-utils.h:60
>     #2 0xa82c25 in call_freefun libiberty/obstack.c:103
>     #3 0xa83098 in _obstack_free libiberty/obstack.c:280
>     #4 0x4367da in auto_obstack::~auto_obstack() gdb/gdb_obstack.h:101
>     #5 0x5ed72c in dwarf2_cu::~dwarf2_cu() gdb/dwarf2read.c:25341
>     #6 0x5fb5bb in std::default_delete<dwarf2_cu>::operator()(dwarf2_cu*) const \
>                    /usr/include/c++/7/bits/unique_ptr.h:78
>     #7 0x5f7334 in std::unique_ptr<dwarf2_cu, \
>                    std::default_delete<dwarf2_cu> >::~unique_ptr() \
> 		   /usr/include/c++/7/bits/unique_ptr.h:268
>     #8 0x5c3ce5 in init_cutu_and_read_dies gdb/dwarf2read.c:7624
>     #9 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
>     #10 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
>     #11 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
>     #12 0x612359 in read_psyms gdb/elfread.c:1311
>     #13 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
>     #14 0x867d7b in read_symbols gdb/symfile.c:821
>     #15 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
>     #16 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
>     #17 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
>     #18 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
>                     enum_flags<symfile_add_flag>, std::vector<other_sections,  \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
>     #19 0x868b64 in symbol_file_add(char const*, \
>                     enum_flags<symfile_add_flag>, std::vector<other_sections, \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>) gdb/symfile.c:1217
>     #20 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
>     #21 0x868bd0 in symbol_file_add_main(char const*, \
>                     enum_flags<symfile_add_flag>) gdb/symfile.c:1231
>     #22 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
>     #23 0x71f10e in catch_command_errors gdb/main.c:372
>     #24 0x71ff5f in captured_main_1 gdb/main.c:1043
>     #25 0x72045d in captured_main gdb/main.c:1163
>     #26 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
>     #27 0x40fd7d in main gdb/gdb.c:32
>     #28 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)
> 
> previously allocated by thread T0 here:
>     #0 0x7f13804526b8 in __interceptor_malloc (/usr/lib64/libasan.so.3+0xc46b8)
>     #1 0x5114b5 in xmalloc gdb/common/common-utils.c:44
>     #2 0xa82bd5 in call_chunkfun libiberty/obstack.c:94
>     #3 0xa82eda in _obstack_newchunk libiberty/obstack.c:206
>     #4 0x477310 in allocate_on_obstack::operator new(unsigned long, obstack*) \
>                    gdb/gdb_obstack.h:117
>     #5 0x5dea8c in load_partial_dies gdb/dwarf2read.c:18571
>     #6 0x5c487f in process_psymtab_comp_unit_reader gdb/dwarf2read.c:8054
>     #7 0x5c3c1f in init_cutu_and_read_dies gdb/dwarf2read.c:7689
>     #8 0x5c4c03 in process_psymtab_comp_unit gdb/dwarf2read.c:8140
>     #9 0x5c58a2 in dwarf2_build_psymtabs_hard gdb/dwarf2read.c:8500
>     #10 0x5c0d03 in dwarf2_build_psymtabs(objfile*) gdb/dwarf2read.c:6337
>     #11 0x612359 in read_psyms gdb/elfread.c:1311
>     #12 0x798a64 in require_partial_symbols(objfile*, int) gdb/psymtab.c:115
>     #13 0x867d7b in read_symbols gdb/symfile.c:821
>     #14 0x8683d9 in syms_from_objfile_1 gdb/symfile.c:1000
>     #15 0x8684a1 in syms_from_objfile gdb/symfile.c:1017
>     #16 0x868873 in symbol_file_add_with_addrs gdb/symfile.c:1124
>     #17 0x868b0a in symbol_file_add_from_bfd(bfd*, char const*, \
>                     enum_flags<symfile_add_flag>, \
> 		    std::vector<other_sections, \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>, objfile*) gdb/symfile.c:1204
>     #18 0x868b64 in symbol_file_add(char const*, enum_flags<symfile_add_flag>, \
>                     std::vector<other_sections, \
> 		    std::allocator<other_sections> >*, \
> 		    enum_flags<objfile_flag>) gdb/symfile.c:1217
>     #19 0x868c39 in symbol_file_add_main_1 gdb/symfile.c:1240
>     #20 0x868bd0 in symbol_file_add_main(char const*, \
>                     enum_flags<symfile_add_flag>) gdb/symfile.c:1231
>     #21 0x71f1b2 in symbol_file_add_main_adapter gdb/main.c:395
>     #22 0x71f10e in catch_command_errors gdb/main.c:372
>     #23 0x71ff5f in captured_main_1 gdb/main.c:1043
>     #24 0x72045d in captured_main gdb/main.c:1163
>     #25 0x7204c8 in gdb_main(captured_main_args*) gdb/main.c:1188
>     #26 0x40fd7d in main gdb/gdb.c:32
>     #27 0x7f137e300f49 in __libc_start_main (/lib64/libc.so.6+0x20f49)
> ...
> 
> This error happens as follows.
> 
> The function find_partial_die has a cu argument, but returns a pdi which may
> or may not be from that cu:
> ...
> /* Find a partial DIE at OFFSET, which may or may not be in CU,
>    except in the case of .debug_types DIEs which do not reference
>    outside their CU (they do however referencing other types via
>    DW_FORM_ref_sig8).  */
> 
> static struct partial_die_info *
> find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
> ...
> 
> So the pdi returned by find_partial_die here in partial_die_parent_scope may
> be from another cu:
> ...
> partial_die_parent_scope (struct partial_die_info *pdi,
>                           struct dwarf2_cu *cu)
> {
>   const char *grandparent_scope;
>   struct partial_die_info *parent, *real_pdi;
> 
>   /* We need to look at our parent DIE; if we have a DW_AT_specification,
>      then this means the parent of the specification DIE.  */
> 
>   real_pdi = pdi;
>   while (real_pdi->has_specification)
>     real_pdi = find_partial_die (real_pdi->spec_offset,
>                                  real_pdi->spec_is_dwz, cu);
> 
>   parent = real_pdi->die_parent;
> ...
> in which case both real_pdi and parent will be not from cu, but from another
> one, say cu2.
> 
> Subsequently, cu's comp_unit_obstack is used to set parent->scope:
> ...
>         parent->scope = typename_concat (&cu->comp_unit_obstack,
>                                          grandparent_scope,
>                                          parent->name, 0, cu);
> ...
> 
> So, we use cu->comp_unit_obstack to assign a value to the scope field of
> a pdi belonging to cu2, and when cu is deleted, the scope field points to a
> freed value.
> 
> Fix this by making find_partial_die return the cu corresponding to the
> returned pdi, and handling this at the call sites.
> 
> Tested on x86_64-linux.
> 
> OK for trunk?
> 
> Thanks,
> - Tom
> 
> [gdb] Fix heap-use-after-free in typename_concat
> 
> gdb/ChangeLog:
> 
> 2019-05-03  Tom de Vries  <tdevries@suse.de>
> 
> 	PR gdb/24094
> 	* dwarf2read.c (struct cu_partial_die_info): New struct.
> 	(find_partial_die): Return cu_partial_die_info.
> 	(partial_die_parent_scope, guess_partial_die_structure_name)
> 	(partial_die_info::fixup): Handle new return type of
> 	find_partial_die.

This all sounds good.  I have a couple of small suggestions inline
below...

> 
> ---
>  gdb/dwarf2read.c | 49 +++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index b0bdecf96f..442b618f6e 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -1518,8 +1518,14 @@ static unsigned int peek_abbrev_code (bfd *, const gdb_byte *);
>  static struct partial_die_info *load_partial_dies
>    (const struct die_reader_specs *, const gdb_byte *, int);
>  
> -static struct partial_die_info *find_partial_die (sect_offset, int,
> -						  struct dwarf2_cu *);
> +struct cu_partial_die_info
> +{
> +  struct dwarf2_cu *cu;
> +  struct partial_die_info *pdi;
> +};

This needs at least a header comment describing its use, and ideally
each field documented too.

I wonder though if you should also provide this with a 2 argument
constructor, and delete the default constructor, like:

  /* blah blah blah...  */

  struct cu_partial_die_info
  {
    /* mumble.. */
    struct dwarf2_cu *cu;

    /* mutter...  */
    struct partial_die_info *pdi;

    cu_partial_die_info (struct dwarf2_cu *cu,
  		       struct partial_die_info *pdi)
      : cu (cu),
        pdi (pdi)
    { /* Nothing.  */ }

  private:
    cu_partial_die_info () = delete;
  };

This will lead to some obvious knock on changes in the rest of the
code, which I think are probably improvements.

Thanks,
Andrew

> +
> +static struct cu_partial_die_info find_partial_die (sect_offset, int,
> +						    struct dwarf2_cu *);
>  
>  static const gdb_byte *read_attribute (const struct die_reader_specs *,
>  				       struct attribute *, struct attr_abbrev *,
> @@ -8771,14 +8777,19 @@ partial_die_parent_scope (struct partial_die_info *pdi,
>  {
>    const char *grandparent_scope;
>    struct partial_die_info *parent, *real_pdi;
> +  struct cu_partial_die_info res;
>  
>    /* We need to look at our parent DIE; if we have a DW_AT_specification,
>       then this means the parent of the specification DIE.  */
>  
>    real_pdi = pdi;
>    while (real_pdi->has_specification)
> -    real_pdi = find_partial_die (real_pdi->spec_offset,
> -				 real_pdi->spec_is_dwz, cu);
> +    {
> +      res = find_partial_die (real_pdi->spec_offset,
> +			      real_pdi->spec_is_dwz, cu);
> +      real_pdi = res.pdi;
> +      cu = res.cu;
> +    }
>  
>    parent = real_pdi->die_parent;
>    if (parent == NULL)
> @@ -18922,7 +18933,7 @@ dwarf2_cu::find_partial_die (sect_offset sect_off)
>     outside their CU (they do however referencing other types via
>     DW_FORM_ref_sig8).  */
>  
> -static struct partial_die_info *
> +static struct cu_partial_die_info
>  find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
>  {
>    struct dwarf2_per_objfile *dwarf2_per_objfile
> @@ -18936,7 +18947,12 @@ find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
>      {
>        pd = cu->find_partial_die (sect_off);
>        if (pd != NULL)
> -	return pd;
> +	{
> +	  struct cu_partial_die_info res;
> +	  res.pdi = pd;
> +	  res.cu = cu;
> +	  return res;
> +	}
>        /* We missed recording what we needed.
>  	 Load all dies and try again.  */
>        per_cu = cu->per_cu;
> @@ -18984,7 +19000,12 @@ find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
>  		    _("could not find partial DIE %s "
>  		      "in cache [from module %s]\n"),
>  		    sect_offset_str (sect_off), bfd_get_filename (objfile->obfd));
> -  return pd;
> +  {
> +    struct cu_partial_die_info res;
> +    res.pdi = pd;
> +    res.cu = per_cu->cu;
> +    return res;
> +  }
>  }
>  
>  /* See if we can figure out if the class lives in a namespace.  We do
> @@ -19003,6 +19024,7 @@ guess_partial_die_structure_name (struct partial_die_info *struct_pdi,
>  
>    struct partial_die_info *real_pdi;
>    struct partial_die_info *child_pdi;
> +  struct cu_partial_die_info res;
>  
>    /* If this DIE (this DIE's specification, if any) has a parent, then
>       we should not do this.  We'll prepend the parent's fully qualified
> @@ -19010,8 +19032,12 @@ guess_partial_die_structure_name (struct partial_die_info *struct_pdi,
>  
>    real_pdi = struct_pdi;
>    while (real_pdi->has_specification)
> -    real_pdi = find_partial_die (real_pdi->spec_offset,
> -				 real_pdi->spec_is_dwz, cu);
> +    {
> +      res = find_partial_die (real_pdi->spec_offset,
> +			      real_pdi->spec_is_dwz, cu);
> +      real_pdi = res.pdi;
> +      cu = res.cu;
> +    }
>  
>    if (real_pdi->die_parent != NULL)
>      return;
> @@ -19056,8 +19082,11 @@ partial_die_info::fixup (struct dwarf2_cu *cu)
>    if (name == NULL && has_specification)
>      {
>        struct partial_die_info *spec_die;
> +      struct cu_partial_die_info res;
>  
> -      spec_die = find_partial_die (spec_offset, spec_is_dwz, cu);
> +      res = find_partial_die (spec_offset, spec_is_dwz, cu);
> +      spec_die = res.pdi;
> +      cu = res.cu;
>  
>        spec_die->fixup (cu);
>
  
Tom Tromey May 16, 2019, 6:53 p.m. UTC | #3
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> When running gdb using AddressSanitizer, and loading a cc1plus binary built
Tom> with profiledbootstrap and -flto, we run into a heap-use-after-free error:

Thanks for finding this.

Tom> +	{
Tom> +	  struct cu_partial_die_info res;
Tom> +	  res.pdi = pd;
Tom> +	  res.cu = cu;
Tom> +	  return res;
Tom> +	}

Can't this be just "return {pd, cu};"?

Tom> +  {
Tom> +    struct cu_partial_die_info res;
Tom> +    res.pdi = pd;
Tom> +    res.cu = per_cu->cu;
Tom> +    return res;
Tom> +  }

If so, another instance of it.

thanks,
Tom
  
Tom de Vries May 17, 2019, 7:43 a.m. UTC | #4
On 16-05-19 20:53, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> When running gdb using AddressSanitizer, and loading a cc1plus binary built
> Tom> with profiledbootstrap and -flto, we run into a heap-use-after-free error:
> 
> Thanks for finding this.
> 
> Tom> +	{
> Tom> +	  struct cu_partial_die_info res;
> Tom> +	  res.pdi = pd;
> Tom> +	  res.cu = cu;
> Tom> +	  return res;
> Tom> +	}
> 
> Can't this be just "return {pd, cu};"?
> 

Indeed.

Addressed at both locations in commit-post at
https://sourceware.org/ml/gdb-patches/2019-05/msg00408.html .

Thanks,
- Tom
  

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b0bdecf96f..442b618f6e 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1518,8 +1518,14 @@  static unsigned int peek_abbrev_code (bfd *, const gdb_byte *);
 static struct partial_die_info *load_partial_dies
   (const struct die_reader_specs *, const gdb_byte *, int);
 
-static struct partial_die_info *find_partial_die (sect_offset, int,
-						  struct dwarf2_cu *);
+struct cu_partial_die_info
+{
+  struct dwarf2_cu *cu;
+  struct partial_die_info *pdi;
+};
+
+static struct cu_partial_die_info find_partial_die (sect_offset, int,
+						    struct dwarf2_cu *);
 
 static const gdb_byte *read_attribute (const struct die_reader_specs *,
 				       struct attribute *, struct attr_abbrev *,
@@ -8771,14 +8777,19 @@  partial_die_parent_scope (struct partial_die_info *pdi,
 {
   const char *grandparent_scope;
   struct partial_die_info *parent, *real_pdi;
+  struct cu_partial_die_info res;
 
   /* We need to look at our parent DIE; if we have a DW_AT_specification,
      then this means the parent of the specification DIE.  */
 
   real_pdi = pdi;
   while (real_pdi->has_specification)
-    real_pdi = find_partial_die (real_pdi->spec_offset,
-				 real_pdi->spec_is_dwz, cu);
+    {
+      res = find_partial_die (real_pdi->spec_offset,
+			      real_pdi->spec_is_dwz, cu);
+      real_pdi = res.pdi;
+      cu = res.cu;
+    }
 
   parent = real_pdi->die_parent;
   if (parent == NULL)
@@ -18922,7 +18933,7 @@  dwarf2_cu::find_partial_die (sect_offset sect_off)
    outside their CU (they do however referencing other types via
    DW_FORM_ref_sig8).  */
 
-static struct partial_die_info *
+static struct cu_partial_die_info
 find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
 {
   struct dwarf2_per_objfile *dwarf2_per_objfile
@@ -18936,7 +18947,12 @@  find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
     {
       pd = cu->find_partial_die (sect_off);
       if (pd != NULL)
-	return pd;
+	{
+	  struct cu_partial_die_info res;
+	  res.pdi = pd;
+	  res.cu = cu;
+	  return res;
+	}
       /* We missed recording what we needed.
 	 Load all dies and try again.  */
       per_cu = cu->per_cu;
@@ -18984,7 +19000,12 @@  find_partial_die (sect_offset sect_off, int offset_in_dwz, struct dwarf2_cu *cu)
 		    _("could not find partial DIE %s "
 		      "in cache [from module %s]\n"),
 		    sect_offset_str (sect_off), bfd_get_filename (objfile->obfd));
-  return pd;
+  {
+    struct cu_partial_die_info res;
+    res.pdi = pd;
+    res.cu = per_cu->cu;
+    return res;
+  }
 }
 
 /* See if we can figure out if the class lives in a namespace.  We do
@@ -19003,6 +19024,7 @@  guess_partial_die_structure_name (struct partial_die_info *struct_pdi,
 
   struct partial_die_info *real_pdi;
   struct partial_die_info *child_pdi;
+  struct cu_partial_die_info res;
 
   /* If this DIE (this DIE's specification, if any) has a parent, then
      we should not do this.  We'll prepend the parent's fully qualified
@@ -19010,8 +19032,12 @@  guess_partial_die_structure_name (struct partial_die_info *struct_pdi,
 
   real_pdi = struct_pdi;
   while (real_pdi->has_specification)
-    real_pdi = find_partial_die (real_pdi->spec_offset,
-				 real_pdi->spec_is_dwz, cu);
+    {
+      res = find_partial_die (real_pdi->spec_offset,
+			      real_pdi->spec_is_dwz, cu);
+      real_pdi = res.pdi;
+      cu = res.cu;
+    }
 
   if (real_pdi->die_parent != NULL)
     return;
@@ -19056,8 +19082,11 @@  partial_die_info::fixup (struct dwarf2_cu *cu)
   if (name == NULL && has_specification)
     {
       struct partial_die_info *spec_die;
+      struct cu_partial_die_info res;
 
-      spec_die = find_partial_die (spec_offset, spec_is_dwz, cu);
+      res = find_partial_die (spec_offset, spec_is_dwz, cu);
+      spec_die = res.pdi;
+      cu = res.cu;
 
       spec_die->fixup (cu);