Mark Wielaard [Thu, 17 Aug 2023 13:40:30 +0000 (15:40 +0200)]
gdb --multi mode stdout redirecting to stderr
When in stdio mode (talking to gdb through stdin/stdout, not
through a socket), redirect stdout to stderr and close stdin
for the inferior. That way at least some output can be seen,
but there will be no input.
This is workaround till we have real terminal handling.
* coregrind/vgdb.c (main): Pass in_port to do_multi_mode.
(do_multi_mode): Pass in_port to fork_and_exec_valgrind.
(fork_and_exec_valgrind): Close stdin, redirect stdout to
stderr if in_port <= 0.
Paul Floyd [Sat, 19 Aug 2023 19:37:33 +0000 (21:37 +0200)]
Always cleanup on exit from ML_(read_elf_object)
I'm still a but baffled as to why this wasn't seen earlier.
A FreeBSD testcase started failing with kernel 13.2 patch 2,
which is quite a minor change. The testcase gets an fd from
pdfork and the parent does a printf with the fd then zaps the
process with pdkill. Standalone the fd is 3, and that's what
the expected contains. However, when it started failing I saw
with lsof that fds 3 and 4 were associated with the guest exe
and ld-elf.so.1.
Aaron Merey [Fri, 30 Jun 2023 22:31:42 +0000 (18:31 -0400)]
Support lazy reading and downloading of DWARF debuginfo
Currently valgrind attempts to read DWARF .debug_* sections as well
as separate debuginfo files for ELF binaries as soon as a shared library
is loaded. This might also result in the downloading of separate debuginfo
files via debuginfod.
This is inefficient when some of this debuginfo never ends up being used
by valgrind while running the client process.
This patch adds support for lazy reading and downloading of DWARF
debuginfo. When an ELF shared library is loaded, the reading of .debug_*
sections as well as separate or alternate debuginfo is deferred until
valgrind handles an instruction pointer corresponding to a text segment
of the shared library. At this point the deferred sections and separate
debug files are loaded.
This feature is only supported on ELF platforms.
https://bugs.kde.org/show_bug.cgi?id=471807
ChangeLog
* debuginfo.c (di_notify_ACHIEVE_ACCEPT_STATE): Replace
read_elf_debug_info with read_elf_object.
(addr_load_di): New function. Attempts to load deferred debuginfo
associated with a given address.
(load_di): New function. Attempts to load a given deferred
debuginfo associated with a given address.
(describe_IP): Add calls to load_di and addr_load_di.
(find_DiCfSI): Add call to load_di.
* priv_readelf.h (read_elf_object): New declaration.
(read_elf_debug): Ditto.
* priv_storage.h (struct _DebugInfo): New field 'bool deferred'.
* readelf.c (read_elf_debug_info): Split into read_elf_object and
read_elf_debug.
(read_elf_object): Read non .debug_* section from an ELF binary.
(read_elf_debug): Read .debug_* sections from an ELF binary as
as well any separate/alternate debuginfo files.
* storage.c (canonicaliseSymtab): Remove assert in order to support
canonicalization of deferred _DebugInfo.
(finish_CFSI_arrays): Add early return if _DebugInfo is
deferred in order to avoid freeing memory that will be needed
when reading debuginfo at a later time.
(canonicaliseTables): Ditto.
* pub_core_debuginfo.h (addr_load_di): New declaration.
(load_di): New declaration.
Andreas Arnez [Mon, 22 May 2023 17:49:08 +0000 (19:49 +0200)]
Bug 470132 - s390x: Increase test coverage for VGM
Add more tests for the VGM instruction, to verify the fix for the VGM
wrap-around case. Also test setting unused bits in the I2 and I3 fields,
to check that Valgrind ignores them as it should.
Andreas Arnez [Mon, 22 May 2023 16:57:35 +0000 (18:57 +0200)]
Bug 470132 - s390x: Fix the wrap-around case in VGM
Valgrind's implementation of VGM is incomplete:
* It doesn't support generating a wrap-around bit mask. Such a mask
should result when the ending bit position is smaller than the starting
bit position. Valgrind runs into an assertion failure instead.
* It doesn't ignore unused bits in the I2 and I3 fields of the
instruction, as it should.
Fix this by re-implementing the main logic in s390_irgen_VGM().
Paul Floyd [Sun, 2 Jul 2023 10:59:40 +0000 (12:59 +0200)]
FreeBSD: add default to configure.ac FreeBSD 13 versions
Also add comment to README.freebsd about ensuring that
jails set "uname -r" to be something compatible with the
normal RELEASE/STABLE/CURRENT releases.
Andreas Arnez [Thu, 15 Jun 2023 15:24:53 +0000 (17:24 +0200)]
Bug 470978 - s390x: Link the tools with -Wl,--s390-pgste
Programs that require the PGSTE mode to be enabled may currently fail
under Valgrind. In particular this affects qemu-kvm.
While it is also possible to enable the PGSTE mode globally with
sysctl vm.allocate_psgte=1
the problem can more easily be prevented by linking the Valgrind tools
with -Wl,--s390-pgste. Add a configure check if the linker supports this,
and activate the flag if it does.
To verify the intended result, the following shell command can be used to
list the executables having this flag set:
The bne instruction expects an absolute target address and it isn't
best-suited for implementing a short range jump, such as the one in
XCHG_M_R().
Replace it with jne which expects a relative address that can be
correctly computed a link time.
Interestingly, the jump is almost never taken. If it would, this would
crash the test. However, linkers may complain when relacating the
target address used in bne.
Paul Floyd [Wed, 7 Jun 2023 20:27:08 +0000 (22:27 +0200)]
Bug 470713 - Failure on the Yosys project: valgrind: m_libcfile.c:1802 (Bool vgPlain_realpath(const HChar *, HChar *)): Assertion 'resolved' failed
When using sysctl kern proc pathname with the pid of the guest or -1
we need to intercept the call otherwise the syscall will return the path
of the memcheck tool and not the path of the guest.
This uses VG_(realpath), which asserts if it doesn't get valid
input pointers.
sysctl kern proc pathname can use a NULL pointer in order to
determine the length of the path (so users can allocate the minumum
necessary). The NULL pointer was being passed on to VG_(realpath)
without being checked, resulting in an assert.
Mark Wielaard [Thu, 1 Jun 2023 14:10:56 +0000 (16:10 +0200)]
memcheck: Handle Err_ReallocSizeZero in MC_(eq_Error)
When an realloc size zero error is emitted MC_(eq_Error) is called to
see if the errors can be deduplicated. This crashed since
Err_ReallocSizeZero wasn't handled. Handle it like Err_Free.
Also add a testcase for this case and test with both
--realloc-zero-bytes-frees=yes and
--realloc-zero-bytes-frees=no.
Which will report a different number of errors.
Mark Wielaard [Sun, 14 May 2023 21:34:05 +0000 (23:34 +0200)]
Add --with-gdbscripts-dir=PATH configure option
Currently the gdb valgrind scripts are installed under VG_LIBDIR
which is normally pkglibexecdir which is likely not in the default
gdb safe-path (a list of directories from which it is safe to
auto-load files). So users will have to add the directory to their
.gdbinit file.
This patch adds a --with-gdbscripts-dir=PATH configure option that
sets VG_GDBSCRIPTS_DIR to the given PATH (${libexecdir}/valgrind if
not given). A user can also configure --without-gdbscripts-dir to
disable adding a .debug_gdb_scripts section to the vgpreload library
and installing the valgrind-monitor python scripts completely.
Use VG_GDBSCRIPTS_DIR as gdbscriptsdir to install the valgrind-monitor
python files and pass it with CPPFLAGS when building vg_preloaded.c
and vgdb.c to use instead of VG_LIBDIR.
Andreas Arnez [Thu, 26 Jan 2023 16:41:18 +0000 (17:41 +0100)]
s390x: XC instruction: clear in 8-byte increments if possible
The XC instruction is frequently executed in many programs, mainly for
clearing memory. It can target from 1 to 256 bytes. If the size is
constant and XC is actually used for clearing memory, Valgrind implements
it as a byte-wise loop and rolls out the loop for <= 8 bytes.
Instead of clearing byte-wise, it is more efficient to clear in 64-bit
increments, so do this for sizes >= 8 bytes. Roll out the loop for up to
32 bytes. Overall, this reduces the number of insns by a few percent and
provides a slight performance improvement for some programs.
Andreas Arnez [Fri, 5 May 2023 15:48:31 +0000 (17:48 +0200)]
s390x: Optimize CLC for 1, 2, 4, and 8 bytes
The CLC instruction compares two memory areas with sizes from 1 up to 256
bytes. Currently Valgrind always implements it with a bytewise loop.
Add special handling for the sizes 1, 2, 4, and 8. Realize CLC with an
8-, 16-, 32-, and 64-bit integer comparison, respectively, in those cases.
Apart from a slight optimization this also improves the diagnostics for
uninitialized values since it avoids the manufactured conditional jump
that breaks out of the loop over the individual bytes.
Andreas Arnez [Wed, 15 Feb 2023 17:02:37 +0000 (18:02 +0100)]
Bug 465782 - s390x: Avoid __builtin_setjmp
Currently Clang doesn't support __builtin_setjmp() on s390x. Since
Valgrind already has an alternate implementation of setjmp/longjmp for
many other platforms, just add one for s390x as well, to get rid of this
dependency.
Andreas Arnez [Thu, 1 Sep 2022 13:03:01 +0000 (15:03 +0200)]
Bug 465782 - s390x: Drop -mzarch -march=z900 from assembler options
The -mzarch flag is unsupported by Clang, and it is redundant on 64-bit
build systems. Remove it.
Also remove '-march=z900', since it is unsupported by Clang as well. It
would only be needed on build systems with a default architecture lower
than z900. Such systems are out of service for some time now.
Paul Floyd [Fri, 5 May 2023 20:05:36 +0000 (22:05 +0200)]
Add Helgrind and DRD tests and suppressions for getaddrinfo on Linux
Bump version to 3.22.0.GIT
The testcase was posted on the freebsd-hackers mailing list.
I had time to get suppressions for FreeBSD into 3.21 but
ran out of time for the test and Linux suppressions.
I did take a look at how thread sanitizer handles this.
Basically it intercepts the call, turns off checking,
calls the resl function then turns checking back on.
I don't see many other similar examples. Might be worth
looking at dlopen and atexit.
Mark Wielaard [Fri, 28 Apr 2023 11:34:48 +0000 (13:34 +0200)]
Support SCV_FLAG also on VGP_ppc64be_linux
Running on a kernel that supports the SCV instruction (sets
PPC_FEATURE2_SCV in auxv AT_HWCAPS2) valgrind will assert: valgrind:
m_syswrap/syswrap-main.c:549 (getSyscallArgsFromGuestState): Assertion
'gst->guest_syscall_flag == SC_FLAG' failed.
Removing that assert makes most things work. But also filter out
PPC_FEATURE2_SCV from AT_HWCAPS2 for the client, so it shouldn't try
using the SCV instruction.
For all the changes I've made recently. And also various other changes
that occurred over the past 20 years that didn't previously make it into
the docs.
Also, this change de-emphasises the cache and branch simulation aspect,
because they're no longer that useful. Instead it emphasises the
precision and reproducibility of instruction count profiling.
Get rid of cache config warnings with `--cache-sim=no`.
By not configuring the caches in that case. This requires moving a few
assertions around, because they currently assume that the caches are
configured.
And deprecate the use of `cg_diff` and `cg_merge`.
Because `cg_annotate` can do a better job, even annotating source files
when doing diffs in some cases.
The user requests merging by passing multiple cgout files to
`cg_annotate`, and diffing by passing two cgout files to `cg_annotate`
along with `--diff`.
- one more comment at the top describing the three usages of vgdb.
- fixed up a few places where tabs were used for indentation (we are
not very consistent in that either, after the release we'll look
into adopting something like clang-format so you don't have to do
all this by hand).
- Add a missing newline in coregrind/m_main.c to make
none/tests/cmdline2 pass.
Mark Wielaard [Thu, 20 Apr 2023 10:59:02 +0000 (12:59 +0200)]
vgdb: Handle EAGAIN in read_buf
The file descriptor is on non-blocking mode and read_buf should only
be called when poll gave us an POLLIN event signaling the file
descriptor is ready for reading from. Still sometimes we do get an
occasional EAGAIN. Just do as told in that case and try to read again.
Also fix an ERROR errno in getpkt. This has never been observed, but
not getting the actual errno if the write fails in that case would be
really confusing.
Mark Wielaard [Wed, 19 Apr 2023 22:42:40 +0000 (00:42 +0200)]
Bug 439685 compiler warning in callgrind/main.c
main.c: In function 'vgCallgrind_post_syscalltime':
main.c:1779:25: warning: '*((void *)&ts_now+8)'
may be used uninitialized in this function [-Wmaybe-uninitialized]
struct vki_timespec ts_now;
main.c:1779:25: warning: 'ts_now'
may be used uninitialized in this function [-Wmaybe-uninitialized]
In function collect_time the conditional expression in the switch
statement has type int (after integral promotions). GCC assumes that
it may have values other than the ones listed in the enumerated type
it was promoted from. In that case the memory pointed to by its 1st
argument remains unintialised. Later on vki_timespec_diff will read
the contents of ts_now undoditionally. Hence the warning.
Using the default case for the tl_assert () removes the warning and
makes the code more robust should another enumerator ever be added to
Collect_Systime.
PowerPC:, Fix test test_isa_3_1_R1_RT.c, test_isa_3_1_R1_XT.c
Fixes an issue with the PAD_ORI used in the the tests by explicitly adding
SAVE_REGS and RESTORE_REGS macros. The macros ensure that the block of
immediate OR instructions don't inadvertently change the contents of the
registers.
John Reiser suggested that the PAD_ORI asm statements in the PAD_ORI
macro be updated to inform the compiler which register the ori instruction
is clobbering. The compiler will then generate the code to save and
restore the register automatically. This is a cleaner solution then
explicitly adding the macros to store and restore the registers. It is
functionally cleaner in that the value fetched by the instruction under
test is not modified by the PAD_ORI instructions.
This patch removes the SAVE_REG and RESTORE_REG macros and updates the
PAD_ORI macro.