-----------------------------------------------------------------------------
overview
-----------------------------------------------------------------------------
This commit introduces an optimisation that speeds up Memcheck by roughly
-3 -- 28%, and Addrcheck by 1 -- 36%, at least for the SPEC2000 benchmarks on
my 1400MHz Athlon.
Basic idea: that handling of A/V bit updates on %esp-adjustments was quite
sub-optimal -- for each "PUT ESP", a function was called that computed the
delta from the old and new ESPs, and then called a looping function to deal
with it.
Improvements:
1. most of the time, the delta can be seen from the code. So there's no need
to compute it.
2. when the delta is known, we can directly call a skin function to handle it.
3. we can specialise for certain common cases (eg. +/- 4, 8, 12, 16, 32),
including having unrolled loops for these.
This slightly bloats UCode because of setting up args for the call, and for
updating ESP in code (previously was done in the called C function). Eg. for
`date' the code expansion ratio goes from 14.2 --> 14.6. But it's much faster.
Note that skins don't have to use the specialised cases, they can just
define the ordinary case if they want; the specialised cases are only used
if present.
-----------------------------------------------------------------------------
details
-----------------------------------------------------------------------------
Removed addrcheck/ac_common.c, put its (minimal) contents in ac_main.c.
Updated the major interface version, because this change isn't binary
compatible with the old core/skin interface.
Removed the hooks {new,die}_mem_stack_aligned, replaced with the better
{new,die}_mem_stack_{4,8,12,16,32}. Still have the generic {die,new}_mem_stack
hooks. These are called directly from UCode, thanks to a new pass that occurs
between instrumentation and register allocation (but only if the skin uses
these stack-adjustment hooks). VG_(unknown_esp_update)() is called from UCode
for the generic case; it determines if it's a stack switch, and calls the
generic {new,die}_stack_mem hooks accordingly. This meant
synth_handle_esp_assignment() could be removed.
The new %esp-delta computation phase is in vg_translate.c.
In Memcheck and Addrcheck, added functions for updating the A and V bits of a
single aligned word and a single aligned doubleword. These are called from the
specialised functions new_mem_stack_4, etc. Could remove the one for the old
hooks new_mem_stack_aligned and die_mem_stack_aligned.
In mc_common.h, added a big macro containing the definitions of new_mem_stack_4
et al. It's ``instantiated'' separately by Memcheck and Addrcheck. The macro
is a bit klugey, but I did it that way because speed is vital for these
functions, so eg. a function pointer would have slowed things down.
Updated the built-in profiling events appropriately for the changes (removed
one old event, added a new one; finding their names is left as an exercise for
the reader).
Fixed memory event profiling in {Addr,Mem}check, which had rotted.
Un-break the head -- the LD_ASSUME_KERNEL thing breaks the entire
universe apart from RH9. This (and thus RH9 out-of-the-box support)
is now waiting for a reliable configure test for NPTL-ness.
A minimal set of changes to make it work on Red Hat 9, at least in the
interim. All hats off to Graydon Hoare for this, plus to whoever
devised the LD_ASSUME_KERNEL trapdoor.
This does not provide NPTL support. Instead it turns out we can ask
for the old LinuxThreads interface to be used (wonderful!)
Other than that we have to deal with kernels with SYSINFO pages at the
top of memory. No big deal, apparently.
the following small patch adds support for the (ancient and horrible)
XLAT instruction. Microsoft in their infinite wisdom use it in their OLE
dispatch code... (from Adam Gundy <arg@cyberscience.com>)
Fix a couple of missing MMX instructions. As far as I know, the only
missing MMX insns now are those which move a value from an MMX reg to
an integer reg, and these seem pretty rare. Easy enough to do if anyone
falls across them.
Julian Seward [Wed, 26 Mar 2003 21:08:13 +0000 (21:08 +0000)]
Implement support for the MMX instruction set. The scheme used is
the same as that for FPU instructions. That is, regard the MMX state
(which is the same as the FPU state) opaquely, and every time we
need to do a MMX instruction, move the simulated MMX state into the
real CPU, do the instruction, and move it back. JeremyF's optimisation
to minimise FPU saves/restores applies automatically here.
So, this scheme is simple. It will cause memcheck to complain bitterly
if uninitialised data is copied through the MMX registers, in the same
way that memcheck complains if you move uninit data through the FPU
registers. Whether this turns out to be a problem remains to be seen.
Most instructions are done, and doing the rest is easy enough, I just
need people to send test cases so I can do them on demand.
(Core) UCode has been extended with 7 new uinstrs:
MMX1 MMX2 MMX3
-- 1/2/3 byte mmx insns, no references to
integer regs or memory, copy exactly to the output stream.
MMX_MemRd MMX_MemWr
-- 2 byte mmx insns which read/write memory and therefore need
to have an address register patched in at code generation
time. These are the analogues to FPU_R / FPU_W.
MMX_RegRd MMX_RegWr
-- These have no analogues in FPU land. They hold 2 byte insns
which move data to/from a normal integer register (%eax etc),
and so this has to be made explicit so that (1) a suitable
int reg can be patched in at codegen time, and (2) so that
memcheck can do suitable magic with the V bits going into/
out of the MMX regs.
Nulgrind (ok, this is a nop, but still ...) and AddrCheck's
instrumenters have been extended to cover these new UInstrs. All
others (cachesim, memcheck, lackey, helgrind, did I forget any)
abort when they see any of them. This may be overkill but at least
it ensures we don't forget to implement it in those skins.
[A bad thing would be that some skin silently passes along
MMX uinstrs because of a default: case, when it should actually
do something with them.]
If this works out well, I propose to backport this to 2_0_BRANCH.
Julian Seward [Sat, 15 Mar 2003 20:01:21 +0000 (20:01 +0000)]
startup_segment_callback: Get rid of completely pointless check re
ASSUMED_EXE_BASE. This is now pointless, and causes problems for people
using Clearcase, since it seems their shared objects may not follow
the convention that they end in .so.
Fixed very minor bug with profiling. With cachegrind, if you specified
--profile=yes without having built it for profiling (by #including
vg_profile.c) it gave a "pushcc" panic, instead of a nice explanation.
This is because I assumed no profiling events would be pushed/popped before
VG_(init_profiling)() was called. But cachegrind malloc's some memory in
post_clo_init(), which takes place before VG_(init_profiling)().
So I changed the dummy pushcc/popcc to give the nice error message, instead of
uninformatively panicking.
vg_profile.c, which is #included by skins that want to profile, was using some
core only functions (vg_assert and VG_(core_panic)), which screwed things up.
Replaced them with the skin versions. I don't know how this ever worked.
Added a stderr filter for new_override, that removes the exact numbers for
malloc, because different glibc versions seem to allocate different amounts of
memory.
Julian Seward [Tue, 25 Feb 2003 23:49:46 +0000 (23:49 +0000)]
Make CXXFLAGS be the same as AM_CXXFLAGS, so the correct options are
used to build the regression tests. I don't know if this is really
the correct way to fix this problem. I don't understand why
AM_CXXFLAGS aren't used by default for C++, seeing as how AM_CFLAGS
evidently are used for C sources.
Julian Seward [Mon, 24 Feb 2003 21:55:34 +0000 (21:55 +0000)]
o I fixed a small bug: the code in coregrind/vg_syscalls.c seems
to assume that all sockaddrs are non-NULL and non-zero in
length. This isn't always true, and when I ran a program that
used a NULL sockaddr through Valgrind it segfaulted. I believe
that the change that I made fixes this bug in general, but I
might be overlooking something.
Fixed demangler bug -- it was relying on glibc for some functions. This
triggered an incredibly obscure bug in my experimental skin -- memcpy() was
called within the demangler at (about?) the same time as the dynamic linker was
fiddling with the memcpy() entry, which caused one word of memory (probably
some counter in the dynamic linker) to be incremented, which my skin didn't
like.
So I removed all (AFAICT) of the demangler's dependencies on glibc. This
required adding macros for memset, memcpy, strlen, strcmp..., to replace them
with their VG_(...) version. The only #includes now are to .h files that are
part of Valgrind.
Also required #defining "size_t" as "Int".
Also required adding VG_(memcmp)() to vg_mylibc.c.
Also removed the "-1 == EOF" part of the compile-time test in safe-ctype.h
that checks the character set is ASCII. This was to remove the dependency
on stdio.h. Slightly dodgy, but should be ok I think/hope.
Julian Seward [Sun, 23 Feb 2003 03:54:59 +0000 (03:54 +0000)]
Initial rough hack to spot the stack segment at startup by looking
for a rwx mapping which contains the startup %esp.
Might be better to look for just rw-. Stack might not be executable
if there's a noexec patch, and x86-64 actually enforces the x bit
distinctly from r.
Julian Seward [Sun, 23 Feb 2003 03:00:29 +0000 (03:00 +0000)]
Apply the following:
The veneers for msgrcv in vg_intercept.c and vg_libpthread.c are not
returning the number of bytes read correctly - they always return zero
for any non-error case, which causes programs using msgrcv to behave
somewhat non-optimally when running under valgrind ;-)
Attached is a patch against 1.9.3 which fixes this.
Julian Seward [Sun, 23 Feb 2003 01:25:51 +0000 (01:25 +0000)]
Initial rough hack to spot the stack segment at startup by looking
for a rwx mapping which contains the startup %esp. This should be
more robust than the previous mechanism, which checked a small number
of known places and gave up if none matched. This change is motivated
by Gentoo Linux's high security mode, in which the stack location is
chosen randomly for each new process.
Thanks to Catherine Allen for helping out on this.
Fixed a minor bug -- the condition for determining whether
VG_(handle_esp_assignment)() was needed by a skin (and thus whether to register
it in the baseBlock) was different to that used when determining whether to
call it in code generation... so it could be (attempted to be) called having
not been registered.
Fixed this by consistifying the conditions, using a function
VG_(need_to_handle_esp_assignment)() that is used in both places. The bug
hadn't been found previously because no existing skin exercised the mismatched
conditions in conflicting ways.
Also took VG_(track).post_mem_write out of consideration because it's no longer
important (due to a change in how stack switching is detected).
----
Improved the error message for when a helper can't be found in the baseBlock --
now looks up the debug info to tell you the name of the not-found function.
----
Increased the number of noncompact helpers allowed from 8 to 24
----
Removed a magic number that was hardcoded all over the place, introducing
VG_MAX_REGS_USED for the size of the arrays needed by VG_(get_reg_usage)()
Made VGOFF_(helper_idiv_64_32) and all the similar helper offsets visible to
skins, so they can determine which helper is being called for CALLM
instructions.
Made the setting of VG_(details).avg_translation_sizeB optional, defaulting to
100 bytes (added VG_DEFAULT_TRANS_SIZEB). Took the now-unnecessary settings
out of Nulgrind and CoreCheck. Also made .avg_translation_sizeB a UInt (from
an Int), to avoid possibility of negatives.
Added some functions for skins to access shadow registers:
VG_(get_shadow_archreg), VG_(set_shadow_archreg), VG_(shadow_archreg_address).
Curiously, the only way skins could previously access them was with
VG_(shadow_reg_offset), which wasn't very flexible.
Renamed VG_(nameCondcode)() as VG_(name_UCondcode)() to make it consistent
with similar functions, and made it visible to skins (useful).
Also bumped up the skin interface minor version number due to this change; this
bumping will cover any other binary-compatible changes between now and the next
release (after 1.9.3).
Made more informative the error message that is shown when liveness problems
occur... this is helpful when writing skins, because it's easy for problems
with SK_(instrument)() to screw it up.
- When recording errors, VG_(dup_extra_and_update)() previously was only
called if the 'extra' field was non-NULL. Now it's always called.
This is for two reasons:
a. The 'extra' field could be holding a non-pointer value that just
happens to be 0
b. The skin might want to update the error, even if it doesn't use
the 'extra' field.
A pretty minor change that shouldn't upset anybody.
- Made the ExeContext 'where' field of an error visible to skins, by
adding VG_(get_error_where)(). This can be useful, eg. for comparing
errors for equality.
Julian Seward [Sun, 5 Jan 2003 12:16:30 +0000 (12:16 +0000)]
Increase default bb limit from 1e+12 to 1e+15. The former limit could
easily be reached in ~12 hours flat out computation on a fast machine
with a simple skin. It happened to me.
Julian Seward [Sat, 28 Dec 2002 13:09:57 +0000 (13:09 +0000)]
When ignoring memcheck-style client requests for block permissions
changes, print a message the first 3 times so the user at least knows
these requests are getting ignored. If I was less lazy I would make
these requests -- at least those pertaining to memory addressibility
-- be done properly. But I'm too lazy.
Julian Seward [Sat, 28 Dec 2002 12:55:48 +0000 (12:55 +0000)]
Remove the mechanism which allowed clients to set block permissions
on their stacks and have those blocks automatically cleared when the
stack retreats past them. This never really worked, certainly didn't
work in a multithreaded setting, and slowed everything down due to
having to do even more stuff at %esp changes.
Julian Seward [Sat, 28 Dec 2002 00:19:00 +0000 (00:19 +0000)]
synth_jcond_lit: fix bug in the sequences generated for LE/NLE.
These assumed that ROR sets the P and Z flags and in fact it
sets neither. Add an extra OR insn to really set those flags.
Julian Seward [Sat, 28 Dec 2002 00:04:08 +0000 (00:04 +0000)]
Minor cleanup and verification of the lazy-eflag optimisation stuff.
As of now it is correct, following several hours study.
- Rename upd_cc parameters to simd_flags since that's what they
really mean: does this insn interact at all with %EFLAGS
(the simulated flags) ?
- Have a convention that calls to new_emit which specify
FlagsEmpty for both the def and use sets should pass False
as the simd_flags parameter; this seems more logical than
saying True. From partial evaluation of new_emit with
these args one can see it does nothing under such circumstances,
as one would hope.
- Add an alternative, unused implementation of new_emit in
which the state space is explicitly enumerated. Instructive.
Julian Seward [Thu, 26 Dec 2002 17:10:12 +0000 (17:10 +0000)]
Fix type errors in args to VG_(new_emit). No practical effect, since
(Int)False == (Int)FlagsEmpty, but still.
Whilst hunting (completely unsuccessfully) for some bug causing
MySQL to malfunction with some skins (memcheck), or with most
skins when --single-step=yes.
Julian Seward [Thu, 26 Dec 2002 11:51:50 +0000 (11:51 +0000)]
Fix longstanding bug in make_thread_jump_to_cancelhdlr, which caused
pthread_join to a cancelled thread not return PTHREAD_CANCELED as it
should. This was due to a mix up with stack offsets.
Fitting this into the core/skin split proved very tricky. Problem is
we want to scan the suppressions list to find Leak suppressions, but
- The core code can't do it because LeakSupp is a skin-specific
suppression kind.
- The skin code can't do it because most (all) of the types and
structures for the suppressions are private to the core.
Eventual "solution" (least-worst thing I could think of) is for the
skins using the leak checker to pass it the value of LeakSupp.
Even that isn't really clean because the skins consider it a value
of type MemCheckSuppKind but the core thinks it must be a
CoreSuppKind, and the two are not to be reconciled. So I kludged
around this by casting it to a UInt.
Nick, perhaps you know some way to smooth this out?
Julian Seward [Sun, 22 Dec 2002 19:11:14 +0000 (19:11 +0000)]
vg_scan_all_valid_memory: Don't prod any page unless we're sure we
need to. Poking pages unnecessarily can cause a page fault which
under some rare circumstances can cause the kernel to extend the stack
segment all the way down to the poked page, which is seriously bad.
Fixes a bug shown by --trace-children=yes --skin=addrcheck
--leak-check=yes when running OpenOffice 1.0.1 on SuSE 8.1. This
distro seems to run with no stack limit, which is one prerequisite of
this bug.
Julian Seward [Mon, 16 Dec 2002 00:10:30 +0000 (00:10 +0000)]
Fix bug in MOV Iv,Ev when Ev is a reg. Noticed by Magnus Christensson
<mch@virtutech.se>. The bug lasted this long because there is an
alternative shorter encoding for the same insn which is usually used
in preference.
Julian Seward [Sun, 15 Dec 2002 13:11:39 +0000 (13:11 +0000)]
Change the behaviour of VG_(get_fnname) back to what it was, viz, not
adding offsets ("+N") to the end of function names. Make a new
function VG_(get_fnname_w_offset) with that behaviour and use it for
%y in VG_(printf) et al.
This is needed so that all addresses within a function generate the
same function name. The offset'd behaviour was breaking the cache
profiler and dependent program kcachegrind.
projects / thirdparty / valgrind.git / log
