This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH][Binutils][AArch64] Fix disassembler bug with out-of-order sections

Hi All,

The AArch64 disassembler has an optimization that it uses to reduce the amount
it has to search for mapping symbols during disassembly.  This optimization
assumes that sections are listed in the section header in monotonic increasing
VMAs.  However this is not a requirement for the ELF specification.

Because of this when such "out of order" sections occur the disassembler would
pick the wrong mapping symbol to disassemble the section with.

This fixes it by explicitly passing along the stop offset for the current
disassembly glob and when this changes compared to the previous one we've seen
the optimization won't be performed.  In effect this restarts the search from
a well defined starting point.  Usually the symbol's address.

The existing stop_vma can't be used for this as it is allowed to be unset and
setting this unconditionally would change the semantics of this field.

build on native hardware and regtested on
  aarch64-none-elf, aarch64-none-elf (32 bit host),
  aarch64-none-linux-gnu, aarch64-none-linux-gnu (32 bit host)

Cross-compiled and regtested on
  aarch64-none-linux-gnu, aarch64_be-none-linux-gnu

and no issues.

Ok for master? and for backport to binutils-2.32?

Thanks,
Tamar

binutils/ChangeLog:

2019-02-22  Tamar Christina  <tamar.christina@arm.com>

	* objdump.c (disassemble_bytes): Pass stop_offset.
	* testsuite/binutils-all/aarch64/out-of-order.T: New test.
	* testsuite/binutils-all/aarch64/out-of-order.d: New test.
	* testsuite/binutils-all/aarch64/out-of-order.s: New test.

include/ChangeLog:

2019-02-22  Tamar Christina  <tamar.christina@arm.com>

	* dis-asm.h (struct disassemble_info): Add stop_offset.

opcodes/ChangeLog:

2019-02-22  Tamar Christina  <tamar.christina@arm.com>

	* aarch64-dis.c (last_stop_offset): New.
	(print_insn_aarch64): Use stop_offset.

-- 

