From patchwork Mon Jan 7 12:42:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 30988 Received: (qmail 9267 invoked by alias); 7 Jan 2019 12:42:44 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 9128 invoked by uid 89); 7 Jan 2019 12:42:43 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=ten, pcs X-HELO: mail-wr1-f68.google.com Received: from mail-wr1-f68.google.com (HELO mail-wr1-f68.google.com) (209.85.221.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 07 Jan 2019 12:42:38 +0000 Received: by mail-wr1-f68.google.com with SMTP id t6so222630wrr.12 for ; Mon, 07 Jan 2019 04:42:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=qscCI6h6CtNXsxK0nxpz1g2sAd0jQV1Di0VFLPT/heg=; b=Ysg5SLnjeo6ENTKWrVLi5ubCDC++qbjHbd46uq8Pz+kVdiD6jyzpRzMzhnmWa9Az27 Somb+8UhezA7Gfe+8NraSLnkhLJN7Krky6qGbihwtSpzbQJW5ZH+O6Gi1brEL8ZRnKDE rysEBgm08QvA6qR37x8Y7d/eR8hlC1akq2XoTC6oZc1fjoI0WYTES+X2NkIIZCo/gAF8 UqdWKMKNFoRXULGDqLFzQf/dsxhfFCFU6WcnMj7quv6g8TWtG1vZBLBrCFeluEB3mdra ovO7nftOwgx64X1m+OrdVDiBWdDjcfMiY3V+P34YovgwgUAVJ9zqWy4BQdOvqdDeixgW bxDg== Return-Path: Received: from localhost (host86-172-198-47.range86-172.btcentralplus.com. [86.172.198.47]) by smtp.gmail.com with ESMTPSA id a62sm6949564wmf.47.2019.01.07.04.42.35 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 07 Jan 2019 04:42:35 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org, Tom Tromey Cc: Andrew Burgess Subject: [PATCH 4/4] gdb: Avoid signed integer overflow when printing source lines Date: Mon, 7 Jan 2019 12:42:24 +0000 Message-Id: <6a2e92c51b7dd83b02473042c9698014325eb9b5.1546860547.git.andrew.burgess@embecosm.com> In-Reply-To: References: In-Reply-To: References: X-IsSubscribed: yes 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/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 9e09c05513c..70d196776ca 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::direction::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::direction::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 f865c8a9508..f7ce18a0532 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::direction::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 fcd83daafcd..3e08238e883 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 class 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. */ + source_lines_range (int startline, direction dir = direction::forward); + + /* Construct a SOURCE_LINES_RANGE from STARTLINE to STOPLINE. */ + 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 *);