[1/2] arm, objdump: Make objdump use bfd's machine detection to drive disassembly

Message ID 20241031113500.3685068-2-andre.simoesdiasvieira@arm.com
State Superseded
Headers
Series arm, objdump: Make objdump use bfd's machine detection to drive disassembly |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm fail Test failed

Commit Message

Andre Vieira (lists) Oct. 31, 2024, 11:34 a.m. UTC
  This patch disables, for any elf target, an old piece of code that forced
disassembly to disassemble for 'unknown architecture' which once upon a time
meant it would disassemble ANY arm instruction.  This is no longer true with
the addition of Armv8.1-M Mainline, as there are conflicting encodings for
different thumb instructions.

BFD however can detect what architecture the object file was assembled for
using information in the notes section.  So if available, we use that,
otherwise we default to the old 'unknown' behaviour.

With the changes above code, a mode changing 'bx lr' assembled for armv4 with
the option --fix-v4bx will result in an object file that is recognized by bfd
as one for the armv4 architecture.  This patch teaches the disassembler to
detect such situations by looking for the combination of the bx encoding bits
and a relocation, and if both are present it will disassemble it as a bx
instead.

This patch removes the unused and wrongfully defined ARM_ARCH_V8A_CRC, and
defines and uses a ARM_ARCH_V8R_CRC to make sure instructions enabled by
-march=armv8-r+crc are disassembled correctly.

This also patches up some of the tests cases, see a brief explanation
for each below.

inst.d:
This test checks the assembly & disassembly of basic instructions in armv3m. I
changed the expected behaviour for teqp, cmnp cmpp and testp instructions to
properly print p when disassembling, whereas before, in the 'unknown' case it
would disassemble these as UNPREDICTABLE as they were changed in later
architectures.

nops.d:
Was missing an -march, added one to make sure we were testing the right
behavior of NOP<c> instructions.

unpredictable.d:
Was missing an -march, added armv6 as that reproduced the behaviour being
tested.
---
 gas/testsuite/gas/arm/inst.d          | 32 +++++++++++++--------------
 gas/testsuite/gas/arm/nops.d          |  1 +
 gas/testsuite/gas/arm/unpredictable.d |  1 +
 include/opcode/arm.h                  |  4 ++--
 opcodes/arm-dis.c                     | 14 +++++++++---
 5 files changed, 31 insertions(+), 21 deletions(-)
  

Patch

diff --git a/gas/testsuite/gas/arm/inst.d b/gas/testsuite/gas/arm/inst.d
index 6f642dbe97f..3fda9465193 100644
--- a/gas/testsuite/gas/arm/inst.d
+++ b/gas/testsuite/gas/arm/inst.d
@@ -95,22 +95,22 @@  Disassembly of section .text:
 0+14c <[^>]*> e1720004 ?	cmn	r2, r4
 0+150 <[^>]*> e1750287 ?	cmn	r5, r7, lsl #5
 0+154 <[^>]*> e1710113 ?	cmn	r1, r3, lsl r1
