Menu
▾
▴
[Valgrind-developers] [valgrind] Bug 452274 memcheck crashes with Assertion 'sci->status.what == SsIdle' failed
|
From: Paul F. <pa...@so...> - 2022-04-12 21:51:35
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=3e7774ea5a5eda11d37bc4670aad8a3cb8a260fe commit 3e7774ea5a5eda11d37bc4670aad8a3cb8a260fe Author: Paul Floyd <pj...@wa...> Date: Tue Apr 12 23:34:41 2022 +0200 Bug 452274 memcheck crashes with Assertion 'sci->status.what == SsIdle' failed FreeBSD (and Darwin) use the carry flag for syscall syscall status. That means that in the assembler for do_syscall_for_client_WRK they have a call to LibVEX_GuestAMD64_put_rflag_c (amd64) or LibVEX_GuestX86_put_eflag_c (x86). These also call WRK functions. The problem is that do_syscall_for_client_WRK has carefully crafted labels correspinding to IP addresses. If a signal interrupts processdings, IP can be compared to these addresses so that VG_(fixup_guest_state_after_syscall_interrupted) can work out how to resume the syscall. But if IP is in the save carry flag functions, the address is not recognized and VG_(fixup_guest_state_after_syscall_interrupted) fails. The crash in the title happens because the interrupted syscall does not reset its status, and on the next syscall it is expected that the status be idle. To fix this I added global variables that get set to 1 just before calling the save carry flag functions, and cleared just after. VG_(fixup_guest_state_after_syscall_interrupted) can then check this and work out which section we are in and resume the syscall correctly. Also: Start a new NEWS section for 3.20 Add a regtest for this and also a similar one for Bug 445032 (x86-freebsd only, new subdir). I saw that this problem also probably exists with macOS, so I made the same changes there (not yet tested) Diff: --- .gitignore | 12 ++++++++ NEWS | 46 +++++++++++++++++++++++++++++ configure.ac | 1 + coregrind/m_syswrap/syscall-amd64-darwin.S | 5 ++++ coregrind/m_syswrap/syscall-amd64-freebsd.S | 40 +++++++++++++++---------- coregrind/m_syswrap/syscall-x86-darwin.S | 4 +++ coregrind/m_syswrap/syscall-x86-freebsd.S | 7 +++++ coregrind/m_syswrap/syswrap-main.c | 16 ++++++++++ none/tests/Makefile.am | 5 +++- none/tests/freebsd/452275.c | 33 +++++++++++++++++++++ none/tests/freebsd/452275.stderr.exp | 0 none/tests/freebsd/452275.vgtest | 3 ++ none/tests/freebsd/Makefile.am | 8 +++-- none/tests/freebsd/filter_452275 | 6 ++++ none/tests/x86-freebsd/445032.c | 31 +++++++++++++++++++ none/tests/x86-freebsd/445032.stderr.exp | 0 none/tests/x86-freebsd/445032.vgtest | 2 ++ none/tests/x86-freebsd/Makefile.am | 15 ++++++++++ none/tests/x86-freebsd/filter_stderr | 3 ++ 19 files changed, 217 insertions(+), 20 deletions(-) diff --git a/.gitignore b/.gitignore index 94ff17d299..78ea5819a6 100644 --- a/.gitignore +++ b/.gitignore @@ -1341,6 +1341,7 @@ /memcheck/tests/freebsd/eventfd2 /memcheck/tests/freebsd/realpathat /memcheck/tests/freebsd/scalar_13_plus +/memcheck/tests/freebsd/452275 # /memcheck/tests/amd64-freebsd /memcheck/tests/amd64-freebsd/*.stderr.diff @@ -2074,6 +2075,7 @@ /none/tests/freebsd/swapcontext /none/tests/freebsd/fexecve /none/tests/freebsd/hello_world +/none/tests/freebsd/452275 # /none/tests/x86/ /none/tests/x86/*.dSYM @@ -2186,6 +2188,16 @@ /none/tests/x86-solaris/coredump_single_thread_sse /none/tests/x86-solaris/syscalls +# /none/tests/x86-freebsd/ +/none/tests/x86-freebsd/*.stderr.diff +/none/tests/x86-freebsd/*.stderr.out +/none/tests/x86-freebsd/*.stdout.diff +/none/tests/x86-freebsd/*.stdout.out +/none/tests/x86-freebsd/.deps +/none/tests/x86-freebsd/Makefile +/none/tests/x86-freebsd/Makefile.in +/none/tests/x86-freebsd/445032 + # /perf/ /perf/*.dSYM /perf/.deps diff --git a/NEWS b/NEWS index dcce409261..752e5c0e7f 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,49 @@ +Release 3.20.0 (?? Oct 2022) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This release supports X86/Linux, AMD64/Linux, ARM32/Linux, ARM64/Linux, +PPC32/Linux, PPC64BE/Linux, PPC64LE/Linux, S390X/Linux, MIPS32/Linux, +MIPS64/Linux, ARM/Android, ARM64/Android, MIPS32/Android, X86/Android, +X86/Solaris, AMD64/Solaris, AMD64/MacOSX 10.12, X86/FreeBSD and +AMD64/FreeBSD. There is also preliminary support for X86/macOS 10.13, +AMD64/macOS 10.13 and nanoMIPS/Linux. + +* ==================== CORE CHANGES =================== + +* Fix Rust v0 name demangling. +* The Linux rseq syscall is now implemented as (silently) returning ENOSYS. +* Add FreeBSD syscall wrappers for __specialfd and __realpathat. +* Remove FreeBSD dependencies on COMPAT10, which fixes compatibility with HardenedBSD + +* ================== PLATFORM CHANGES ================= + +* arm64: + +* s390: + +* ppc64: + +* ==================== TOOL CHANGES =================== + + +* ==================== FIXED BUGS ==================== + +The following bugs have been fixed or resolved. Note that "n-i-bz" +stands for "not in bugzilla" -- that is, a bug that was reported to us +but never got a bugzilla entry. We encourage you to file bugs in +bugzilla (https://bugs.kde.org/enter_bug.cgi?product=valgrind) rather +than mailing the developers (or mailing lists) directly -- bugs that +are not entered into bugzilla tend to get forgotten about or ignored. + +452274 memcheck crashes with Assertion 'sci->status.what == SsIdle' failed + +To see details of a given bug, visit + https://bugs.kde.org/show_bug.cgi?id=XXXXXX +where XXXXXX is the bug number as listed above. + +(3.20.0.RC1: ?? )ct 2022) + + Release 3.19.0 (11 Apr 2022) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/configure.ac b/configure.ac index 7fd7ff18c6..5dcd4894a3 100755 --- a/configure.ac +++ b/configure.ac @@ -5357,6 +5357,7 @@ AC_CONFIG_FILES([ none/tests/x86-darwin/Makefile none/tests/amd64-solaris/Makefile none/tests/x86-solaris/Makefile + none/tests/x86-freebsd/Makefile exp-bbv/Makefile exp-bbv/tests/Makefile exp-bbv/tests/x86/Makefile diff --git a/coregrind/m_syswrap/syscall-amd64-darwin.S b/coregrind/m_syswrap/syscall-amd64-darwin.S index ed93a851eb..8a970fa088 100644 --- a/coregrind/m_syswrap/syscall-amd64-darwin.S +++ b/coregrind/m_syswrap/syscall-amd64-darwin.S @@ -248,6 +248,11 @@ ML_(blksys_complete_UNIX): .quad MK_L_SCCLASS_N(UNIX,3) ML_(blksys_committed_UNIX): .quad MK_L_SCCLASS_N(UNIX,4) ML_(blksys_finished_UNIX): .quad MK_L_SCCLASS_N(UNIX,5) +.data +globl ML_(blksys_saving_cflag) +ML_(blksys_saving_cflag): .quad 0 +.previous + #endif // defined(VGP_amd64_darwin) /* Let the linker know we don't need an executable stack */ diff --git a/coregrind/m_syswrap/syscall-amd64-freebsd.S b/coregrind/m_syswrap/syscall-amd64-freebsd.S index 414371967f..b435842c81 100644 --- a/coregrind/m_syswrap/syscall-amd64-freebsd.S +++ b/coregrind/m_syswrap/syscall-amd64-freebsd.S @@ -106,23 +106,23 @@ ML_(do_syscall_for_client_WRK): /* 6 register parameters */ movq -16(%rbp), %r11 /* r11 = VexGuestAMD64State * */ - movq OFFSET_amd64_RDI(%r11), %rdi - movq OFFSET_amd64_RSI(%r11), %rsi - movq OFFSET_amd64_RDX(%r11), %rdx - movq OFFSET_amd64_R10(%r11), %r10 - movq OFFSET_amd64_R8(%r11), %r8 - movq OFFSET_amd64_R9(%r11), %r9 - /* 2 stack parameters plus return address (ignored by syscall) */ - movq OFFSET_amd64_RSP(%r11), %r11 /* r11 = simulated RSP */ - movq 16(%r11), %rax - pushq %rax - movq 8(%r11), %rax - pushq %rax + movq OFFSET_amd64_RDI(%r11), %rdi + movq OFFSET_amd64_RSI(%r11), %rsi + movq OFFSET_amd64_RDX(%r11), %rdx + movq OFFSET_amd64_R10(%r11), %r10 + movq OFFSET_amd64_R8(%r11), %r8 + movq OFFSET_amd64_R9(%r11), %r9 + /* 2 stack parameters plus return address (ignored by syscall) */ + movq OFFSET_amd64_RSP(%r11), %r11 /* r11 = simulated RSP */ + movq 16(%r11), %rax + pushq %rax + movq 8(%r11), %rax + pushq %rax /* (fake) return address. */ - movq 0(%r11), %rax - pushq %rax - /* syscallno */ - movq -8(%rbp), %rax + movq 0(%r11), %rax + pushq %rax + /* syscallno */ + movq -8(%rbp), %rax /* If rip==2, then the syscall was either just about to start, or was interrupted and the kernel was @@ -144,7 +144,9 @@ ML_(do_syscall_for_client_WRK): movq %rax, %rdi /* arg1 = new flag */ movq %r11, %rsi /* arg2 = vex state */ addq $24, %rsp /* remove syscall parameters */ + movq $0x1, ML_(blksys_saving_cflag) call LibVEX_GuestAMD64_put_rflag_c + movq $0x0, ML_(blksys_saving_cflag) 4: /* Re-block signals. If eip is in [4,5), then the syscall is complete and we needn't worry about it. */ @@ -197,6 +199,12 @@ ML_(blksys_committed): .quad 4b ML_(blksys_finished): .quad 5b .previous + .data + .globl ML_(blksys_saving_cflag) + ML_(blksys_saving_cflag): .quad 0 + .previous + + #endif /* defined(VGP_amd64_freebsd) */ /* Let the linker know we don't need an executable stack */ diff --git a/coregrind/m_syswrap/syscall-x86-darwin.S b/coregrind/m_syswrap/syscall-x86-darwin.S index 9032422685..cbeadc52f8 100644 --- a/coregrind/m_syswrap/syscall-x86-darwin.S +++ b/coregrind/m_syswrap/syscall-x86-darwin.S @@ -246,6 +246,10 @@ ML_(blksys_complete_UNIX): .long MK_L_SCCLASS_N(UNIX,3) ML_(blksys_committed_UNIX): .long MK_L_SCCLASS_N(UNIX,4) ML_(blksys_finished_UNIX): .long MK_L_SCCLASS_N(UNIX,5) +.data +.globl ML_(blksys_saving_cflag) +ML_(blksys_saving_cflag): .long 0 +.previous #endif // defined(VGP_x86_darwin) /* Let the linker know we don't need an executable stack */ diff --git a/coregrind/m_syswrap/syscall-x86-freebsd.S b/coregrind/m_syswrap/syscall-x86-freebsd.S index 9437a34675..d4a6c90812 100644 --- a/coregrind/m_syswrap/syscall-x86-freebsd.S +++ b/coregrind/m_syswrap/syscall-x86-freebsd.S @@ -143,7 +143,9 @@ ML_(do_syscall_for_client_WRK): movl $0, 0(%esp) movb 12(%esp), %al movb %al, 0(%esp) + movl $0x1, ML_(blksys_saving_cflag) call LibVEX_GuestX86_put_eflag_c + movl $0x0, ML_(blksys_saving_cflag) addl $12, %esp 4: /* Re-block signals. If eip is in [4,5), then the syscall is @@ -191,6 +193,11 @@ ML_(blksys_committed): .long 4b ML_(blksys_finished): .long 5b .previous + .data + .globl ML_(blksys_saving_cflag) + ML_(blksys_saving_cflag): .long 0 + .previous + #endif // defined(VGP_x86_freebsd) /* Let the linker know we don't need an executable stack */ diff --git a/coregrind/m_syswrap/syswrap-main.c b/coregrind/m_syswrap/syswrap-main.c index 2982e27669..333fa09b1d 100644 --- a/coregrind/m_syswrap/syswrap-main.c +++ b/coregrind/m_syswrap/syswrap-main.c @@ -2564,6 +2564,9 @@ void VG_(post_syscall) (ThreadId tid) extern const Addr ML_(blksys_complete); extern const Addr ML_(blksys_committed); extern const Addr ML_(blksys_finished); +#if defined(VGO_freebsd) + extern const Addr ML_(blksys_saving_cflag); +#endif #elif defined(VGO_darwin) /* Darwin requires extra uglyness */ extern const Addr ML_(blksys_setup_MACH); @@ -2581,6 +2584,7 @@ void VG_(post_syscall) (ThreadId tid) extern const Addr ML_(blksys_complete_UNIX); extern const Addr ML_(blksys_committed_UNIX); extern const Addr ML_(blksys_finished_UNIX); + extern const Addr ML_(blksys_saving_cflag); #elif defined(VGO_solaris) extern const Addr ML_(blksys_setup); extern const Addr ML_(blksys_complete); @@ -3111,6 +3115,18 @@ VG_(fixup_guest_state_after_syscall_interrupted)( ThreadId tid, # error "Unknown OS" # endif +#if defined(VGO_freebsd) || defined(VGO_darwin) + if (outside_range) + { + if (ML_(blksys_saving_cflag)) + { + outside_range = False; + in_complete_to_committed = True; + } + } +#endif + + /* Figure out what the state of the syscall was by examining the (real) IP at the time of the signal, and act accordingly. */ if (outside_range) { diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am index 24a81b3be9..2ac09e6202 100644 --- a/none/tests/Makefile.am +++ b/none/tests/Makefile.am @@ -70,10 +70,13 @@ endif if VGCONF_PLATFORMS_INCLUDE_X86_SOLARIS SUBDIRS += x86-solaris endif +if VGCONF_PLATFORMS_INCLUDE_X86_FREEBSD +SUBDIRS += x86-freebsd +endif DIST_SUBDIRS = x86 amd64 ppc32 ppc64 arm arm64 s390x mips32 mips64 nanomips \ linux darwin solaris freebsd amd64-linux x86-linux amd64-darwin \ - x86-darwin amd64-solaris x86-solaris scripts . + x86-darwin amd64-solaris x86-solaris x86-freebsd scripts . dist_noinst_SCRIPTS = \ filter_cmdline0 \ diff --git a/none/tests/freebsd/452275.c b/none/tests/freebsd/452275.c new file mode 100644 index 0000000000..6207729f52 --- /dev/null +++ b/none/tests/freebsd/452275.c @@ -0,0 +1,33 @@ +#include <fcntl.h> +#include <signal.h> +#include <stdio.h> +#include <sys/time.h> +#include <unistd.h> + +volatile int ticks = 0; +struct itimerval timert; +struct sigaction timer_action; + + +void handle_vtalrm(int sig) { + ticks++; +} + +void setup_timer() { + timer_action.sa_handler = handle_vtalrm; + sigemptyset(&timer_action.sa_mask); + timer_action.sa_flags = SA_RESTART; + + sigaction(SIGVTALRM, &timer_action, NULL); + + timert.it_interval.tv_sec = timert.it_value.tv_sec = 0; + timert.it_interval.tv_usec = timert.it_value.tv_usec = 100; + setitimer(ITIMER_VIRTUAL, &timert, NULL); +} + +int main(int argc, char *argv[]) { + setup_timer(); + while (ticks < 100) { + write(2, " ", 1); + } +} diff --git a/none/tests/freebsd/452275.stderr.exp b/none/tests/freebsd/452275.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/none/tests/freebsd/452275.vgtest b/none/tests/freebsd/452275.vgtest new file mode 100644 index 0000000000..047d618bbe --- /dev/null +++ b/none/tests/freebsd/452275.vgtest @@ -0,0 +1,3 @@ +prog: 452275 +vgopts: -q +stderr_filter: filter_452275 diff --git a/none/tests/freebsd/Makefile.am b/none/tests/freebsd/Makefile.am index f3dabc9a9b..345990c627 100644 --- a/none/tests/freebsd/Makefile.am +++ b/none/tests/freebsd/Makefile.am @@ -1,7 +1,7 @@ include $(top_srcdir)/Makefile.tool-tests.am -dist_noinst_SCRIPTS = filter_stderr test.sh +dist_noinst_SCRIPTS = filter_stderr test.sh filter_452275 EXTRA_DIST = \ auxv.vgtest \ auxv.stderr.exp \ @@ -25,10 +25,12 @@ EXTRA_DIST = \ fexecve_script2.stdout.exp \ fexecve_script2.stderr.exp \ fexecve_txt.vgtest \ - fexecve_txt.stderr.exp + fexecve_txt.stderr.exp \ + 452275.vgtest \ + 452275.stderr.out check_PROGRAMS = \ - auxv osrel swapcontext hello_world fexecve + auxv osrel swapcontext hello_world fexecve 452275 AM_CFLAGS += $(AM_FLAG_M3264_PRI) AM_CXXFLAGS += $(AM_FLAG_M3264_PRI) diff --git a/none/tests/freebsd/filter_452275 b/none/tests/freebsd/filter_452275 new file mode 100755 index 0000000000..73bbad0e52 --- /dev/null +++ b/none/tests/freebsd/filter_452275 @@ -0,0 +1,6 @@ +#! /bin/sh + +./filter_stderr | + +sed 's/ //g' + diff --git a/none/tests/x86-freebsd/445032.c b/none/tests/x86-freebsd/445032.c new file mode 100644 index 0000000000..6fe828cb4d --- /dev/null +++ b/none/tests/x86-freebsd/445032.c @@ -0,0 +1,31 @@ +#include <fcntl.h> +#include <signal.h> +#include <stdio.h> +#include <sys/time.h> +#include <unistd.h> + +volatile int ticks = 0; +struct itimerval timert; +struct sigaction timer_action; + + +void handle_vtalrm(int sig) { + ticks++; +} + +void setup_timer() { + timer_action.sa_handler = handle_vtalrm; + sigemptyset(&timer_action.sa_mask); + timer_action.sa_flags = SA_RESTART; + + sigaction(SIGVTALRM, &timer_action, NULL); + + timert.it_interval.tv_sec = timert.it_value.tv_sec = 0; + timert.it_interval.tv_usec = timert.it_value.tv_usec = 100; + setitimer(ITIMER_VIRTUAL, &timert, NULL); +} + +int main(int argc, char *argv[]) { + setup_timer(); + sleep(5); +} diff --git a/none/tests/x86-freebsd/445032.stderr.exp b/none/tests/x86-freebsd/445032.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/none/tests/x86-freebsd/445032.vgtest b/none/tests/x86-freebsd/445032.vgtest new file mode 100644 index 0000000000..06fb233915 --- /dev/null +++ b/none/tests/x86-freebsd/445032.vgtest @@ -0,0 +1,2 @@ +prog: 445032 +vgopts: -q diff --git a/none/tests/x86-freebsd/Makefile.am b/none/tests/x86-freebsd/Makefile.am new file mode 100644 index 0000000000..5dc444b6d4 --- /dev/null +++ b/none/tests/x86-freebsd/Makefile.am @@ -0,0 +1,15 @@ +include $(top_srcdir)/Makefile.tool-tests.am + +dist_noinst_SCRIPTS = filter_stderr + +EXTRA_DIST = \ + 445032.vgtest \ + 445032.stderr.exp + +check_PROGRAMS = \ + 445032 + +# Linux also adds $(FLAG_MMMX) $(FLAG_MSSE) to the first two +AM_CFLAGS += @FLAG_M32@ +AM_CXXFLAGS += @FLAG_M32@ +AM_CCASFLAGS += @FLAG_M32@ diff --git a/none/tests/x86-freebsd/filter_stderr b/none/tests/x86-freebsd/filter_stderr new file mode 100755 index 0000000000..a778e971fc --- /dev/null +++ b/none/tests/x86-freebsd/filter_stderr @@ -0,0 +1,3 @@ +#! /bin/sh + +../filter_stderr "$@" |
