[OB PATCH] Fix NULL pointer dereference

[OB PATCH] Fix NULL pointer dereference

[Gary Benson]

This commit fixes a segmentation fault on tab completion when
certain debuginfo is installed:

  https://bugzilla.redhat.com/show_bug.cgi?id=1398387

gdb/ChangeLog:

	* symtab.c (add_symtab_completions): Prevent NULL pointer
	dereference.
---
 gdb/ChangeLog | 5 +++++
 gdb/symtab.c  | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 356f480..2c141e5 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5163,6 +5163,9 @@ add_symtab_completions (struct compunit_symtab *cust,
   struct block_iterator iter;
   int i;
 
+  if (cust == NULL)
+    return;
+
   for (i = GLOBAL_BLOCK; i <= STATIC_BLOCK; i++)
     {
       QUIT;
-- 
1.8.3.1

Re: [OB PATCH] Fix NULL pointer dereference

[Pedro Alves]

On 02/09/2017 03:39 PM, Gary Benson wrote:
> This commit fixes a segmentation fault on tab completion when
> certain debuginfo is installed:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1398387

Can you provide a bit more detail please?  What was the backtrace
that lead to this?  It seems odd to me to reach here with
a NULL cust.

Thanks,
Pedro Alves

Re: [OB PATCH] Fix NULL pointer dereference

[Gary Benson]

Pedro Alves wrote:
> On 02/09/2017 03:39 PM, Gary Benson wrote:
> > This commit fixes a segmentation fault on tab completion when
> > certain debuginfo is installed:
> > 
> >   https://bugzilla.redhat.com/show_bug.cgi?id=1398387
> 
> Can you provide a bit more detail please?  What was the backtrace
> that lead to this?  It seems odd to me to reach here with a NULL
> cust.

Me too, I suppose, though that symtab.c is full of checks for it
so I assumed it meant something.  Maybe I was rash!  Anyway, here's
the backtrace:

#0  0x00000000007271be in add_symtab_completions (cust=0x0, sym_text=0x7fffffffdb02 "si", sym_text_len=2, text=0x7fffffffdb02 "si", word=0x7fffffffdb02 "si", 
    code=TYPE_CODE_UNDEF) at ../../src/gdb/symtab.c:5172
#1  0x00000000007272cc in symtab_expansion_callback (symtab=0x0, user_data=0x7fffffffd7a0) at ../../src/gdb/symtab.c:5197
#2  0x00000000006d959a in psym_expand_symtabs_matching (objfile=0x1717540, file_matcher=0x0, symbol_matcher=0x727144 <symbol_completion_matcher(char const*, void*)>, 
    expansion_notify=0x72727c <symtab_expansion_callback(compunit_symtab*, void*)>, kind=ALL_DOMAIN, data=0x7fffffffd7a0) at ../../src/gdb/psymtab.c:1444
#3  0x00000000007194c6 in expand_symtabs_matching (file_matcher=0x0, symbol_matcher=0x727144 <symbol_completion_matcher(char const*, void*)>, 
    expansion_notify=0x72727c <symtab_expansion_callback(compunit_symtab*, void*)>, kind=ALL_DOMAIN, data=0x7fffffffd7a0) at ../../src/gdb/symfile.c:3897
#4  0x00000000007276cf in default_make_symbol_completion_list_break_on_1 (text=0x7fffffffdb02 "si", word=0x7fffffffdb02 "si", break_on=0x9a5280 "", code=TYPE_CODE_UNDEF)
    at ../../src/gdb/symtab.c:5330
#5  0x0000000000727a4e in default_make_symbol_completion_list_break_on (text=0x7fffffffdb02 "si", word=0x7fffffffdb02 "si", break_on=0x9a5280 "", code=TYPE_CODE_UNDEF)
    at ../../src/gdb/symtab.c:5424