-0+158 <[^>]*> e330f00a ?	teq	r0, #10	@ <UNPREDICTABLE>
-0+15c <[^>]*> e132f004 ?	teq	r2, r4	@ <UNPREDICTABLE>
-0+160 <[^>]*> e135f287 ?	teq	r5, r7, lsl #5	@ <UNPREDICTABLE>
-0+164 <[^>]*> e131f113 ?	teq	r1, r3, lsl r1	@ <UNPREDICTABLE>
-0+168 <[^>]*> e370f00a ?	cmn	r0, #10	@ <UNPREDICTABLE>
-0+16c <[^>]*> e172f004 ?	cmn	r2, r4	@ <UNPREDICTABLE>
-0+170 <[^>]*> e175f287 ?	cmn	r5, r7, lsl #5	@ <UNPREDICTABLE>
-0+174 <[^>]*> e171f113 ?	cmn	r1, r3, lsl r1	@ <UNPREDICTABLE>
-0+178 <[^>]*> e350f00a ?	cmp	r0, #10	@ <UNPREDICTABLE>
-0+17c <[^>]*> e152f004 ?	cmp	r2, r4	@ <UNPREDICTABLE>
-0+180 <[^>]*> e155f287 ?	cmp	r5, r7, lsl #5	@ <UNPREDICTABLE>
-0+184 <[^>]*> e151f113 ?	cmp	r1, r3, lsl r1	@ <UNPREDICTABLE>
-0+188 <[^>]*> e310f00a ?	tst	r0, #10	@ <UNPREDICTABLE>
-0+18c <[^>]*> e112f004 ?	tst	r2, r4	@ <UNPREDICTABLE>
-0+190 <[^>]*> e115f287 ?	tst	r5, r7, lsl #5	@ <UNPREDICTABLE>
-0+194 <[^>]*> e111f113 ?	tst	r1, r3, lsl r1	@ <UNPREDICTABLE>
+0+158 <[^>]*> e330f00a ?	teqp	r0, #10
+0+15c <[^>]*> e132f004 ?	teqp	r2, r4
+0+160 <[^>]*> e135f287 ?	teqp	r5, r7, lsl #5
+0+164 <[^>]*> e131f113 ?	teqp	r1, r3, lsl r1
+0+168 <[^>]*> e370f00a ?	cmnp	r0, #10
+0+16c <[^>]*> e172f004 ?	cmnp	r2, r4
+0+170 <[^>]*> e175f287 ?	cmnp	r5, r7, lsl #5
+0+174 <[^>]*> e171f113 ?	cmnp	r1, r3, lsl r1
+0+178 <[^>]*> e350f00a ?	cmpp	r0, #10
+0+17c <[^>]*> e152f004 ?	cmpp	r2, r4
+0+180 <[^>]*> e155f287 ?	cmpp	r5, r7, lsl #5
+0+184 <[^>]*> e151f113 ?	cmpp	r1, r3, lsl r1
+0+188 <[^>]*> e310f00a ?	tstp	r0, #10
+0+18c <[^>]*> e112f004 ?	tstp	r2, r4
+0+190 <[^>]*> e115f287 ?	tstp	r5, r7, lsl #5
+0+194 <[^>]*> e111f113 ?	tstp	r1, r3, lsl r1
 0+198 <[^>]*> e0000291 ?	mul	r0, r1, r2
 0+19c <[^>]*> e0110392 ?	muls	r1, r2, r3
 0+1a0 <[^>]*> 10000091 ?	mulne	r0, r1, r0
diff --git a/gas/testsuite/gas/arm/nops.d b/gas/testsuite/gas/arm/nops.d
index bda0c307dce..0f5de019bbe 100644
--- a/gas/testsuite/gas/arm/nops.d
+++ b/gas/testsuite/gas/arm/nops.d
@@ -1,4 +1,5 @@ 
 # name: NOP<c> instructions
+# as: -march=armv7-a
 # objdump: -dr --prefix-addresses --show-raw-insn
 # skip: *-*-pe *-*-wince
 
diff --git a/gas/testsuite/gas/arm/unpredictable.d b/gas/testsuite/gas/arm/unpredictable.d
index 0781c18377c..0d3c14b6234 100644
--- a/gas/testsuite/gas/arm/unpredictable.d
+++ b/gas/testsuite/gas/arm/unpredictable.d
@@ -1,4 +1,5 @@ 
 # name: Upredictable Instructions
+# as: -march=armv6
 # objdump: -D --prefix-addresses --show-raw-insn
 
 .*: +file format .*arm.*
diff --git a/include/opcode/arm.h b/include/opcode/arm.h
index a89c215faff..de1fcd49adb 100644
--- a/include/opcode/arm.h
+++ b/include/opcode/arm.h
@@ -354,8 +354,6 @@ 
 #define ARM_ARCH_V7M	 ARM_FEATURE_CORE (ARM_AEXT_V7M, ARM_EXT2_V6T2_V8M)
 #define ARM_ARCH_V7EM	 ARM_FEATURE_CORE (ARM_AEXT_V7EM, ARM_EXT2_V6T2_V8M)
 #define ARM_ARCH_V8A	 ARM_FEATURE_CORE (ARM_AEXT_V8A, ARM_AEXT2_V8A)