diff --git a/binutils/objdump.c b/binutils/objdump.c
index 872539068cab8276db3c043287431cbe5a46f856..4b98e7b6909d3b8b37da73b0ad6718bf645a6c54 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -1971,6 +1971,7 @@ disassemble_bytes (struct disassemble_info * inf,
 		   disassembling code of course, and when -D is in effect.  */
 		inf->stop_vma = section->vma + stop_offset;
 
+	      inf->stop_offset = stop_offset;
 	      octets = (*disassemble_fn) (section->vma + addr_offset, inf);
 
 	      inf->stop_vma = 0;
diff --git a/binutils/testsuite/binutils-all/aarch64/out-of-order.T b/binutils/testsuite/binutils-all/aarch64/out-of-order.T
new file mode 100644
index 0000000000000000000000000000000000000000..489ae80190e785f25e77a3db34e532db57b03da7
--- /dev/null
+++ b/binutils/testsuite/binutils-all/aarch64/out-of-order.T
@@ -0,0 +1,14 @@
+ENTRY(v1)
+SECTIONS
+{
+  . = 0xffe00000;
+  .global : { *(.global) }
+  . = 0x4018280;
+  .func2 : { *(.func2) }
+  . = 0x4005000;
+  .func1 : { *(.func1) }
+  . = 0x4015000;
+  .func3 : { *(.func3) }
+  .data : { *(.data) }
+  .rodata : { *(.rodata) }
+}
\ No newline at end of file
diff --git a/binutils/testsuite/binutils-all/aarch64/out-of-order.d b/binutils/testsuite/binutils-all/aarch64/out-of-order.d
new file mode 100644
index 0000000000000000000000000000000000000000..410f37f68ea21f9e16e2319b5048c123cec99910
--- /dev/null
+++ b/binutils/testsuite/binutils-all/aarch64/out-of-order.d
@@ -0,0 +1,27 @@
+#PROG: objcopy
+#ld: -T out-of-order.T
+#objdump: -d
+#name: Check if disassembler can handle sections in different order than header
+
+.*: +file format .*aarch64.*
+
+Disassembly of section \.func2:
+
+0000000004018280 <\.func2>:
+ 4018280:	8b010000 	add	x0, x0, x1
+
+Disassembly of section \.func1:
+
+0000000004005000 <v1>:
+ 4005000:	8b010000 	add	x0, x0, x1
+ 4005004:	00000000 	\.word	0x00000000
+
+Disassembly of section \.func3:
+
+0000000004015000 <\.func3>:
+ 4015000:	8b010000 	add	x0, x0, x1
+ 4015004:	8b010000 	add	x0, x0, x1
+ 4015008:	8b010000 	add	x0, x0, x1
+ 401500c:	8b010000 	add	x0, x0, x1
+ 4015010:	8b010000 	add	x0, x0, x1
+ 4015014:	00000000 	\.word	0x00000000
diff --git a/binutils/testsuite/binutils-all/aarch64/out-of-order.s b/binutils/testsuite/binutils-all/aarch64/out-of-order.s
new file mode 100644
index 0000000000000000000000000000000000000000..6c52e857df485f71ca650338ead40ee387459e87
--- /dev/null
+++ b/binutils/testsuite/binutils-all/aarch64/out-of-order.s
@@ -0,0 +1,28 @@
+	.text
+	.global v1
+	.section .func1,"ax",@progbits
+	.type v1 %function
+	.size v1, 4
+v1:
+	add x0, x0, x1
+	.word 0
+
+	.section .func2,"ax",@progbits
+	add x0, x0, x1
+
+	.section .func3,"ax",@progbits
+	add x0, x0, x1
+	add x0, x0, x1
+	add x0, x0, x1
+	add x0, x0, x1
+	add x0, x0, x1
+	.word 0
+
+	.data
+	.section .global,"aw",@progbits
+	.xword 1
+	.xword 1
+	.xword 1
+
+	.section .rodata
+	.word 4
diff --git a/include/dis-asm.h b/include/dis-asm.h
index 22c593ece6e1d13214987f98f8dfee271d527dce..4e1263c90e3bed06195ad3481d0a4d1885f744e5 100644
--- a/include/dis-asm.h
+++ b/include/dis-asm.h
@@ -221,6 +221,12 @@ typedef struct disassemble_info
      file being disassembled.  */
   bfd_vma stop_vma;
 
+  /* The end range of the current range being disassembled.  This is required
+     in order to notify the disassembler when it's currently handling a
+     different range than it was before.  This prevent unsafe optimizations when
+     disassembling such as the way mapping symbols are found on AArch64.  */
+  bfd_vma stop_offset;
+
 } disassemble_info;
 
 /* This struct is used to pass information about valid disassembler
diff --git a/opcodes/aarch64-dis.c b/opcodes/aarch64-dis.c
index 4c31f57451d20d8029624e601b15e41cf56f5a19..fc7e95d066807f2909d1a1c891d7a89a5f31204b 100644
--- a/opcodes/aarch64-dis.c
+++ b/opcodes/aarch64-dis.c
@@ -37,6 +37,7 @@ enum map_type
 
 static enum map_type last_type;
 static int last_mapping_sym = -1;
+static bfd_vma last_stop_offset = 0;
 static bfd_vma last_mapping_addr = 0;
 
 /* Other options */
@@ -3333,7 +3334,10 @@ print_insn_aarch64 (bfd_vma pc,
       /* Start scanning at the start of the function, or wherever
 	 we finished last time.  */
       n = info->symtab_pos + 1;
-      if (n < last_mapping_sym)
+      /* If the last stop offset is different from the current one it means we
+	 are disassembling a different glob of bytes.  As such the optimization
+	 would not be safe and we should start over.  */
+      if (n < last_mapping_sym && info->stop_offset == last_stop_offset)
 	n = last_mapping_sym;
 
       /* Scan up to the location being disassembled.  */
@@ -3370,6 +3374,7 @@ print_insn_aarch64 (bfd_vma pc,
 
       last_mapping_sym = last_sym;
       last_type = type;
+      last_stop_offset = info->stop_offset;
 
       /* Look a little bit ahead to see if we should print out
 	 less than four bytes of data.  If there's a symbol,
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]