At various places, there were either some assumption that the 'end'
boundary (highest address) was either not included, included,
or was the highest addressable word, or the highest addressable byte.
This e.g. was very visible when doing:
./vg-in-place -d -d ./helgrind/tests/tc01_simple_race|&grep regi
giving
--24040:2:stacks register 0xBEDB4000-0xBEDB4FFF as stack 0
--24040:2:stacks register 0x402C000-0x4A2C000 as stack 1
showing that the main stack end was (on x86) not the highest word
but the highest byte, while for the thread 1, the registered end
was a byte not part of the stack.
The attached patch ensures that stack bounds semantic are documented and
consistent. Also, some of the stack handling code is factorised.
The convention that the patch ensures and documents is:
start is the lowest addressable byte, end is the highest addressable byte.
(the words 'min' and 'max' have been kept when already used, as this wording is
consistent with the new semantic of start/end).
In various debug log, used brackets [ and ] to make clear that
both bounds are included.
The code to guess and register the client stack was duplicated
in all the platform specific syswrap-<plat>-<os>.c files.
Code has been factorised in syswrap-generic.c
The patch has been regression tested on
x86, amd64, ppc32/64, s390x.
It has been compiled and one test run on arm64.
Not compiled/not tested on darwin, android, mips32/64, arm
More in details, the patch does the following:
coregrind/pub_core_aspacemgr.h
include/valgrind.h
include/pub_tool_machine.h
coregrind/pub_core_scheduler.h
coregrind/pub_core_stacks.h
- document start/end semantic in various functions
also in pub_tool_machine.h:
- replaces unclear 'bottommost address' by 'lowest address'
(unclear as stack bottom is or at least can be interpreted as
the 'functional' bottom of the stack, which is the highest
address for 'stack growing downwards').
coregrind/pub_core_initimg.h
replace unclear clstack_top by clstack_end
coregrind/m_main.c
updated to clstack_end
coregrind/pub_core_threadstate.h
renamed client_stack_highest_word to client_stack_highest_byte
coregrind/m_scheduler/scheduler.c
computes client_stack_highest_byte as the highest addressable byte
Update comments in call to VG_(show_sched_status)
coregrind/m_machine.c
coregrind/m_stacktrace.c
updated to client_stack_highest_byte, and switched
stack_lowest/highest_word to stack_lowest/highest_byte accordingly
coregrind/m_stacks.c
clarify semantic of start/end,
added a comment to indicate why we invert start/end in register call
(note that the code find_stack_by_addr was already assuming that
end was included as the checks were doing e.g.
sp >= i->start && sp <= i->end
coregrind/pub_core_clientstate.h
coregrind/m_clientstate.c
renames Addr VG_(clstk_base) to Addr VG_(clstk_start_base)
(start to indicate it is the lowest address, base suffix kept
to indicate it is the initial lowest address).
coregrind/m_initimg/initimg-darwin.c
updated to VG_(clstk_start_base)
replace unclear iicii.clstack_top by iicii.clstack_end
updated clstack_max_size computation according to both bounds included.
coregrind/m_initimg/initimg-linux.c
updated to VG_(clstk_start_base)
updated VG_(clstk_end) computation according to both bounds included.
replace unclear iicii.clstack_top by iicii.clstack_end
coregrind/pub_core_aspacemgr.h
extern Addr VG_(am_startup) : clarify semantic of the returned value
coregrind/m_aspacemgr/aspacemgr-linux.c
removed a copy of a comment that was already in pub_core_aspacemgr.h
(avoid double maintenance)
renamed unclear suggested_clstack_top to suggested_clstack_end
(note that here, it looks like suggested_clstack_top was already
the last addressable byte)
* factorisation of the stack guessing and registration causes
mechanical changes in the following files:
coregrind/m_syswrap/syswrap-ppc64-linux.c
coregrind/m_syswrap/syswrap-x86-darwin.c
coregrind/m_syswrap/syswrap-amd64-linux.c
coregrind/m_syswrap/syswrap-arm-linux.c
coregrind/m_syswrap/syswrap-generic.c
coregrind/m_syswrap/syswrap-mips64-linux.c
coregrind/m_syswrap/syswrap-ppc32-linux.c
coregrind/m_syswrap/syswrap-amd64-darwin.c
coregrind/m_syswrap/syswrap-mips32-linux.c
coregrind/m_syswrap/priv_syswrap-generic.h
coregrind/m_syswrap/syswrap-x86-linux.c
coregrind/m_syswrap/syswrap-s390x-linux.c
coregrind/m_syswrap/syswrap-darwin.c
coregrind/m_syswrap/syswrap-arm64-linux.c
Some files to look at more in details:
syswrap-darwin.c : the handling of sysctl(kern.usrstack) looked
buggy to me, and has probably be made correct by the fact that
VG_(clstk_end) is now the last addressable byte. However,unsure
about this, as I could not find any documentation about
sysctl(kern.usrstack). I only find several occurences on the web,
showing that the result of this is page aligned, which I guess
means it must be 1+ the last addressable byte.
syswrap-x86-darwin.c and syswrap-amd64-darwin.c
I suspect the code that was computing client_stack_highest_word
was wrong, and the patch makes it correct.
syswrap-mips64-linux.c
not sure what to do for this code. This is the only code
that was guessing the stack differently from others.
Kept (almost) untouched. To be discussed with mips maintainers.
coregrind/pub_core_libcassert.h
coregrind/m_libcassert.c
* void VG_(show_sched_status):
renamed Bool valgrind_stack_usage to Bool stack_usage
if stack_usage, shows both the valgrind stack usage and
the client stack boundaries
coregrind/m_scheduler/scheduler.c
coregrind/m_gdbserver/server.c
coregrind/m_gdbserver/remote-utils.c
Updated comments in callers to VG_(show_sched_status)
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14392
getdents has been deprecated since linux 2.4 and newer arches (arm64)
might no longer provide the getdents syscall. Use getdents64 for reading
the /proc/self/fd/ dir so --track-fds=yes works reliable on all arches.
Without this the none/tests/fdleak*vgtest might fail.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14384
pre_mem_read_sockaddr: in the case where the caller doesn't
specify any address family (that is, the family is AF_UNSPEC)
don't perform any further checks on the supplied |sa| address
block, since doing so merely gives rise to false uninitialised
value errors.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14320
of clo which are (or should be) 'enum set'.
* pub_tool_options.h : add new macrox VG_USET_CLO and VG_USETX_CLO to
parse an 'enum set' command line option (with or without "all" keyword).
* use VG_USET_CLO for existing enum set clo options:
memcheck --errors-for-leak-kinds, --show-leak-kinds, --leak-check-heuristics
coregrind --vgdb-stop-at
* change --sim-hints and --kernel-variants to enum set
(this allows to detect user typos: currently, a typo in a sim-hint
or kernel variant is silently ignored. Now, an error will be given
to the user)
* The 2 new sets (--sim-hints and --kernel-variants) should not make
use of the 'all' keyword => VG_(parse_enum_set) has a new argument
to enable/disable the use of the "all" keyword.
* The macros defining an 'all enum' set definition was duplicating
all enum values (so addition of a new enum value could easily
give a bug). Removing these macros as they are unused
(to the exception of the leak-kind set).
For this set, the 'all macro' has been replaced by an 'all function',
coded using parse_enum_set parsing the "all" keyword.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14301
rather than throwing to the default case. This stops Memcheck
reporting false positives for the NETLINK case.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14237
Some syscall verification code is allocating memory to generate
the string used to build an error, e.g. syswrap-generic.c verifying fields of
e.g socket addresses (pre_mem_read_sockaddr) or sendmsg/recvmsg args
(msghdr_foreachfield)
The allocated pointer was copied in the error created by VG_(maybe_record_error).
This was wrong for 2 reasons:
1. If the error is a new error, it is stored in a list of errors,
but the string memory was freed by pre_mem_read_sockaddr, msghdr_foreachfield, ...
This causes a dangling reference. Was at least visible when giving -v, which
re-prints all errors at the end of execution.
Probably this could have some consequences during run while generating new errors,
and comparing for equality with a recorded error having a dangling reference.
2. the same allocated string is re-used for each piece/field of the verified struct.
The code in mc_errors.c that checks that 2 errors are identical was then wrongly
considereing that 2 successive errors for 2 different fields for the same syscall
arg are identical, just because the error string happened to be produced at
the same address.
(it is believed that initially, the error string was assumed to be a static
string, which is not the case anymore, causing the above 2 problems).
Changes:
* The fix consists in duplicating in m_errormgr.c the given error string when
the error is recorded. In other words, the error string is now duplicated similarly
to the (optional) extra component of the error.
* memcheck/tests/linux/rfcomm.c test modified as now an error is reported
for each uninit field.
* socketaddr unknown family is also better reported (using sa_data field name,
rather than an empty field name.
* minor reformatting in m_errormgr.c, to be below 80 characters.
Some notes:
1. the string is only duplicated if the error is recorded
(ie. printed or the first time an error matches a suppression).
The string is not duplicated for duplicated errors or following errors
matching the first (suppressed) error.
The string is also not duplicated for 'unique errors' (that are printed
and then not recorded).
2. duplicating the string for each recorded error is not deemed to
use a lot of memory:
* error strings are usually NULL or short (often 10 bytes or so).
* we expect no program has a huge number of errors
If ever this string duplicate would be significant, having a DedupPoolAlloc
in m_errormgr.c for these strings would reduce this memory (as we expect to
have very few different strings, even with millions of errors).
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14214
Call ML_(safe_to_deref) before using msghdr msg_name, msg_iov or msg_control.
Fixes bug #334705.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13963
Bug #320116. sockaddr_rc might contain some padding which might not be
initialized. Explicitly check the sockaddr_rc fields are set. That also
produces better diagnostics about which field is unitialized.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13404
(allows to have the list of opened fds and the associated info
on request from GDB or from the shell, using vgdb)
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13072
mremap3.c based on testcase provided by Jan Engelhardt
* coregrind/m_syswrap/syswrap-generic.c
- The two 'no-thrash checks' that were introduced to fix bug #129866
were (probably) broken when adress space manager was reworked.
The new VG_(am_get_advisory_client_simple) returns NULL for a free
segment, but the check was based on checking not NULL and then
that the state is free.
=> replaces these two local checks by a call to the new
am Bool VG_(am_covered_by_single_free_segment) function.
* coregrind/pub_core_aspacemgr.h
coregrind/m_aspacemgr/aspacemgr-linux.c
- new function Bool VG_(am_covered_by_single_free_segment)
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12314
(Rusty Russell, rusty@rustcorp.com.au)
tdb uses fcntl locks and mmap, and some of the tests fail under valgrind.
strace showed valgrind opening the tdb file, reading 1024 bytes, then closing
it. This is not allowed: POSIX says if you open and close a file, all fcntl
locks on it are dropped (insane, yes).
Finally got around to hacking the source to track this down: di_notify_mmap is
doing the damage. The simplest fix was to hand in an optional fd for it to
use, then have it do pread.
I had to fix your pread; surely this should seek back even if the platform
doesn't have pread support?
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12224
a bunch of file-related syscalls to be handled on the might-block
syscall path rather than the fast syscall path. This fixes deadlocks
when running some FUSE-specific filesystem codes. Fixes#278057.
(Mike Shal, marfey@gmail.com)
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11993
ensure proper cleanup of gdbsrv FIFOs/shmem files with untraced fork/exec
* syswrap-{generic|darwin|aix5}.c : in PRE(sys_execve) : terminate gdbserver
* pub_core_gdbserver.h and m_gdbserver.c : add VG_(gdbserver_prerun_action),
factorising the actions to do by gdbserver at "startup" (i.e. a traced
fork or a traced exec).
* scheduler.c : implement startup action using VG_(gdbserver_prerun_action)
(Philippe Waroquiers, philippe.waroquiers@skynet.be)
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11771
__builtin_setjmp and __builtin_longjmp so that they can be selectively
replaced, on a platform by platform basis. Does not change any
functionality. Related to #259977.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11687
is how mmap() sizes are treated. It fixes an assertion failure in Massif
with --pages-as-heap=yes.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11485
chase/nochase decisions for child processes to be made on the basis
of their argv[] entries rather than on the name of their executables.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11483
account rounding requirements to SHMLBA. Modified version of a patch
by Kirill Batuzov, batuzovk@ispras.ru. This fixes the main bug in
#222545. Temporarily breaks the build on all other platforms though.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11399
except on amd64-linux. This fixes a secondary problem discussed
in bug 222545. (Kirill Batuzov, batuzovk@ispras.ru)
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11398
for the old segment so we need to save the permissions from it before
the call so that we can use them when notifying tools of the new space
afterwards, or we will notify them of the wrong permissions.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11384
platforms. Also add MERGE64_FIRST and MERGE64_SECOND macros to help
produce the right argument names in error messages on big/little
endian platforms.
Based on patch from Dodji Seketeli. Part fix for #215973.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10951
Specifies a comma-separated list of executable-names
(with "*" and "?" wildcards allowed) that should not be traced into
even when --trace-children=yes. Modified version of a patch
from Bill Hoffman. Fixes#148932.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10927
is mounted to compile-time logic in order to minimize the differences
in behavior with Valgrind version 3.5.0.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10868
