This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH][ARM][1/2] Cleanup mixed use of "cpu_variant" and selected_cpu"
- From: Will Newton <will dot newton at linaro dot org>
- To: Jiong Wang <jiong dot wang at arm dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Thu, 14 Aug 2014 11:03:10 +0100
- Subject: Re: [PATCH][ARM][1/2] Cleanup mixed use of "cpu_variant" and selected_cpu"
- Authentication-results: sourceware.org; auth=none
- References: <53EC7B48 dot 5030507 at arm dot com>
On 14 August 2014 10:03, Jiong Wang <email@example.com> 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
> * 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
Toolchain Working Group, Linaro