[binutils-gdb] [ARC] Fix handling of cpu=... disassembler option value

[Anton Kolesov <akolesov at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e1e94c4994151ebe0e3a103fd0d27f60bd806bbe

commit e1e94c4994151ebe0e3a103fd0d27f60bd806bbe
Author: Anton Kolesov <Anton.Kolesov@synopsys.com>
Date:   Thu Jun 15 15:58:32 2017 +0300

    [ARC] Fix handling of cpu=... disassembler option value
    
    There is a bug in handling of cpu=... disassembler option in case there are
    other options after it, for example, `cpu=EM,dsp'.  In this case `EM,dsp' is
    treated as an option value, and strcasecmp reports is as non-equal to "EM".
    This is fixed by using disassembler_options_cmp function, which compares string
    treating `,' the same way as `\0'.
    
    This function also solves a problem with option order in parse_option.
    Previously, if several option had same prefix (e.g. fpud, fpuda), then the
    longer one should have been compared first, otherwise when longer option is
    passed it would be treated as a short one, because
    
      CONST_STRNEQ ("fpud", "fpuda")
    
    would be true.  The order of options was correct for ARC, so there were no
    bugs per se, but with disassembler_option_cmp there is no risk of such a bug
    being introduced in the future.
    
    opcodes/ChangeLog:
    
    yyyy-mm-dd  Anton Kolesov  <Anton.Kolesov@synopsys.com>
    
    	* arc-dis.c (parse_option): Use disassembler_options_cmp to compare
    	disassembler option strings.
    	(parse_cpu_option): Likewise.
    
    binutils/ChangeLog
    
    yyyy-mm-dd  Anton Kolesov  <Anton.Kolesov@synopsys.com>
    
    	* testsuite/binutils-all/arc/double_store.s: New file.
    	* testsuite/binutils-all/arc/objdump.exp: Tests for disassembler
    	options.
    	(do_objfile): New function.
    	(check_assembly): Likewise.

Diff:
---
 binutils/ChangeLog                                 |  8 +++
 binutils/testsuite/binutils-all/arc/double_store.s |  6 ++
 binutils/testsuite/binutils-all/arc/objdump.exp    | 73 +++++++++++++++++-----
 opcodes/ChangeLog                                  |  6 ++
 opcodes/arc-dis.c                                  | 16 ++---
 5 files changed, 84 insertions(+), 25 deletions(-)

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 14cfcff..d8577b0 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,11 @@
+2017-06-29  Anton Kolesov  <Anton.Kolesov@synopsys.com>
+
+	* testsuite/binutils-all/arc/double_store.s: New file.
+	* testsuite/binutils-all/arc/objdump.exp: Tests for disassembler
+	options.
+	(do_objfile): New function.
+	(check_assembly): Likewise.
+
 2017-06-29  Andreas Arnez  <arnez@linux.vnet.ibm.com>
 
 	* readelf.c (get_note_type): Add NT_S390_GS_CB and NT_S390_GS_BC.
diff --git a/binutils/testsuite/binutils-all/arc/double_store.s b/binutils/testsuite/binutils-all/arc/double_store.s
new file mode 100644
index 0000000..794d7e8
--- /dev/null
+++ b/binutils/testsuite/binutils-all/arc/double_store.s
@@ -0,0 +1,6 @@
+.cpu HS
+.section .text
+.global __start
+__start:
+    std r0r1, [r3]
+
diff --git a/binutils/testsuite/binutils-all/arc/objdump.exp b/binutils/testsuite/binutils-all/arc/objdump.exp
index ca36431..2037b2b 100644
--- a/binutils/testsuite/binutils-all/arc/objdump.exp
+++ b/binutils/testsuite/binutils-all/arc/objdump.exp
@@ -25,31 +25,70 @@ if {[which $OBJDUMP] == 0} then {
 
 send_user "Version [binutil_version $OBJDUMP]"
 
-###########################
-# Set up the test of dsp.s
-###########################
+# Helper functions
 
-if {![binutils_assemble $srcdir/$subdir/dsp.s tmpdir/dsp.o]} then {
-    return
+# Create object file from the assembly source.
+proc do_objfile { srcfile } {
+    global srcdir
+    global subdir
+
+    set objfile [regsub -- "\.s$" $srcfile ".o"]
+
+    if {![binutils_assemble $srcdir/$subdir/$srcfile tmpdir/$objfile]} then {
+	return
+    }
+
+    if [is_remote host] {
+	set objfile [remote_download host tmpdir/$objfile]
+    } else {
+	set objfile tmpdir/$objfile
+    }
+
+    return $objfile
 }
 
-if [is_remote host] {
-    set objfile [remote_download host tmpdir/dsp.o]
-} else {
-    set objfile tmpdir/dsp.o
+# Ensure that disassembler output includes EXPECTED lines.
+proc check_assembly { testname objfile expected { disas_flags "" } } {
+    global OBJDUMP
+    global OBJDUMPFLAGS
+
+    set got [binutils_run $OBJDUMP "$OBJDUMPFLAGS --disassemble $disas_flags \
+	$objfile"]
+
+    if [regexp $expected $got] then {
+	pass $testname
+    } else {
+	fail $testname
+    }
 }
 
 # Make sure that a warning message is generated (because the disassembly does
 # not match the assembled instructions, which has happened because the user
 # has not specified a -M option on the disassembler command line, and so the
 # disassembler has had to guess as the instruction class in use).
-
-set got [binutils_run $OBJDUMP "$OBJDUMPFLAGS --disassemble $objfile"]
-
 set want "Warning: disassembly.*vmac2hnfr\[ \t\]*r0,r2,r4.*dmulh12.f\[ \t\]*r0,r2,r4.*dmulh11.f"
+check_assembly "Warning test" [do_objfile dsp.s] $want
+
+set double_store_hs_expected {std\s*r0r1,\[r3\]}
+set objfile [do_objfile double_store.s]
+check_assembly "arc double_store default -M" $objfile \
+    $double_store_hs_expected
+check_assembly "arc double_store -Mcpu=hs" $objfile \
+    $double_store_hs_expected "-Mcpu=hs"
+check_assembly "arc double_store -Mcpu=hs38_linux" $objfile \
+    $double_store_hs_expected "-Mcpu=hs38_linux"
+set double_store_em_expected ".long 0x1b000006"
+check_assembly "arc double_store -Mcpu=em" $objfile \
+    $double_store_em_expected "-Mcpu=em"
+check_assembly "arc double_store -Mcpu=em4_dmips" $objfile \
+    $double_store_em_expected "-Mcpu=em4_dmips"
+# Test to ensure that only value up to the next `,' is checked.  There used to
+# be a bug, where whole `em,fpus' was compared against known CPU values, and
+# that comparison would fail.  When this bug is present, whole cpu= option will
+# be ignored and instruction will be disassembled as ARC HS.
+check_assembly "arc double_store -Mcpu=em,fpus" $objfile \
+    $double_store_em_expected "-Mcpu=em,fpus"
+# Make sure that the last cpu= value is used.
+check_assembly "arc double_store -Mcpu=hs,cpu=em" $objfile \
+    $double_store_em_expected "-Mcpu=hs,cpu=em"
 
-if [regexp $want $got] then {
-    pass "Warning test"
-} else {
-    fail "Warning test"
-}
diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
index db23da3..1442a71 100644
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,3 +1,9 @@
+2017-06-29  Anton Kolesov  <Anton.Kolesov@synopsys.com>
+
+	* arc-dis.c (parse_option): Use disassembler_options_cmp to compare
+	disassembler option strings.
+	(parse_cpu_option): Likewise.
+
 2017-06-28  Tamar Christina  <tamar.christina@arm.com>
 
 	* aarch64-asm.c (aarch64_ins_reglane): Added 4B dotprod.
diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
index edd0c07..b46424a 100644
--- a/opcodes/arc-dis.c
+++ b/opcodes/arc-dis.c
@@ -740,16 +740,16 @@ operand_iterator_next (struct arc_operand_iterator *iter,
 static void
 parse_option (const char *option)
 {
-  if (CONST_STRNEQ (option, "dsp"))
+  if (disassembler_options_cmp (option, "dsp") == 0)
     add_to_decodelist (DSP, NONE);
 
-  else if (CONST_STRNEQ (option, "spfp"))
+  else if (disassembler_options_cmp (option, "spfp") == 0)
     add_to_decodelist (FLOAT, SPX);
 
-  else if (CONST_STRNEQ (option, "dpfp"))
+  else if (disassembler_options_cmp (option, "dpfp") == 0)
     add_to_decodelist (FLOAT, DPX);
 
-  else if (CONST_STRNEQ (option, "quarkse_em"))
+  else if (disassembler_options_cmp (option, "quarkse_em") == 0)
     {
       add_to_decodelist (FLOAT, DPX);
       add_to_decodelist (FLOAT, SPX);
@@ -757,16 +757,16 @@ parse_option (const char *option)
       add_to_decodelist (FLOAT, QUARKSE2);
     }
 
-  else if (CONST_STRNEQ (option, "fpuda"))
+  else if (disassembler_options_cmp (option, "fpuda") == 0)
     add_to_decodelist (FLOAT, DPA);
 
-  else if (CONST_STRNEQ (option, "fpus"))
+  else if (disassembler_options_cmp (option, "fpus") == 0)
     {
       add_to_decodelist (FLOAT, SP);
       add_to_decodelist (FLOAT, CVT);
     }
 
-  else if (CONST_STRNEQ (option, "fpud"))
+  else if (disassembler_options_cmp (option, "fpud") == 0)
     {
       add_to_decodelist (FLOAT, DP);
       add_to_decodelist (FLOAT, CVT);
@@ -808,7 +808,7 @@ parse_cpu_option (const char *option)
 
   for (i = 0; cpu_types[i].name; ++i)
     {
-      if (!strcasecmp (cpu_types[i].name, option))
+      if (!disassembler_options_cmp (cpu_types[i].name, option))
 	{
 	  return cpu_types[i].flags;
 	}