Re: [Tinycc-devel] Regression on ARM

[Kirill Smelkov]

On Wed, Nov 21, 2012 at 12:30:04PM +0100, Thomas Preud'homme wrote:
> On Wed, 21 Nov 2012 10:03:58 +0400, Kirill Smelkov wrote:
> >On Tue, Nov 20, 2012 at 03:29:26PM +0100, address@hidden wrote:
> >>Another regression is due to the unconditional activation of btest
> >>in the list of tests to run. Indeed, there is no bound checking on
> >>arm (and also C67 and il). This test should thus be run only for
> >>architecture supporting it.
> >
> >Sorry, my fault, that was late at night. Does the following patch
> >fixes
> >btest issue for you?
> >
> >---- 8< ----
> >From: Kirill Smelkov <address@hidden>
> >Date: Wed, 21 Nov 2012 09:47:23 +0400
> >Subject: [PATCH] tests: btest should only run on targets
> >supporting bcheck
> >
> >After 40a54c43 (Repair bounds-checking runtime), and in particular
> >5d648485 (Now btest pass!) `make test` was broken on ARCH != i386,
> >because I've changed btest to unconditionally run on all arches.
> >
> >But bounds-checking itself is only supported on i386 and oops...
> >
> >My fault, fix it.
> 
> No need to blame yourself, this line is not necessary. Besides, this
> information can be extracted from git log/blame.

I don't insist on leaving it, let's drop it :)


> >Reported-by: Thomas Preud'homme <address@hidden>
> >---
> > tests/Makefile | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> >diff --git a/tests/Makefile b/tests/Makefile
> >index af1fdb8..ca581b0 100644
> >--- a/tests/Makefile
> >+++ b/tests/Makefile
> >@@ -11,12 +11,16 @@ TESTS = libtest \
> >             test2 \
> >             test3 \
> >             speedtest \
> >-            btest \
> >             weaktest
> >
> >             # test4 # this test does not seem to work on any platform
> >             # asmtest # this test does not seem to work on any platform
> >
> >+# bounds-checking is supported only on i386
> >+ifeq ($(ARCH),i386)
> >+    TESTS += btest
> >+endif
> >+
> > # these should work too
> > # TESTS += test1 test2 speedtest btest weaktest
> 
> >
> >--
> >1.8.0.289.g7a667bc
> 
> Yep it works but shouldn't you use the same filter-out method to be
> consistent with other disabled tests?

Maybe yes, but that filter-outs and a lot of ifdefs inside tcc sources
looks not so good to me. Ideally, here I would like to write

    if CONFIG_TCC_BCHECK
        tests += btest
    endif

or better

    tests-$(CONFIG_TCC_BCHECK) += btest

and that CONFIG_TCC_BCHECK should be inferred by configure from cpu,
arch, etc... to y or n, and then only tests-y run. (this scheme is used
by linux kernel).

Also, doing it positively correllates on how at least me thinks - on
such-and-such environment this works, instead of on this long list of
condition this is broken.


Only I'm out of spare time to refresh the configure, so I don't insist.


> >>There was a test regression on ARM recently which I managed to
> >>pinpoint to commit b2a02961b4407ca76db5a66ca5ae855cbfba4e8e. All I
> >>can tell so far is that it's related to the addition of the test
> >>builtin_frame_address_test.
> >
> >Thanks for the report.
> >
> >I'm new to arm - will try to setup arm cross-compiler toolchain
> >and see
> >what is going on there, but that will probably take time. In the
> >meantime, I'm having problem to build tcc arm cross compiler on i386:
> 
> I can maybe fix it myself if you have problem setting up your
> environment.

Studying arm was long on my todo list. I've started learning its
instructions yesterday evening. It will probablt take some more
evenings, then the next step is to setup cross-toolchain - some more
evenings again. That's not a problem for me, but others would need to
wait, or if there is no time, your help with fixing it is appreciated.


