Message ID | 20200104114312.165656-1-shahab.vahedi@gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 126736 invoked by alias); 4 Jan 2020 11:43:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 126720 invoked by uid 89); 4 Jan 2020 11:43:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.8 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=H*Ad:U*claziss, HContent-Transfer-Encoding:8bit X-HELO: mail-wr1-f66.google.com Received: from mail-wr1-f66.google.com (HELO mail-wr1-f66.google.com) (209.85.221.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 04 Jan 2020 11:43:48 +0000 Received: by mail-wr1-f66.google.com with SMTP id z7so44621585wrl.13; Sat, 04 Jan 2020 03:43:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=2743KgVvOX9U335djXF/TBIIMJRkA2356IT3ZHUTQZk=; b=u6mSGzQBIu6Ag6Qaov/TPvrGJfY08oadn5pLjPnuN8ko9fxnirqpha22OUpZXiBYvx Ib2NOkUHW2HvB3SrzdTxVQC0sCGqQD3hFyW3pP9xGOf1HvAXKmYC1NTuiIbQwfDvQdCn cXxQssIIPiccss1edmnA8FJDD6uLKRmWOZXTYh2lq9wOa1yLefQ6BH2YZb77ylXjXgmO PQQhtUivGb8l9Yuzl+ndSep5slSzTnI+u0bywBGGxDz98PQmT0H7WwKTgldbSMEyXpTQ ri/tFN5uFzSxtJtWw9c6EZt1dEoYzRnC1Z/VyMzqKxUB/yG8g9Vi9SckOnDtz/f7TUSZ 8jVQ== Return-Path: <shahab.vahedi@gmail.com> Received: from localhost.localdomain (ip-217-103-128-141.ip.prioritytelecom.net. [217.103.128.141]) by smtp.gmail.com with ESMTPSA id 2sm66076584wrq.31.2020.01.04.03.43.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 04 Jan 2020 03:43:45 -0800 (PST) From: Shahab Vahedi <shahab.vahedi@gmail.com> To: gdb-patches@sourceware.org Cc: gdb@sourceware.org, Shahab Vahedi <shahab@synopsys.com>, Claudiu Zissulescu <claziss@synopsys.com>, Francois Bedard <fbedard@synopsys.com> Subject: [PATCH] GDB: Fix the overflow in addr_is_displayed() Date: Sat, 4 Jan 2020 12:43:12 +0100 Message-Id: <20200104114312.165656-1-shahab.vahedi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit |
Commit Message
Shahab Vahedi
Jan. 4, 2020, 11:43 a.m. UTC
From: Shahab Vahedi <shahab@synopsys.com>
In a corner case scenario, where the height of the assembly TUI is
bigger than the number of instructions in the whole program, GDB
dumps core. The problem roots in this condition check:
int i = 0;
while (i < content. size() - threshold ...) {
... content[i] ...
}
"threshold" is 2 and there are times that "content. size()" is 0.
This results into an overflow and the loop is entered whereas it
should have been skipped.
This has been discussed at length in bug 25345:
https://sourceware.org/bugzilla/show_bug.cgi?id=25345
gdb/ChangeLog:
2020-01-04 Shahab Vahedi <shahab@synopsys.com>
* tui/tui-disasm.c (tui_disasm_window::addr_is_displayed):
Treat "content.size()" as "int" to avoid overflow.
---
gdb/tui/tui-disasm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
* Shahab Vahedi <shahab.vahedi@gmail.com> [2020-01-04 12:43:12 +0100]: > From: Shahab Vahedi <shahab@synopsys.com> > > In a corner case scenario, where the height of the assembly TUI is > bigger than the number of instructions in the whole program, GDB > dumps core. The problem roots in this condition check: > > int i = 0; > while (i < content. size() - threshold ...) { > ... content[i] ... > } > > "threshold" is 2 and there are times that "content. size()" is 0. > This results into an overflow and the loop is entered whereas it > should have been skipped. I didn't quite understand the problem description. I can see how 'content.size() - threshold' would overflow when the size is 0 or 1, but I don't understand the part about the tui height being bigger than the number of instructions. I tried to reproduce the failure on native x86-64 but failed. I can get the initial stage reproduced, where the asm window is empty and I see the error about "Cannot access memory at address ....", but then when I attach to a remote the asm window fills in fine. I guess this is because on Linux the page the code is on is readable, and under QEMU only the specific program instructions are readable maybe? Anyway, the change looks reasonable, though I had one comment, inline below... > > This has been discussed at length in bug 25345: > https://sourceware.org/bugzilla/show_bug.cgi?id=25345 > > gdb/ChangeLog: > 2020-01-04 Shahab Vahedi <shahab@synopsys.com> > * tui/tui-disasm.c (tui_disasm_window::addr_is_displayed): > Treat "content.size()" as "int" to avoid overflow. > --- > gdb/tui/tui-disasm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c > index c72b50730b0..a0921eb09d1 100644 > --- a/gdb/tui/tui-disasm.c > +++ b/gdb/tui/tui-disasm.c > @@ -349,10 +349,10 @@ bool > tui_disasm_window::addr_is_displayed (CORE_ADDR addr) const > { > bool is_displayed = false; > - int threshold = SCROLL_THRESHOLD; > + int nr_of_lines = int(content.size()) - int(SCROLL_THRESHOLD); I had a look through our code and I couldn't find any examples of us using 'int (xxx)' syntax to cast to int, this is probably a result of our C heritage, but we should probably try to be consistent I think. I'd suggest maybe this become: int nr_of_lines = (int) content.size () - SCROLL_THRESHOLD; Thanks, Andrew > > int i = 0; > - while (i < content.size () - threshold && !is_displayed) > + while (i < nr_of_lines && !is_displayed) > { > is_displayed > = (content[i].line_or_addr.loa == LOA_ADDRESS > -- > 2.24.1 >
diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c index c72b50730b0..a0921eb09d1 100644 --- a/gdb/tui/tui-disasm.c +++ b/gdb/tui/tui-disasm.c @@ -349,10 +349,10 @@ bool tui_disasm_window::addr_is_displayed (CORE_ADDR addr) const { bool is_displayed = false; - int threshold = SCROLL_THRESHOLD; + int nr_of_lines = int(content.size()) - int(SCROLL_THRESHOLD); int i = 0; - while (i < content.size () - threshold && !is_displayed) + while (i < nr_of_lines && !is_displayed) { is_displayed = (content[i].line_or_addr.loa == LOA_ADDRESS