public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] gdb, opcodes: Add non-enum disassembler options
@ 2022-08-31  2:15 Tsukasa OI
  2022-08-31  2:15 ` [PATCH 1/2] " Tsukasa OI
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Tsukasa OI @ 2022-08-31  2:15 UTC (permalink / raw)
  To: Tsukasa OI, Andrew Burgess, Palmer Dabbelt, Claudiu Zissulescu,
	Chenghua Xu, Nelson Chu
  Cc: binutils, gdb-patches

Hello,

This is a part of my work: implement `arch' disassembler option in RISC-V.
However, it requires technical changes also affecting opcodes:ARC and MIPS
and GDB.  It will take some time because we have to wait many Binutils
prerequisites but this technical change can be discussed now (due to it
affects both Binutils and GDB).

PATCH 1/2: Binutils changes
PATCH 2/2: GDB changes

Independently applying Binutils/GDB changes is completely safe because we
haven't implemented any actual non-enum options.


[Example: Implement `arch' disassembler option]

$ objdump -b binary -m riscv:rv32 -M arch=rv32i_zfinx -D sample.bin
(... analyze a binary file with 'RV32I_Zfinx' ISA)

You can try my modified version at:
<https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_arch_priv_spec>
<https://github.com/a4lg/binutils-gdb/tree/riscv-dis-arch-priv-spec>


[Technical Changes]

There is a portable mechanism for disassembler options and used on some
architectures:

-   ARC
-   Arm
-   MIPS
-   PowerPC
-   RISC-V
-   S/390

However, it only supports following forms:

-   [NAME]
-   [NAME]=[ENUM_VALUE]

Valid values for [ENUM_VALUE] must be predefined in
`disasm_option_arg_t.values'.  For instance, for -M cpu=[CPU] in ARC
architecture, opcodes/arc-dis.c builds valid CPU model list from
include/elf/arc-cpu.def.

This patchset adds following third format:

-   [NAME]=[ARBITRARY_VALUE] (cannot contain "," though)

This is identified by `NULL' value of `disasm_option_arg_t.values'
(normally, this is a non-NULL pointer to a NULL-terminated list).

Note that this patch modifies following architectures (that use
similar code to print disassembler help message) for consistency:

-   ARC
-   MIPS
-   RISC-V

In the future, adding "verify" function to disasm_option_arg_t (or some)
might be an option as it may provide flexible argument validation.

Thanks,
Tsukasa




Tsukasa OI (2):
  opcodes: Add non-enum disassembler options
  gdb: Add non-enum disassembler options

 gdb/disasm.c        | 4 ++++
 include/dis-asm.h   | 3 ++-
 opcodes/arc-dis.c   | 2 ++
 opcodes/mips-dis.c  | 2 ++
 opcodes/riscv-dis.c | 2 ++
 5 files changed, 12 insertions(+), 1 deletion(-)


base-commit: 803584b96d97e1f6ea50b0a0064d2a03ab0baa60
-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] opcodes: Add non-enum disassembler options
  2022-08-31  2:15 [PATCH 0/2] gdb, opcodes: Add non-enum disassembler options Tsukasa OI
@ 2022-08-31  2:15 ` Tsukasa OI
  2022-09-01 12:03   ` Nick Clifton
  2022-08-31  2:15 ` [PATCH 2/2] gdb: " Tsukasa OI
  2022-09-04  8:03 ` [PATCH v2 0/2] gdb, opcodes: " Tsukasa OI
  2 siblings, 1 reply; 10+ messages in thread
From: Tsukasa OI @ 2022-08-31  2:15 UTC (permalink / raw)
  To: Tsukasa OI, Andrew Burgess, Palmer Dabbelt, Claudiu Zissulescu,
	Chenghua Xu, Nelson Chu
  Cc: binutils, gdb-patches

This is paired with "gdb: Add non-enum disassembler options".

There is a portable mechanism for disassembler options and used on some
architectures:
- ARC
- Arm
- MIPS
- PowerPC
- RISC-V
- S/390

However, it only supports following forms:

- [NAME]
- [NAME]=[ENUM_VALUE]