> 
> >
> >    gcc -o arm-fpa-tcc tcc.c -DONE_SOURCE -DTCC_TARGET_ARM  -Wall -g
> >-O2 -fno-strict-aliasing -Wno-pointer-sign -Wno-sign-compare
> >-Wno-unused-result -mpreferred-stack-boundary=2 -march=i386
> >-falign-functions=0 -m32 -lm -ldl
> >    In file included from libtcc.c:50:0,
> >                     from tcc.c:22:
> >    arm-gen.c: In function ‘gfunc_call’:
> >    arm-gen.c:1058:7: warning: implicit declaration of function
> >‘vfpr’ [-Wimplicit-function-declaration]
> >    In file included from libtcc.c:42:0,
> >                     from tcc.c:22:
> >    tccpp.c: In function ‘macro_subst’:
> >    tccpp.c:2806:12: warning: ‘*((void *)&cval+4)’ is used
> >uninitialized in this function [-Wuninitialized]
> >    /tmp/ccWBLnLc.o: In function `gfunc_call':
> >    /home/kirr/src/tools/tinycc/arm-gen.c:1058: undefined
> >reference to `vfpr'
> >    /home/kirr/src/tools/tinycc/arm-gen.c:1058: undefined
> >reference to `vfpr'
> >    collect2: error: ld returned 1 exit status
> >    Makefile:194: recipe for target 'arm-fpa-tcc' failed
> >    make: *** [arm-fpa-tcc] Error 1
> >
> >
> >could you please fix it, or tell me what is the workaround, so I
> >could
> >try fixing tcc while working on cross-version of it?
> 
> Yes sure, I'm sorry. I broke build without vfp. I always tested with
> VFP in my tests.
>
> (following up)
> Forgot to mention it's fixed.

Thanks.


> >Also, while I'm studying how to setup arm toolchain, what is
> >`objdump -d`
> >output on arm for gcc and tcc for the next source?
> >
> >---- 8< ----
> >void *bfa0()
> >{
> >    return __builtin_frame_address(0);
> >}
> >
> >void *bfa1()
> >{
> >    return __builtin_frame_address(1);
> >}
> >
> >void *bfa2()
> >{
> >    return __builtin_frame_address(2);
> >}
> >---- 8< ----
> 
> *** tcc ***
> 
> 00000000 <bfa0>:
>    0:   e1a0c00d        mov     ip, sp
>    4:   e92d5800        push    {fp, ip, lr}
>    8:   e28db00c        add     fp, sp, #12
>    c:   e1a00000        nop                     ; (mov r0, r0)
>   10:   e28b0000        add     r0, fp, #0
>   14:   e1a00000        nop                     ; (mov r0, r0)
>   18:   e91ba800        ldmdb   fp, {fp, sp, pc}
> 
> 0000001c <bfa1>:
>   1c:   e1a0c00d        mov     ip, sp
>   20:   e92d5800        push    {fp, ip, lr}
>   24:   e28db00c        add     fp, sp, #12
>   28:   e1a00000        nop                     ; (mov r0, r0)
>   2c:   e59b0000        ldr     r0, [fp]
>   30:   e1a00000        nop                     ; (mov r0, r0)
>   34:   e91ba800        ldmdb   fp, {fp, sp, pc}
> 
> 00000038 <bfa2>:
>   38:   e1a0c00d        mov     ip, sp
>   3c:   e92d5800        push    {fp, ip, lr}
>   40:   e28db00c        add     fp, sp, #12
>   44:   e24bd010        sub     sp, fp, #16
>   48:   e59b0000        ldr     r0, [fp]
>   4c:   e50b0010        str     r0, [fp, #-16]
>   50:   e51be010        ldr     lr, [fp, #-16]
>   54:   e59e0000        ldr     r0, [lr]
>   58:   e1a00000        nop                     ; (mov r0, r0)
>   5c:   e91ba800        ldmdb   fp, {fp, sp, pc}
> 
> *** gcc ***
> 
> 00000000 <bfa0>:
>    0:   b480            push    {r7}
>    2:   af00            add     r7, sp, #0
>    4:   463b            mov     r3, r7
>    6:   4618            mov     r0, r3
>    8:   46bd            mov     sp, r7
>    a:   bc80            pop     {r7}
>    c:   4770            bx      lr
>    e:   bf00            nop
> 
> 00000010 <bfa1>:
>   10:   b480            push    {r7}
>   12:   af00            add     r7, sp, #0
>   14:   683b            ldr     r3, [r7, #0]
>   16:   4618            mov     r0, r3
>   18:   46bd            mov     sp, r7
>   1a:   bc80            pop     {r7}
>   1c:   4770            bx      lr
>   1e:   bf00            nop
> 
> 00000020 <bfa2>:
>   20:   b480            push    {r7}
>   22:   af00            add     r7, sp, #0
>   24:   683b            ldr     r3, [r7, #0]
>   26:   681b            ldr     r3, [r3, #0]
>   28:   4618            mov     r0, r3
>   2a:   46bd            mov     sp, r7
>   2c:   bc80            pop     {r7}
>   2e:   4770            bx      lr

That's the thumb instructions in gcc output, if I see it correctly.
Couly you please ask gcc to generate usual arm 4-byte wide instructions
and show the disassembly.

Also, what is the output of ./tests/tcctest.gcc for
__builtin_frame_address() tests? It should be

    str: __builtin_frame_address
    bfa1: __builtin_frame_address
    bfa2: __builtin_frame_address
    bfa3: __builtin_frame_address


> >By the way, what arm variant I should test against - it seems
> >there are
> >many? Another question: maybe you know, what is the most
> >convenient way
> >to setup arm cross-toolchain on Debian GNU/Linux on i386?
> 
> The bug I experienced was with the hardfloat variant (that is,
> TCC_ARM_HARDFLOAT, TCC_ARM_EABI and TCC_ARM_VFP must be defined).
> Maybe the bug also happens on EABI but I can't tell for sure since I
> haven't tried.

thanks, will try to setup armhf then.


> >Thanks,
> >Kirill
> 
> Thank to you :)

Nevermind :)