-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Opt out of inline assembly in libc/sysdeps/linux/*/bits/select.h #34
base: klee_0_9_29
Are you sure you want to change the base?
Conversation
Thanks, @jordr. This patch is clearly fine, but nevertheless I wouldn't like to merge any patches before the CI is up again. Could you or @MartinNowack perhaps add here the scripts for GitHub Actions? We don't need all the targets, even the standard one with uclibc + posix would be enough. |
@ccadar I will take care of the script - wanted to try something out in the first place to simplify the build script. |
@MartinNowack great, thanks! |
@jordr Can you rebase this PR so we can merge it. Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jordr it would be great for this to be accompanied by a test in KLEE which currently would fail but would pass after this patch.
@ccadar sounds good, but this PR should be merged before the KLEE PR, otherwise the KLEE PR will have failing tests and merging blocked, right? |
No matter how I twist it the klee-uclibc changes seem to have no impact. When I run this test: #include <sys/select.h>
int main() {
fd_set fds;
FD_ZERO(&fds);
FD_SET(0, &fds);
} I get
Looking into the LLVM bytecode, there is this: ; Function Attrs: nounwind uwtable
define i32 @__klee_posix_wrapped_main() #0 !dbg !891 {
entry:
%fds = alloca %struct.fd_set.0, align 8
%__d0 = alloca i32, align 4
%__d1 = alloca i32, align 4
call void @llvm.dbg.declare(metadata %struct.fd_set.0* %fds, metadata !7852, metadata !2637), !dbg !7860
call void @llvm.dbg.declare(metadata i32* %__d0, metadata !7861, metadata !2637), !dbg !7863
call void @llvm.dbg.declare(metadata i32* %__d1, metadata !7864, metadata !2637), !dbg !7863
%__fds_bits = getelementptr inbounds %struct.fd_set.0, %struct.fd_set.0* %fds, i32 0, i32 0, !dbg !7865
%arrayidx = getelementptr inbounds [16 x i64], [16 x i64]* %__fds_bits, i64 0, i64 0, !dbg !7865
%0 = call { i64, i64* } asm sideeffect "cld; rep; stosq", "={cx},={di},{ax},0,1,~{memory},~{dirflag},~{fpsr},~{flags}"(i32 0, i64 16, i64* %arrayidx) #8, !dbg !7865, !srcloc !7867
%asmresult = extractvalue { i64, i64* } %0, 0, !dbg !7865
%asmresult1 = extractvalue { i64, i64* } %0, 1, !dbg !7865
%1 = trunc i64 %asmresult to i32, !dbg !7865
store i32 %1, i32* %__d0, align 4, !dbg !7865
%2 = ptrtoint i64* %asmresult1 to i64, !dbg !7865
%3 = trunc i64 %2 to i32, !dbg !7865
store i32 %3, i32* %__d1, align 4, !dbg !7865
ret i32 0, !dbg !7868
} and when I look at the debug locations, it references typedef __fd_mask fd_mask;
# 101 "/usr/include/x86_64-linux-gnu/sys/select.h" 3 4
extern int select (int __nfds, fd_set *__restrict __readfds,
fd_set *__restrict __writefds,
fd_set *__restrict __exceptfds,
struct timeval *__restrict __timeout);
# 113 "/usr/include/x86_64-linux-gnu/sys/select.h" 3 4
extern int pselect (int __nfds, fd_set *__restrict __readfds,
fd_set *__restrict __writefds,
fd_set *__restrict __exceptfds,
const struct timespec *__restrict __timeout,
const __sigset_t *__restrict __sigmask);
# 2 "inlineasm.c" 2
int main() { fd_set fds;
do { int __d0, __d1; __asm__ __volatile__ ("cld; rep; " "stosq" : "=c" (__d0), "=D" (__d1) : "a" (0), "0" (sizeof (fd_set) / sizeof (__fd_mask)), "1" (&((&fds)->__fds_bits)[0]) : "memory"); } while (0);
(0, &fds);
} Am I doing something wrong? It seems like there is already inline assembly when KLEE is run, so there is no way the klee-uclibc changes have an impact, right? |
Right, by default we're using the system headers. I need to look closer at this, but that's why I wanted a test ;) |
@ccadar This test file passes for me if you run it in the KLEE test suite, but I hardcoded the klee-uclibc directory path. // RUN: %clang %s -emit-llvm -O0 -c -o %t1.bc -I/home/jruiz/code/klee-uclibc-3.8.0/include/
// RUN: %clang %s -emit-llvm -O0 -c -o %t2.bc
// RUN: rm -rf %t1.klee-out %t2.klee-out
// RUN: %klee --output-dir=%t1.klee-out %t1.bc > %t1.out 2>&1
// RUN: %klee --output-dir=%t2.klee-out %t2.bc > %t2.out 2>&1
// RUN: test ! -f %t1.klee-out/test000001.exec.err
// RUN: test -f %t2.klee-out/test000001.exec.err
#include <sys/select.h>
int main() {
fd_set fds;
FD_ZERO(&fds);
FD_SET(0, &fds);
} |
FD_ZERO
andFD_SET
use inline assembly that is not supported by KLEE. With this patch, we force the fallback path to C code.