Valid values for [ENUM_VALUE] must be predefined in
`disasm_option_arg_t.values'. For instance, for -M cpu=[CPU] in ARC
architecture, opcodes/arc-dis.c builds valid CPU model list from
include/elf/arc-cpu.def.

In this commit, it adds following format:

- [NAME]=[ARBITRARY_VALUE] (cannot contain "," though)

This is identified by NULL' value of disasm_option_arg_t.values'
(normally, this is a non-NULL pointer to a NULL-terminated list).

gdb/ChangeLog:

    * gdb/disasm.c (set_disassembler_options): Add support for
    non-enum disassembler options.
    (show_disassembler_options_sfunc): Likewise.

include/ChangeLog:

    * dis-asm.h (disasm_option_arg_t): Update comment of `values'
    to allow non-enum disassembler options.

opcodes/ChangeLog:

    * riscv-dis.c (print_riscv_disassembler_options): Support
    non-enum disassembler options on printing disassembler help.
    * arc-dis.c (print_arc_disassembler_options): Likewise.
    * mips-dis.c (print_mips_disassembler_options): Likewise.
---
 include/dis-asm.h   | 3 ++-
 opcodes/arc-dis.c   | 2 ++
 opcodes/mips-dis.c  | 2 ++
 opcodes/riscv-dis.c | 2 ++
 4 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/dis-asm.h b/include/dis-asm.h
index f1a83dc84e5..4921c040710 100644
--- a/include/dis-asm.h
+++ b/include/dis-asm.h
@@ -318,7 +318,8 @@ typedef struct
   /* Option argument name to use in descriptions.  */
   const char *name;
 
-  /* Vector of acceptable option argument values, NULL-terminated.  */
+  /* Vector of acceptable option argument values, NULL-terminated.
+     NULL if any values are accepted.  */
   const char **values;
 } disasm_option_arg_t;
 
diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
index 3490bad4f66..c8dc525f64d 100644
--- a/opcodes/arc-dis.c
+++ b/opcodes/arc-dis.c
@@ -1611,6 +1611,8 @@ print_arc_disassembler_options (FILE *stream)
   for (i = 0; args[i].name != NULL; ++i)
     {
       size_t len = 3;
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
diff --git a/opcodes/mips-dis.c b/opcodes/mips-dis.c
index 9db604ffb39..faeebccfc3b 100644
--- a/opcodes/mips-dis.c
+++ b/opcodes/mips-dis.c
@@ -2809,6 +2809,8 @@ with the -M switch (multiple options should be separated by commas):\n\n"));
 
   for (i = 0; args[i].name != NULL; i++)
     {
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 164fd209dbd..b2debd3dd97 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -1181,6 +1181,8 @@ with the -M switch (multiple options should be separated by commas):\n"));
 
   for (i = 0; args[i].name != NULL; i++)
     {
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] gdb: Add non-enum disassembler options
  2022-08-31  2:15 [PATCH 0/2] gdb, opcodes: Add non-enum disassembler options Tsukasa OI
  2022-08-31  2:15 ` [PATCH 1/2] " Tsukasa OI
@ 2022-08-31  2:15 ` Tsukasa OI
  2022-09-02 10:00   ` Andrew Burgess
  2022-09-04  8:03 ` [PATCH v2 0/2] gdb, opcodes: " Tsukasa OI
  2 siblings, 1 reply; 10+ messages in thread
From: Tsukasa OI @ 2022-08-31  2:15 UTC (permalink / raw)
  To: Tsukasa OI, Andrew Burgess, Palmer Dabbelt, Claudiu Zissulescu,
	Chenghua Xu, Nelson Chu
  Cc: binutils, gdb-patches

This is paired with "opcodes: Add non-enum disassembler options".

There is a portable mechanism for disassembler options and used on some
architectures:
- ARC
- Arm
- MIPS
- PowerPC
- RISC-V
- S/390

However, it only supports following forms:

- [NAME]
- [NAME]=[ENUM_VALUE]

Valid values for [ENUM_VALUE] must be predefined in
`disasm_option_arg_t.values'. For instance, for -M cpu=[CPU] in ARC
architecture, opcodes/arc-dis.c builds valid CPU model list from
include/elf/arc-cpu.def.

In this commit, it adds following format:

- [NAME]=[ARBITRARY_VALUE] (cannot contain "," though)

This is identified by NULL' value of disasm_option_arg_t.values'
(normally, this is a non-NULL pointer to a NULL-terminated list).

gdb/ChangeLog:

    * gdb/disasm.c (set_disassembler_options): Add support for
    non-enum disassembler options.
    (show_disassembler_options_sfunc): Likewise.
---
 gdb/disasm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/disasm.c b/gdb/disasm.c
index db6724757ac..fe4eed2d524 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -1270,6 +1270,8 @@ set_disassembler_options (const char *prospective_options)
 	    if (memcmp (opt, valid_options->name[i], len) != 0)
 	      continue;
 	    arg = opt + len;
+	    if (valid_options->arg[i]->values == NULL)
+	      break;
 	    for (j = 0; valid_options->arg[i]->values[j] != NULL; j++)
 	      if (disassembler_options_cmp
 		    (arg, valid_options->arg[i]->values[j]) == 0)
@@ -1391,6 +1393,8 @@ The following disassembler options are supported for use with the\n\
 
       for (i = 0; valid_args[i].name != NULL; i++)
 	{
+	  if (valid_args[i].values == NULL)
+	    continue;
 	  gdb_printf (file, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 		      valid_args[i].name);
-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] opcodes: Add non-enum disassembler options
  2022-08-31  2:15 ` [PATCH 1/2] " Tsukasa OI
@ 2022-09-01 12:03   ` Nick Clifton
  0 siblings, 0 replies; 10+ messages in thread
From: Nick Clifton @ 2022-09-01 12:03 UTC (permalink / raw)
  To: Tsukasa OI, Andrew Burgess, Palmer Dabbelt, Claudiu Zissulescu,
	Chenghua Xu, Nelson Chu
  Cc: binutils, gdb-patches

Hi Tsukasa,

> gdb/ChangeLog:
> 
>      * gdb/disasm.c (set_disassembler_options): Add support for
>      non-enum disassembler options.
>      (show_disassembler_options_sfunc): Likewise.
> 
> include/ChangeLog:
> 
>      * dis-asm.h (disasm_option_arg_t): Update comment of `values'
>      to allow non-enum disassembler options.
> 
> opcodes/ChangeLog:
> 
>      * riscv-dis.c (print_riscv_disassembler_options): Support
>      non-enum disassembler options on printing disassembler help.
>      * arc-dis.c (print_arc_disassembler_options): Likewise.
>      * mips-dis.c (print_mips_disassembler_options): Likewise.

The binutils parts of this patch are approved.

Cheers
   Nick



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] gdb: Add non-enum disassembler options
  2022-08-31  2:15 ` [PATCH 2/2] gdb: " Tsukasa OI
@ 2022-09-02 10:00   ` Andrew Burgess
  0 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2022-09-02 10:00 UTC (permalink / raw)
  To: Tsukasa OI, Tsukasa OI, Palmer Dabbelt, Claudiu Zissulescu,
	Chenghua Xu, Nelson Chu
  Cc: binutils, gdb-patches

Tsukasa OI <research_trasio@irq.a4lg.com> writes:

> This is paired with "opcodes: Add non-enum disassembler options".
>
> There is a portable mechanism for disassembler options and used on some
> architectures:
> - ARC
> - Arm
> - MIPS
> - PowerPC
> - RISC-V
> - S/390
>
> However, it only supports following forms:
>
> - [NAME]
> - [NAME]=[ENUM_VALUE]
>
> Valid values for [ENUM_VALUE] must be predefined in
> `disasm_option_arg_t.values'. For instance, for -M cpu=[CPU] in ARC
> architecture, opcodes/arc-dis.c builds valid CPU model list from
> include/elf/arc-cpu.def.
>
> In this commit, it adds following format:
>
> - [NAME]=[ARBITRARY_VALUE] (cannot contain "," though)
>
> This is identified by NULL' value of disasm_option_arg_t.values'
> (normally, this is a non-NULL pointer to a NULL-terminated list).
>
> gdb/ChangeLog:
>
>     * gdb/disasm.c (set_disassembler_options): Add support for
>     non-enum disassembler options.
>     (show_disassembler_options_sfunc): Likewise.
> ---
>  gdb/disasm.c | 4 ++++
>  1 file changed, 4 insertions(+)

The GDB parts are OK.

Thanks,
Andrew

>
> diff --git a/gdb/disasm.c b/gdb/disasm.c
> index db6724757ac..fe4eed2d524 100644
> --- a/gdb/disasm.c
> +++ b/gdb/disasm.c
> @@ -1270,6 +1270,8 @@ set_disassembler_options (const char *prospective_options)
>  	    if (memcmp (opt, valid_options->name[i], len) != 0)
>  	      continue;
>  	    arg = opt + len;
> +	    if (valid_options->arg[i]->values == NULL)
> +	      break;
>  	    for (j = 0; valid_options->arg[i]->values[j] != NULL; j++)
>  	      if (disassembler_options_cmp
>  		    (arg, valid_options->arg[i]->values[j]) == 0)
> @@ -1391,6 +1393,8 @@ The following disassembler options are supported for use with the\n\
>  
>        for (i = 0; valid_args[i].name != NULL; i++)
>  	{
> +	  if (valid_args[i].values == NULL)
> +	    continue;
>  	  gdb_printf (file, _("\n\
>    For the options above, the following values are supported for \"%s\":\n   "),
>  		      valid_args[i].name);
> -- 
> 2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 0/2] gdb, opcodes: Add non-enum disassembler options
  2022-08-31  2:15 [PATCH 0/2] gdb, opcodes: Add non-enum disassembler options Tsukasa OI
  2022-08-31  2:15 ` [PATCH 1/2] " Tsukasa OI
  2022-08-31  2:15 ` [PATCH 2/2] gdb: " Tsukasa OI
@ 2022-09-04  8:03 ` Tsukasa OI
  2022-09-04  8:03   ` [PATCH v2 1/2] " Tsukasa OI
  2022-09-04  8:03   ` [PATCH v2 2/2] gdb: " Tsukasa OI
  2 siblings, 2 replies; 10+ messages in thread
From: Tsukasa OI @ 2022-09-04  8:03 UTC (permalink / raw)
  To: Tsukasa OI, Nick Clifton, Andrew Burgess; +Cc: binutils, gdb-patches

Hello,

This is a part of my work: implement `arch' disassembler option in RISC-V.
However, it requires technical changes also affecting opcodes:ARC and MIPS
and GDB.  It will take some time because we have to wait many Binutils
prerequisites but this technical change can be discussed now (due to it
affects both Binutils and GDB).

PATCH v1
    <https://sourceware.org/pipermail/binutils/2022-August/122677.html>
    <https://sourceware.org/pipermail/gdb-patches/2022-August/191611.html>
... is approved as follows:

PATCH 1/2: Binutils changes
    <https://sourceware.org/pipermail/binutils/2022-September/122697.html> (2022-09-01)
PATCH 2/2: GDB changes
    <https://sourceware.org/pipermail/gdb-patches/2022-September/191668.html> (2022-09-02)

The code part of PATCH v2 is unchanged from (approved) PATCH v1 but minor
commit message issues (including excess ChangeLog and quoting) are fixed.

Since I don't have write access yet, it's my preasure to merge the commits
if approved.

Thanks,
Tsukasa




Tsukasa OI (2):
  opcodes: Add non-enum disassembler options
  gdb: Add non-enum disassembler options

 gdb/disasm.c        | 4 ++++
 include/dis-asm.h   | 3 ++-
 opcodes/arc-dis.c   | 2 ++
 opcodes/mips-dis.c  | 2 ++
 opcodes/riscv-dis.c | 2 ++
 5 files changed, 12 insertions(+), 1 deletion(-)


base-commit: 148b68a56c57d69bd97a5f40c34fc4700693596a
-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] opcodes: Add non-enum disassembler options
  2022-09-04  8:03 ` [PATCH v2 0/2] gdb, opcodes: " Tsukasa OI
@ 2022-09-04  8:03   ` Tsukasa OI
  2022-09-06  2:36     ` Tsukasa OI
  2022-09-04  8:03   ` [PATCH v2 2/2] gdb: " Tsukasa OI
  1 sibling, 1 reply; 10+ messages in thread
From: Tsukasa OI @ 2022-09-04  8:03 UTC (permalink / raw)
  To: Tsukasa OI, Nick Clifton, Andrew Burgess; +Cc: binutils, gdb-patches

This is paired with "gdb: Add non-enum disassembler options".

There is a portable mechanism for disassembler options and used on some
architectures:

-   ARC
-   Arm
-   MIPS
-   PowerPC
-   RISC-V
-   S/390

However, it only supports following forms:

-   [NAME]
-   [NAME]=[ENUM_VALUE]

Valid values for [ENUM_VALUE] must be predefined in
disasm_option_arg_t.values. For instance, for -M cpu=[CPU] in ARC
architecture, opcodes/arc-dis.c builds valid CPU model list from
include/elf/arc-cpu.def.

In this commit, it adds following format:

-   [NAME]=[ARBITRARY_VALUE] (cannot contain "," though)

This is identified by NULL value of disasm_option_arg_t.values
(normally, this is a non-NULL pointer to a NULL-terminated list).

include/ChangeLog:

	* dis-asm.h (disasm_option_arg_t): Update comment of values
	to allow non-enum disassembler options.

opcodes/ChangeLog:

	* riscv-dis.c (print_riscv_disassembler_options): Support
	non-enum disassembler options on printing disassembler help.
	* arc-dis.c (print_arc_disassembler_options): Likewise.
	* mips-dis.c (print_mips_disassembler_options): Likewise.
---
 include/dis-asm.h   | 3 ++-
 opcodes/arc-dis.c   | 2 ++
 opcodes/mips-dis.c  | 2 ++
 opcodes/riscv-dis.c | 2 ++
 4 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/dis-asm.h b/include/dis-asm.h
index f1a83dc84e5..4921c040710 100644
--- a/include/dis-asm.h
+++ b/include/dis-asm.h
@@ -318,7 +318,8 @@ typedef struct
   /* Option argument name to use in descriptions.  */
   const char *name;
 
-  /* Vector of acceptable option argument values, NULL-terminated.  */
+  /* Vector of acceptable option argument values, NULL-terminated.
+     NULL if any values are accepted.  */
   const char **values;
 } disasm_option_arg_t;
 
diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
index 3490bad4f66..c8dc525f64d 100644
--- a/opcodes/arc-dis.c
+++ b/opcodes/arc-dis.c
@@ -1611,6 +1611,8 @@ print_arc_disassembler_options (FILE *stream)
   for (i = 0; args[i].name != NULL; ++i)
     {
       size_t len = 3;
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
diff --git a/opcodes/mips-dis.c b/opcodes/mips-dis.c
index 9db604ffb39..faeebccfc3b 100644
--- a/opcodes/mips-dis.c
+++ b/opcodes/mips-dis.c
@@ -2809,6 +2809,8 @@ with the -M switch (multiple options should be separated by commas):\n\n"));
 
   for (i = 0; args[i].name != NULL; i++)
     {
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 160cc40f865..7ae6e709290 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -1195,6 +1195,8 @@ with the -M switch (multiple options should be separated by commas):\n"));
 
   for (i = 0; args[i].name != NULL; i++)
     {
+      if (args[i].values == NULL)
+	continue;
       fprintf (stream, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 	       args[i].name);
-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] gdb: Add non-enum disassembler options
  2022-09-04  8:03 ` [PATCH v2 0/2] gdb, opcodes: " Tsukasa OI
  2022-09-04  8:03   ` [PATCH v2 1/2] " Tsukasa OI
@ 2022-09-04  8:03   ` Tsukasa OI
  1 sibling, 0 replies; 10+ messages in thread
From: Tsukasa OI @ 2022-09-04  8:03 UTC (permalink / raw)
  To: Tsukasa OI, Nick Clifton, Andrew Burgess; +Cc: binutils, gdb-patches

This is paired with "opcodes: Add non-enum disassembler options".

There is a portable mechanism for disassembler options and used on some
architectures:

-   ARC
-   Arm
-   MIPS
-   PowerPC
-   RISC-V
-   S/390

However, it only supports following forms:

-   [NAME]
-   [NAME]=[ENUM_VALUE]

Valid values for [ENUM_VALUE] must be predefined in
disasm_option_arg_t.values. For instance, for -M cpu=[CPU] in ARC
architecture, opcodes/arc-dis.c builds valid CPU model list from
include/elf/arc-cpu.def.

In this commit, it adds following format:

-   [NAME]=[ARBITRARY_VALUE] (cannot contain "," though)

This is identified by NULL value of disasm_option_arg_t.values
(normally, this is a non-NULL pointer to a NULL-terminated list).

gdb/ChangeLog:

	* gdb/disasm.c (set_disassembler_options): Add support for
	non-enum disassembler options.
	(show_disassembler_options_sfunc): Likewise.
---
 gdb/disasm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/disasm.c b/gdb/disasm.c
index db6724757ac..fe4eed2d524 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -1270,6 +1270,8 @@ set_disassembler_options (const char *prospective_options)
 	    if (memcmp (opt, valid_options->name[i], len) != 0)
 	      continue;
 	    arg = opt + len;
+	    if (valid_options->arg[i]->values == NULL)
+	      break;
 	    for (j = 0; valid_options->arg[i]->values[j] != NULL; j++)
 	      if (disassembler_options_cmp
 		    (arg, valid_options->arg[i]->values[j]) == 0)
@@ -1391,6 +1393,8 @@ The following disassembler options are supported for use with the\n\
 
       for (i = 0; valid_args[i].name != NULL; i++)
 	{
+	  if (valid_args[i].values == NULL)
+	    continue;
 	  gdb_printf (file, _("\n\
   For the options above, the following values are supported for \"%s\":\n   "),
 		      valid_args[i].name);
-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] opcodes: Add non-enum disassembler options
  2022-09-04  8:03   ` [PATCH v2 1/2] " Tsukasa OI
@ 2022-09-06  2:36     ` Tsukasa OI
  2022-09-06  8:31       ` Tsukasa OI
  0 siblings, 1 reply; 10+ messages in thread
From: Tsukasa OI @ 2022-09-06  2:36 UTC (permalink / raw)
  To: Nick Clifton, Andrew Burgess; +Cc: binutils, gdb-patches

PATCH v2 1/2 is committed as a combination of:

-   Changes from PATCH v1 1/2 (that is approved)
-   Obvious copy-editing on the commit message (no code changes)

Under the "write after approval" access to the Binutils project.  This
is my very first commit by myself (as a new committer).  Thanks, Nick!

Technically, it's not impossible to push 2/2 myself but this is
definitely not Binutils and GDB should go.  Andrew, if you don't mind,
could you push the PATCH v2 2/2 for me?

Thanks,
Tsukasa

On 2022/09/04 17:03, Tsukasa OI wrote:
> This is paired with "gdb: Add non-enum disassembler options".
> 
> There is a portable mechanism for disassembler options and used on some
> architectures:
> 
> -   ARC
> -   Arm
> -   MIPS
> -   PowerPC
> -   RISC-V
> -   S/390
> 
> However, it only supports following forms:
> 
> -   [NAME]
> -   [NAME]=[ENUM_VALUE]
> 
> Valid values for [ENUM_VALUE] must be predefined in
> disasm_option_arg_t.values. For instance, for -M cpu=[CPU] in ARC
> architecture, opcodes/arc-dis.c builds valid CPU model list from
> include/elf/arc-cpu.def.
> 
> In this commit, it adds following format:
> 
> -   [NAME]=[ARBITRARY_VALUE] (cannot contain "," though)
> 
> This is identified by NULL value of disasm_option_arg_t.values
> (normally, this is a non-NULL pointer to a NULL-terminated list).
> 
> include/ChangeLog:
> 
> 	* dis-asm.h (disasm_option_arg_t): Update comment of values
> 	to allow non-enum disassembler options.
> 
> opcodes/ChangeLog:
> 
> 	* riscv-dis.c (print_riscv_disassembler_options): Support
> 	non-enum disassembler options on printing disassembler help.
> 	* arc-dis.c (print_arc_disassembler_options): Likewise.
> 	* mips-dis.c (print_mips_disassembler_options): Likewise.
> ---
>  include/dis-asm.h   | 3 ++-
>  opcodes/arc-dis.c   | 2 ++
>  opcodes/mips-dis.c  | 2 ++
>  opcodes/riscv-dis.c | 2 ++
>  4 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/include/dis-asm.h b/include/dis-asm.h
> index f1a83dc84e5..4921c040710 100644
> --- a/include/dis-asm.h
> +++ b/include/dis-asm.h
> @@ -318,7 +318,8 @@ typedef struct
>    /* Option argument name to use in descriptions.  */
>    const char *name;
>  
> -  /* Vector of acceptable option argument values, NULL-terminated.  */
> +  /* Vector of acceptable option argument values, NULL-terminated.
> +     NULL if any values are accepted.  */
>    const char **values;
>  } disasm_option_arg_t;
>  
> diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
> index 3490bad4f66..c8dc525f64d 100644
> --- a/opcodes/arc-dis.c
> +++ b/opcodes/arc-dis.c
> @@ -1611,6 +1611,8 @@ print_arc_disassembler_options (FILE *stream)
>    for (i = 0; args[i].name != NULL; ++i)
>      {
>        size_t len = 3;
> +      if (args[i].values == NULL)
> +	continue;
>        fprintf (stream, _("\n\
>    For the options above, the following values are supported for \"%s\":\n   "),
>  	       args[i].name);
> diff --git a/opcodes/mips-dis.c b/opcodes/mips-dis.c
> index 9db604ffb39..faeebccfc3b 100644
> --- a/opcodes/mips-dis.c
> +++ b/opcodes/mips-dis.c
> @@ -2809,6 +2809,8 @@ with the -M switch (multiple options should be separated by commas):\n\n"));
>  
>    for (i = 0; args[i].name != NULL; i++)
>      {
> +      if (args[i].values == NULL)
> +	continue;
>        fprintf (stream, _("\n\
>    For the options above, the following values are supported for \"%s\":\n   "),
>  	       args[i].name);
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 160cc40f865..7ae6e709290 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -1195,6 +1195,8 @@ with the -M switch (multiple options should be separated by commas):\n"));
>  
>    for (i = 0; args[i].name != NULL; i++)
>      {
> +      if (args[i].values == NULL)
> +	continue;
>        fprintf (stream, _("\n\
>    For the options above, the following values are supported for \"%s\":\n   "),
>  	       args[i].name);

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] opcodes: Add non-enum disassembler options
  2022-09-06  2:36     ` Tsukasa OI
@ 2022-09-06  8:31       ` Tsukasa OI
  0 siblings, 0 replies; 10+ messages in thread
From: Tsukasa OI @ 2022-09-06  8:31 UTC (permalink / raw)
  To: gdb-patches, Binutils

Sorry, because of my misunderstandings, I thought I don't have any write
permission to the GDB part.  Because that was not true (according to a
Binutils maintainer), I also pushed PATCH 2/2 myself.

Thanks,
Tsukasa

On 2022/09/06 11:36, Tsukasa OI via Gdb-patches wrote:
> PATCH v2 1/2 is committed as a combination of:
> 
> -   Changes from PATCH v1 1/2 (that is approved)
> -   Obvious copy-editing on the commit message (no code changes)
> 
> Under the "write after approval" access to the Binutils project.  This
> is my very first commit by myself (as a new committer).  Thanks, Nick!
> 
> Technically, it's not impossible to push 2/2 myself but this is
> definitely not Binutils and GDB should go.  Andrew, if you don't mind,
> could you push the PATCH v2 2/2 for me?
> 
> Thanks,
> Tsukasa
> 
> On 2022/09/04 17:03, Tsukasa OI wrote:
>> This is paired with "gdb: Add non-enum disassembler options".
>>
>> There is a portable mechanism for disassembler options and used on some
>> architectures:
>>
>> -   ARC
>> -   Arm
>> -   MIPS
>> -   PowerPC
>> -   RISC-V
>> -   S/390
>>
>> However, it only supports following forms:
>>
>> -   [NAME]
>> -   [NAME]=[ENUM_VALUE]
>>
>> Valid values for [ENUM_VALUE] must be predefined in
>> disasm_option_arg_t.values. For instance, for -M cpu=[CPU] in ARC
>> architecture, opcodes/arc-dis.c builds valid CPU model list from
>> include/elf/arc-cpu.def.
>>
>> In this commit, it adds following format:
>>
>> -   [NAME]=[ARBITRARY_VALUE] (cannot contain "," though)
>>
>> This is identified by NULL value of disasm_option_arg_t.values
>> (normally, this is a non-NULL pointer to a NULL-terminated list).
>>
>> include/ChangeLog:
>>
>> 	* dis-asm.h (disasm_option_arg_t): Update comment of values
>> 	to allow non-enum disassembler options.
>>
>> opcodes/ChangeLog:
>>
>> 	* riscv-dis.c (print_riscv_disassembler_options): Support
>> 	non-enum disassembler options on printing disassembler help.
>> 	* arc-dis.c (print_arc_disassembler_options): Likewise.
>> 	* mips-dis.c (print_mips_disassembler_options): Likewise.
>> ---
>>  include/dis-asm.h   | 3 ++-
>>  opcodes/arc-dis.c   | 2 ++
>>  opcodes/mips-dis.c  | 2 ++
>>  opcodes/riscv-dis.c | 2 ++
>>  4 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/dis-asm.h b/include/dis-asm.h
>> index f1a83dc84e5..4921c040710 100644
>> --- a/include/dis-asm.h
>> +++ b/include/dis-asm.h
>> @@ -318,7 +318,8 @@ typedef struct
>>    /* Option argument name to use in descriptions.  */
>>    const char *name;
>>  
>> -  /* Vector of acceptable option argument values, NULL-terminated.  */
>> +  /* Vector of acceptable option argument values, NULL-terminated.
>> +     NULL if any values are accepted.  */
>>    const char **values;
>>  } disasm_option_arg_t;
>>  
>> diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
>> index 3490bad4f66..c8dc525f64d 100644
>> --- a/opcodes/arc-dis.c
>> +++ b/opcodes/arc-dis.c
>> @@ -1611,6 +1611,8 @@ print_arc_disassembler_options (FILE *stream)
>>    for (i = 0; args[i].name != NULL; ++i)
>>      {
>>        size_t len = 3;
>> +      if (args[i].values == NULL)
>> +	continue;
>>        fprintf (stream, _("\n\
>>    For the options above, the following values are supported for \"%s\":\n   "),
>>  	       args[i].name);
>> diff --git a/opcodes/mips-dis.c b/opcodes/mips-dis.c
>> index 9db604ffb39..faeebccfc3b 100644
>> --- a/opcodes/mips-dis.c
>> +++ b/opcodes/mips-dis.c
>> @@ -2809,6 +2809,8 @@ with the -M switch (multiple options should be separated by commas):\n\n"));
>>  
>>    for (i = 0; args[i].name != NULL; i++)
>>      {
>> +      if (args[i].values == NULL)
>> +	continue;
>>        fprintf (stream, _("\n\
>>    For the options above, the following values are supported for \"%s\":\n   "),
>>  	       args[i].name);
>> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
>> index 160cc40f865..7ae6e709290 100644
>> --- a/opcodes/riscv-dis.c
>> +++ b/opcodes/riscv-dis.c
>> @@ -1195,6 +1195,8 @@ with the -M switch (multiple options should be separated by commas):\n"));
>>  
>>    for (i = 0; args[i].name != NULL; i++)
>>      {
>> +      if (args[i].values == NULL)
>> +	continue;
>>        fprintf (stream, _("\n\
>>    For the options above, the following values are supported for \"%s\":\n   "),
>>  	       args[i].name);
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-09-06  8:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-31  2:15 [PATCH 0/2] gdb, opcodes: Add non-enum disassembler options Tsukasa OI
2022-08-31  2:15 ` [PATCH 1/2] " Tsukasa OI
2022-09-01 12:03   ` Nick Clifton
2022-08-31  2:15 ` [PATCH 2/2] gdb: " Tsukasa OI
2022-09-02 10:00   ` Andrew Burgess
2022-09-04  8:03 ` [PATCH v2 0/2] gdb, opcodes: " Tsukasa OI
2022-09-04  8:03   ` [PATCH v2 1/2] " Tsukasa OI
2022-09-06  2:36     ` Tsukasa OI
2022-09-06  8:31       ` Tsukasa OI
2022-09-04  8:03   ` [PATCH v2 2/2] gdb: " Tsukasa OI

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).