Re: [Tinycc-devel] Call for testing

[Thomas Preud'homme]

Le dimanche 6 janvier 2013 17:03:28, grischka a écrit :
> > /usr/bin/gcc-4.0.4 -o tcc.o -c tcc.c -DTCC_TARGET_I386     -I.
> > -I/home/didier/documents/tech/dev/tcc/tinycc -Wall -g -O2
> 
> I think some people might not feel comfortable to see their absolute
> paths exposed.  Also it makes command lines longer/uglier than necessary
> and is redundant here anyway (same as -I.).
> 
> > -fno-strict-aliasing -Wno-pointer-sign -Wno-sign-compare
> > -Wno-unused-result -mpreferred-stack-boundary=2 -march=i386
> > -falign-functions=0 -m32
> 
> Do we need both -march=i386 and -m32?
> 
> Also, what about removing
>      -mpreferred-stack-boundary=2
> and
>      -falign-functions=0
> ?

We can. I don't know why they were added in the first place and when that's 
the case I just leave things untouched.

> 
> > cc1: error: unrecognized command line option "-Wno-unused-result"
> > make: *** [tcc.o] Error 1
> 
> Maybe we could test supported switches by running the compiler in
> configure.

Normally all tests about the environment should go to configure. Also it 
should be possible to set everything in configure as well since tcc could be 
built by cross-compilation and in this case the host environment is 
irrelevant. But at least that part should wait for after the release. As to 
testing the switch, I don't have much motivation right now and I prefer to 
spend my free time on fixing bugs first.

> 
> Also what about using -D_FORTIFY_SOURCE=0 when -Wno-unused-result
> is not supported?

I don't think that's a good idea. It seems I built with a custom CFLAGS 
because I don't have any warnings if I build without that switch so we can 
just remove -Wno-unused-result.

In the case we had some warnings I'd still prefer to not use -D_FORTIFY_SOURCE 
0. The first reason is that _FORTIFY_SOURCE does 2 things: it hardens the 
source by adding some buffer overflow checks and it adds more warnings. 
Setting _FORTIFY_SOURCE to 0 would stop the warning but also stop all the 
hardening which could be wanted. Secondly, turning off warnings globally to 
avoid some false positives would mean we miss real cases where someone forgot 
to test the return value of a function.

> 
> Also, someone please test the functionality of the install.  (I suspect
> libtcc.h might be missing).

Good catch. It seems it comes from commit 
e79281f58ec302e8cc9dfc7d00e06f426fcd2acd.

> 
> Compilation on Windows with MSC fails in tccgen.c:vswap():
> ../tccgen.c(476) : error C2143: syntax error : missing ';' before 'type'
> [more ...]

Oh, C99ism. Fixed

> 
> After moving declarations before statements, it fails like this:
> ../tccgen.c(490) : warning C4308: negative integral constant converted to
> unsigned type ../tccgen.c(490) : warning C4307: '*' : integral constant
> overflow ../tccgen.c(490) : warning C4307: '+' : integral constant
> overflow ../tccgen.c:481: error: incompatible types for redefinition of
> '__static_assert_t' [more ...]
> 
> I'd suggest to move that optimization into its own function
>       void memswap(void *p1, void *p2, size_t n);
> or otherwise to revert it.

I'll check this and the rest later. Free time over for today :)

> 
> Other candidates for reversion (OTOH):
> -
> http://repo.or.cz/w/tinycc.git/commitdiff/fc574f14984d11f1ead50560d1bdc5ae
> 0eaf6d8d The headers from include are copied to win32/include during
> install
> 
> -
> http://repo.or.cz/w/tinycc.git/commitdiff/943574aba54713ca4a17fe33aadde5ab
> ee233b53 Obviously (if you look at the code) this was not a bug but
> intended behavior.
> 
> -
> http://repo.or.cz/w/tinycc.git/commitdiff/3d409b08893873b917ccb8c34398bc41
> a4e84d7c The patch doesn't fix the real problem which is not duplicate -I's
> by the user but the very obviously crappy handling of #include_next
> itself.  Of course the patch fixes something, but if we keep it chances
> are that we'll never see a correct solution.

Thanks for fixing this one. I didn't look at the diff yet, even less tried the 
fix but I know you did some good job. I'll take a look in a day or two, -
ETOOBUSY right now.

> 
> --- grischka

Best regards,

Thomas

signature.asc
Description: This is a digitally signed message part.