From 0e2a21335b6fc4a5b6bed19d9623916c52918b72 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Mon, 7 Jan 2019 07:26:35 +0000 Subject: [PATCH] gdb: Avoid signed integer overflow when printing source lines When printing source lines with calls to print_source_lines we need to pass a start line number and an end line number. The end line number is calculated by calling get_lines_to_list and adding this value to the start line number. For example this code from list_command: print_source_lines (cursal.symtab, first, first + get_lines_to_list (), 0); The problem is that get_lines_to_list returns a value based on the GDB setting `set listsize LISTSIZE`. By default LISTSIZE is 10, however, its also possible to set LISTSIZE to unlimited, in which case get_lines_to_list will return INT_MAX. As the parameter signature for print_source_lines is: void print_source_lines (struct symtab *, int, int, print_source_lines_flags); and `first` in the above code is an `int`, then when LISTSIZE is `unlimited` the above code will result in signed integer overflow, which is undefined. The solution in this patch is a new class source_lines_range that can be constructed from a single line number and a direction (forward or backward). The range is then constructed from the line number and the value of get_lines_to_list. gdb/ChangeLog: * cli/cli-cmds.c (list_command): Pass a source_lines_range to print_source_lines. * source.c (print_source_lines_base): Update line number check. (print_source_lines): New function. (source_lines_range::source_lines_range): New function. * source.h (class source_lines_range): New class. (print_source_lines): New declaration. --- gdb/ChangeLog | 10 ++++++++++ gdb/cli/cli-cmds.c | 35 ++++++++++++++++----------------- gdb/source.c | 48 +++++++++++++++++++++++++++++++++++++++------- gdb/source.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 26 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 7d6d5978e0..c89c86be4e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,13 @@ +2019-01-09 Andrew Burgess + + * cli/cli-cmds.c (list_command): Pass a source_lines_range to + print_source_lines. + * source.c (print_source_lines_base): Update line number check. + (print_source_lines): New function. + (source_lines_range::source_lines_range): New function. + * source.h (class source_lines_range): New class. + (print_source_lines): New declaration. + 2019-01-09 Philippe Waroquiers * linespec.c (linespec_state_destructor): Free self->canonical_names. diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 9e09c05513..57cfad441c 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -895,14 +895,13 @@ list_command (const char *arg, int from_tty) && get_lines_to_list () == 1 && first > 1) first -= 1; - print_source_lines (cursal.symtab, first, - first + get_lines_to_list (), 0); + print_source_lines (cursal.symtab, source_lines_range (first), 0); } /* "l" or "l +" lists next ten lines. */ else if (arg == NULL || arg[0] == '+') - print_source_lines (cursal.symtab, cursal.line, - cursal.line + get_lines_to_list (), 0); + print_source_lines (cursal.symtab, + source_lines_range (cursal.line), 0); /* "l -" lists previous ten lines, the ones before the ten just listed. */ @@ -911,10 +910,9 @@ list_command (const char *arg, int from_tty) if (get_first_line_listed () == 1) error (_("Already at the start of %s."), symtab_to_filename_for_display (cursal.symtab)); - print_source_lines (cursal.symtab, - std::max (get_first_line_listed () - - get_lines_to_list (), 1), - get_first_line_listed (), 0); + source_lines_range range (get_first_line_listed (), + source_lines_range::BACKWARD); + print_source_lines (cursal.symtab, range, 0); } return; @@ -1059,9 +1057,11 @@ list_command (const char *arg, int from_tty) if (dummy_beg && sal_end.symtab == 0) error (_("No default source file yet. Do \"help list\".")); if (dummy_beg) - print_source_lines (sal_end.symtab, - std::max (sal_end.line - (get_lines_to_list () - 1), 1), - sal_end.line + 1, 0); + { + source_lines_range range (sal_end.line + 1, + source_lines_range::BACKWARD); + print_source_lines (sal_end.symtab, range, 0); + } else if (sal.symtab == 0) error (_("No default source file yet. Do \"help list\".")); else if (no_end) @@ -1074,17 +1074,14 @@ list_command (const char *arg, int from_tty) first_line = 1; if (sals.size () > 1) print_sal_location (sal); - print_source_lines (sal.symtab, - first_line, - first_line + get_lines_to_list (), - 0); + print_source_lines (sal.symtab, source_lines_range (first_line), 0); } } + else if (dummy_end) + print_source_lines (sal.symtab, source_lines_range (sal.line), 0); else - print_source_lines (sal.symtab, sal.line, - (dummy_end - ? sal.line + get_lines_to_list () - : sal_end.line + 1), + print_source_lines (sal.symtab, + source_lines_range (sal.line, (sal_end.line + 1)), 0); } diff --git a/gdb/source.c b/gdb/source.c index f865c8a950..1f10379a07 100644 --- a/gdb/source.c +++ b/gdb/source.c @@ -1331,13 +1331,8 @@ print_source_lines_base (struct symtab *s, int line, int stopline, last_source_error = 0; /* If the user requested a sequence of lines that seems to go backward - (from high to low line numbers) then we don't print anything. - The use of '- 1' here instead of '<=' is currently critical, we rely - on the undefined wrap around behaviour of 'int' for stopline. When - the use has done: 'set listsize unlimited' then stopline can overflow - and appear as MIN_INT. This is a long-standing bug that needs - fixing. */ - if (stopline - 1 < line) + (from high to low line numbers) then we don't print anything. */ + if (stopline <= line) return; std::string lines; @@ -1399,6 +1394,18 @@ print_source_lines (struct symtab *s, int line, int stopline, { print_source_lines_base (s, line, stopline, flags); } + +/* See source.h. */ + +void +print_source_lines (struct symtab *s, source_lines_range line_range, + print_source_lines_flags flags) +{ + print_source_lines_base (s, line_range.startline (), + line_range.stopline (), flags); +} + + /* Print info on range of pc's in a specified line. */ @@ -1822,6 +1829,33 @@ set_substitute_path_command (const char *args, int from_tty) forget_cached_source_info (); } +/* See source.h. */ + +source_lines_range::source_lines_range (int startline, + source_lines_range::direction dir) +{ + if (dir == source_lines_range::FORWARD) + { + LONGEST end = static_cast (startline) + get_lines_to_list (); + + if (end > INT_MAX) + end = INT_MAX; + + m_startline = startline; + m_stopline = static_cast (end); + } + else + { + LONGEST start = static_cast (startline) - get_lines_to_list (); + + if (start < 1) + start = 1; + + m_startline = static_cast (start); + m_stopline = startline; + } +} + void _initialize_source (void) diff --git a/gdb/source.h b/gdb/source.h index fcd83daafc..f1b5f6e8f7 100644 --- a/gdb/source.h +++ b/gdb/source.h @@ -157,6 +157,54 @@ DEF_ENUM_FLAGS_TYPE (enum print_source_lines_flag, print_source_lines_flags); extern void print_source_lines (struct symtab *s, int line, int stopline, print_source_lines_flags flags); +/* Wrap up the logic to build a line number range for passing to + print_source_lines when using get_lines_to_list. An instance of this + class can be built from a single line number and a direction (forward or + backward) the range is then computed using get_lines_to_list. */ +class source_lines_range +{ +public: + /* When constructing the range from a single line number, does the line + range extend forward, or backward. */ + enum direction + { + FORWARD, + BACKWARD + }; + + /* Construct a SOURCE_LINES_RANGE starting at STARTLINE and extending in + direction DIR. The number of lines is from GET_LINES_TO_LIST. If the + direction is backward then the start is actually (STARTLINE - + GET_LINES_TO_LIST). There is also logic in place to ensure the start + is always 1 or more, and the end will be at most INT_MAX. */ + explicit source_lines_range (int startline, direction dir = FORWARD); + + /* Construct a SOURCE_LINES_RANGE from STARTLINE to STOPLINE. */ + explicit source_lines_range (int startline, int stopline) + : m_startline (startline), + m_stopline (stopline) + { /* Nothing. */ } + + /* Return the line to start listing from. */ + int startline () const + { return m_startline; } + + /* Return the line after the last line that should be listed. */ + int stopline () const + { return m_stopline; } + +private: + + /* The start and end of the range. */ + int m_startline; + int m_stopline; +}; + +/* Variation of previous print_source_lines that takes a range instead of a + start and end line number. */ +extern void print_source_lines (struct symtab *s, source_lines_range r, + print_source_lines_flags flags); + /* Forget line positions and file names for the symtabs in a particular objfile. */ extern void forget_cached_source_info_for_objfile (struct objfile *); -- 2.34.1