Julian Seward [Sun, 18 May 2003 10:05:38 +0000 (10:05 +0000)]
gcc-3.3 as supplied with SuSE 8.2 ("gcc version 3.3 20030226
(prerelease) (SuSE Linux)") seems to complain about signed-vs-unsigned
comparisons, when -Wall is on. This commit fixes (most of) those
complaints.
Julian Seward [Mon, 5 May 2003 22:18:17 +0000 (22:18 +0000)]
Merge patch from JosefW:
Hi,
recently I found that there is sometimes cost attributed to some
strange lines (with cachegrind/calltree) with GCC 3.x (using the
DWARF2 debug info format).
I had time to look at this. There is a bug in the DWARF2 debug line
info loader when instructions following each other have source lines
far from each other (e.g. with inlined functions).
Julian Seward [Sun, 4 May 2003 12:32:56 +0000 (12:32 +0000)]
Unchain translations when doing VALGRIND_DISCARD_TRANSLATIONS. Otherwise
the tt/tc are left in an inconsistent state afterwards. (Adam Gundy
<arg@cyberscience.com>)
Moved the CLIENT_tstCALL[0123] requests out of valgrind.h into vg_skin.h,
because there was no point exposing them to clients, as they don't know the
ThreadState type.
Also, removed the LOGMESSAGE request type, replaced it with calls to
VG_(message) via the generic VALGRIND_NON_SIMD_CALL2.
In fact, almost every single pthread client request could be removed in this
same way. That would result in less code, which would be nice... yeah, real
nice.
Removed magic numbers for VG_(stack) size and VG_(sigstack) size, made into
#defined constants. I hope I got all the right places. I also hope that they
can be different sizes; experiments seem to indicate so. Also if I reduce the
size of the main stack at all below its current 10000 I get problems, but that
was happening before anyway, I think.
Fixed a bug in Cachegrind -- if the profiled program changes working directory,
the output file wouldn't get written. No longer creating the file at startup
and then writing it at the end; just writing it at the end. Also recording
the start directory at the start so that the output ends up in it even if the
program does change directory.
1. No longer printing the "MMX-only" or "SSE-capable" string at startup
("if (0)"'d it out).
2. Fixed the SSE state alignment in the baseBlock -- it worked for Memcheck,
Nulgrind and Corecheck, but not for Addrcheck, Cachegrind, Lackey, Helgrind,
due to variable numbers of shadow registers and registered compact helpers
coming before the SSE/FPU state.
Extend the state components in VG_(m_state_static) and VG_(baseBlock)
to include the SSE/SSE2 architectural state. Automagically detect
at startup, in vg_startup.S, whether or not this is a SSE-enabled
CPU and act accordingly. All subsequent FPU/SSE state transfers
between the simulated and real machine are then done either with
fsave/frstor (as before) or fxsave/fxrstor (the SSE equivalents).
Fragile and fiddly; (1) the SSE state needs to be stored on a 16-byte
boundary, and (2) certain bits in the saved MXCSR reg in a state
written by fxsave need to be anded out before we can safely restore
using fxrstor.
It does appear to work. I'd appreciate people trying it out on
various CPUs to establish whether the SSE / not-SSE check works
right, and/or anything else is broken.
Unfortunately makes some programs run significantly slower.
I don't know why. Perhaps due to copying around more processor
state than there was before (SSE state is 512 bytes, FPU state
was only 108). I will look into this.
Couple of minor fixes:
- don't keel over if run on an empty file
- abort if the "summary:" line is missing; previously it gave a warning
and tried to keep going but then other things broke.
Don't trash %eflags before snapshotting the state of the machine at
startup. I don't know if this is actually of interest, since surely
the flags are not live at the point where the section's .init function
is called, but still, it's a form of state pollution.
Don't call VG_(mash_LD_PRELOAD_and_LD_LIBRARY_PATH) if all we're doing
is exiting normally. There's no point, and it causes breakage when
programs which write their environment variables, finish normally.
Fix threading problems on glibc-2.3.2 or later. Note this is *not*
NPTL support.
The behaviour of weak vs strong symbols seems to have changed in
glibc-2.3.2. This caused problems in coregrind/vg_intercept.c,
wherein strong symbols in vg_libpthread.c were intended to
override weak symbols in vg_intercept.c, in order to give alternative
thread-safe implementations of some functions, poll(), select(), etc.
The change involves moving the nonblocking implementations of poll, etc
into vg_intercept.c, renaming them to (eg) VGR_(poll), and routing
all calls to poll to VGR_(poll) [dually for other such fns]. This
means even single-threaded programs now use these functions, but
that doesn't strike me as harmful.
Fixed
Fixed bug in strncpy() and strncat() -- they were giving the right answers, but
could read the (n+1)th char, which could possibly be inaccessible. Thanks to
Adam Gundy <arg@cyberscience.com> for spotting it.
Changed things so that the regression test script (vg_regtest) is no longer
installed, as it was silly since the reg tests themselves aren't installed.
Updated the "make regtest" target to use the uninstalled version in tests/.
Also, vg_regtest now no longer uses the installed version of Valgrind to do the
testing, but instead coregrind/valgrind. This means that you don't have to do
make install with your buggy changes before running the regtests on them :)
Record the correct address of the initial thread's stack, as determined
by the initial scan of /proc/self/maps, so that we correctly identify
addresses in it. This fix is thanks to Dirk Mueller.
According to Greg Hosler <hosler@lugs.org.sg>, these entries need to be added,
otherwise rpm on RH9 will complain that these files are in the payload area,
but not saved to the rpm.
In my big malloc-out-of-core changes of last week, I broke Helgrind by
forgetting to change one of the skin functions. It ran fine until it tried to
generate an error message, whereupon it complained about a missing skin
function. (We should really have more than one regression test for
Helgrind...)
Added two client requests: VALGRIND_COUNT_ERRORS and VALGRIND_COUNT_LEAKS.
The first returns the number of errors found so far, and is a core request.
The second returns the number of bytes found
reachable/dubious/leaked/suppressed by all leak checks so far, for Memcheck and
Addrcheck.
Both are useful for using Valgrind in regression test suites where multiple
tests are present in a single file -- one can run Valgrind with no output
(using --logfile-fd=-1) and use the requests after each test to determine if
any errors happened.
Had to rename and make public vg_n_errs_found --> VG_(n_errs_found) to do so.
Nb: leak errors are not counted as errors for the purposes of
VALGRIND_COUNT_ERRORS. This was decided as the best thing to do after
discussion with Olly Betts, who original suggested these changes.
Pulled out common client request code shared between Memcheck and Addrcheck.
Better way to specify where gdb is -- now GDB_PATH is defined in config.h,
which is global, rather than passing it in with -D to the file that needed it.
-----------------------------------------------------------------------------
overview
-----------------------------------------------------------------------------
Previously Valgrind had its own versions of malloc() et al that replaced
glibc's. This is necessary for various reasons for Memcheck, but isn't needed,
and was actually detrimental, to some other skins. I never managed to treat
this satisfactorily w.r.t the core/skin split.
Now I have. If a skin needs to know about malloc() et al, it must provide its
own replacements. But because this is not uncommon, the core provides a module
vg_replace_malloc.c which a skin can link with, which provides skeleton
definitions, to reduce the amount of work a skin must do. The skeletons handle
the transfer of control from the simd CPU to the real CPU, and also the
--alignment, --sloppy-malloc and --trace-malloc options. These skeleton
definitions subsequently call functions SK_(malloc), SK_(free), etc, which the
skin must define; in these functions the skin can do the things it needs to do
about tracking heap blocks.
For skins that track extra info about malloc'd blocks -- previously done with
ShadowChunks -- there is a new file vg_hashtable.c that implements a
generic-ish hash table (using dodgy C-style inheritance using struct overlays)
which allows skins to continue doing this fairly easily.
Skins can also replace other functions too, eg. Memcheck has its own versions
of strcpy(), memcpy(), etc.
Overall, it's slightly more work now for skins that need to replace malloc(),
but other skins don't have to use Valgrind's malloc(), so they're getting a
"purer" program run, which is good, and most of the remaining rough edges from
the core/skin split have been removed.
-----------------------------------------------------------------------------
details
-----------------------------------------------------------------------------
Moved malloc() et al intercepts from vg_clientfuncs.c into vg_replace_malloc.c.
Skins can link to it if they want to replace malloc() and friends; it does
some stuff then passes control to SK_(malloc)() et al which the skin must
define. They can call VG_(cli_malloc)() and VG_(cli_free)() to do the actual
allocation/deallocation. Redzone size for the client (the CLIENT arena) is
specified by the static variable VG_(vg_malloc_redzone_szB).
vg_replace_malloc.c thus represents a kind of "mantle" level service.
To get automake to build vg_replace_malloc.o, had to resort to a similar trick
as used for the demangler -- ask for a "no install" library (which is never
used) to be built from it.
Note that all malloc, calloc, realloc, builtin_new, builtin_vec_new, memalign
are now aware of --alignment, when running on simd CPU or real CPU.
This means the new_mem_heap, die_mem_heap, copy_mem_heap and ban_mem_heap
events no longer exist, since the core doesn't control malloc() any more, and
skins can watch for these events themselves.
This required moving all the ShadowChunk stuff out of the core, which meant
the sizeof_shadow_block ``need'' could be removed, yay -- it was a horrible
hack. Now ShadowChunks are done with a generic HashTable type, in
vg_hashtable.c, which skins can "inherit from" (in a dodgy C-only fashion by
using structs with similar layouts). Also, the free_list stuff was all moved
as a part of this. Also, VgAllocKind was moved out of core into
Memcheck/Addrcheck and renamed MAC_AllocKind.
Moved these options out of core into vg_replace_malloc.c:
--trace-malloc
--sloppy-malloc
--alignment
The alternative_free ``need'' could go, too, since Memcheck is now in complete
control of free(), yay -- another horribility.
The bad_free and free_mismatch events could go too, since they're now not
detected by core, yay -- yet another horribility.
Moved malloc() et al wrappers for Memcheck out of vg_clientmalloc.c into
mac_malloc_wrappers.c. Helgrind has its own wrappers now too.
Introduced VG_USERREQ__CLIENT_CALL[123] client requests. When a skin function
is operating on the simd CPU, this will call a given function and run it on the
real CPU. The macros VG_NON_SIMD_CALL[123] in valgrind.h present a cleaner
interface to actually use. Also introduce analogues of these that pass 'tst'
from the scheduler as the first arg to the called function -- needed for
MC_(client_malloc)() et al.
Fiddled with USERREQ_{MALLOC,FREE} etc. in vg_scheduler.c; they call
SK_({malloc,free})() which by default call VG_(cli_malloc)() -- can't call
glibc's malloc() here. All the other default SK_(calloc)() etc. instantly
panic; there's a lock variable to ensure that the default SK_({malloc,free})()
are only called from the scheduler, which prevents a skin from forgetting to
override SK_({malloc,free})(). Got rid of the unused USERREQ_CALLOC,
USERREQ_BUILTIN_NEW, etc.
Moved special versions of strcpy/strlen, etc, memcpy() and memchr() into
mac_replace_strmem.c -- they are only necessary for memcheck, because the
hyper-optimised normal glibc versions confuse it, and for memcpy() etc. overlap
checking.
Also added dst/src overlap checks to strcpy(), memcpy(), strcat(). They are
reported not as proper errors, but just with single line warnings, as for silly
args to malloc() et al; this is mainly because they're on the simulated CPU
and proper error handling would be a pain; hopefully they're rare enough to
not be a problem. The strcpy check is done after the copy, because it would
require counting the length of the string beforehand. Also added strncpy() and
strncat(), which have overlap checks too. Note that addrcheck doesn't do
overlap checking.
Put USERREQ__LOGMESSAGE in vg_skin.h to do the overlap check error messages.
After removing malloc() et al and strcpy() et al out of vg_clientfuncs.c, moved
the remaining three things (sigsuspend, VG_(__libc_freeres_wrapper),
__errno_location) into vg_intercept.c, since it contains things that run on the
simulated CPU too. Removed vg_clientfuncs.c altogether.
Moved regression test "malloc3" out of corecheck into memcheck, since corecheck
no longer looks for silly (eg. negative) args to malloc().
Removed the m_eip, m_esp, m_ebp fields from the `Error' type. They were being
set up, and then read immediately only once, only if GDB attachment was done.
So now they're just being held in local variables. This saves 12 bytes per
Error.
Made replacement calloc() check for --sloppy-malloc; previously it didn't.
Added "silly" negative size arg check to realloc(), it didn't have one.
Changed VG_(read_selfprocmaps)() so it can parse the file directly, or from a
previously read buffer. Buffer can be filled with the new
VG_(read_selfprocmaps_contents)(). Using this at start-up to snapshot
/proc/self/maps before the skins do anything, and then parsing it once they
have done their setup stuff. Skins can now safely call VG_(malloc)() in
SK_({pre,post}_clo_init)() without the mmap'd superblock erroneously being
identified as client memory.
Changed the --help usage message slightly, now divided into four sections: core
normal, skin normal, core debugging, skin debugging. Changed the interface for
the command_line_options need slightly -- now two functions, VG_(print_usage)()
and VG_(print_debug_usage)(), and they do the printing themselves, instead of
just returning a string -- that's more flexible.
Removed DEBUG_CLIENTMALLOC code, it wasn't being used and was a pain.
Added a regression test testing leak suppressions (nanoleak_supp), and another
testing strcpy/memcpy/etc overlap warnings (overlap).
Also changed Addrcheck to link with the files shared with Memcheck, rather than
#including the .c files directly.
Commoned up a little more shared Addrcheck/Memcheck code, for the usage
message, and initialisation/finalisation.
Added a Bool param to VG_(unique_error)() dictating whether it should allow
GDB to be attached; for leak checks, because we don't want to attach GDB on
leak errors (causes seg faults). A bit hacky, but it will do.
Had to change lots of the expected outputs from regression files now that
malloc() et al are in vg_replace_malloc.c rather than vg_clientfuncs.c.
Removed support for the 1.0.X series from the regression test suite -- this
was present from before the core/skin split, which is now dead. Means the
script is slightly simpler, and we can dispense with lots of expected
foo.stderr.hd files.
Also undid accidental change to required Automake version in main Makefile.am
from my last commit, whoops.
This commit moves some skin-specific stuff out of core, and generally
neatens other things up.
Also, it adds the --gen-suppressions option for automatically generating
suppressions for each error.
Note that it changes the core/skin interface:
SK_(dup_extra_and_update)() is replaced by SK_(update_extra)(), and
SK_(get_error_name)() and SK_(print_extra_suppression_info)() are added.
-----------------------------------------------------------------------------
details
-----------------------------------------------------------------------------
Removed ac_common.c -- it just #included another .c file; moved the
#include into ac_main.c.
Introduced "mac_" prefixes for files shared between Addrcheck and Memcheck,
to make it clearer which code is shared. Also using a "MAC_" prefix for
functions and variables and types that are shared. Addrcheck doesn't see
the "MC_" prefix at all.
Factored out almost-identical mc_describe_addr() and describe_addr()
(AddrCheck's version) into MAC_(describe_addr)().
Got rid of the "pp_ExeContext" closure passed to SK_(pp_SkinError)(), it
wasn't really necessary.
Introduced MAC_(pp_shared_SkinError)() for the error printing code shared by
Addrcheck and Memcheck. Fixed some bogus stuff in Addrcheck error messages
about "uninitialised bytes" (there because of an imperfect conversion from
Memcheck).
Moved the leak checker out of core (vg_memory.c), into mac_leakcheck.c.
- This meant the hacky way of recording Leak errors, which was different to
normal errors, could be changed to something better: introduced a
function VG_(unique_error)(), which unlike VG_(maybe_record_error)() just
prints the error (unless suppressed) but doesn't record it. Used for
leaks; a much better solution all round as it allowed me to remove a lot
of almost-identical code from leak handling (is_suppressible_leak(),
leaksupp_matches_callers()).
- As part of this, changed the horrible SK_(dup_extra_and_update) into the
slightly less horrible SK_(update_extra), which returns the size of the
`extra' part for the core to duplicate.
- Also renamed it from VG_(generic_detect_memory_leaks)() to
MAC_(do_detect_memory_leaks). In making the code nicer w.r.t suppressions
and error reporting, I tied it a bit more closely to Memcheck/Addrcheck,
and got rid of some of the args. It's not really "generic" any more, but
then it never really was. (This could be undone, but there doesn't seem
to be much point.)
STREQ and STREQN were #defined in several places, and in two different ways.
Made global macros VG_STREQ, VG_CLO_STREQ and VG_CLO_STREQN in vg_skin.h.
Added the --gen-suppressions code. This required adding the functions
SK_(get_error_name)() and SK_(print_extra_suppression_info)() for skins that
use the error handling need.
Added documentation for --gen-suppressions, and fixed some other minor document
problems.
Add and use a configure test to determine if this is an NPTL based
system. Hopefully this will allow it to build out of the box and
(sort-of) work on RH9.