#6  0x0000000000727b3a in default_make_symbol_completion_list (text=0x7fffffffdb02 "si", word=0x7fffffffdb02 "si", code=TYPE_CODE_UNDEF) at ../../src/gdb/symtab.c:5441
#7  0x0000000000727b6c in make_symbol_completion_list (text=0x7fffffffdb02 "si", word=0x7fffffffdb02 "si") at ../../src/gdb/symtab.c:5452
#8  0x00000000005b9ad1 in linespec_location_completer (ignore=0xee7d60, text=0x7fffffffdb02 "si", word=0x7fffffffdb02 "si") at ../../src/gdb/completer.c:287
#9  0x00000000005ba02b in location_completer (ignore=0xee7d60, text=0x7fffffffdb02 "si", word=0x7fffffffdb02 "si") at ../../src/gdb/completer.c:525
#10 0x00000000005ba53e in expression_completer (ignore=0xee7d60, text=0x7fffffffdb02 "si", word=0x7fffffffdb02 "si") at ../../src/gdb/completer.c:650
#11 0x00000000005bab76 in complete_line_internal (text=0x239aee0 "si", line_buffer=0xf14090 "p si", point=4, reason=handle_completions) at ../../src/gdb/completer.c:958
#12 0x00000000005bad95 in complete_line (text=0x239aee0 "si", line_buffer=0xf14090 "p si", point=4) at ../../src/gdb/completer.c:1067
#13 0x00000000005bb34f in line_completion_function (text=0x239aee0 "si", matches=0, line_buffer=0xf14090 "p si", point=4) at ../../src/gdb/completer.c:1286
#14 0x00000000005b9615 in readline_line_completion_function (text=0x239aee0 "si", matches=0) at ../../src/gdb/completer.c:121
#15 0x0000000000791862 in rl_completion_matches (text=0x239aee0 "si", entry_function=0x5b95ea <readline_line_completion_function(char const*, int)>)
    at ../../src/readline/complete.c:2011
#16 0x000000000078faeb in gen_completion_matches (text=0x239aee0 "si", start=2, end=4, our_func=0x5b95ea <readline_line_completion_function(char const*, int)>, found_quote=0, 
    quote_char=0) at ../../src/readline/complete.c:1094
#17 0x00000000007913e4 in rl_complete_internal (what_to_do=9) at ../../src/readline/complete.c:1849
#18 0x000000000078eb69 in rl_complete (ignore=1, invoking_key=9) at ../../src/readline/complete.c:408
#19 0x0000000000788c05 in _rl_dispatch_subseq (key=9, map=0xdc4200 <emacs_standard_keymap>, got_subseq=0) at ../../src/readline/readline.c:774
#20 0x0000000000788a55 in _rl_dispatch (key=-9470, map=0xdc4200 <emacs_standard_keymap>) at ../../src/readline/readline.c:724
#21 0x00000000007886fa in readline_internal_char () at ../../src/readline/readline.c:552
#22 0x00000000007a1281 in rl_callback_read_char () at ../../src/readline/callback.c:201
#23 0x000000000062c42c in gdb_rl_callback_read_char_wrapper (client_data=0xe13ac0) at ../../src/gdb/event-top.c:173
#24 0x000000000062ca0d in stdin_event_handler (error=0, client_data=0xe13ac0) at ../../src/gdb/event-top.c:505
#25 0x000000000062b04b in handle_file_event (file_ptr=0x17eaf90, ready_mask=1) at ../../src/gdb/event-loop.c:733
#26 0x000000000062b5ee in gdb_wait_for_event (block=1) at ../../src/gdb/event-loop.c:859
#27 0x000000000062a4c9 in gdb_do_one_event () at ../../src/gdb/event-loop.c:347
#28 0x000000000062a501 in start_event_loop () at ../../src/gdb/event-loop.c:371
#29 0x000000000069b763 in captured_command_loop (data=0x0) at ../../src/gdb/main.c:325
#30 0x000000000062dea4 in catch_errors (func=0x69b721 <captured_command_loop(void*)>, func_args=0x0, errstring=0x981395 "", mask=RETURN_MASK_ALL)
    at ../../src/gdb/exceptions.c:236
