public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>
To: Christophe Lyon <christophe.lyon@arm.com>, <gcc-patches@gcc.gnu.org>
Subject: Re: PING^1 [PATCH] testsuite: 'b' instruction can't do long enough jumps
Date: Wed, 28 Sep 2022 15:39:19 +0200	[thread overview]
Message-ID: <5c21c50f-c75c-6cc7-4994-85a1f9892020@foss.st.com> (raw)
In-Reply-To: <f39ac203-b833-9c11-5ab9-608c2edef40f@arm.com>

Hi Christophe!

On 2022-09-28 13:55, Christophe Lyon wrote:
> Hi!
> 
> 
> On 9/28/22 11:17, Torbjorn SVENSSON via Gcc-patches wrote:
>> Hi,
>>
>> Ping: 
>> https://gcc.gnu.org/pipermail/gcc-patches/2022-September/601829.html
>>
>> Kind regards,
>> Torbjörn
>>
>> On 2022-09-19 18:30, Torbjörn SVENSSON wrote:
>>> After moving the testglue in commit 9d503515cee, the jump to exit and
>>> abort is too far for the 'b' instruction on Cortex-M0. As most of the
> I am not sure I understand why that commit changed the distance between 
> 'exit' and the branch instruction?

The change was that the gcc_tg.o (the DejaGNU testglue.c object file) is 
now put last on the command line. In the previous versions of GCC, it 
was put before the ldflags flag etc, so now the code is placed in a way 
that the distance might be too big.

This could also be related to that we in ST are using QEMU in system 
mode and not user mode and as a result, our test environment is slightly 
larger and might perhaps be placed in between the code for the test case 
and the testglue.

>>> C code would generate a 'bl' instruction instead of a 'b'
>>> instruction, lets do the same for the inline assembler.
>>>
>>> The error seen without this patch:
>>>
>>> /tmp/cccCRiCl.o: in function `main':
>>> stack-protector-1.c:(.text+0x4e): relocation truncated to fit: 
>>> R_ARM_THM_JUMP11 against symbol `__wrap_exit' defined in .text 
>>> section in gcc_tg.o
>>> stack-protector-1.c:(.text+0x50): relocation truncated to fit: 
>>> R_ARM_THM_JUMP11 against symbol `__wrap_abort' defined in .text 
>>> section in gcc_tg.o
>>> collect2: error: ld returned 1 exit status
>>>
> Anyway the change seems sensible to me, I suppose it's not worth adding 
> support in the linker to insert long branch stubs for these relocations.

If a simple 'bl' instead of 'b' is enough, I think that this trivial 
change is the right one as the test case is supposed to test the stack 
protection, not branching, right?

Kind regards,
Torbjörn

> 
> Christophe
> 
>>> gcc/testsuite/ChangeLog:
>>>
>>>          * gcc/testsuite/gcc.target/arm/stack-protector-1.c: Use 'bl'
>>>     instead of 'b' instruction.
>>>     * gcc/testsuite/gcc.target/arm/stack-protector-3.c: Likewise.
>>>
>>> Co-Authored-By: Yvan ROUX  <yvan.roux@foss.st.com>
>>> Signed-off-by: Torbjörn SVENSSON  <torbjorn.svensson@foss.st.com>
>>> ---
>>>   gcc/testsuite/gcc.target/arm/stack-protector-1.c | 4 ++--
>>>   gcc/testsuite/gcc.target/arm/stack-protector-3.c | 2 +-
>>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-1.c 
>>> b/gcc/testsuite/gcc.target/arm/stack-protector-1.c
>>> index 8d28b0a847c..3f0ffc9c3f3 100644
>>> --- a/gcc/testsuite/gcc.target/arm/stack-protector-1.c
>>> +++ b/gcc/testsuite/gcc.target/arm/stack-protector-1.c
>>> @@ -56,8 +56,8 @@ asm (
>>>   "    ldr    r1, [sp, #4]\n"
>>>       CHECK (r1)
>>>   "    mov    r0, #0\n"
>>> -"    b    exit\n"
>>> +"    bl    exit\n"
>>>   "1:\n"
>>> -"    b    abort\n"
>>> +"    bl    abort\n"
>>>   "    .size    main, .-main"
>>>   );
>>> diff --git a/gcc/testsuite/gcc.target/arm/stack-protector-3.c 
>>> b/gcc/testsuite/gcc.target/arm/stack-protector-3.c
>>> index b8f77fa2309..2f710529b8f 100644
>>> --- a/gcc/testsuite/gcc.target/arm/stack-protector-3.c
>>> +++ b/gcc/testsuite/gcc.target/arm/stack-protector-3.c
>>> @@ -26,7 +26,7 @@ asm (
>>>   "    .type    __stack_chk_fail, %function\n"
>>>   "__stack_chk_fail:\n"
>>>   "    movs    r0, #0\n"
>>> -"    b    exit\n"
>>> +"    bl    exit\n"
>>>   "    .size    __stack_chk_fail, .-__stack_chk_fail"
>>>   );

  reply	other threads:[~2022-09-28 13:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-19 16:30 Torbjörn SVENSSON
2022-09-28  9:17 ` PING^1 " Torbjorn SVENSSON
2022-09-28 11:55   ` Christophe Lyon
2022-09-28 13:39     ` Torbjorn SVENSSON [this message]
2022-09-28 14:32       ` Christophe Lyon
     [not found]   ` <f11762f1-774d-1054-0c51-8c4082d9607a@foss.st.com>
2022-10-05  9:51     ` Kyrylo Tkachov
2022-10-05 10:03       ` Torbjorn SVENSSON

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5c21c50f-c75c-6cc7-4994-85a1f9892020@foss.st.com \
    --to=torbjorn.svensson@foss.st.com \
    --cc=christophe.lyon@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).