Menu
▾
▴
valgrind-developers
|
From: Paul F. <pa...@so...> - 2024-10-08 11:21:36
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=fc20240c84b5f61e4bf590d69eb50475fa2c119c commit fc20240c84b5f61e4bf590d69eb50475fa2c119c Author: Paul Floyd <pj...@wa...> Date: Tue Oct 8 13:19:08 2024 +0200 sigaltstack syscall: improve error messages Previously the same message was generated for the 3 members of stack_t. Also on FreeBSD I get a Conditional jump error with this syscall but not on all platforms, so I've added a suppression. Diff: --- coregrind/m_syswrap/syswrap-generic.c | 6 +++--- memcheck/tests/freebsd/Makefile.am | 1 + memcheck/tests/freebsd/scalar.c | 8 +------- memcheck/tests/freebsd/scalar.stderr.exp | 30 ++++++------------------------ memcheck/tests/freebsd/scalar.supp | 11 +++++++++++ memcheck/tests/freebsd/scalar.vgtest | 2 +- 6 files changed, 23 insertions(+), 35 deletions(-) diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c index 9093b831d6..5434a68369 100644 --- a/coregrind/m_syswrap/syswrap-generic.c +++ b/coregrind/m_syswrap/syswrap-generic.c @@ -5033,9 +5033,9 @@ PRE(sys_sigaltstack) const vki_stack_t *, ss, vki_stack_t *, oss); if (ARG1 != 0) { const vki_stack_t *ss = (vki_stack_t *)(Addr)ARG1; - PRE_MEM_READ( "sigaltstack(ss)", (Addr)&ss->ss_sp, sizeof(ss->ss_sp) ); - PRE_MEM_READ( "sigaltstack(ss)", (Addr)&ss->ss_flags, sizeof(ss->ss_flags) ); - PRE_MEM_READ( "sigaltstack(ss)", (Addr)&ss->ss_size, sizeof(ss->ss_size) ); + PRE_MEM_READ( "sigaltstack(ss->ss_sp)", (Addr)&ss->ss_sp, sizeof(ss->ss_sp) ); + PRE_MEM_READ( "sigaltstack(ss->ss_size)", (Addr)&ss->ss_size, sizeof(ss->ss_size) ); + PRE_MEM_READ( "sigaltstack(ss->ss_flags)", (Addr)&ss->ss_flags, sizeof(ss->ss_flags) ); } if (ARG2 != 0) { PRE_MEM_WRITE( "sigaltstack(oss)", ARG2, sizeof(vki_stack_t) ); diff --git a/memcheck/tests/freebsd/Makefile.am b/memcheck/tests/freebsd/Makefile.am index ae4b1aa531..8329a4377c 100644 --- a/memcheck/tests/freebsd/Makefile.am +++ b/memcheck/tests/freebsd/Makefile.am @@ -91,6 +91,7 @@ EXTRA_DIST = \ scalar.h scalar.vgtest \ scalar.stderr.exp \ scalar.stderr.exp-x86 \ + scalar.supp \ scalar_abort2.vgtest \ scalar_13_plus.vgtest \ scalar_13_plus.stderr.exp \ diff --git a/memcheck/tests/freebsd/scalar.c b/memcheck/tests/freebsd/scalar.c index ac8edd42aa..76a9f651cc 100644 --- a/memcheck/tests/freebsd/scalar.c +++ b/memcheck/tests/freebsd/scalar.c @@ -257,14 +257,8 @@ int main(void) struct our_sigaltstack oss; VALGRIND_MAKE_MEM_NOACCESS(&ss, sizeof(struct our_sigaltstack)); VALGRIND_MAKE_MEM_NOACCESS(&oss, sizeof(struct our_sigaltstack)); - GO(SYS_sigaltstack, "0s 2m"); + GO(SYS_sigaltstack, "2s 4m"); SY(SYS_sigaltstack, x0+&ss, x0+&oss); FAIL; - - GO(SYS_sigaltstack, "2s 0m"); - SY(SYS_sigaltstack, x0, x0); SUCC; - - GO(SYS_sigaltstack, "2s 2m"); - SY(SYS_sigaltstack, x0+1, x0+1); FAIL; } /* SYS_ioctl 54 */ diff --git a/memcheck/tests/freebsd/scalar.stderr.exp b/memcheck/tests/freebsd/scalar.stderr.exp index f8209228e2..681bb8f2ad 100644 --- a/memcheck/tests/freebsd/scalar.stderr.exp +++ b/memcheck/tests/freebsd/scalar.stderr.exp @@ -440,7 +440,7 @@ Syscall param acct(filename) points to unaddressable byte(s) Address 0x........ is not stack'd, malloc'd or (recently) free'd --------------------------------------------------------- - 53: SYS_sigaltstack 0s 2m + 53: SYS_sigaltstack 2s 4m --------------------------------------------------------- Syscall param sigaltstack(ss) contains uninitialised byte(s) ... @@ -448,40 +448,22 @@ Syscall param sigaltstack(ss) contains uninitialised byte(s) Syscall param sigaltstack(oss) contains uninitialised byte(s) ... -Syscall param sigaltstack(ss) points to unaddressable byte(s) +Syscall param sigaltstack(ss->ss_sp) points to unaddressable byte(s) ... Address 0x........ is on thread 1's stack -Syscall param sigaltstack(oss) points to unaddressable byte(s) +Syscall param sigaltstack(ss->ss_size) points to unaddressable byte(s) ... Address 0x........ is on thread 1's stack ---------------------------------------------------------- - 53: SYS_sigaltstack 2s 0m ---------------------------------------------------------- -Syscall param sigaltstack(ss) contains uninitialised byte(s) - ... - - -Syscall param sigaltstack(oss) contains uninitialised byte(s) - ... - ---------------------------------------------------------- - 53: SYS_sigaltstack 2s 2m ---------------------------------------------------------- -Syscall param sigaltstack(ss) contains uninitialised byte(s) - ... - -Syscall param sigaltstack(oss) contains uninitialised byte(s) +Syscall param sigaltstack(ss->ss_flags) points to unaddressable byte(s) ... + Address 0x........ is on thread 1's stack -Syscall param sigaltstack(ss) points to unaddressable byte(s) - ... - Address 0x........ is not stack'd, malloc'd or (recently) free'd Syscall param sigaltstack(oss) points to unaddressable byte(s) ... - Address 0x........ is not stack'd, malloc'd or (recently) free'd + Address 0x........ is on thread 1's stack --------------------------------------------------------- 54: SYS_ioctl 3s 1m diff --git a/memcheck/tests/freebsd/scalar.supp b/memcheck/tests/freebsd/scalar.supp new file mode 100644 index 0000000000..f6b15a33ea --- /dev/null +++ b/memcheck/tests/freebsd/scalar.supp @@ -0,0 +1,11 @@ +# not sure what causes this exactly# on x86 and amd64 (but not arm64) +# there is an conditional uninit read +# there are several if statements that use the two pointers to stack +# structures that probably generate the error + +{ + internal uninitilized read + Memcheck:Cond + fun:syscall + fun:main +} diff --git a/memcheck/tests/freebsd/scalar.vgtest b/memcheck/tests/freebsd/scalar.vgtest index 170c1576e1..2f433038c7 100644 --- a/memcheck/tests/freebsd/scalar.vgtest +++ b/memcheck/tests/freebsd/scalar.vgtest @@ -1,5 +1,5 @@ prog: scalar -vgopts: -q --error-limit=no +vgopts: -q --error-limit=no --suppressions=scalar.supp stderr_filter: filter_scalar # Remove all frames from the stack trace except the first one. # This is important because syscall() function on x86 isn't ABI conformant |
|
From: Mark W. <ma...@kl...> - 2024-10-08 12:49:30
|
Hi Paul, On Tue, 2024-10-08 at 11:21 +0000, Paul Floyd wrote: > https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=fc20240c84b5f61e4bf590d69eb50475fa2c119c > > commit fc20240c84b5f61e4bf590d69eb50475fa2c119c > Author: Paul Floyd <pj...@wa...> > Date: Tue Oct 8 13:19:08 2024 +0200 > > sigaltstack syscall: improve error messages > > Previously the same message was generated for the 3 members of > stack_t. > [...] > diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c > index 9093b831d6..5434a68369 100644 > --- a/coregrind/m_syswrap/syswrap-generic.c > +++ b/coregrind/m_syswrap/syswrap-generic.c > @@ -5033,9 +5033,9 @@ PRE(sys_sigaltstack) > const vki_stack_t *, ss, vki_stack_t *, oss); > if (ARG1 != 0) { > const vki_stack_t *ss = (vki_stack_t *)(Addr)ARG1; > - PRE_MEM_READ( "sigaltstack(ss)", (Addr)&ss->ss_sp, sizeof(ss->ss_sp) ); > - PRE_MEM_READ( "sigaltstack(ss)", (Addr)&ss->ss_flags, sizeof(ss->ss_flags) ); > - PRE_MEM_READ( "sigaltstack(ss)", (Addr)&ss->ss_size, sizeof(ss->ss_size) ); > + PRE_MEM_READ( "sigaltstack(ss->ss_sp)", (Addr)&ss->ss_sp, sizeof(ss->ss_sp) ); > + PRE_MEM_READ( "sigaltstack(ss->ss_size)", (Addr)&ss->ss_size, sizeof(ss->ss_size) ); > + PRE_MEM_READ( "sigaltstack(ss->ss_flags)", (Addr)&ss->ss_flags, sizeof(ss->ss_flags) ); > } > if (ARG2 != 0) { > PRE_MEM_WRITE( "sigaltstack(oss)", ARG2, sizeof(vki_stack_t) ); This is clearly a bug fix and I agree with the new names. Thanks. But you might want to explicitly mention this in NEWS since it does change how suppressions work for sigaltstack. Previously a suppression like { my-weird-sigaltstack-call Memcheck:Param sigaltstack(ss) fun:sigaltstack fun:main } would work, now that needs to be rewritten to refer to one of the new messages. Most likely nobody did the above, but it is technically a user visible change, so would be good to explicitly mention. Also I accidentially changed some syscall Param messages in the past without realizing it is a user visible change, so I just mention it to remind myself to be careful here. Cheers, Mark |
|
From: Paul F. <pj...@wa...> - 2024-10-08 19:52:06
|
On 08-10-24 12:49, Mark Wielaard wrote: > This is clearly a bug fix and I agree with the new names. Thanks. > > But you might want to explicitly mention this in NEWS since it does > change how suppressions work for sigaltstack. OK, I added it as an n-i-bz entry. A+ Paul |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X