This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] ARM: objdump produces incorrect disassembly on multiple inputs
On 4/7/2011 3:19 PM, Paul Brook wrote:
Rubbish.
The disassemble() function is documented to return an appropriate disassembly
callback. Having it also reset unspecified state is at somewhat surprising.
Putting the state in private_data will do exactly what you want. If fact it's
more reliable as it's linked to the actual disassembler state (of which there
may be many), rather than when the user happens to request the callback
function. Note how a new instance of struct disassemble_info is created for
each object.
Looks like I either didn't dig deep enough or I got confused with the
BFD 'info' structure.
In any event, yes, moving the global variables into the private data
structure and initializing them there results in the exact same behavior
as my earlier fix, with less impact on other files.
Here would be the alternative patch, with the test case:
2011-04-07 Paul Carroll<pcarroll@codesourcery.com>
opcodes/
* arm-dis.c (print_insn): init vars moved into private_data structure
binutils/testsuite/
* binutils-all/arm/simple.s: Demo issue with objdump with
multiple input files
* binutils-all/arm/objdump.exp: added new ARM test case code
Index: src/binutils/testsuite/binutils-all/arm/objdump.exp
===================================================================
RCS file: /cvs/src/src/binutils/testsuite/binutils-all/arm/objdump.exp,v
retrieving revision 1.6
diff -u -p -r1.6 objdump.exp
--- src/binutils/testsuite/binutils-all/arm/objdump.exp 2 Sep 2009
07:22:33 -0000 1.6
+++ src/binutils/testsuite/binutils-all/arm/objdump.exp 7 Apr 2011
23:33:17 -0000
@@ -61,3 +61,29 @@ if [regexp $want $got] then {
} else {
fail "thumb2-cond test2"
}
+
+###########################
+# Set up the test of multiple disassemblies
+###########################
+
+if {![binutils_assemble $srcdir/$subdir/simple.s tmpdir/simple.o]} then {
+ return
+}
+
+if [is_remote host] {
+ set objfile [remote_download host tmpdir/simple.o]
+} else {
+ set objfile tmpdir/simple.o
+}
+
+# Make sure multiple disassemblies come out the same
+
+set got [binutils_run $OBJDUMP "-dr $objfile $objfile"]
+
+set want "$objfile:\[ \]*file format.*$objfile:\[ \]*file
format.*push.*add.*sub.*str.*add.*pop"
+
+if [regexp $want $got] then {
+ pass "multiple input files"
+} else {
+ fail "multiple input files"
+}
Index: src/binutils/testsuite/binutils-all/arm/simple.s
===================================================================
RCS file: src/binutils/testsuite/binutils-all/arm/simple.s
diff -N src/binutils/testsuite/binutils-all/arm/simple.s
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ src/binutils/testsuite/binutils-all/arm/simple.s 7 Apr 2011
23:33:17 -0000
@@ -0,0 +1,35 @@
+ .cpu arm7tdmi-s
+ .fpu softvfp
+ .file "y.c"
+ .bss
+ .align 2
+l:
+ .space 4
+ .text
+ .align 2
+ .global f1
+ .type f1, %function
+f1:
+ str fp, [sp, #-4]!
+ add fp, sp, #0
+ sub sp, sp, #12
+ str r0, [fp, #-8]
+ add sp, fp, #0
+ ldmfd sp!, {fp}
+ bx lr
+ .align 2
+ .word l
+ .size f1, .-f1
+ .align 2
+ .global main
+ .type main, %function
+main:
+ stmfd sp!, {fp, lr}
+ add fp, sp, #4
+ bx lr
+ .align 2
+ .word 1717986919
+ .word -1840700269
+ .word l
+ .size main, .-main
+ .ident "GCC: (Sourcery G++ 2011.03) 4.5.1"
Index: src/opcodes/arm-dis.c
===================================================================
RCS file: /cvs/src/src/opcodes/arm-dis.c,v
retrieving revision 1.138
diff -u -p -r1.138 arm-dis.c
--- src/opcodes/arm-dis.c 14 Mar 2011 16:04:08 -0000 1.138
+++ src/opcodes/arm-dis.c 7 Apr 2011 23:33:18 -0000
@@ -45,6 +45,14 @@
#define NUM_ELEM(a) (sizeof (a) / sizeof (a)[0])
#endif
+/* Cached mapping symbol state. */
+enum map_type
+{
+ MAP_ARM,
+ MAP_THUMB,
+ MAP_DATA
+};
+
struct arm_private_data
{
/* The features to use when disassembling optional instructions. */
@@ -53,6 +61,13 @@ struct arm_private_data
/* Whether any mapping symbols are present in the provided symbol
table. -1 if we do not know yet, otherwise 0 or 1. */
int has_mapping_symbols;
+
+ /* Track the last type (although this doesn't seem to be useful) */
+ enum map_type last_type;
+
+ /* Tracking symbol table information */
+ int last_mapping_sym;
+ bfd_vma last_mapping_addr;
};
struct opcode32
@@ -1642,18 +1657,6 @@ static unsigned int ifthen_next_state;
static bfd_vma ifthen_address;
#define IFTHEN_COND ((ifthen_state >> 4) & 0xf)
-/* Cached mapping symbol state. */
-enum map_type
-{
- MAP_ARM,
- MAP_THUMB,
- MAP_DATA
-};
-
-enum map_type last_type;
-int last_mapping_sym = -1;
-bfd_vma last_mapping_addr = 0;
-
/* Functions. */
int
@@ -4635,6 +4638,8 @@ print_insn (bfd_vma pc, struct disassemb
select_arm_features (info->mach, & private.features);
private.has_mapping_symbols = -1;
+ private.last_mapping_sym = -1;
+ private.last_mapping_addr = 0;
info->private_data = & private;
}
@@ -4658,8 +4663,8 @@ print_insn (bfd_vma pc, struct disassemb
/* Start scanning at the start of the function, or wherever
we finished last time. */
start = info->symtab_pos + 1;
- if (start < last_mapping_sym)
- start = last_mapping_sym;
+ if (start < private_data->last_mapping_sym)
+ start = private_data->last_mapping_sym;
found = FALSE;
/* First, look for mapping symbols. */
@@ -4754,10 +4759,10 @@ print_insn (bfd_vma pc, struct disassemb
}
}
- last_mapping_sym = last_sym;
- last_type = type;
- is_thumb = (last_type == MAP_THUMB);
- is_data = (last_type == MAP_DATA);
+ private_data->last_mapping_sym = last_sym;
+ private_data->last_type = type;
+ is_thumb = (private_data->last_type == MAP_THUMB);
+ is_data = (private_data->last_type == MAP_DATA);
/* Look a little bit ahead to see if we should print out
two or four bytes of data. If there's a symbol,