From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13599 invoked by alias); 26 Mar 2013 18:07:14 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 13310 invoked by uid 89); 26 Mar 2013 18:07:05 -0000 X-Spam-SWARE-Status: No, score=-8.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 26 Mar 2013 18:07:02 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2QI6uAM007814 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 26 Mar 2013 14:06:58 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r2QI6tnQ004172; Tue, 26 Mar 2013 14:06:56 -0400 Message-ID: <5151E3BE.90603@redhat.com> Date: Tue, 26 Mar 2013 19:35:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130311 Thunderbird/17.0.4 MIME-Version: 1.0 To: "Metzger, Markus T" CC: GDB Patches Subject: Re: "set record instruction-history" ? References: <51507403.6030208@redhat.com> <51508D60.6040906@redhat.com> <5151836D.90401@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-03/txt/msg00978.txt.bz2 On 03/26/2013 11:50 AM, Metzger, Markus T wrote: >> -----Original Message----- >> From: Pedro Alves [mailto:palves@redhat.com] >> Sent: Tuesday, March 26, 2013 12:16 PM > > >>> The functionality should be covered by the gdb.btrace test suite. Do you want >>> to commit your patch or do you want me to test and then commit it? >> >> Could you test and report back please? I believe my laptop's cpu is >> one of those where branch tracing is borked and ended up disabled. > > No regressions. Thanks! Taking into account the push back in making the set history size command signed, I've adjusted the patch to keep this command signed as well and checked it in. I tried the different numbers that should be affected manually and I believe it to be an equivalent patch from what you tested. Let me know, if something broke. ---------------- Subject: [PATCH] "set record instruction-history-size"/"set record function-call-history-size" range validation. While the commands are uinteger, the target interfaces are limited to INT_MAX. Don't let the user request more than we can handle. gdb/ 2013-03-26 Pedro Alves * record.c (record_insn_history_size_setshow_var) (record_call_history_size_setshow_var): New globals. (command_size_to_target_size): New function. (cmd_record_insn_history, cmd_record_call_history): Use command_size_to_target_size instead of cast. (validate_history_size, set_record_insn_history_size) (set_record_call_history_size): New functions. (_initialize_record): Install set_record_insn_history_size and set_record_call_history_size as "set" hooks of "set record instruction-history-size" and "set record function-call-history-size". --- gdb/record.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 87 insertions(+), 16 deletions(-) diff --git a/gdb/record.c b/gdb/record.c index 6bc1704..b64f3bc 100644 --- a/gdb/record.c +++ b/gdb/record.c @@ -35,9 +35,18 @@ unsigned int record_debug = 0; /* The number of instructions to print in "record instruction-history". */ static unsigned int record_insn_history_size = 10; +/* The variable registered as control variable in the "record + instruction-history" command. Necessary for extra input + validation. */ +static unsigned int record_insn_history_size_setshow_var; + /* The number of functions to print in "record function-call-history". */ static unsigned int record_call_history_size = 10; +/* The variable registered as control variable in the "record + call-history" command. Necessary for extra input validation. */ +static unsigned int record_call_history_size_setshow_var; + struct cmd_list_element *record_cmdlist = NULL; struct cmd_list_element *set_record_cmdlist = NULL; struct cmd_list_element *show_record_cmdlist = NULL; @@ -428,6 +437,25 @@ get_insn_history_modifiers (char **arg) return modifiers; } +/* The "set record instruction-history-size / set record + function-call-history-size" commands are unsigned, with UINT_MAX + meaning unlimited. The target interfaces works with signed int + though, to indicate direction, so map "unlimited" to INT_MAX, which + is about the same as unlimited in practice. If the user does have + a log that huge, she can can fetch it in chunks across several + requests, but she'll likely have other problems first... */ + +static int +command_size_to_target_size (unsigned int *command) +{ + gdb_assert (*command <= INT_MAX || *command == UINT_MAX); + + if (record_call_history_size == UINT_MAX) + return INT_MAX; + else + return *command; +} + /* The "record instruction-history" command. */ static void @@ -439,11 +467,7 @@ cmd_record_insn_history (char *arg, int from_tty) flags = get_insn_history_modifiers (&arg); - /* We use a signed size to also indicate the direction. Make sure that - unlimited remains unlimited. */ - size = (int) record_insn_history_size; - if (size < 0) - size = INT_MAX; + size = command_size_to_target_size (&record_insn_history_size); if (arg == NULL || *arg == 0 || strcmp (arg, "+") == 0) target_insn_history (size, flags); @@ -559,11 +583,7 @@ cmd_record_call_history (char *arg, int from_tty) flags = get_call_history_modifiers (&arg); - /* We use a signed size to also indicate the direction. Make sure that - unlimited remains unlimited. */ - size = (int) record_call_history_size; - if (size < 0) - size = INT_MAX; + size = command_size_to_target_size (&record_call_history_size); if (arg == NULL || *arg == 0 || strcmp (arg, "+") == 0) target_call_history (size, flags); @@ -617,6 +637,51 @@ cmd_record_call_history (char *arg, int from_tty) } } +/* Helper for "set record instruction-history-size" and "set record + function-call-history-size" input validation. COMMAND_VAR is the + variable registered in the command as control variable. *SETTING + is the real setting the command allows changing. */ + +static void +validate_history_size (unsigned int *command_var, int *setting) +{ + if (*command_var != UINT_MAX && *command_var > INT_MAX) + { + unsigned int new_value = *command_var; + + /* Restore previous value. */ + *command_var = *setting; + error (_("integer %u out of range"), new_value); + } + + /* Commit new value. */ + *setting = *command_var; +} + +/* Called by do_setshow_command. We only want values in the + [0..INT_MAX] range, while the command's machinery accepts + [0..UINT_MAX]. See command_size_to_target_size. */ + +static void +set_record_insn_history_size (char *args, int from_tty, + struct cmd_list_element *c) +{ + validate_history_size (&record_insn_history_size_setshow_var, + &record_insn_history_size); +} + +/* Called by do_setshow_command. We only want values in the + [0..INT_MAX] range, while the command's machinery accepts + [0..UINT_MAX]. See command_size_to_target_size. */ + +static void +set_record_call_history_size (char *args, int from_tty, + struct cmd_list_element *c) +{ + validate_history_size (&record_call_history_size_setshow_var, + &record_call_history_size); +} + /* Provide a prototype to silence -Wmissing-prototypes. */ extern initialize_file_ftype _initialize_record; @@ -634,18 +699,20 @@ _initialize_record (void) &showdebuglist); add_setshow_uinteger_cmd ("instruction-history-size", no_class, - &record_insn_history_size, _("\ + &record_insn_history_size_setshow_var, _("\ Set number of instructions to print in \"record instruction-history\"."), _("\ Show number of instructions to print in \"record instruction-history\"."), - NULL, NULL, NULL, &set_record_cmdlist, - &show_record_cmdlist); + NULL, + set_record_insn_history_size, NULL, + &set_record_cmdlist, &show_record_cmdlist); add_setshow_uinteger_cmd ("function-call-history-size", no_class, - &record_call_history_size, _("\ + &record_call_history_size_setshow_var, _("\ Set number of function to print in \"record function-call-history\"."), _("\ Show number of functions to print in \"record function-call-history\"."), - NULL, NULL, NULL, &set_record_cmdlist, - &show_record_cmdlist); + NULL, + set_record_call_history_size, NULL, + &set_record_cmdlist, &show_record_cmdlist); c = add_prefix_cmd ("record", class_obscure, cmd_record_start, _("Start recording."), @@ -727,4 +794,8 @@ from the first argument.\n\ The number of functions to print can be defined with \"set record \ function-call-history-size\"."), &record_cmdlist); + + /* Sync command control variables. */ + record_insn_history_size_setshow_var = record_insn_history_size; + record_call_history_size_setshow_var = record_call_history_size; } -- 1.7.11.7