[OPCODE][AARCH64]Check mapping symbol while backward searching for IT block
Renlin Li
renlin.li@foss.arm.com
Fri Jan 22 13:34:00 GMT 2016
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 .
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