[PATCH] libm/x86: use call instead of jump for wrappers

Denys Vlasenko vda.linux at googlemail.com
Sun Oct 31 15:10:33 UTC 2010


On Sunday 31 October 2010 07:57, Timo Teräs wrote:
> >> If we used jump in such cases, we'd corrupt the call stack and
> >> crash.
> > 
> > Yes, but using call will push additional word on the stack
> > and the called function will use wrong offset to access
> > the parameter on stack.
> 
> Oh right. Good catch.
> 
> I somehow mixed cdecl with the other calling conventions that pass first
> four floating point arguments in FPU registers. Should have checked this...
> 
> > So instead of this, you need to update #if guard:
> > 
> > #if defined __i386__ && defined __OPTIMIZE__
> > 
> > Add more conditions so that it kicks in only when safe.
> 
> And thank you for breaking my build again with d726ada135...

Since by breaking long double wrappers I proved that no one actually
runs math testsuite (does anyone runs ANY part of testsuite?),
I decided that now it is my obligation to fix math testsuite first,
so that in the future it will be easier to catch breakatge.

I fixed it to a certain degree already.

Then I reverted your fix, and _verified_ that it works with both
static and shared build.


> You added only the case if uclibc supports SSP. Technically that's
> irrelevant.

You are right, I was not sure whether I am using right conditional.

> We should check:
>  - __PIC__ for PIC build, it causes ebx reload (and ebx needs to be
> saved/restored in prologue/epilogue on PIC).

It needs to be saved/restored *if ebx is used*. In these stubs it is
not used, thus it is not saved by gcc, and therefore tail jump is ok
even with __PIC__.

I do not merely think so. I tested it. Try in the directory
where you successfully built shared uclibc using PIC:

cd test
rm -rf `find -type d -a ! -name math`  # don't want all other dirs
make check UCLIBC_ONLY=1 VERBOSE=1 2>&1 | tee LOG


It will fail on long double test stage. Looking
ant the failures, you will see that they are all
induced by long double -> double truncation:

make -C  math compile
...
---------------------------------
TEST math/ test-ldouble
---------------------------------
./test-ldouble   > "test-ldouble.out" 2>&1 ; ret=$? ; expected_ret="" ; test -z "$expected_ret" && export expected_ret=0 ; if ! test $ret -eq $expected_ret ; then echo "ret == $r
ret == 1 ; expected_ret == 0
The output of failed test is:
testing long double (without inline functions)
Failure: Test: acos (0) == pi/2
Result:
 is:          1.57079632679489655799e+00   0x1.921fb54442d180000000p+0
 should be:   1.57079632679489661925e+00   0x1.921fb54442d184600000p+0
 difference:  6.12574227454310005181e-17   0x4.6a000000000000000000p-56
 ulp       :  565.0000
 max.ulp   :  0.0000
...
...
...
Failure: Test: rint (-72057594037927937.5) == -72057594037927938.0
Result:
 is:         -7.20575940379279360001e+16  -0x100000000000000000000p+56
 should be:  -7.20575940379279380024e+16  -0x1.00000000000002000000p+56
 difference:  2.00000000000000000000e+00   0x200000000000000000000p+0
 ulp       :  256.0000
 max.ulp   :  0.0000
Maximal error of `rint'
 is      : 1024 ulp
 accepted: 0 ulp

Test suite completed:
  770 test cases plus 680 tests for exception flags executed.
  170 errors occurred.
make[1]: *** [test-ldouble.exe] Error 1
make[1]: Leaving directory `/.1/usr/srcdevel/uclibc/fix/uClibc.t9/test/math'
make: *** [_dirrun_math] Error 2


With tail call instead of tail jump, there will be much more errors
and their nature will be far worse: the results will be widely off.

If you see more failures (or if ldouble test SEGVs outright),
please post your .config.


>  - __SSP_ALL__ (SSP build can be enabled with UCLIBC_BUILD_SSP or adding
> manually the extra CFLAG) to check if even small functions get stack
> smashing protection
> 
> We don't need __UCLIBC_HAS_SSP__, it means "uclibc supports applications
> built with SSP", not that "uclibc itself is built with SSP enabled".

Thanks, replaced:

-#if defined __i386__ && defined __OPTIMIZE__ && !defined __UCLIBC_HAS_SSP__
+#if defined __i386__ && defined __OPTIMIZE__ && !defined __SSP_ALL__

and pushed to git.


> Alternatively we could write the wrappers to cope with PIC properly and
> use the GCC attribute naked. Or write the wrappers as .S file.

Alternatively, we may just nuke them. Maybe my idea to be "clever"
with them wasn't such a good idea in the first place.

-- 
vda


More information about the uClibc mailing list