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
to add PPC64 LE support. The other two patches can be found in Bugzillas
334834 and 334836. The commit does not have a VEX commit associated with it.
POWER PC, add initial Little Endian support
The IBM POWER processor now supports both Big Endian and Little Endian.
This patch renames the #defines with the name ppc64 to ppc64be for the BE
specific code. This patch adds the Little Endian #define ppc64le to the
Additionally, a few functions are renamed to remove BE from the name if the
function is used by BE and LE. Functions that are BE specific have BE put
in the name.
The goals of this patch is to make sure #defines, function names and
variables consistently use PPC64/ppc64 if it refers to BE and LE,
PPC64BE/ppc64be if it is specific to BE, PPC64LE/ppc64le if it is LE
specific. The patch does not break the code for PPC64 Big Endian.
The test files memcheck/tests/atomic_incs.c, tests/power_insn_available.c
and tests/power_insn_available.c are also updated to the new #define
definition for PPC64 BE.
Signed-off-by: Carl Love <carll@us.ibm.com>
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14238
showing inlined function calls.
See 278972 valgrind stacktraces and suppression do not handle inlined function call debuginfo
Reading the inlined dwarf call info is activated using the new clo
--read-inline-info=yes
Default is currently no but an objective is to optimise the performance
and memory in order to possibly set it on by default.
(see below discussion about performances).
Basically, the patch provides the following pieces:
1. Implement a new dwarf3 reader that reads the inlined call info
2. Some performance improvements done for this new parser, and
on some common code between the new parser and the var info parser.
3. Use the parsed inlined info to produce stacktrace showing inlined calls
4. Use the parsed inlined info in the suppression matching and suppression generation
5. and of course, some reg tests
1. new dwarf3 reader:
---------------------
Two options were possible: add the reading of the inlined info
in the current var info dwarf reader, or add a 2nd reader.
The 2nd approach was preferred, for the following reasons:
The var info reader is slow, memory hungry and quite complex.
Having a separate parsing phase for the inlined information
is simpler/faster when just reading the inlined info.
Possibly, a single parser would be faster when using both
--read-var-info=yes and --read-inline-info=yes.
However, var-info being extremely memory/cpu hungry, it is unlikely
to be used often, and having a separate parsing for inlined info
does in any case make not much difference.
(--read-var-info=yes is also now less interesting thanks to commit
r13991, which provides a fast and low memory "reasonable" location
for an address).
The inlined info parser reads the dwarf info to make calls
to priv_storage.h ML_(addInlInfo).
2. performance optimisations
----------------------------
* the abbrev cache has been improved in revision r14035.
* The new parser skips the non interesting DIEs
(the var-info parser has no logic to skip uninteresting DIEs).
* Some other minor perf optimisation here and there.
In total now, on a big executable, 15 seconds CPU are needed to
create the inlined info (on my slow x86 pentium).
With regards to memory, the dinfo arena:
with inlined info: 172281856/121085952 max/curr mmap'd
without : 157892608/106721280 max/curr mmap'd,
So, basically, inlined information costs about 15Mb of memory for
my big executable (compared to first version of the patch, this is
already using less memory, thanks to the strpool deduppoolalloc.
The needed memory can probably be decreased somewhat more.
3. produce better stack traces
------------------------------
VG_(describe_IP) has a new argument InlIPCursor *iipc which allows
to describe inlined function calls by doing repetitive calls
to describe_IP. See pub_tool_debuginfo.h for a description.
4. suppression generation and matching
--------------------------------------
* suppression generation now also uses an InlIPCursor *iipc
to generate a line for each inlined fn call.
* suppression matching: to allow suppression matching to
match one IP to several function calls in a suppression entry,
the 'inputCompleter' object (that allows to lazily generate
function or object names for a stacktrace when matching
an error with a suppression) has been generalised a little bit
more to also lazily generate the input sequence.
VG_(generic_match) has been updated so as to be more generic
with respect to the input completer : when providing an
input completer, VG_(generic_match) does not need anymore
to produce/compute any input itself : this is all delegated
to the input completer.
5. various regtests
-------------------
to test stack traces with inlined calls, and suppressions
of (some of) these errors using inlined fn calls matching.
Work still to do:
-----------------
* improve parsing performance
* improve the memory overhead.
* handling the directory name for files of the inlined function calls is not yet done.
(probably implies to refactor some code)
* see if m_errormgr.c *offsets arrays cannot be managed via xarray
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14036
Necessary changes to Valgrind to support MIPS64LE on Linux.
Minor cleanup/style changes embedded in the patch as well.
The change corresponds to r2687 in VEX.
Patch written by Dejan Jevtic and Petar Jovanovic.
More information about this issue:
https://bugs.kde.org/show_bug.cgi?id=313267
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13292
gcc reports a warning:
m_stacktrace.c:183: warning: ‘xip_verified’ may be used uninitialized in this function
This warning is a false positive:
xip_verified is assigned in the following branch:
if (UNLIKELY(xip_verif >= CFUNWIND)) {
if (xip_verif == CFUNWIND) {
...
} else {
<<<< here xip_verified is initialised >>>>
}
}
xip_verified is then used only if xip_verif > CFUNWIND.
Assign a rubish value to xip_verified to silence gcc.
(??? there are GCC pragmas that can be used to
disable a warning only on a specific line e.g.
something like:
#pragma GCC diagnostic ignored "-Wuninitialized"
Addr xip_verified; // xip for which we have calculated fpverif_uregs
#pragma GCC diagnostic warning "-Wuninitialized"
instead of
Addr xip_verified = 0; // xip for which we have calculated fpverif_uregs
// 0 assigned to silence false positive -Wuninitialized warning
but the #pragma technique seems not used currently.
So, using the bypass by assigning a rubbish value
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13282
* other platforms (e.g. amd64) are first trying to unwind
with cfi info, then with the fp chain.
* fp unwind when code is compiled without frame pointer can
fail and give incomplete stack traces (often terminating
with a random program counter, causing a huge amount of
recorded stack traces).
This patch improves unwinding on x86 by:
* first time an IP is unwound, do the unwind both with
CFI technique and with fp technique.
If results are identical, IP is inserted in a cache of
'fp unwindable' IP
* following unwind of the same IP are then done directly
either with fp unwind or with cfi, depending on the
cached result of the check done during first unwind.
The cache is needed so as to avoid as much as possible cfi unwind,
as this is significantly slower than fp unwind.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13280
In a big applications, some recursive algorithms have created
hundreds of thousands of stacktraces, taking a lot of memory.
Option --merge-recursive-frames=<number> tells Valgrind to
detect and merge (collapse) recursive calls when recording stack traces.
The value is changeable using the monitor command
'v.set merge-recursive-frames'.
Also, this provides a new client request: VALGRIND_MONITOR_COMMAND
allowing to execute a gdbsrv monitor command from the client
program.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13246
If VG_(use_CF_info) fails to find the next frame using loaded debug symbols, it
will still change the data in uregs. Thus, we need to have uregs_copy before
calling VG_(use_CF_info), and restore uregs if the call returns wrong data.
This fixes drd/tests/tc04_free_lock on MIPS.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12962
ARM by simply scanning up and looking for words that look like they
might be return addresses. Last-ditch hack for when the CFI trail
goes cold.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12641
__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
side components. (Florian Krohm <britzel@acm.org> and Christian
Borntraeger <borntraeger@de.ibm.com>). Fixes#243404.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11604
bogus, and produces essentially useless traces from them. With
gcc-4.4 and later, some valid thread stacks really are smaller than
this. Hence change the limit down to 256 bytes. Investigated by
Evgeniy Stepanov, eugeni.stepanov@gmail.com.
See bug 243270 comment 21.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11403
knows how to unwind. This is important when unwinding Thumb code
the CFA is often stated as being at some offset from r7.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11377
too. This is a first step towards making not be completely
x86/amd64-linux specific, and so replaces some x86/amd64-specific
stuff with more general constructions:
* structure 'DiCfSI', into which the info is summarised, has been
made target-specific (ugh), since the sets of registers to be
unwound differ on different targets.
* enum CfiReg and the CFIC_ constants have been expanded
accordingly, to handle both arm and x86/amd64 registers.
The abbreviation "IA" (Intel Architecture) has been used in a
few places where the x86 and amd64 definitions are shared.
* the CFI reader/summariser in readdwarf.c has been expanded &
generalised appropriately.
* the DiCfSI evaluator in debuginfo.c, VG_(use_CFI_info), has
also been generalised appropriately.
The main change is that instead of passing around triples
of (IP, SP, BP) values, a new structure 'D3UnwindRegs' is
passed around instead. This is defined differently for IA and
ARM and succeeds in hiding at least some of the differences
where we don't care about them.
Note also, D3UnwindRegs duplicates, in purpose and structure,
structure 'RegSummary' in priv_d3basics.h. This will be tidied
up in due course.
This commit almost certainly breaks stack unwinding on amd64-linux.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10986
the changes to do with reading and using ELF and DWARF3 info.
This breaks all targets except amd64-linux and x86-linux.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10982
frames other than the first one found.
This is taken from Jakub Jelinek's second patch on bug #210479.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10938
both wrapped up in XML tags (as before) but also in plain text in a
sequence of CDATA blocks. Normally only one, but in the worst case
the raw data will have ]]> in it, in which case it needs to be split
across two CDATA blocks.
This apparently simple change involved a lot of refactoring of the
suppression printing machinery:
* in the core-tool iface, change "print_extra_suppression_info" (which
prints any auxiliary info) to "get_extra_suppression_info", which
parks the text in a caller-supplied buffer. Adjust tools to match.
* VG_(apply_StackTrace): accept a void* argument, which is passed to
each invokation of the functional parameter (a poor man's closure
implementation).
* move PRINTF_CHECK into put_tool_basics.h, where it should have been
all along
* move private printf-into-an-XArray-of-character functions from
m_debuginfo into m_xarray, and make them public
* gen_suppression itself: use all the above changes. Basically we
always generate the plaintext version into an XArray. In text mode
that's just printed. In XML mode, we print the XMLery as before,
but the plaintext version is dumped into a CDATA block too.
* update the Protocol 4 specification to match all this.
This still isn't 100% right in the sense that the CDATA block data
needs to be split across multiple blocks if it should ever contain the
CDATA end mark "]]>". The Protocol 4 spec has this right even though
the implementation currently doesn't.
Fixes#191189.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10822
This commit tidies up and rationalises what could be called the
"messaging" system -- that part of V to do with presenting output to
the user. In particular it brings significant improvements to XML
output.
Changes are:
* XML and normal text output now have separate file descriptors,
which solves longstanding problems for XML consumers caused by
the XML output getting polluted by unexpected non-XML output.
* This also means that we no longer have to hardwire all manner
of output settings (verbosity, etc) when XML is requested.
* The XML output format has been revised, cleaned up, and made
more suitable for use by error detecting tools in general
(various Memcheck-specific features have been removed). XML
output is enabled for Ptrcheck and Helgrind, and Memcheck is
updated to the new format.
* One side effect is that the behaviour of VG_(message) has been
made to be consistent with printf: it no longer automatically
adds a newline at the end of the output. This means multiple
calls to it can be used to build up a single line message; or a
single call can write a multi-line message. The ==pid==
preamble is automatically inserted at each newline.
* VG_(message)(Vg_UserMsg, ..args..) now has the abbreviated form
VG_(UMSG)(..args..); ditto VG_(DMSG) for Vg_DebugMsg and
VG_(EMSG) for Vg_DebugExtraMsg. A couple of other useful
printf derivatives have been added to pub_tool_libcprint.h,
most particularly VG_(vcbprintf).
* There's a small change in the core-tool interface to do with
error handling: VG_(needs_tool_errors) has a new method
void (*before_pp_Error)(Error* err) which, if non-NULL, is
called just before void (*pp_Error)(Error* err). This is to
give tools the chance to look at errors before any part of them
is printed, so they can print any XML preamble they like.
* coregrind/m_errormgr.c has been overhauled and cleaned up, and
is a bit simpler and more commented. In particular pp_Error
and VG_(maybe_record_error) are significantly changed.
The diff is huge, but mostly very boring. Most of the changes
are of the form
- VG_(message)(Vg_UserMsg, "this is a message %d", n);
+ VG_(message)(Vg_UserMsg, "this is a message %d\n", n);
Unfortunately as a result of this, it touches a large number
of source files.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10465
I tried using 'svn merge' to do the merge but it did a terrible job and
there were bazillions of conflicts. So instead I just took the diff between
the branch and trunk at r10155, applied the diff to the trunk, 'svn add'ed
the added files (no files needed to be 'svn remove'd) and committed.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10156
- When printing suppressions, never print more entries than there are in the
stack. This avoids bogus suppressions in some cases! (I haven't seen
them on Linux, but I have seen them on Darwin.)
- When getting a stack trace, stop if we get an IP of zero or one; that
means we've hit the end of the stack. And don't include that entry in the
stack trace, because it's a guaranteed "???" if it's ever printed which is
useless.
- In VG_(apply_StackTrace), we can now rely entirely on the n_ip parameter
rather than looking for 0 or -1, because that check is done when the stack
trace is first obtained. In other words, stack traces all use an n_ip
parameter to record their size, whereas previously they used an odd
mixture of n_ip and null-termination.
- Rename 'n_ips' variables as 'max_n_ips' where appropriate; those left as
'n_ips' truly describe how many IPs there are in the stack trace.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@9793
numbers) when Valgrind is running Wine. Modified version of a
patch by John Reiser (vgsvn+wine-load-pdb-debuginfo.patch) with
extensions to read a second format of line number tables.
Wine uses a new client request, VG_USERREQ__LOAD_PDB_DEBUGINFO,
to tell Valgrind when to read PDB info. Wine's implementation
of module loading is vastly different from that used by
ld-linux.so, and it is too difficult to recognize what is going
on just by observing the calls to mmap and mprotect.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@9580
- Now more clearly distinguishing between C++-demangling, Z-demangling, and
below-main renaming, particularly in 'get_sym_name'.
- --demangle=no no longer prevents Z-demangling, which makes more sense,
although it's unlikely to affect anyone.
- Broke the circular dependency between m_demangle and m_debuginfo by moving
below-main renaming into m_debuginfo.
- Renamed some get_fnname_* functions to make their effect clearer, and
improved their comments.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@9138
'__libc_start_main', in Massif, m_debuginfo and m_stacktrace. As part of
this, --show-below-main is now visible to tools, and Massif pays attention
to it.
Improved the description of --show-below-main=yes in the manual.
Replaced some instances of "__libc_start_main" in the test *.exp files with
"(below main)", which is what will actually be seen. Also updated
scalar.stderr.exp*, which should make it get closer to actually passing.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@9131
as denoting the logical end of the stack. This change stops printing
of a lot of junk below the logical "-1" end mark. See added comments
for details.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@9051
stack unwind mechanism (the function VG_(record_ExeContext) et al),
clears up some associated kludges, and makes suppression matching work
more reliably.
Prior to this commit, a stack snapshot contained, at [0], the IP of
the relevant thread, and at all positions [1] and above, the return
addresses for the open calls.
When showing a snapshot to the user (in VG_(apply_StackTrace)), and
searching the stack for stack blocks (in VG_(get_data_description)), 1
is subtracted from positions [1] and above, so as to move these return
addresses back to the last byte of the calling instruction. This
subtraction is also done even in VG_(get_StackTrace_wrk) itself, in
order to make the stack unwinding work at all.
It turns out that suppression-vs-function-name matching requires the
same hack, and sometimes failed to match suppressions that should
match, because of this self-same problem.
So the commit changes the stack unwinder itself, so that entries [1]
and above point to the last byte of the call instruction, rather than
the return address. The associated kludges in VG_(apply_StackTrace)
and VG_(get_StackTrace_wrk) are removed, and suppression matching is
observed to work in a case where it failed before.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@8818
minor changes to make stack unwinding on amd64-linux approximately
twice as fast as it was before.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@8707
