Paul Floyd [Sun, 3 Nov 2024 19:42:43 +0000 (20:42 +0100)]
Bug 494327 - Crash when running Helgrind built with #define TRACE_PTH_FNS 1
Use write() rather than 'fprintf()' for the TRACE_PTH_FNS blocks for
pthread_mutex_lock and pthread_mutex_lock. Mixing FILE and fd isn't
great, but this is to stderr which gets flushed on every line, and
it is only for developer builds that modify that TRACE_PTH_FNS macro.
Report track-fd errors for fd used which was not opened or already closed
Add (optional) pathname, description, where_closed and where_opened
fields to struct FdBadUse. Print those fields when set in fd_pp_Error.
Add a new function ML_(find_OpenFd) that provides a recorded OpenFd
given an fd (or NULL when the fd was never recorded).
In ML_(fd_allowed) when using a file descriptor use ML_(find_OpenFd)
to see if the fd was ever created, if not create an "was never
created" FdBadUse error. If it was created, but already closed create
an "was closed already", filling in as much details as we can.
Add none/tests/use_after_close.vgtest to test, already closed, never
created, invalid, double (double) close and invalid close issues.
Adjust error message in none/tests/fdbaduse.stderr.exp.
Florian Krohm [Tue, 29 Oct 2024 15:24:31 +0000 (16:24 +0100)]
Bug 493959 - s390x: Fix regtest failure for op00 with /bin/dash
On different machines /bin/sh may be impersonated by different shells, and
those behave differently as to whether they write "Illegal instruction ..."
to stderr. While newer versions of bash do not, dash does.
For the op00 test case this means that an additional line may be written
to `op00.stderr.out', depending on which shell is being used. Hence
adding "Illegal instruction ..." as an expected line to `op00.stderr.exp'
wouldn't work on all systems.
Instead, fix this issue by adding the case of "illegal instruction" to the
general filtering logic in filter_stderr_basic.in, where various other
messages of this kind are already filtered out.
Carl Love [Wed, 23 Oct 2024 23:25:23 +0000 (18:25 -0500)]
PowerPC, dcbf instruction
ISA 2.7 and ISA 3.0 "accepts" L = 0 to 3, ISA 3.1 "accepts: L = 0 to 7.
Removed the L field check so valgrind will match the real hardware. For
the purposes of Valgrind the dcbf instruction is a NOP anyway so it will
not change the behavior of Valgrind.
Paul Floyd [Sat, 12 Oct 2024 07:10:21 +0000 (09:10 +0200)]
FreeBSD helgrind: temporary (?) fix for Bug 494337
FreeBSD 15 has added a pthread lock to exit() to ensure that atexit handling
is thread safe. Unfortunately that lock gets leaked which messes up just about
all of the Helgrind tests.
Supression won't work as the callstack is the same for both genuine leaks
and this deliberate leak.
This change simply turns off the check for FreeBSD >= 15.
I see two possible proper fixes. One would be to allow one lock on exit.
The problem with that is that we will need to tell apart a clean exit
(1 lock allowed) and any kind of abort that doesn't call exit (no locks
allowed). That's going to be tricky as the Helgrind check is done before
we get back to core and know whether it is an abort or a clean exit.
The other thing would be to hack the lock counting. If we can detect that
it's a pthread_mutex_lock called from exit() then we could ignore that for
counting purposes. That would mean a possibly significant overhead for
each call to pthread_mutex_lock on FreeBSD.
Introduce a new FdBadFd type with associated extra info struct.
Which for now just holds the fd number (no path or description).
fd_pp_Error and fd_update_extra have been updated to handle the
new type and produce xml when requested.
Rename showing_core_errors to showing_core_warning
(returns yes when the tools wants to show core errors,
-q isn't given and we aren't producing xml).
In ML_(fd_allowed) we now call VG_(maybe_record_error) to
generate a real error (that can be suppressed and shows up
in the xml output with full execution backtrace). For now
we also produce the legacy warnings when --track-fds=yes
isn't given.
Add none/tests/fdbaduse.vgtest to test the new FdBadUse
core error.
This is the first part of reporting bad fd usage errors.
We are also tracking already closed file descriptors which
should also produce errors like these. The current bad file
descriptors are just those that are negative or above the
current file limit of the process.
Andreas Arnez [Wed, 9 Oct 2024 15:10:08 +0000 (17:10 +0200)]
s390x: Add missing early-clobber to GET_STARTREGS
The inline assembly for GET_STARTREGS in m_libcassert.c writes to its
output before using the input argument. But since the compiler doesn't
know this, it is allowed to allocate the same register for both, causing
problems. This has been seen when compiling Valgrind with -O0, after
which memcheck/tests/leak-autofreepool-5 fails due to SIGSEGV.
Fix this by declaring the output as early-clobber, so the compiler knows
about the restriction.
Andreas Arnez [Wed, 9 Oct 2024 15:10:08 +0000 (17:10 +0200)]
Bug 493970 - s390x: Drop saving/restoring FPC upon helper call
Saving the FPC before each helper call and restoring it afterwards creates
unnecessary overhead, and it may also not be desirable.
Drop it. Also remove the functions in host_s390_defs.c responsible for
emitting LFPC and STFPC instructions. And since this frees up the FPC
save slot on the stack, adjust the stack layout accordingly.
Andreas Arnez [Wed, 9 Oct 2024 15:10:08 +0000 (17:10 +0200)]
Fix some issues with GSL for `make auxchecks'
When trying to reproduce Bug 423908, I ran into some trouble performing
`make auxchecks', due to problems in GSL:
Newer compilers complain about incompatible pointer types in argument
passing between
unsigned int *
and
size_t *
This affects the fifth argument of function gsl_eigen_jacobi() in
eigen/jacobi.c. Fix this by passing the right pointer type at invocation.
Also, the configure checks for IEEE comparisons and for IEEE denormalized
values don't work as intended, because they call exit() without declaring
it and thus fail independently from those features being supported. Fix
this by using `return' instead.
Paul Floyd [Tue, 8 Oct 2024 19:03:17 +0000 (21:03 +0200)]
macOS regtest: give up trying to build bug492210.c
macOS has to make everything difficult. Global names need
to be decorated with an underscore prefix. And you can't
just read from global variables, you have to do something
like a rip-relative lea.
Paul Floyd [Mon, 7 Oct 2024 05:34:59 +0000 (07:34 +0200)]
FreeBSD regtest: add FAKE macros for scalar
FreeBSD 15 removed the never-implemented sbrk syscall.
Arm64 also has a few missing syscalls (backward compat ones
that predate the arm64 port). Rather than having an ever
increasing number of expecteds the aim is to use these
FAKE macros. It's a bit fiddly to get the matching text.
Mark Wielaard [Sun, 22 Sep 2024 21:24:34 +0000 (23:24 +0200)]
Implement /proc/self/exe readlink[at] fallback in POST handler
Calling the readlink[at] syscall directly from the PRE handler defeats
the FUSE_COMPATIBLE_MAY_BLOCK (SfMayBlock) flag. Add a POST handler
that only explicitly calls the readlink[at] handler for the
/proc/self/exe fallback (this should be fine unless /proc is also
implemented as fuse in this process).
Adjust readlink[at] GENX_ and LINX_ syswrap macros to GENXY and LINXY.
Mark Wielaard [Sat, 21 Sep 2024 20:27:24 +0000 (22:27 +0200)]
Add missing FUSE_COMPATIBLE_MAY_BLOCKs
Various syscalls (in particular "at" variants) PRE handlers were
missing a FUSE_COMPATIBLE_MAY_BLOCK statement.
Add it to the generic PRE handlers of access and statfs64. And the
linux PRE handlers of mknodat, fchownat, futimesat, utimensat,
utimensat_time64, renameat, renameat2, readlinkat, fchmodat,
fchmodat2, faccessat and faccessat2.
Mark Wielaard [Sat, 31 Aug 2024 17:47:27 +0000 (19:47 +0200)]
Implement stable variant of sync_file ioctls
We implemented an old staging android variant of the sync_file
ioctls. But the data structures and ioctl numbers changed when these
were upstreamed in the table linux kernel.
This implements the SYNC_IOC_MERGE, SYNC_IOC_FILE_INFO and
SYNC_IOC_SET_DEADLINE ioctls. And makes sure to record the new file
descriptor created by SYNC_IOC_MERGE.
Andreas Arnez [Tue, 1 Oct 2024 11:12:44 +0000 (13:12 +0200)]
s390x: Add bug 440180 to NEWS
As reported in Bug 440180, the s390x disassembler could run into an
assertion failure due to the maximum mnemonic length being exceeded. This
was fixed with commit 67a2bb759a7c9c76fd6aa142bdb6fe342a5998e2.
Paul Floyd [Mon, 30 Sep 2024 19:09:15 +0000 (21:09 +0200)]
FreeBSD regtest: remove unneeded expected file
Previously I had #ifdef'd the freebsd7 compatibility syscalls
in the scalar test for all platforms when adding the arm64
port. They are now back for amd64 and x86 so the previous
expected also covers FreeBSD >= 14.0 and the extra expected
file is now superfluous and removed.
Paul Floyd [Sun, 29 Sep 2024 08:02:33 +0000 (10:02 +0200)]
FreeBSD procctl syscall: change arg name in error messages
The man page was inconsistent in the use of 'data' or 'arg'
for the fourth argument. I chose to use 'arg'. The manpage
has now been cleaned up and uses 'data'. So I'm switching to
use the same name.
Paul Floyd [Sat, 28 Sep 2024 06:20:25 +0000 (08:20 +0200)]
Compiler warning in ML_(check_elf_and_get_rw_loads)
GCC 12.2 complains that
previous_rw_a_phdr.p_vaddr + previous_rw_a_phdr.p_filesz
may be using p_filesz uninitialized
That's only possible if ML_(img_get) somehow fails to read all
of a program header such that p_memsz is greater than 0 but
p_filesz remains uninitialized. Hardly likely since p_memsz
comes after p_filesz in the structure.
Paul Floyd [Fri, 27 Sep 2024 20:18:24 +0000 (22:18 +0200)]
FreeBSD: remove code for FreeBSD 10
FreeBSD 10 was never really tested - fully working FreeBSD support
arrived around the time of FreeBSD 11.3 and 12.1. FreeBSD had
already been EOL around 2 years by then.
Paul Floyd [Sun, 15 Sep 2024 07:52:56 +0000 (09:52 +0200)]
Bug 492210 - False positive on x86/amd64 with ZF taken directly from addition
Also adds similar checks for short and char equivalents to the
original int reproducer.
Initial fix provided by
Alexander Monakov <amonakov@gmail.com>
Two versions of the testcase, one with default options and one with
-expensive-definedness-checks=yes because the byte operations
subb and addb need the flag turned on explicitly.
Paul Floyd [Sat, 14 Sep 2024 18:56:54 +0000 (20:56 +0200)]
FreeBSD: add file descriptor tracking for _umtx_op
UMTX_OP_SHM with a sub request of UMTX_SHM_CREAT creates
an anonymous shared memory object and returns a file
descriptor. This fd is now tracked when required.
Andreas Arnez [Tue, 10 Sep 2024 16:38:49 +0000 (18:38 +0200)]
s390x: Add MSA support
Handle instructions that were added to z/Architecture with the
message-security assist (MSA) facility or with one of its extensions up to
MSA extension 9:
km -- ``cipher message''
kmc -- ``cipher message with chaining''
kimd -- ``compute intermediate message digest''
klmd -- ``compute last message digest''
kmac -- ``compute message authentication code''
kmf -- ``cipher message with cipher feedback''
kmctr -- ``cipher message with counter''
kmo -- ``cipher message with output feedback''
pcc -- ``perform cryptographic computation''
kma -- ``cipher message with authentication''
kdsa -- ``compute digital signature authentication''
Each of these instructions has multiple functions. Support all functions
described by MSA levels up to extension 9. Handle the instructions as
"extensions" and essentially forward them to the instructions themselves,
as long as they are available on the host.
will not be handled by this change, since it is privileged and should not
occur in user-space programs.
The MSA facilities are typically used by cryptographic libraries like
OpenSSL or openCryptoki. So far Valgrind suppresses the facility bits
indicating any MSA support, which causes such libraries to revert to a
software implementation.
This change enables running cryptographic applications under Valgrind
without reverting to an alternate code path.
Miao Wang [Mon, 26 Aug 2024 14:08:43 +0000 (22:08 +0800)]
sys_statx: support for statx(fd, NULL, AT_EMPTY_PATH)
statx(fd, NULL, AT_EMPTY_PATH) is supported since Linux 6.11 and this
patch adds the support to valgrind, so that it won't complain when
NULL is used as |filename| and |flags| includes AT_EMPTY_PATH.
Ref: commit 0ef625bba6fb ("vfs: support statx(..., NULL, AT_EMPTY_PATH, ...)")
Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
valgrind testing: extend vg_regtest to emit automake-style .trs/.log files
Extend vg_regtest to produce automake-style log files for each vgtest
case, so that developers and testsuite archiving/analysis tools such
as bunsen can examine passing as well as non-passing test outputs in
detail. The build-tree test-suite-overall.log file holds all the key
information about tests, especially failures.
Andreas Arnez [Mon, 19 Aug 2024 13:22:40 +0000 (15:22 +0200)]
s390x: Fix PC calculations with EX/EXRL
When executing under EX or EXRL, some instructions yield wrong results
under Valgrind. This affects
* PC-relative instructions such as LARL or BRC
* instructions that set a link register, such as BASR
The issue is caused by confusions about the various instruction addresses
involved. When executing an instruction under EX or EXRL, the following
addresses are relevant:
(1) The address of the execute instruction (guest_IA_curr_instr). This is
needed when restarting the instruction or iterating over it.
(2) The address following the execute instruction (guest_IA_next_instr).
This is what a link register needs to be set to.
(3) The address of the target instruction. This is the base for relative
addressing.
The latter isn't handled at all when translating for EX/EXRL. And the
instructions that set a link register don't use guest_IA_next_instr, but
add their own instruction length to guest_IA_curr_instr. This is wrong
whenever the target instruction and the EX/EXRL instruction have different
lengths.
Fix all this and enhance the test cases accordingly. The updated test
cases fail before this patch and succeed afterwards.
Andreas Arnez [Tue, 13 Aug 2024 11:52:07 +0000 (13:52 +0200)]
s390x: Fix performance issue with EXRL
Valgrind can currently run into a situation where a code block containing
EXRL is re-translated over and over, potentially causing extreme
slow-down. Such a slow-down has been observed when running the following
command under Valgrind:
z/Architecture has the "execute" instructions EX and EXRL. Valgrind
handles EX by translating it at least twice. The first translation just
copies the target instruction to the variable `last_execute_target' and
triggers a "restart", invalidating the current BB and creating a new BB
that starts with EX. The second translation contains the IR for the
instruction in `last_execute_target', but first checks if this still
matches the instruction to be executed. If not, it initiates a "restart",
as above. For EXRL there is a shortcut that sets `last_execute_target'
without going through the first translation.
Now the combination of two issues in the current implemenation typically
leads to an EXRL being translated every time:
(1) An EXRL can appear in the middle of a BB. If so, a "restart" will
discard everything in the BB up to this point. And when getting back
to the same instructions, everything will be re-translated again.
(2) After commit 7e9113cb7a249e0fae2a365462c6b016 (handling Bug 405403),
the shortcut in s390_irgen_EXRL() only fills 6 instead of 8 bytes into
`last_execute_target', while the check still compares this to 8 bytes
from the target location. Thus the check usually fails, triggering a
"restart" of EXRL.
The first issue does not apply to EX, because there was already logic for
terminating a BB before an EX instruction. Just extend that logic and
treat EXRL the same way.
The second issue is caused by the discrepancy of reading 6 versus 8 bytes
and comparing these two. But in fact, reading 6 or 8 bytes are both
incorrect. Only the bytes that belong to the instruction should be read
and compared. The instruction length can be determined from the first
byte `b' at the target location (2 bytes if b < 0x40, 4 bytes if b < 0xc0,
and 6 bytes otherwise), so do this.
Andreas Arnez [Thu, 8 Aug 2024 12:56:50 +0000 (14:56 +0200)]
s390x: Fix disassembly of locfh/locfhr, update S390_MAX_MNEMONIC_LEN
The length of the "longest mnemonic" for the s390x disassembler is
currently defined in s390_defs.h to be 8 characters, where in fact it
should be 9. Update the constant to reflect that.
Also fix the disassembly of the instructions locfh and locfhr, changing
them from their current wrong representations `locgh' and `locghr'.
Sam James [Mon, 22 Jul 2024 11:26:39 +0000 (12:26 +0100)]
configure: drop -flto-partition=one
For me, -flto-partition=one takes ~35m to build + test, while the default
(which is 'balanced') takes ~5m.
The reason that -flto-partition=one is slower is because it disables all
of gcc's LTO parallelisation. This can produce better code, at the cost
of (far) more expensive build times. If users want that, they can still
pass it in their *FLAGS, but I don't think it's a suitable default.