From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16350 invoked by alias); 8 Apr 2011 00:21:19 -0000 Received: (qmail 16334 invoked by uid 22791); 8 Apr 2011 00:21:17 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,TW_BJ,TW_DM,TW_TV,TW_VF,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 08 Apr 2011 00:21:11 +0000 Received: (qmail 4491 invoked from network); 8 Apr 2011 00:21:10 -0000 Received: from unknown (HELO ?192.168.2.6?) (pcarroll@127.0.0.2) by mail.codesourcery.com with ESMTPA; 8 Apr 2011 00:21:10 -0000 Message-ID: <4D9E54F8.6070606@codesourcery.com> Date: Fri, 08 Apr 2011 00:21:00 -0000 From: Paul Carroll User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.15) Gecko/20110303 Thunderbird/3.1.9 MIME-Version: 1.0 To: Paul Brook CC: binutils@sourceware.org Subject: Re: [PATCH] ARM: objdump produces incorrect disassembly on multiple inputs References: <4D76BA21.90708@codesourcery.com> <201104071427.24516.paul@codesourcery.com> <4D9DEF45.7010504@codesourcery.com> <201104072219.32532.paul@codesourcery.com> In-Reply-To: <201104072219.32532.paul@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2011-04/txt/msg00112.txt.bz2 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 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,