From patchwork Tue Nov 8 18:56:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Arnez X-Patchwork-Id: 17296 Received: (qmail 7702 invoked by alias); 8 Nov 2016 18:58:31 -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 7685 invoked by uid 89); 8 Nov 2016 18:58:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 spammy=displaying, 1727, 1323, 1312 X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 08 Nov 2016 18:58:22 +0000 Received: from pps.filterd (m0098421.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id uA8IrsxY076968 for ; Tue, 8 Nov 2016 13:58:20 -0500 Received: from e06smtp13.uk.ibm.com (e06smtp13.uk.ibm.com [195.75.94.109]) by mx0a-001b2d01.pphosted.com with ESMTP id 26kh3mnpy8-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 08 Nov 2016 13:58:20 -0500 Received: from localhost by e06smtp13.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 8 Nov 2016 18:58:18 -0000 Received: from d06dlp01.portsmouth.uk.ibm.com (9.149.20.13) by e06smtp13.uk.ibm.com (192.168.101.143) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 8 Nov 2016 18:58:16 -0000 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 9414317D8056 for ; Tue, 8 Nov 2016 19:00:36 +0000 (GMT) Received: from d06av10.portsmouth.uk.ibm.com (d06av10.portsmouth.uk.ibm.com [9.149.37.251]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id uA8IwF0715008020 for ; Tue, 8 Nov 2016 18:58:15 GMT Received: from d06av10.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av10.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id uA8HwH3G020145 for ; Tue, 8 Nov 2016 10:58:17 -0700 Received: from oc1027705133.ibm.com (dyn-9-152-212-182.boeblingen.de.ibm.com [9.152.212.182]) by d06av10.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id uA8Hvan8019676 for ; Tue, 8 Nov 2016 10:58:17 -0700 From: Andreas Arnez To: gdb-patches@sourceware.org Subject: [PATCH 1/4] tui-disasm: Fix window content buffer overrun Date: Tue, 8 Nov 2016 19:56:51 +0100 In-Reply-To: <1478631454-9447-1-git-send-email-arnez@linux.vnet.ibm.com> References: <1478631454-9447-1-git-send-email-arnez@linux.vnet.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16110818-0012-0000-0000-000004873789 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16110818-0013-0000-0000-00001622E5CA Message-Id: <1478631454-9447-2-git-send-email-arnez@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-11-08_05:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1611080343 X-IsSubscribed: yes A user reported a GDB crash with TUI when trying to debug a function with a long demangled C++ method name. It turned out that the logic for displaying the TUI disassembly window has a bug that can cause a buffer overrun, possibly overwriting GDB-internal data structures. In particular, the logic performs an unguarded strcpy. Another (harmless) bug in tui_alloc_source_buffer causes the buffer to be two lines longer than needed. This may have made the crash appear less frequently. gdb/ChangeLog: * tui/tui-disasm.c (tui_set_disassem_content): Fix line buffer overrun due to unchecked strcpy. gdb/testsuite/ChangeLog: * gdb.base/tui-layout.c: New file. * gdb.base/tui-layout.exp: Use tui-layout.c, to ensure that the disassembly window contains very long lines. --- gdb/testsuite/gdb.base/tui-layout.c | 30 ++++++++++++++++++++++++++++++ gdb/testsuite/gdb.base/tui-layout.exp | 17 ++++++++++++++--- gdb/tui/tui-disasm.c | 24 ++++++++++-------------- 3 files changed, 54 insertions(+), 17 deletions(-) create mode 100644 gdb/testsuite/gdb.base/tui-layout.c diff --git a/gdb/testsuite/gdb.base/tui-layout.c b/gdb/testsuite/gdb.base/tui-layout.c new file mode 100644 index 0000000..951bea7 --- /dev/null +++ b/gdb/testsuite/gdb.base/tui-layout.c @@ -0,0 +1,30 @@ +#define LONGER_NAME(x) x ## x +#define LONGER(x) LONGER_NAME(x) +#define LONGNAME1 d_this_identifier_of_32_chars_an +#define LONGNAME2 LONGER (LONGER (LONGER (LONGER (LONGER (LONGNAME1))))) + +/* Construct a long identifier name. If SHORT_IDENTIFIERS is set, limit + it to 1024 chars. */ + +#ifdef SHORT_IDENTIFIERS +#define LONGNAME3 LONGNAME2 +#else +#define LONGNAME3 LONGER (LONGER (LONGER (LONGER (LONGER (LONGNAME2))))) +#endif + +void LONGNAME3 (void); + +int +main () +{ + LONGNAME3 (); + return 0; +} + +/* Function with a long name. Placing it after main makes it more likely + to be shown in the disassembly window on startup. */ + +void +LONGNAME3 (void) +{ +} diff --git a/gdb/testsuite/gdb.base/tui-layout.exp b/gdb/testsuite/gdb.base/tui-layout.exp index 43b3a4f..0c7169e 100644 --- a/gdb/testsuite/gdb.base/tui-layout.exp +++ b/gdb/testsuite/gdb.base/tui-layout.exp @@ -13,12 +13,23 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -standard_testfile start.c +standard_testfile -if { [prepare_for_testing ${testfile}.exp ${testfile} $srcfile] } { - return -1 +set ccopts {debug quiet} +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "$binfile" \ + executable $ccopts] != "" } { + # Maybe the compiler can't handle arbitrarily long identfier names. + # Try with a shorter version. + lappend ccopts "additional_flags=-DSHORT_IDENTIFIERS" + if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "$binfile" \ + executable $ccopts] != "" } { + fail "compile" + return -1 + } } +clean_restart "$binfile" + if {[skip_tui_tests]} { # TUI support is disabled. Check for error message. gdb_test "layout asm" "Undefined command: \"layout\". Try \"help\"." diff --git a/gdb/tui/tui-disasm.c b/gdb/tui/tui-disasm.c index 29c1443..5368aa4 100644 --- a/gdb/tui/tui-disasm.c +++ b/gdb/tui/tui-disasm.c @@ -172,7 +172,7 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc) enum tui_status ret = TUI_FAILURE; int i; int offset = TUI_DISASM_WIN->detail.source_info.horizontal_offset; - int max_lines; + int max_lines, line_width; CORE_ADDR cur_pc; struct tui_gen_win_info *locator = tui_locator_win_info_ptr (); int tab_len = tui_default_tab_len (); @@ -193,8 +193,9 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc) TUI_DISASM_WIN->detail.source_info.start_line_or_addr.u.addr = pc; cur_pc = locator->content[0]->which_element.locator.addr; - max_lines = TUI_DISASM_WIN->generic.height - 2; /* Account for - hilite. */ + /* Window size, excluding highlight box. */ + max_lines = TUI_DISASM_WIN->generic.height - 2; + line_width = TUI_DISASM_WIN->generic.width - 2; /* Get temporary table that will hold all strings (addr & insn). */ asm_lines = XALLOCAVEC (struct tui_asm_line, max_lines); @@ -233,20 +234,15 @@ tui_set_disassem_content (struct gdbarch *gdbarch, CORE_ADDR pc) src = &element->which_element.source; strcpy (line, asm_lines[i].addr_string); cur_len = strlen (line); - - /* Add spaces to make the instructions start on the same - column. */ - while (cur_len < insn_pos) - { - strcat (line, " "); - cur_len++; - } - - strcat (line, asm_lines[i].insn); + memset (line + cur_len, ' ', insn_pos - cur_len); + strcpy (line + insn_pos, asm_lines[i].insn); /* Now copy the line taking the offset into account. */ if (strlen (line) > offset) - strcpy (src->line, &line[offset]); + { + strncpy (src->line, &line[offset], line_width); + src->line[line_width] = '\0'; + } else src->line[0] = '\0';