-#define ARM_ARCH_V8A_CRC ARM_FEATURE (ARM_AEXT_V8A,	   \
-				      ARM_AEXT2_V8A | ARM_EXT2_CRC)
 #define ARM_ARCH_V8_1A	 ARM_FEATURE (ARM_AEXT_V8A, ARM_AEXT2_V8_1A	   \
 				      | ARM_EXT2_CRC,  FPU_NEON_EXT_RDMA)
 #define ARM_ARCH_V8_2A	 ARM_FEATURE (ARM_AEXT_V8A, ARM_AEXT2_V8_2A	   \
@@ -381,6 +379,8 @@ 
 #define ARM_ARCH_V8M_MAIN_DSP  ARM_FEATURE_CORE (ARM_AEXT_V8M_MAIN_DSP,	   \
 						 ARM_AEXT2_V8M_MAIN_DSP)
 #define ARM_ARCH_V8R	       ARM_FEATURE_CORE (ARM_AEXT_V8R, ARM_AEXT2_V8R)
+#define ARM_ARCH_V8R_CRC       ARM_FEATURE_CORE (ARM_AEXT_V8R,		   \
+						 ARM_AEXT2_V8R | ARM_EXT2_CRC)
 #define ARM_ARCH_V8_1M_MAIN    ARM_FEATURE_CORE (ARM_AEXT_V8_1M_MAIN,	   \
 						 ARM_AEXT2_V8_1M_MAIN)
 #define ARM_ARCH_V9A	       ARM_FEATURE_ALL(ARM_AEXT_V8A,	   \
diff --git a/opcodes/arm-dis.c b/opcodes/arm-dis.c
index d1d7ca30993..2bcc1d2c828 100644
--- a/opcodes/arm-dis.c
+++ b/opcodes/arm-dis.c
@@ -10050,7 +10050,14 @@  print_insn_arm (bfd_vma pc, struct disassemble_info *info, long given)
       if ((given & insn->mask) != insn->value)
 	continue;
 
-      if (! ARM_CPU_HAS_FEATURE (insn->arch, private_data->features))
+      if (! ARM_CPU_HAS_FEATURE (insn->arch, private_data->features)
+	  /* If we are dealing with a bx instruction and arvm4, with a
+	     relocation, then it is likely this was assembled with --fix-v4bx
+	     and thus intended as a mode changing instruction, so disassemble
+	     it as such.  */
+	  && (insn->value != 0x12fff10
+	      || info->mach != bfd_mach_arm_4
+	      || ((info->flags & INSN_HAS_RELOC) == 0)))
 	continue;
 
       /* Special case: an instruction with all bits set in the condition field
@@ -12292,7 +12299,7 @@  select_arm_features (unsigned long mach,
 	  ARM_MERGE_FEATURE_SETS (arch_fset, arch_fset, armv8_6_ext_fset);
 	  break;
 	}
-    case bfd_mach_arm_8R:	 ARM_SET_FEATURES (ARM_ARCH_V8R); break;
+    case bfd_mach_arm_8R:	 ARM_SET_FEATURES (ARM_ARCH_V8R_CRC); break;
     case bfd_mach_arm_8M_BASE:	 ARM_SET_FEATURES (ARM_ARCH_V8M_BASE); break;
     case bfd_mach_arm_8M_MAIN:	 ARM_SET_FEATURES (ARM_ARCH_V8M_MAIN); break;
     case bfd_mach_arm_8_1M_MAIN:
@@ -12358,7 +12365,8 @@  print_insn (bfd_vma pc, struct disassemble_info *info, bool little)
     {
       static struct arm_private_data private;
 
-      if ((info->flags & USER_SPECIFIED_MACHINE_TYPE) == 0)
+      if (info->flavour != bfd_target_elf_flavour
+	  && (info->flags & USER_SPECIFIED_MACHINE_TYPE) == 0)
 	/* If the user did not use the -m command line switch then default to
 	   disassembling all types of ARM instruction.