Message ID | 20190918030533.365417-1-simon.marchi@polymtl.ca |
---|---|
State | New, archived |
Headers |
Received: (qmail 20894 invoked by alias); 18 Sep 2019 03:05:39 -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 20886 invoked by uid 89); 18 Sep 2019 03:05:39 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL autolearn=ham version=3.3.1 spammy=HContent-Transfer-Encoding:8bit X-HELO: barracuda.ebox.ca Received: from barracuda.ebox.ca (HELO barracuda.ebox.ca) (96.127.255.19) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 18 Sep 2019 03:05:36 +0000 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id ELNsPzL5cdQ9GdIW (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 17 Sep 2019 23:05:34 -0400 (EDT) Received: from simark.lan (unknown [192.222.164.54]) by smtp.ebox.ca (Postfix) with ESMTP id 8B62D441B21; Tue, 17 Sep 2019 23:05:34 -0400 (EDT) From: Simon Marchi <simon.marchi@polymtl.ca> To: gdb-patches@sourceware.org Cc: Simon Marchi <simon.marchi@polymtl.ca> Subject: [PATCH] gdb: remove local extern declaration of cli_styling Date: Tue, 17 Sep 2019 23:05:33 -0400 Message-Id: <20190918030533.365417-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes |
Commit Message
Simon Marchi
Sept. 18, 2019, 3:05 a.m. UTC
Following the int -> bool conversion of boolean options (commit 491144b5e21b ("Change boolean options to bool instead of int")), I see this ASAN error: ==357543==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55555b25d440 at pc 0x5555583ce9e1 bp 0x7fffffffd390 sp 0x7fffffffd380 READ of size 4 at 0x55555b25d440 thread T0 #0 0x5555583ce9e0 in term_cli_styling /home/simark/src/binutils-gdb/gdb/ui-file.c:111 #1 0x5555583cf8b0 in stdio_file::can_emit_style_escape() /home/simark/src/binutils-gdb/gdb/ui-file.c:308 #2 0x5555584450d2 in set_output_style /home/simark/src/binutils-gdb/gdb/utils.c:1442 #3 0x5555584491af in fprintf_styled(ui_file*, ui_file_style const&, char const*, ...) /home/simark/src/binutils-gdb/gdb/utils.c:2143 #4 0x5555582fa13c in print_gdb_version(ui_file*, bool) /home/simark/src/binutils-gdb/gdb/top.c:1339 #5 0x555557b723ab in captured_main_1 /home/simark/src/binutils-gdb/gdb/main.c:981 #6 0x555557b7353b in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1172 #7 0x555557b735d0 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1197 #8 0x55555700a53d in main /home/simark/src/binutils-gdb/gdb/gdb.c:32 #9 0x7ffff64c9ee2 in __libc_start_main (/usr/lib/libc.so.6+0x26ee2) #10 0x55555700a30d in _start (/home/simark/build/binutils-gdb/gdb/gdb+0x1ab630d) 0x55555b25d441 is located 0 bytes to the right of global variable 'cli_styling' defined in '/home/simark/src/binutils-gdb/gdb/cli/cli-style.c:31:6' (0x55555b25d440) of size 1 The reason of this is that we have this declaration of cli_styling in cli/cli-style.h: extern bool cli_styling; but ui-file.c uses its own local declaration: extern int cli_styling; Because of that, the code in ui-file.c thinks the variable is 4 bytes long, when it is actually 1 byte long, so the generated code reads past the variable. Fix it by removing the declaration and making ui-file.c include cli/cli-style.h. gdb/ChangeLog: * ui-file.c: Include cli/cli-style.h. (term_cli_styling): Remove cli_styling declaration. --- gdb/ui-file.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Comments
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
Simon> * ui-file.c: Include cli/cli-style.h.
Simon> (term_cli_styling): Remove cli_styling declaration.
Thanks for doing this.
I think as a rule it's best to always have declarations in a header and
never in the .c code. That way they can be checked against the
definition.
Tom
On 2019-09-18 12:18 p.m., Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes: > > Simon> * ui-file.c: Include cli/cli-style.h. > Simon> (term_cli_styling): Remove cli_styling declaration. > > Thanks for doing this. > > I think as a rule it's best to always have declarations in a header and > never in the .c code. That way they can be checked against the > definition. Yes, agreed. Actually, I didn't push this patch directly, even though it seemed obvious, because I was wondering if there was some good reason I was missing for not doing it like this in the first place. I pushed it now. Simon
diff --git a/gdb/ui-file.c b/gdb/ui-file.c index 042b13ca3b18..71b74bba1905 100644 --- a/gdb/ui-file.c +++ b/gdb/ui-file.c @@ -24,6 +24,7 @@ #include "gdb_obstack.h" #include "gdb_select.h" #include "gdbsupport/filestuff.h" +#include "cli/cli-style.h" null_file null_stream; @@ -106,8 +107,6 @@ ui_file_isatty (struct ui_file *file) static bool term_cli_styling () { - extern int cli_styling; - if (!cli_styling) return false;