This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] x86: reject architecture settings that are invalid to be set from the command line
>>> On 10.06.10 at 16:32, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> On Thu, Jun 10, 2010 at 7:25 AM, Jan Beulich <JBeulich@novell.com> wrote:
>>>>> On 10.06.10 at 16:19, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>>> On Thu, Jun 10, 2010 at 12:05 AM, Jan Beulich <JBeulich@novell.com> wrote:
>>>>>>> On 09.06.10 at 18:02, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>>>>> On Wed, Jun 9, 2010 at 8:36 AM, Jan Beulich <JBeulich@novell.com> wrote:
>>>>>> So far, options like -march=i8086 were accepted despite the assembler
>>>>>> subsequently choking on other consistency checks, leading to reasonably
>>>>>> cryptic error messages. This patch makes it so that impossible
>>>>>> architecure settings are neither accepted nor displayed (i.e. it is now
>>>>>> made sure that those settings can only be used via directives).
>>>>>>
>>>>>> gas/
>>>>>> 2010-06-09 Jan Beulich <jbeulich@novell.com>
>>>>>>
>>>>>> * config/tc-i386.c (md_parse_option): Ignore impossible processor
>>>>>> types.
>>>>>> (show_arch): New parameter 'check'.
>>>>>> (md_show_usage): Adjust calls to show_arch().
>>>>>>
>>>>>> --- 2010-06-09/gas/config/tc-i386.c 2010-06-09 17:04:12.000000000 +0200
>>>>>> +++ 2010-06-09/gas/config/tc-i386.c 2010-06-09 17:24:59.000000000 +0200
>>>>>> @@ -8166,6 +8166,11 @@ md_parse_option (int c, char *arg)
>>>>>> if (strcmp (arch, cpu_arch [j].name) == 0)
>>>>>> {
>>>>>> /* Processor. */
>>>>>> + if (! (strcmp (default_arch, "i386")
>>>>>> + ? cpu_arch[j].flags.bitfield.cpulm
>>>>>> + : cpu_arch[j].flags.bitfield.cpui386))
>>>>>> + continue;
>>>>>> +
>>>>>> cpu_arch_name = cpu_arch[j].name;
>>>>>> cpu_sub_arch_name = NULL;
>>>>>> cpu_arch_flags = cpu_arch[j].flags;
>>>>>> @@ -8297,7 +8302,7 @@ md_parse_option (int c, char *arg)
>>>>>> "
>>>>> "
>>>>>>
>>>>>> static void
>>>>>> -show_arch (FILE *stream, int ext)
>>>>>> +show_arch (FILE *stream, int ext, int check)
>>>>>> {
>>>>>> static char message[] = MESSAGE_TEMPLATE;
>>>>>> char *start = message + 27;
>>>>>> @@ -8334,6 +8339,13 @@ show_arch (FILE *stream, int ext)
>>>>>> /* It is an processor. Skip if we show only extension. */
>>>>>> continue;
>>>>>> }
>>>>>> + else if (check && ! (strcmp (default_arch, "i386")
>>>>>> + ? cpu_arch[j].flags.bitfield.cpulm
>>>>>> + : cpu_arch[j].flags.bitfield.cpui386))
>>>>>> + {
>>>>>> + /* It is an impossible processor - skip. */
>>>>>> + continue;
>>>>>> + }
>>>>>>
>>>>>>
>>>>>
>>>>> Do we need to check cpu_arch[j].flags.bitfield.cpulm? Can we
>>>>> just check cpu_arch[j].flags.bitfield.cpui386 like
>>>>>
>>>>> if (check && !cpu_arch[j].flags.bitfield.cpui386)
>>>>> continue?
>>>>>
>>>>
>>>> I'm of the opinion that when the assembler is in 64-bit mode it
>>>> should reject those architectures that aren't 64-bit capable,
>>>> otherwise specifying e.g. -march=i386 has the same ugly effect
>>>> as has passing -march=i8086 in 32-bit mode. And if we reject
>>>> them, we should also not display them as available.
>>>>
>>>
>>> On Linux/x86-64, your patch gave me
>>>
>>> ../as-new --help
>>>
>>> --32/--64 generate 32bit/64bit code
>>> --divide ignored
>>> -march=CPU[,+EXTENSION...]
>>> generate code for CPU and EXTENSION, CPU is one
>>> of:
>>> generic64, nocona, core2, corei7, l1om, opteron,
>>> k8,
>>> amdfam10, bdver1
>>>
>>> I don't see how it can be correct since "--32 -march=i386" works fine.
>>>
>>
>> Just try ../as-new --32 --help - it'll show the 32-bit possibilities
>> as well.
>
> It doesn't look right to me.
>
I'm afraid I'm not following: Without --32, -march=i386 doesn't work.
What's wrong then with not displaying the option in the first place
without --32?
Or if you insist on your point, would you clarify how you think people
can know that they can't pass -march=i386 without --32 on a 64-bit
installation, when the help screen doesn't tell them so? If I can
understand your thinking here, I could perhaps agree to not doing
the check in the help function...
Jan