[PATCH][ARM][1/2] Cleanup mixed use of "cpu_variant" and selected_cpu"

Jiong Wang jiong.wang@arm.com
Thu Aug 14 09:03:00 GMT 2014


currently, gas for arm32 have several variables to track cpu features:

cpu_variant
selected_cpu
arm_arch_used
thumb_arch_used

and there are some wrong usage of cpu_variant and selected_cpu which actually affect
binary correctness.

I checked the log, there was cpu_variant only, but later a new variable "selected_cpu"
was introduced for TAG_cpu attribute generation only, since then people start to
use them in a mixed way.

for selected_cpu, its value depends on arch/cpu options or directives, if none of
them found, then its default value is zero.

while for cpu_variant, its default value is a mask contains all features, and will be
updated to the value of selected_cpu *if the value of selected_cpu is not zero*.

so if no arch/cpu options/directives specified, then cpu_variant *accept* everything
while *selected_cpu* reject everything.

then if the code in parsing stage use cpu_variant, it will accept some features, but
reject them in later fixup stage silently if this is not a error/warning report. this
is a *logic error* affect correctness which should be fixed.

for example, given the following .s

.syntax unified

.thumb
.type entry, %function
.global entry
entry:
   blx label           <=== A

.type label, %function
label:
   bx  lr

.arm
.type label2, %function
label2:
   blx label3

.type label3, %function
label3:
   bx  lr

as -o 1.o 1.s
objdump -d 1.o

00000000 <entry>:
    0:   f7ff fffe       bl      0 <entry>

at location A, a detected it's a thumb->thumb blx, then we convert it to bl, but the relocated
destination point to itself which is wrong. this bug is just caused by mixed use of cpu_variant
and selected_cpu.

this patch and the following tries to clean up all the wrong usage of selected_cpu, to make
feature check be consistent in the assembler.

New variable usage
=======================

   * the first patch tries the following variable renaming:

   s/cpu_variant/parse_arch/
   s/selected_cpu_name/intended_arch_name/
   s/selected_cpu/intended_arch/


   * the second patch update intended_arch based on the info we got
     during parsing, and thus naturally fix this bug.


   "parse_arch" are to be used during gas parsing stage, for example those
    do_XXX encoder in tc-arm.c.

   "intended_arch" are to be used during all stage after parsing.

   new variable calculation logic is:
     * intended_arch default to be arm_arch_none which is empty.
     * intended_arch initialized to the value of arch if any of
       them is specified by cmdline or directive.

     * if (inteneded_arch not empty) then
         parse_arch = intended_arch
       else
         parse_arch = accept_all

   the idea behind is if none arch info specified by the user, then
   we should be permissive in parsing stage, while be strict in later
   fixup stage. because accept unsupported instructions in parsing stage
   will normally cause ill-instruction exception on silicon which is
   obvious for user to fix, while wrong relocations may introduce silent
   bugs which is hard to hunt.

   any comments on this ?

   thanks.

   gas/
     * config/tc-arm.c : Rename "cpu_variant" to "parse_arch",
     "selected_cpu" to intended_arch", "selected_cpu_name" to
     "intended_arch_name"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ARM-GAS-Rename-two-internal-variables.patch
Type: text/x-patch
Size: 31924 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20140814/a1e0cd27/attachment.bin>


More information about the Binutils mailing list