[OPCODE][AARCH64]Check mapping symbol while backward searching for IT block
Renlin Li
renlin.li@foss.arm.com
Thu Jan 28 10:28:00 GMT 2016
Hi all,
Here is the patch to make the test to skip non-elf target.
The test case here is only applicable to elf target which has mapping
symbol defined.
arm-none-eabi bintuils regression test checked Okay, Okay to commit?
Regards,
Renlin Li
gas/ChangeLog:
2016-01-28 Renlin Li <renlin.li@arm.com>
* testsuite/gas/arm/thumb2_it_search.d: Skip non-elf targets.
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.
>>>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rb5225.patch
Type: text/x-patch
Size: 610 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20160128/351a9a37/attachment.bin>
More information about the Binutils
mailing list