From e1e94c4994151ebe0e3a103fd0d27f60bd806bbe Mon Sep 17 00:00:00 2001 From: Anton Kolesov Date: Thu, 15 Jun 2017 15:58:32 +0300 Subject: [PATCH] [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 * 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 * 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. --- 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(-) create mode 100644 binutils/testsuite/binutils-all/arc/double_store.s 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 + + * 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 * 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 + + * arc-dis.c (parse_option): Use disassembler_options_cmp to compare + disassembler option strings. + (parse_cpu_option): Likewise. + 2017-06-28 Tamar Christina * 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; } -- 2.7.4