[5/5] ARM: asm-source.exp link options in case of armv7b target

Message ID 1413853021-4393-6-git-send-email-victor.kamensky@linaro.org
State New
Headers show

Commit Message

vkamensky Oct. 21, 2014, 12:57 a.m.
gdb.asm/asm-source.exp fails in armv7b case, because it does
not pass --be8 option to link, as result instructions in asm-source
executable are in big endian order and crash with SIGILL.

Solution is to add --be8 option to link command during test creation.
---
 gdb/testsuite/ChangeLog              | 4 ++++
 gdb/testsuite/gdb.asm/asm-source.exp | 4 ++++
 2 files changed, 8 insertions(+)

Comments

Yao Qi Oct. 24, 2014, 6:05 a.m. | #1
Victor Kamensky <victor.kamensky@linaro.org> writes:

> +    "armv7b-*-*" {
> +	set asm-arch arm
> +	append link-flags " -be8"
> +    }

We can't tell whether "-be8" is needed from the target triplet.
Considering multi-lib, "-be8" is needed for one multilib, but not
for the other.

Maybe, you can fix your problem by running tests via LDFLAGS_FOR_TARGET,

$ make check RUNTESTFLAGS='LDFLAGS_FOR_TARGET=-be8'

or you can create your own board file foo.exp, and add

set_board_info ldflags  "-be8"

$ make check RUNTESTFLAGS='--target_board=foo'
vkamensky Oct. 24, 2014, 6:35 a.m. | #2
Hi Yao,

On 23 October 2014 23:05, Yao Qi <yao@codesourcery.com> wrote:
> Victor Kamensky <victor.kamensky@linaro.org> writes:
>
>> +    "armv7b-*-*" {
>> +     set asm-arch arm
>> +     append link-flags " -be8"
>> +    }
>
> We can't tell whether "-be8" is needed from the target triplet.
> Considering multi-lib, "-be8" is needed for one multilib, but not
> for the other.

Any executable/library that runs on big endian V7 *must* be linked
with -be8 option. Otherwise it simply won't run. In any other multilib
option vfp, neon, etc -be8 must be set. Basically, in big endian case
gcc/gas generates data and instructions in big endian
format but ARM V7 requires that instruction should be little endian
format. It is linker that does instructions byte swap. If -be8 flag
is not passed during link while running on ARM V7 big endian target
executable with crash with SIGILL. If link happens through gcc, then
-be8 always passed for non relocatable code by compiler. In this
particular case link happens directly with linker and -be8 is not
default, so it is needed. One may argue that -be8 for final
executables in ARM V7 BE target should be default even for
linker, but it is not the current case ...

Also note that you have plenty examples in the same test
gdb/testsuite/gdb.asm/asm-source.exp
that do very similar things. For example:

    "powerpc64le-*" {
        set asm-arch powerpc64le
        set asm-flags "-a64 -I${srcdir}/${subdir} $obj_include"
        append link-flags " -m elf64lppc"
    }

Why "-m elf64lppc" is set for powerpc64le target? I suspect
by very similar reasons.

> Maybe, you can fix your problem by running tests via LDFLAGS_FOR_TARGET,
>
> $ make check RUNTESTFLAGS='LDFLAGS_FOR_TARGET=-be8'
>
> or you can create your own board file foo.exp, and add
>
> set_board_info ldflags  "-be8"

I don't feel very strong about it, and definitely I can workaround
this issue or just ignore the failure. It just seemed that it was very
easy to fix.

If you are still not convinced by above argument, yes,
let's just drop it. Please let me know you final thoughts. We
will proceed accordingly. I am fine either way.

Thanks,
Victor

