Re: [Tinycc-devel] Tinycc from git still can't compile fossil-scm

[Domingo Alvarez Duarte]

clang and pcc also generates inline assembly instead of call/alloc.

gcc -E -o check-va-gcc.c check-va.c

gcc -S -o check-va-gcc.asm check-va.c

gcc -o check-va-gcc check-va.c

clang -E -o check-va-clang.c check-va.c

clang -S -o check-va-clang.asm check-va.c

clang -o check-va-clang check-va.c

pcc -E -o check-va-pcc.c check-va.c

pcc -S -o check-va-pcc.asm check-va.c

#pcc -o check-va-pcc check-va.c

../tcc -B.. -I.. -I../include -E -o check-va-tcc.c check-va.c

../tcc -B.. -I.. -I../include  -o check-va-tcc check-va.c

./check-va-gcc

./check-va-tcc

On Sat, Mar 29, 2014 at 9:38 PM, Domingo Alvarez Duarte <address@hidden> wrote:

Asking gcc to generate assembler code from your test code I can see that gcc do not call/malloc any builtin it generates inline code so there is nothing to free.

-----

.file "check-va.c"

.text

.type passdown, @function

passdown:

.LFB0:

.cfi_startproc

pushq %rbp

.cfi_def_cfa_offset 16

.cfi_offset 6, -16

movq %rsp, %rbp

.cfi_def_cfa_register 6

subq $48, %rsp

movq %rdi, -40(%rbp)

movq %rsi, -48(%rbp)

leaq -32(%rbp), %rax

movq -48(%rbp), %rdx

movq (%rdx), %rcx

movq %rcx, (%rax)

movq 8(%rdx), %rcx

movq %rcx, 8(%rax)

movq 16(%rdx), %rdx

movq %rdx, 16(%rax)

leaq -32(%rbp), %rdx

movq -40(%rbp), %rax

movq %rdx, %rsi

movq %rax, %rdi

call vprintf

movl %eax, -4(%rbp)

movl -4(%rbp), %eax

leave

.cfi_def_cfa 7, 8

ret

.cfi_endproc

.LFE0:

.size passdown, .-passdown

.type myprintf, @function

myprintf:

.LFB1:

.cfi_startproc

pushq %rbp

.cfi_def_cfa_offset 16

.cfi_offset 6, -16

movq %rsp, %rbp

.cfi_def_cfa_register 6

subq $224, %rsp

movq %rsi, -168(%rbp)

movq %rdx, -160(%rbp)

movq %rcx, -152(%rbp)

movq %r8, -144(%rbp)

movq %r9, -136(%rbp)

testb %al, %al

je .L3

movaps %xmm0, -128(%rbp)

movaps %xmm1, -112(%rbp)

movaps %xmm2, -96(%rbp)

movaps %xmm3, -80(%rbp)

movaps %xmm4, -64(%rbp)

movaps %xmm5, -48(%rbp)

movaps %xmm6, -32(%rbp)

movaps %xmm7, -16(%rbp)

.L3:

movq %rdi, -216(%rbp)

movl $8, -200(%rbp)

movl $48, -196(%rbp)

leaq 16(%rbp), %rax

movq %rax, -192(%rbp)

leaq -176(%rbp), %rax

movq %rax, -184(%rbp)

leaq -200(%rbp), %rdx

movq -216(%rbp), %rax

movq %rdx, %rsi

movq %rax, %rdi

call passdown

leave

.cfi_def_cfa 7, 8

ret

.cfi_endproc

.LFE1:

.size myprintf, .-myprintf

.section .rodata

.LC1:

.string "bla"

.LC2:

.string "%s %i %f\n"

.text

.globl main

.type main, @function

main:

.LFB2:

.cfi_startproc

pushq %rbp

.cfi_def_cfa_offset 16

.cfi_offset 6, -16

movq %rsp, %rbp

.cfi_def_cfa_register 6

movsd .LC0(%rip), %xmm0

movl $42, %edx

movl $.LC1, %esi

movl $.LC2, %edi

movl $1, %eax

call myprintf

movl $0, %eax

popq %rbp

.cfi_def_cfa 7, 8

ret

.cfi_endproc

.LFE2:

.size main, .-main

.section .rodata

.align 8

.LC0:

.long 2576980378

.long 1071225241

.ident "GCC: (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3"

.section .note.GNU-stack,"",@progbits

 

On Sat, Mar 29, 2014 at 9:33 PM, Domingo Alvarez Duarte <address@hidden> wrote:

Thanks for pointing it and show an example to test !

Now going back to the original problem the original tcc implementation leaks memory on:

----

void *__va_copy(struct __va_list_struct *src)

{

    struct __va_list_struct *dest =

        (struct __va_list_struct *)malloc(sizeof(struct __va_list_struct));

    *dest = *src;

    return dest;

}

----

And I'll continue investigating a way to make it work with fossil-scm for the X86_64, the problem that I saw is that there is a double free when the process fork somehow the fossil compiled by tcc seem to not duplicate the malloced strioneng and both the parent and child free the same string.

Sounds crazy because the os should do that.

Any idea on the memory leak and the process fork ?

Again thanks for you time and attention !

On Sat, Mar 29, 2014 at 5:53 PM, Michael Matz <address@hidden> wrote:

Hello,

On Fri, 28 Mar 2014, Domingo Alvarez Duarte wrote:

I found that on X86_64 linux if I do not free the memory on __va_end(), the
compiled fossil-scm server works, I suspect is something with the
fork/threads.---
void __va_end(struct __va_list_struct *ap)
{
    //free(ap);
}

Cheers !

Errr.  I see you now fiddled with that on mob.  Commit c025478d7c03, rewriting va* to not use malloc.  That's completely wrong.  You've effectively changed the ABI of stdarg, and hence interoperability with every non-TCC compiler.  The public va_list on x86_64 _must_ be a pointer.
To see it breaking try e.g. this:

% cat vatest.c
#include <stdio.h>
#include <stdarg.h>

static int passdown (const char *str, va_list ap)
{
  int ret;
  va_list ap2;
  va_copy (ap2, ap);
  ret = vprintf (str, ap2);
  va_end (ap2);
  return ret;
}

static int myprintf (const char *str, ...)
{
  va_list ap;
  va_start (ap, str);
  passdown (str, ap);
  va_end (ap);
}

int main ()
{
  myprintf ("%s %i %f\n", "bla", 42, 0.4);
  return 0;
}

When executed it must print:
bla 42 0.400000

Before your patch it does, after your patch it prints garbage (on my system " 134514261 0.000000") (without the va_copy and ap2 it even just segfaults now, though strictly speaking that's invalid stdarg usage). Please revert.

If you could please _discuss_ changes in parts you don't completely understand on the list before making nilly-willy changes?  Just because fossil-scm "works" after your patching doesn't mean much if you don't know _why_ fossil-scm didn't work before, and especially doesn't mean that the change was even correct.


Ciao,
Michael.

_______________________________________________
Tinycc-devel mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/tinycc-devel