diff mbox

[RFA] Fix 'Invalid read of size 4' in search_command_helper

Message ID 20190101150151.3177-1-philippe.waroquiers@skynet.be
State New, archived
Headers

Commit Message

Philippe Waroquiers Jan. 1, 2019, 3:01 p.m. UTC 
  Valgrind detects the below error in gdb.base/list.exp.
==14763== Invalid read of size 4
==14763==    at 0x60B584: search_command_helper(char const*, int, bool) [clone .constprop.91] (source.c:1601)
==14763==    by 0x408888: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:1892)
==14763==    by 0x668550: execute_command(char const*, int) (top.c:630)
==14763==    by 0x4B2F7B: command_handler(char const*) (event-top.c:583)
==14763==    by 0x4B326C: command_line_handler(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) (event-top.c:772)
...
==14763==  Address 0x6d9f09c is 4 bytes before a block of size 156 alloc'd
==14763==    at 0x4C2E2B3: realloc (vg_replace_malloc.c:836)
==14763==    by 0x41904C: xrealloc (common-utils.c:62)
==14763==    by 0x60A300: find_source_lines(symtab*, int) (source.c:1203)
==14763==    by 0x608219: source_cache::get_plain_source_lines(symtab*, int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) (source-cache.c:51)
==14763==    by 0x60A46B: print_source_lines_base(symtab*, int, int, enum_flags<print_source_lines_flag>) (source.c:1350)
==14763==    by 0x404E2D: list_command(char const*, int) (cli-cmds.c:1080)
....

Add the missing condition to end the loop once line 1 has been
reversed-searched.

gdb/ChangeLog
2019-01-01  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* source.c (search_command_helper): Stop reverse search
	when line 1 has been searched.
---
 gdb/source.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Tom Tromey Jan. 1, 2019, 6:37 p.m. UTC | #1 
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> gdb/ChangeLog
Philippe> 2019-01-01  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* source.c (search_command_helper): Stop reverse search
Philippe> 	when line 1 has been searched.

This is ok.  Some of these could probably have gone in under the obvious
rule, though I completely understand if you'd rather not do that.

Tom
Philippe Waroquiers Jan. 1, 2019, 7:35 p.m. UTC | #2 
On Tue, 2019-01-01 at 11:37 -0700, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Philippe> gdb/ChangeLog
> Philippe> 2019-01-01  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> Philippe> 	* source.c (search_command_helper): Stop reverse search
> Philippe> 	when line 1 has been searched.
> 
> This is ok.  Some of these could probably have gone in under the obvious
> rule, though I completely understand if you'd rather not do that.
Effectively, some fixes are easy enough to be obvious, but still sometimes
an explicit review helps to see e.g. when it is time to restructure
the code (e.g. to restructure it using c++).

Thanks for the reviews, I pushed all 3 RFA fixes.

Philippe
diff mbox

Patch

diff --git a/gdb/source.c b/gdb/source.c
index 5c300db3ad..ad6c6466b4 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1598,6 +1598,8 @@  search_command_helper (const char *regex, int from_tty, bool forward)
       else
 	{
 	  line--;
+	  if (line < 1)
+	    break;
 	  if (fseek (stream.get (),
 		     current_source_symtab->line_charpos[line - 1], 0) < 0)
 	    {