> $ make check RUNTESTFLAGS='--target_board=foo'
>
> --
> Yao (齐尧)
Andrew Pinski Oct. 24, 2014, 6:38 a.m. | #3
On Thu, Oct 23, 2014 at 11:35 PM, Victor Kamensky
<victor.kamensky@linaro.org> wrote:
> Hi Yao,
>
> On 23 October 2014 23:05, Yao Qi <yao@codesourcery.com> wrote:
>> Victor Kamensky <victor.kamensky@linaro.org> writes:
>>
>>> +    "armv7b-*-*" {
>>> +     set asm-arch arm
>>> +     append link-flags " -be8"
>>> +    }
>>
>> We can't tell whether "-be8" is needed from the target triplet.
>> Considering multi-lib, "-be8" is needed for one multilib, but not
>> for the other.
>
> Any executable/library that runs on big endian V7 *must* be linked
> with -be8 option. Otherwise it simply won't run. In any other multilib
> option vfp, neon, etc -be8 must be set. Basically, in big endian case
> gcc/gas generates data and instructions in big endian
> format but ARM V7 requires that instruction should be little endian
> format. It is linker that does instructions byte swap. If -be8 flag
> is not passed during link while running on ARM V7 big endian target
> executable with crash with SIGILL. If link happens through gcc, then
> -be8 always passed for non relocatable code by compiler. In this
> particular case link happens directly with linker and -be8 is not
> default, so it is needed. One may argue that -be8 for final
> executables in ARM V7 BE target should be default even for
> linker, but it is not the current case ...
>
> Also note that you have plenty examples in the same test
> gdb/testsuite/gdb.asm/asm-source.exp
> that do very similar things. For example:
>
>     "powerpc64le-*" {
>         set asm-arch powerpc64le
>         set asm-flags "-a64 -I${srcdir}/${subdir} $obj_include"
>         append link-flags " -m elf64lppc"
>     }
>
> Why "-m elf64lppc" is set for powerpc64le target? I suspect
> by very similar reasons.


Yes and no.  For PowerPC64 little-endian is Linux only so it will
never have a multi-libs that support both little-endian and
big-endian.  While for arm*-*-*, you can have a bare metal env and
that could have a multi-lib for both little and big endian.

This is true for MIPS too.

Thanks,
Andrew Pinski


>
>> Maybe, you can fix your problem by running tests via LDFLAGS_FOR_TARGET,
>>
>> $ make check RUNTESTFLAGS='LDFLAGS_FOR_TARGET=-be8'
>>
>> or you can create your own board file foo.exp, and add
>>
>> set_board_info ldflags  "-be8"
>
> I don't feel very strong about it, and definitely I can workaround
> this issue or just ignore the failure. It just seemed that it was very
> easy to fix.
>
> If you are still not convinced by above argument, yes,
> let's just drop it. Please let me know you final thoughts. We
> will proceed accordingly. I am fine either way.
>
> Thanks,
> Victor
>
>> $ make check RUNTESTFLAGS='--target_board=foo'
>>
>> --
>> Yao (齐尧)
Yao Qi Oct. 24, 2014, 8:52 a.m. | #4
Andrew Pinski <pinskia@gmail.com> writes:

>> Any executable/library that runs on big endian V7 *must* be linked
>> with -be8 option. Otherwise it simply won't run. In any other multilib
>> option vfp, neon, etc -be8 must be set. Basically, in big endian case
>> gcc/gas generates data and instructions in big endian
>> format but ARM V7 requires that instruction should be little endian
>> format. It is linker that does instructions byte swap. If -be8 flag
>> is not passed during link while running on ARM V7 big endian target
>> executable with crash with SIGILL. If link happens through gcc, then
>> -be8 always passed for non relocatable code by compiler. In this
>> particular case link happens directly with linker and -be8 is not
>> default, so it is needed. One may argue that -be8 for final
>> executables in ARM V7 BE target should be default even for
>> linker, but it is not the current case ...
>>
>> Also note that you have plenty examples in the same test
>> gdb/testsuite/gdb.asm/asm-source.exp
>> that do very similar things. For example:
>>
>>     "powerpc64le-*" {
>>         set asm-arch powerpc64le
>>         set asm-flags "-a64 -I${srcdir}/${subdir} $obj_include"
>>         append link-flags " -m elf64lppc"
>>     }
>>
>> Why "-m elf64lppc" is set for powerpc64le target? I suspect
>> by very similar reasons.
>
>
> Yes and no.  For PowerPC64 little-endian is Linux only so it will
> never have a multi-libs that support both little-endian and
> big-endian.  While for arm*-*-*, you can have a bare metal env and
> that could have a multi-lib for both little and big endian.

