[OPCODE][AARCH64]Check mapping symbol while backward searching for IT block
Renlin Li
renlin.li@foss.arm.com
Mon Jan 25 18:35:00 GMT 2016
Hi Christophe,
Thank you for reporting this!
The related patch searches for proper mapping symbols. They are defined
in arm elf document.
So the test case should be limited on elf target only. I will post a fix.
Regards,
Renlin
On 25/01/16 18:07, Christophe Lyon wrote:
> On 22 January 2016 at 14:34, Renlin Li <renlin.li@foss.arm.com> wrote:
>> Hi,
>>
>> Sorry for the late reply.
>>
>> After discussion with Richard, we agreed that, this patch is fixing a BUG
>> which gives undesired behavior provided a valid input.
>> The patch is Okay as it is. I will commit it then.
>>
>> For an invalid input which leads to unexpected result, it's another issue. I
>> will try to sum up and create a public ticket .
>>
> Hi,
> I've noticed that this patch makes:
> ./gas/testsuite/gas.sum:FAIL: 32-bit Thumb conditional instructions
> backward search
> on arm-wince-pe.
>
> Can you have a look?
>
>
>> Regards,
>> Renlin Li
>>
>>
>>
>>
>> On 15/09/15 11:16, Richard Earnshaw wrote:
>>> On 10/09/15 18:19, Renlin Li wrote:
>>>> Hi all,
>>>>
>>>> For the following simple assembly code.
>>>>
>>>> .text
>>>> .thumb
>>>> .syntax unified
>>>> .thumb_func
>>>> f:
>>>> nop.w
>>>> .long 0xbf080000
>>>> nop.w
>>>>
>>>> When objdumped, the following code assembly will be generated.
>>>>
>>>> 0: f3af 8000 nop.w
>>>> 4: bf080000 .word 0xbf080000
>>>> 8: f3af 8000 nopeq.w
>>>>
>>>> The instruction at pc=0x8 is treated as conditional executed instruction
>>>> within an IT block. This is because objdump misinterprets data (at
>>>> pc=0x4) as an IT instruction.
>>>> During the backward search for a proper IT instruction, mapping state
>>>> for the current instruction is not checked.
>>>>
>>>>
>>>> In this patch, I create a new function mapping_symbol_for_insn to search
>>>> the mapping state given an addr.
>>>> It's used in find_ifthen_state to further guard the check for an IT
>>>> instruction.
>>>>
>>>> A new testcase is also added. Binutils checked without any new issues.
>>>> Is Okay to commit on trunk?
>>>>
>>> Hmm, I'm not entirely sure about this. Use of .word in this sort of
>>> case is most likely a mistake by the programmer rather than deliberate.
>>> Furthermore, if you're going to change this case, shouldn't you also
>>> consider:
>>>
>>> itttt eq
>>> .word xxxxxxxxx // 32-bit opcode
>>> nop
>>>
>>> ?
>>>
>>> R.
>>>
>>>> Kind regards,
>>>> Renlin Li
>>>>
>>>>
>>>>
>>>>
>>>> opcodes/ChangeLog:
>>>>
>>>> 2015-09-10 Renlin Li <renlin.li@arm.com>
>>>>
>>>> * arm-dis.c (mapping_symbol_for_insn): New function.
>>>> (find_ifthen_state): Call mapping_symbol_for_insn().
>>>>
>>>>
>>>>
>>>> gas/testsuite/ChangeLog:
>>>>
>>>> 2015-09-10 Renlin Li <renlin.li@arm.com>
>>>>
>>>> * gas/arm/thumb2_it_search.d: New.
>>>> * gas/arm/thumb2_it_search.s: New.
>>>>
>>>>
>>>>
>>>> new-new.diff
>>>>
>>>>
>>>> commit 347a76f708bfb31b38ce787bd444005412655ae0
>>>> Author: Renlin Li <renlin.li@arm.com>
>>>> Date: Thu Sep 10 16:13:22 2015 +0100
>>>>
>>>> tmp
>>>> Change-Id: I74905754d21022ba9513f12649e80cd65d4425a9
>>>>
>>>> diff --git a/gas/testsuite/gas/arm/thumb2_it_search.d
>>>> b/gas/testsuite/gas/arm/thumb2_it_search.d
>>>> new file mode 100644
>>>> index 0000000..6758ef8
>>>> --- /dev/null
>>>> +++ b/gas/testsuite/gas/arm/thumb2_it_search.d
>>>> @@ -0,0 +1,12 @@
>>>> +#name: 32-bit Thumb conditional instructions backward search
>>>> +#as: -march=armv6kt2
>>>> +#skip: *-*-*aout*
>>>> +#source: thumb2_it_search.s
>>>> +#objdump: -dr --prefix-addresses --show-raw-insn
>>>> +
>>>> +.*: +file format .*arm.*
>>>> +
>>>> +Disassembly of section .text:
>>>> +0+0 <[^>]+> f3af 8000 nop.w
>>>> +0+4 <[^>]+> bf080000 .word 0xbf080000
>>>> +0+8 <[^>]+> f3af 8000 nop.w
>>>> diff --git a/gas/testsuite/gas/arm/thumb2_it_search.s
>>>> b/gas/testsuite/gas/arm/thumb2_it_search.s
>>>> new file mode 100644
>>>> index 0000000..a29cb51
>>>> --- /dev/null
>>>> +++ b/gas/testsuite/gas/arm/thumb2_it_search.s
>>>> @@ -0,0 +1,8 @@
>>>> + .text
>>>> + .thumb
>>>> + .syntax unified
>>>> + .thumb_func
>>>> +f:
>>>> + nop.w
>>>> + .long 0xbf080000
>>>> + nop.w
>>>> diff --git a/opcodes/arm-dis.c b/opcodes/arm-dis.c
>>>> index 430da08..6a097f5 100644
>>>> --- a/opcodes/arm-dis.c
>>>> +++ b/opcodes/arm-dis.c
>>>> @@ -5930,6 +5930,10 @@ parse_disassembler_options (char *options)
>>>> }
>>>> }
>>>> +static bfd_boolean
>>>> +mapping_symbol_for_insn (bfd_vma pc, struct disassemble_info *info,
>>>> + enum map_type *map_symbol);
>>>> +
>>>> /* Search back through the insn stream to determine if this instruction
>>>> is
>>>> conditionally executed. */
>>>> @@ -5992,9 +5996,15 @@ find_ifthen_state (bfd_vma pc,
>>>> }
>>>> if ((insn & 0xff00) == 0xbf00 && (insn & 0xf) != 0)
>>>> {
>>>> - /* This could be an IT instruction. */
>>>> - seen_it = insn;
>>>> - it_count = count >> 1;
>>>> + enum map_type type = MAP_ARM;
>>>> + bfd_boolean found = mapping_symbol_for_insn (addr, info,
>>>> &type);
>>>> +
>>>> + if (!found || (found && type == MAP_THUMB))
>>>> + {
>>>> + /* This could be an IT instruction. */
>>>> + seen_it = insn;
>>>> + it_count = count >> 1;
>>>> + }
>>>> }
>>>> if ((insn & 0xf800) >= 0xe800)
>>>> count++;
>>>> @@ -6078,6 +6088,71 @@ get_sym_code_type (struct disassemble_info *info,
>>>> return FALSE;
>>>> }
>>>> +/* Search the mapping symbol state for instruction at pc. This is
>>>> only
>>>> + applicable for elf target.
>>>> +
>>>> + There is an assumption Here, info->private_data contains the correct
>>>> AND
>>>> + up-to-date information about current scan process. The information
>>>> will be
>>>> + used to speed this search process.
>>>> +
>>>> + Return TRUE if the mapping state can be determined, and map_symbol
>>>> + will be updated accordingly. Otherwise, return FALSE. */
>>>> +
>>>> +static bfd_boolean
>>>> +mapping_symbol_for_insn (bfd_vma pc, struct disassemble_info *info,
>>>> + enum map_type *map_symbol)
>>>> +{
>>>> + bfd_vma addr;
>>>> + int n, start = 0;
>>>> + bfd_boolean found = FALSE;
>>>> + enum map_type type = MAP_ARM;
>>>> + struct arm_private_data *private_data;
>>>> +
>>>> + if (info->private_data == NULL || info->symtab_size == 0
>>>> + || bfd_asymbol_flavour (*info->symtab) != bfd_target_elf_flavour)
>>>> + return FALSE;
>>>> +
>>>> + private_data = info->private_data;
>>>> + if (pc == 0)
>>>> + start = 0;
>>>> + else
>>>> + start = private_data->last_mapping_sym;
>>>> +
>>>> + start = (start == -1)? 0 : start;
>>>> + addr = bfd_asymbol_value (info->symtab[start]);
>>>> +
>>>> + if (pc >= addr)
>>>> + {
>>>> + if (get_map_sym_type (info, start, &type))
>>>> + found = TRUE;
>>>> + }
>>>> + else
>>>> + {
>>>> + for (n = start - 1; n >= 0; n--)
>>>> + {
>>>> + if (get_map_sym_type (info, n, &type))
>>>> + {
>>>> + found = TRUE;
>>>> + break;
>>>> + }
>>>> + }
>>>> + }
>>>> +
>>>> + /* No mapping symbols were found. A leading $d may be
>>>> + omitted for sections which start with data; but for
>>>> + compatibility with legacy and stripped binaries, only
>>>> + assume the leading $d if there is at least one mapping
>>>> + symbol in the file. */
>>>> + if (!found && private_data->has_mapping_symbols == 1)
>>>> + {
>>>> + type = MAP_DATA;
>>>> + found = TRUE;
>>>> + }
>>>> +
>>>> + *map_symbol = type;
>>>> + return found;
>>>> +}
>>>> +
>>>> /* Given a bfd_mach_arm_XXX value, this function fills in the fields
>>>> of the supplied arm_feature_set structure with bitmasks indicating
>>>> the support base architectures and coprocessor extensions.
>>>>
More information about the Binutils
mailing list