#31 0x000000000069ca75 in captured_main (data=0x7fffffffe110) at ../../src/gdb/main.c:1148
#32 0x000000000069ca9e in gdb_main (args=0x7fffffffe110) at ../../src/gdb/main.c:1158
#33 0x000000000040ca1d in main (argc=2, argv=0x7fffffffe218) at ../../src/gdb/gdb.c:32

-- 
http://gbenson.net/

Re: [OB PATCH] Fix NULL pointer dereference

[Pedro Alves]

On 02/10/2017 11:19 AM, Gary Benson wrote:

> Me too, I suppose, though that symtab.c is full of checks for it
> so I assumed it meant something.  Maybe I was rash!  Anyway, here's
> the backtrace:
> 
> #0  0x00000000007271be in add_symtab_completions (cust=0x0, sym_text=0x7fffffffdb02 "si", sym_text_len=2, text=0x7fffffffdb02 "si", word=0x7fffffffdb02 "si", 
>     code=TYPE_CODE_UNDEF) at ../../src/gdb/symtab.c:5172
> #1  0x00000000007272cc in symtab_expansion_callback (symtab=0x0, user_data=0x7fffffffd7a0) at ../../src/gdb/symtab.c:5197
> #2  0x00000000006d959a in psym_expand_symtabs_matching (objfile=0x1717540, file_matcher=0x0, symbol_matcher=0x727144 <symbol_completion_matcher(char const*, void*)>, 
>     expansion_notify=0x72727c <symtab_expansion_callback(compunit_symtab*, void*)>, kind=ALL_DOMAIN, data=0x7fffffffd7a0) at ../../src/gdb/psymtab.c:1444
> #3  0x00000000007194c6 in expand_symtabs_matching (file_matcher=0x0, symbol_matcher=0x727144 <symbol_completion_matcher(char const*, void*)>, 
>     expansion_notify=0x72727c <symtab_expansion_callback(compunit_symtab*, void*)>, kind=ALL_DOMAIN, data=0x7fffffffd7a0) at ../../src/gdb/symfile.c:3897

So we reach this call of the 'expansion_notify' function pointer:

      if (recursively_search_psymtabs (ps, objfile, kind, symbol_matcher, data))
	{
	  struct compunit_symtab *symtab =
	    psymtab_to_symtab (objfile, ps);

	  if (expansion_notify != NULL)
	    expansion_notify (symtab, data);
	}
    }

So why can recursively_search_psymtabs find a matching partial symbol
and thus return true, and then psymtab_to_symtab returns NULL,
indicating the symtab is empty?  That sounds like a bug?

Thanks,
Pedro Alves

Re: [OB PATCH] Fix NULL pointer dereference

[Gary Benson]

Pedro Alves wrote:
> So we reach this call of the 'expansion_notify' function pointer:
> 
>       if (recursively_search_psymtabs (ps, objfile, kind, symbol_matcher, data))
> 	{
> 	  struct compunit_symtab *symtab =
> 	    psymtab_to_symtab (objfile, ps);
> 
> 	  if (expansion_notify != NULL)
> 	    expansion_notify (symtab, data);
> 	}
>     }
> 
> So why can recursively_search_psymtabs find a matching partial
> symbol and thus return true, and then psymtab_to_symtab returns
> NULL, indicating the symtab is empty?  That sounds like a bug?

So the first time psymtab_to_symtab returns NULL, ps->filename is
"src/basic/string-util.h"; the only string-util.h on my system is
/usr/src/debug/systemd-231/src/basic/string-util.h from
systemd-debuginfo-231-10.fc25.x86_64 so I'm assuming it's that.

But, recursively_search_psymtabs is returning 1 not for that psymtab
but for one of its dependencies (the first, as it happens).  That has
no filename, and its user->filename is "<artificial>".  So
recursively_search_psymtabs is saying string-util.h matches because
it matches because of some (shared?) symbol table it references, but
psymtab_to_symtab is being called on the string-util.h psymtab which
doesn't match (or exist?!)

I have no idea what I'm looking at here :(

Thanks,
Gary

-- 
http://gbenson.net/