Andrew is right.  We can have a arm-linux-gnueabi toolchain which has
multilibs for the combination of {le, be} x {armv7, armv6, armv5}, and
this test still fails on armv7 be multilib.
vkamensky Oct. 24, 2014, 5:11 p.m. | #5
On 24 October 2014 01:52, Yao Qi <yao@codesourcery.com> wrote:
> Andrew Pinski <pinskia@gmail.com> writes:
>
>>> Any executable/library that runs on big endian V7 *must* be linked
>>> with -be8 option. Otherwise it simply won't run. In any other multilib
>>> option vfp, neon, etc -be8 must be set. Basically, in big endian case
>>> gcc/gas generates data and instructions in big endian
>>> format but ARM V7 requires that instruction should be little endian
>>> format. It is linker that does instructions byte swap. If -be8 flag
>>> is not passed during link while running on ARM V7 big endian target
>>> executable with crash with SIGILL. If link happens through gcc, then
>>> -be8 always passed for non relocatable code by compiler. In this
>>> particular case link happens directly with linker and -be8 is not
>>> default, so it is needed. One may argue that -be8 for final
>>> executables in ARM V7 BE target should be default even for
>>> linker, but it is not the current case ...
>>>
>>> Also note that you have plenty examples in the same test
>>> gdb/testsuite/gdb.asm/asm-source.exp
>>> that do very similar things. For example:
>>>
>>>     "powerpc64le-*" {
>>>         set asm-arch powerpc64le
>>>         set asm-flags "-a64 -I${srcdir}/${subdir} $obj_include"
>>>         append link-flags " -m elf64lppc"
>>>     }
>>>
>>> Why "-m elf64lppc" is set for powerpc64le target? I suspect
>>> by very similar reasons.
>>
>>
>> Yes and no.  For PowerPC64 little-endian is Linux only so it will
>> never have a multi-libs that support both little-endian and
>> big-endian.  While for arm*-*-*, you can have a bare metal env and
>> that could have a multi-lib for both little and big endian.
>
> Andrew is right.  We can have a arm-linux-gnueabi toolchain which has
> multilibs for the combination of {le, be} x {armv7, armv6, armv5}, and
> this test still fails on armv7 be multilib.

I am not sure that I follow your argument, Your point that for
arm-linux-gnueab that has big endian multilib test will fail, and because
of that we want it to keep failing on armv7b-unknown-linux-gnueabihf
target? The fix I suggested will be activate only if target triplet
starts with 'armv7b'. The situation when target name starts
with 'armv7b' and has little endian multilib support seems very
hypothetical to me.

In case of arm-linux-gnueabi with big endian multilib pretty much
all tests will fail, unless caller will not specify correct compile/link
option (i.e compile -mbig-endian -Wl,--be8, link --be8, etc).

OK, it seems that the patch causes too much controversy, I am
going to drop it. I'll re-post series without it with final version of
other 3 patches, the one that I believe approved by you. And I
will wait for few days for additional feedback.

Thanks,
Victor

> --
> Yao (齐尧)

Patch

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b7cc1c6..1d5fefa 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@ 
+2014-10-13  Victor Kamensky  <victor.kamensky@linaro.org>
+
+	* gdb.asm/asm-source.exp: add armv7b case for target.
+
 2014-09-30  Yao Qi  <yao@codesourcery.com>
 
 	* lib/prelink-support.exp (build_executable_own_libs): Error if
diff --git a/gdb/testsuite/gdb.asm/asm-source.exp b/gdb/testsuite/gdb.asm/asm-source.exp
index fa4585c..153bc50 100644
--- a/gdb/testsuite/gdb.asm/asm-source.exp
+++ b/gdb/testsuite/gdb.asm/asm-source.exp
@@ -37,6 +37,10 @@  switch -glob -- [istarget] {
         set asm-flags "-no-mdebug -I${srcdir}/${subdir} $obj_include"
 	set debug-flags "-gdwarf-2"
     }
+    "armv7b-*-*" {
+	set asm-arch arm
+	append link-flags " -be8"
+    }
     "arm*-*-*" {
         set asm-arch arm
     }