From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106479 invoked by alias); 2 Nov 2016 23:28:53 -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 106456 invoked by uid 89); 2 Nov 2016 23:28:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.6 required=5.0 tests=BAYES_50,RCVD_IN_DNSWL_LOW,RCVD_IN_SEMBACKSCATTER,SPF_PASS autolearn=ham version=3.3.2 spammy=r13, Assume, 85706, strrchr X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 02 Nov 2016 23:28:42 +0000 Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id uA2NOENo001855 for ; Wed, 2 Nov 2016 19:28:41 -0400 Received: from e19.ny.us.ibm.com (e19.ny.us.ibm.com [129.33.205.209]) by mx0a-001b2d01.pphosted.com with ESMTP id 26fmrtjr3j-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 02 Nov 2016 19:28:40 -0400 Received: from localhost by e19.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 2 Nov 2016 19:28:39 -0400 Received: from d01dlp02.pok.ibm.com (9.56.250.167) by e19.ny.us.ibm.com (146.89.104.206) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 2 Nov 2016 19:28:36 -0400 Received: from b01cxnp22034.gho.pok.ibm.com (b01cxnp22034.gho.pok.ibm.com [9.57.198.24]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 3B06B6E803F; Wed, 2 Nov 2016 19:28:11 -0400 (EDT) Received: from b01ledav005.gho.pok.ibm.com (b01ledav005.gho.pok.ibm.com [9.57.199.110]) by b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id uA2NSZem28967098; Wed, 2 Nov 2016 23:28:35 GMT Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id CDE39AE04B; Wed, 2 Nov 2016 19:28:35 -0400 (EDT) Received: from otta.local (unknown [9.80.227.229]) by b01ledav005.gho.pok.ibm.com (Postfix) with ESMTP id 46157AE03B; Wed, 2 Nov 2016 19:28:35 -0400 (EDT) Subject: Re: [PATCH, RFC] Add support for choosing disassembler cpu in GDB for POWER. To: Ulrich Weigand , Pedro Alves References: <20161028184714.6B7EC10B927@oc8523832656.ibm.com> Cc: Alan Modra , gdb-patches@sourceware.org, binutils From: Peter Bergner Date: Wed, 02 Nov 2016 23:28:00 -0000 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161028184714.6B7EC10B927@oc8523832656.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16110223-0056-0000-0000-000001CFD3C7 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006022; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000189; SDB=6.00775993; UDB=6.00373196; IPR=6.00553113; BA=6.00004852; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00013193; XFM=3.00000011; UTC=2016-11-02 23:28:37 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16110223-0057-0000-0000-00000602F0A2 Message-Id: <5fe2e203-d607-285b-fe28-a273cf4a132f@vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-11-02_07:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1611020426 X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg00023.txt.bz2 On 10/28/16 1:47 PM, Ulrich Weigand wrote: > Peter Bergner wrote: >> On 10/28/16 9:15 AM, Ulrich Weigand wrote: >>> It's probably not that important to exactly match objdump >>> behavior here. B.t.w. how do you even enter a space as >>> separator with the -M option? >> >> bergner@genoa:~$ objdump -d -M'power5 power6' wait.o | grep warning >> warning: ignoring unknown -Mpower5 power6 option > > OK, well :-) As I said, it's probably not important to > exactly match *this* detail in the GDB command ... I actually ended up adding code to clean up white space and extra commas (ala ARM's arm-dis.c handling). I did this for two reasons. The first reason is that tab completion seems to add some white space to the end of the disassembler option and that was causing us to not match the expected string. Secondly, I wanted a way for the user to go back to the default (ie, NULL) disassembler option. I could only do that first I removed white space. I added the comma cleanup since it was easy. I'll note I modified the completion function slightly. On POWER, a disassembler_option value of "power8,any" is valid. If I have typed "power8,a" and hit tab, I wasn't getting any completions. I modified the completion to complete only on the text after the last ',', which allows me to complete "a" to "any", even if I have "power8,a". >> But not all arches have descriptions paired with the option name. >> Looking at Pedro's objdump output from before: [snip] > Hmm, I see. I guess we could have NULL description string for those, > and then the common printing routine could choose the compact output ... Ok, if an architecture registers a descriptions array, then they get the long form. Otherwise, we emit a compact form. I ported ppc, arm and s390 over to use the new support. S390 was fairly easy, but I had to modify the initialization code in s390-dis.c, since the current code has no way to rest the current_arch_mask and option_use_insn_len_bits_p values. Without the current code, I was able to set and change them, but there was no way to go back to the default values. ARM was a little trickier, since it already has a "set arm disassembler" command. I had to make both commands set the same gdbarch value. Note that the "set disassenbler-options ..." command uses long reg names (eg, reg-names-std) rather than the short (eg, "std") names that the "set arm disassembler ..." option uses. I had to do this, since arm-dis.c expects the info->disassembler_options string to contain long names. How does this iteration of the patch look? Peter include/ * dis-asm.h (parse_arm_disassembler_option): Remove prototype. (set_arm_regname_option): Likewise. (disassemble_init_s390): New prototype. (disassembler_options_names_powerpc): Likewise. (disassembler_options_names_arm): Likewise. (disassembler_options_desc_arm): Likewise. (disassembler_options_names_s390): Likewise. (disassembler_options_desc_s390): Likewise. opcodes/ * disassemble.c (disassemble_init_for_target): Handle s390 init. * ppc-dis.c: Include "libiberty.h". (ppc_opts): Add "32" and "64" entries. (parse_ppc_dis_option): New function. (disassembler_options_names_powerpc): Likewise. (powerpc_init_dialect): Use parse_ppc_dis_option(). Add break to switch statement. (print_ppc_disassembler_options): Remove printing of "32" and "64". * arm-dis.c: Include "libiberty.h". (struct arm_regname): Add 'long_name' field. (regnames): Initialize it. (set_arm_regname_option): Remove function. (parse_arm_disassembler_option): Make static. (disassembler_options_names_arm): New function. (disassembler_options_desc_arm): Likewise. * s390-dis.c: Include "libiberty.h". (struct options_t): New structure type. (options): New structure. (init_disasm): Rename from this... (disassemble_init_s390): ...to this. Add initializations for current_arch_mask and option_use_insn_len_bits_p. Remove init_flag. (disassembler_options_names_s390): New function. (disassembler_options_desc_s390): Likewise. (print_s390_disassembler_options): Print using information from struct 'options'. gdb/ * gdbarch.sh (gdbarch_disassembler_options): New variable. (gdbarch_disassembler_options_names): Likewise. (gdbarch_disassembler_options_descriptions): Likewise. * gdbarch.c: Regenerate. * gdbarch.h: Likewise. * disasm.c: Include "arch-utils.h", "gdbcmd.h", "gdbcmd.h" and "safe-ctype.h". (gdb_disassemble_info): Initilize di.disassembler_options. (gdb_buffered_insn_length_init_dis): Initilize di->application_data and di->disassembler_options. (cleanup_disassembler_options): New function. (parse_disassembler_options): Likewise. (set_disassembler_options): Likewise. (show_disassembler_options): Likewise. (disassembler_options_completer): Likewise. (_initialize_disasm): Likewise. * disasm.h (set_disassembler_options): New prototype. (show_disassembler_options): Likewise. * rs6000-tdep.c (rs6000_gdbarch_init): Call set_gdbarch_disassembler_options_names. * arm-tdep.c: Include "disasm.h" and "cli/cli-decode.h". (disassembly_style): Delete static variable. (set_disassembly_style): Delete function and prototype. (show_disassembly_style_sfunc): New function. (set_disassembly_style_sfunc): Call set_disassembler_options. (arm_gdbarch_init): Call set_gdbarch_disassembler_options, set_gdbarch_disassembler_options_names and set_gdbarch_disassembler_options_descriptions. (_initialize_arm_tdep): New static variable 'disassembly_style'; Remove calls to parse_arm_disassembler_option & set_arm_regname_option. Pass show_disassembly_style_sfunc to the "disassembler" setshow command. * s390-tdep.c (s390_gdbarch_init): Call functions set_gdbarch_disassembler_options_names and set_gdbarch_disassembler_options_descriptions. diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 645825f..acdd0bb 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -27,6 +27,8 @@ #include "gdbcmd.h" #include "gdbcore.h" #include "dis-asm.h" /* For register styles. */ +#include "disasm.h" +#include "cli/cli-decode.h" #include "regcache.h" #include "reggroups.h" #include "doublest.h" @@ -217,14 +219,13 @@ static const char *const arm_register_names[] = /* Valid register name styles. */ static const char **valid_disassembly_styles; -/* Disassembly style to use. Default to "std" register names. */ -static const char *disassembly_style; - /* This is used to keep the bfd arch_info in sync with the disassembly style. */ static void set_disassembly_style_sfunc(char *, int, struct cmd_list_element *); -static void set_disassembly_style (void); +static void show_disassembly_style_sfunc (struct ui_file *, int, + struct cmd_list_element *, + const char *); static void convert_from_extended (const struct floatformat *, const void *, void *, int); @@ -8508,9 +8509,27 @@ arm_show_force_mode (struct ui_file *file, int from_tty, static void set_disassembly_style_sfunc (char *args, int from_tty, - struct cmd_list_element *c) + struct cmd_list_element *c) +{ + /* Convert the short style name into the long style name (eg, reg-names-*) + before calling the generic set_disassembler_options() function. */ + char long_name[256], *style = long_name; + snprintf (style, 256, "reg-names-%s", *(char **)c->var); + c->var = &style; + set_disassembler_options (args, from_tty, c); +} + +static void +show_disassembly_style_sfunc (struct ui_file *file, int from_tty, + struct cmd_list_element *c, const char *value) { - set_disassembly_style (); + struct gdbarch *gdbarch = get_current_arch (); + const char *style = gdbarch_disassembler_options (gdbarch); + if (style == NULL) + style = "default"; + else if (CONST_STRNEQ (style, "reg-names-")) + style += strlen ("reg-names-"); + fprintf_unfiltered (file, "The disassembly style is \"%s\".\n", style); } /* Return the ARM register name corresponding to register I. */ @@ -8551,21 +8570,6 @@ arm_register_name (struct gdbarch *gdbarch, int i) return arm_register_names[i]; } -static void -set_disassembly_style (void) -{ - int current; - - /* Find the style that the user wants. */ - for (current = 0; current < num_disassembly_options; current++) - if (disassembly_style == valid_disassembly_styles[current]) - break; - gdb_assert (current < num_disassembly_options); - - /* Synchronize the disassembler. */ - set_arm_regname_option (current); -} - /* Test whether the coff symbol specific value corresponds to a Thumb function. */ @@ -9529,6 +9533,12 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) user_reg_add (gdbarch, arm_register_aliases[i].name, value_of_arm_user_reg, &arm_register_aliases[i].regnum); + set_gdbarch_disassembler_options (gdbarch, xstrdup ("reg-names-std")); + set_gdbarch_disassembler_options_names (gdbarch, + disassembler_options_names_arm ()); + set_gdbarch_disassembler_options_descriptions + (gdbarch, disassembler_options_desc_arm ()); + return gdbarch; } @@ -9549,6 +9559,9 @@ extern initialize_file_ftype _initialize_arm_tdep; /* -Wmissing-prototypes */ void _initialize_arm_tdep (void) { + /* Disassembly style to use. Default to "std" register names. */ + static const char *disassembly_style; + struct ui_file *stb; long length; const char *setname; @@ -9595,9 +9608,6 @@ _initialize_arm_tdep (void) _("Various ARM-specific commands."), &showarmcmdlist, "show arm ", 0, &showlist); - /* Sync the opcode insn printer with our register viewer. */ - parse_arm_disassembler_option ("reg-names-std"); - /* Initialize the array that will be passed to add_setshow_enum_cmd(). */ valid_disassembly_styles = XNEWVEC (const char *, @@ -9609,13 +9619,6 @@ _initialize_arm_tdep (void) length = snprintf (rdptr, rest, "%s - %s\n", setname, setdesc); rdptr += length; rest -= length; - /* When we find the default names, tell the disassembler to use - them. */ - if (!strcmp (setname, "std")) - { - disassembly_style = setname; - set_arm_regname_option (i); - } } /* Mark the end of valid options. */ valid_disassembly_styles[num_disassembly_options] = NULL; @@ -9635,8 +9638,7 @@ _initialize_arm_tdep (void) _("Show the disassembly style."), helptext, set_disassembly_style_sfunc, - NULL, /* FIXME: i18n: The disassembly style is - \"%s\". */ + show_disassembly_style_sfunc, &setarmcmdlist, &showarmcmdlist); add_setshow_boolean_cmd ("apcs32", no_class, &arm_apcs_32, diff --git a/gdb/disasm.c b/gdb/disasm.c index 07c3abe..9a3d6af 100644 --- a/gdb/disasm.c +++ b/gdb/disasm.c @@ -18,13 +18,17 @@ along with this program. If not, see . */ #include "defs.h" +#include "arch-utils.h" #include "target.h" #include "value.h" #include "ui-out.h" #include "disasm.h" #include "gdbcore.h" +#include "gdbcmd.h" #include "dis-asm.h" +#include "cli/cli-decode.h" #include "source.h" +#include "safe-ctype.h" #include /* Disassemble functions. @@ -785,6 +789,7 @@ gdb_disassemble_info (struct gdbarch *gdbarch, struct ui_file *file) di.endian = gdbarch_byte_order (gdbarch); di.endian_code = gdbarch_byte_order_for_code (gdbarch); di.application_data = gdbarch; + di.disassembler_options = gdbarch_disassembler_options (gdbarch); disassemble_init_for_target (&di); return di; } @@ -901,6 +906,8 @@ gdb_buffered_insn_length_init_dis (struct gdbarch *gdbarch, di->endian = gdbarch_byte_order (gdbarch); di->endian_code = gdbarch_byte_order_for_code (gdbarch); + di->application_data = gdbarch; + di->disassembler_options = gdbarch_disassembler_options (gdbarch); disassemble_init_for_target (di); } @@ -917,3 +924,222 @@ gdb_buffered_insn_length (struct gdbarch *gdbarch, return gdbarch_print_insn (gdbarch, addr, &di); } + +/* Remove whitespace and consecutive commas from OPTIONS. */ + +char * +cleanup_disassembler_options (char *options) +{ + char *str; + size_t i, len; + + if (options == NULL) + return NULL; + + /* Strip off all trailing whitespace and commas. */ + for (len = strlen (options); len > 0; len--) + { + if (!ISSPACE (options[len - 1]) && options[len - 1] != ',') + break; + options[len - 1] = '\0'; + } + + /* Convert all remaining whitespace to commas. */ + for (i = 0; options[i] != '\0'; i++) + if (ISSPACE (options[i])) + options[i] = ','; + + /* Remove consecutive commas. */ + for (str = options; *str != '\0'; str++) + if (*str == ',' && *(str + 1) == ',') + { + char *next = str++; + while (*next == ',') + next++; + len = strlen (next); + memmove (str, next, len); + next[len - (size_t)(next - str)] = '\0'; + } + return (strlen (options) != 0) ? options : NULL; +} + +/* Parse OPTIONS looking for ',' seperated disassembler options and + verify each option is valid. Return NULL if all options are valid. + Otherwise, return a pointer to the first invalid disassembler option. */ + +static char * +parse_disassembler_options (const char *options, const char **valid_names) +{ + static char opt[256]; + size_t i; + + if (options == NULL) + return NULL; + + while (*options) + { + /* Copy the current disassembler option into OPT. */ + const char *separator = strchr (options, ','); + if (separator != NULL) + { + strncpy (opt, options, (size_t) (separator - options)); + opt[(size_t) (separator - options)] = 0; + options = separator; + /* Skip to the next disassembler option. */ + while (*options == ',') + options++; + } + else + options = stpcpy (opt, options); + + /* Verify OPT is a valid disassembler option. */ + for (i = 0; valid_names[i] != NULL; i++) + if (strcmp (opt, valid_names[i]) == 0) + break; + if (valid_names[i] == NULL) + return opt; + } + + return NULL; +} + +void +set_disassembler_options (char *args, int from_tty, struct cmd_list_element *c) +{ + struct gdbarch *gdbarch = get_current_arch (); + const char **valid_names = gdbarch_disassembler_options_names (gdbarch); + if (valid_names == NULL) + { + fprintf_filtered (gdb_stdlog, _("\ +'set disassembler' is not supported on this architecture.\n")); + return; + } + + char *options = *(char **)c->var; + options = cleanup_disassembler_options (options); + char *opt = parse_disassembler_options (options, valid_names); + if (opt != NULL) + { + fprintf_filtered (gdb_stdlog, + _("Invalid disassembler option value: '%s'.\n"), opt); + return; + } + + free (gdbarch_disassembler_options (gdbarch)); + if (options != NULL) + set_gdbarch_disassembler_options (gdbarch, xstrdup (options)); + else + set_gdbarch_disassembler_options (gdbarch, NULL); +} + +void +show_disassembler_options (struct ui_file *file, int from_tty, + struct cmd_list_element *c, const char *value) +{ + struct gdbarch *gdbarch = get_current_arch (); + + const char *options = gdbarch_disassembler_options (gdbarch); + if (options == NULL) + options = "default"; + + fprintf_filtered (file, _("The current disassembler options are '%s'\n"), + options); + + const char **names = gdbarch_disassembler_options_names (gdbarch); + const char **desc = gdbarch_disassembler_options_descriptions (gdbarch); + + if (names == NULL) + return; + + fprintf_filtered (file, _("\n\ +The following disassembler options are supported for use with\n\ +the 'set disassembler-options