Didn't the compiler give you a possible uninitialized use (of tsc) at line
415 of syscall.c?
That longjmp based waserror, the more I think, the more it scares me.
IMO all the local variables accessed on the longjmp return path, should be
made volatile.
No matter how hard you tell GCC to clobber registers and memory.
Here it is an example, compiled with GCC 4.8.4, which uses returns_twice
and all the trimmings on setjmp():
#include <setjmp.h>
jmp_buf env;
extern int goo(int);
int foo(int l)
{
int k = goo(l);
if (setjmp(env)) {
return k;
}
k += 11;
goo(9);
return k;
}
$ gcc -O2 -S
foo:
pushq %rbx
subq $16, %rsp
call goo
movl $env, %edi
movl %eax, 12(%rsp) ; Stores goo(l) call result (k) to rsp[12]
call _setjmp
testl %eax, %eax ; Checks setjmp() return code
movl 12(%rsp), %edx ; Loads back rsp[12] (k) to %edx
jne .L2 ; L2 is the longjmp() return path
leal 11(%rdx), %ebx ; Stores k+11 to %ebx. L2 will never see this
+11 operation
movl $9, %edi
call goo
movl %ebx, %edx
.L2:
addq $16, %rsp
movl %edx, %eax ; Loads the original k (simply the result of
goo(l), from the %edx load after testl)
popq %rbx
ret
Pretty messed up. Easily verifiable by putting goo() in a separate file,
and printing results of foo() calls.
That is Kevin what you do with tsc = read_tsc() at line 421.
IMO, all the local variable accessed within a waserror() return path, and
(re)assigned after the waserror(), must be marked as volatile.
Or bad things happen.
One more thing Kevin about the timing tests.
These can become jittery with high load, if the test run on a node where
the test app is not the only thing running.
Ask me how I know ... (Jenkins build going red at 0300 in the morning) 😀
OTOH some testing needs to be there. What I resorted in the past, was to
run that test at high priority, do the time();sleep(n);time() sequence 5
times, and take the lowest of the deltas as sleep time.
Not sure we have to care of these cases for Akaros.
On Thu, Nov 19, 2015 at 6:18 PM, Kevin Klues <[email protected]> wrote:
> The changes in this request can be viewed online at:
>
> https://github.com/brho/akaros/compare/63c72d8...6b9f50f
>
> The following changes since commit
> 63c72d8a58dc11457604164207075d6351e9c0bb:
>
> Create ak-kill-9pserver.sh that kills ufs server (2015-11-18 10:37:28
> -0800)
>
> are available in the git repository at:
>
> [email protected]:klueska/akaros sleep-support
>
> for you to fetch changes up to 6b9f50f49f873c663c29b6f2234af23b99de4125:
>
> A utest to test nanosleep, sleep, and usleep (2015-11-19 18:15:46 -0800)
>
> ----------------------------------------------------------------
> Kevin Klues (3):
> Add the nanosleep syscall
> Add support for glibc sleep, nanosleep, and usleep
> A utest to test nanosleep, sleep, and usleep
>
> kern/include/ros/bits/syscall.h | 1 +
> kern/src/syscall.c | 50 +++++++++
> .../glibc-2.19-akaros/sysdeps/akaros/nanosleep.c | 34 ++++++
> .../glibc-2.19-akaros/sysdeps/akaros/sleep.c | 75 -------------
> .../glibc-2.19-akaros/sysdeps/akaros/usleep.c | 25 +++++
> user/utest/sleep.c | 120
> +++++++++++++++++++++
> 6 files changed, 230 insertions(+), 75 deletions(-)
> create mode 100644
> tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/nanosleep.c
> delete mode 100644
> tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/sleep.c
> create mode 100644
> tools/compilers/gcc-glibc/glibc-2.19-akaros/sysdeps/akaros/usleep.c
> create mode 100644 user/utest/sleep.c
>
> --
> You received this message because you are subscribed to the Google Groups
> "Akaros" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To post to this group, send email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
>
--
You received this message because you are subscribed to the Google Groups
"Akaros" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
For more options, visit https://groups.google.com/d/optout.
