This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

On 14/08/14 11:03, Will Newton wrote:
On 14 August 2014 10:03, Jiong Wang <> wrote:
currently, gas for arm32 have several variables to track cpu features:


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
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

so if no arch/cpu options/directives specified, then cpu_variant *accept*
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

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

.type label, %function
   bx  lr

.type label2, %function
   blx label3

.type label3, %function
   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:

Why are we replacing "cpu" with "arch"? Most of the manipulations of
these variables include with word "cpu" so it seems strange to name
them "arch".

   thanks for your comment.

   it's OK to change "arch" back to "cpu".

my intention is to find idea names that help people know immediately which one to use.

-- Jiong

   * 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
         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 ?


     * config/tc-arm.c : Rename "cpu_variant" to "parse_arch",
     "selected_cpu" to intended_arch", "selected_cpu_name" to

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]