This has unfortunately become necessary because optimising compilers are
generating 64-bit equality comparisons on partially defined values on this
target. There will shortly be two followup commits which partially mitigate
the resulting performance loss.
This branch contains code which avoids Memcheck false positives resulting from
gcc and clang creating branches on uninitialised data. For example:
bool isClosed;
if (src.isRect(..., &isClosed, ...) && isClosed) {
clang9 -O2 compiles this as:
callq 7e7cdc0 <_ZNK6SkPath6isRectEP6SkRectPbPNS_9DirectionE>
cmpb $0x0,-0x60(%rbp) // "if (isClosed) { .."
je 7ed9e08 // "je after"
test %al,%al // "if (return value of call is nonzero) { .."
je 7ed9e08 // "je after"
..
after:
That is, the && has been evaluated right-to-left. This is a correct
transformation if the compiler can prove that the call to |isRect| returns
|false| along any path on which it does not write its out-parameter
|&isClosed|.
In general, for the lazy-semantics (L->R) C-source-level && operator, we have
|A && B| == |B && A| if you can prove that |B| is |false| whenever A is
undefined. I assume that clang has some kind of interprocedural analysis that
tells it that. The compiler is further obliged to show that |B| won't trap,
since it is now being evaluated speculatively, but that's no big deal to
prove.
A similar result holds, per de Morgan, for transformations involving the C
language ||.
Memcheck correctly handles bitwise &&/|| in the presence of undefined inputs.
It has done so since the beginning. However, it assumes that every
conditional branch in the program is important -- any branch on uninitialised
data is an error. However, this idiom demonstrates otherwise. It defeats
Memcheck's existing &&/|| handling because the &&/|| is spread across two
basic blocks, rather than being bitwise.
This initial commit contains a complete initial implementation to fix that.
The basic idea is to detect the && condition spread across two blocks, and
transform it into a single block using bitwise &&. Then Memcheck's existing
accurate instrumentation of bitwise && will correctly handle it. The
transformation is
<contents of basic block A>
C1 = ...
if (!C1) goto after
.. falls through to ..
<contents of basic block B>
C2 = ...
if (!C2) goto after
.. falls through to ..
after:
===>
<contents of basic block A>
C1 = ...
<contents of basic block B, conditional on C1>
C2 = ...
if (!C1 && !C2) goto after
.. falls through to ..
after:
This assumes that <contents of basic block B> can be conditionalised, at the
IR level, so that the guest state is not modified if C1 is |false|. That's
not possible for all IRStmt kinds, but it is possible for a large enough
subset to make this transformation feasible.
There is no corresponding transformation that recovers an || condition,
because, per de Morgan, that merely corresponds to swapping the side exits vs
fallthoughs, and inverting the sense of the tests, and the pattern-recogniser
as implemented checks all possible combinations already.
The analysis and block-building is performed on the IR returned by the
architecture specific front ends. So they are almost not modified at all: in
fact they are simplified because all logic related to chasing through
unconditional and conditional branches has been removed from them, redone at
the IR level, and centralised.
The only file with big changes is the IRSB constructor logic,
guest_generic_bb_to_IR.c (a.k.a the "trace builder"). This is a complete
rewrite.
There is some additional work for the IR optimiser (ir_opt.c), since that
needs to do a quick initial simplification pass of the basic blocks, in order
to reduce the number of different IR variants that the trace-builder has to
pattern match on. An important followup task is to further reduce this cost.
There are two new IROps to support this: And1 and Or1, which both operate on
Ity_I1. They are regarded as evaluating both arguments, consistent with AndXX
and OrXX for all other sizes. It is possible to synthesise at the IR level by
widening the value to Ity_I8 or above, doing bitwise And/Or, and re-narrowing
it, but this gives inefficient code, so I chose to represent them directly.
The transformation appears to work for amd64-linux. In principle -- because
it operates entirely at the IR level -- it should work for all targets,
providing the initial pre-simplification pass can normalise the block ends
into the required form. That will no doubt require some tuning. And1 and Or1
will have to be implemented in all instruction selectors, but that's easy
enough.
Remaining FIXMEs in the code:
* Rename `expr_is_speculatable` et al to `expr_is_conditionalisable`. These
functions merely conditionalise code; the speculation has already been done
by gcc/clang.
* `expr_is_speculatable`: properly check that Iex_Unop/Binop don't contain
operatins that might trap (Div, Rem, etc).
* `analyse_block_end`: recognise all block ends, and abort on ones that can't
be recognised. Needed to ensure we don't miss any cases.
* maybe: guest_amd64_toIR.c: generate better code for And1/Or1
* ir_opt.c, do_iropt_BB: remove the initial flattening pass since presimp
will already have done it
* ir_opt.c, do_minimal_initial_iropt_BB (a.k.a. presimp). Make this as
cheap as possible. In particular, calling `cprop_BB_wrk` is total overkill
since we only need copy propagation.
* ir_opt.c: once the above is done, remove boolean parameter for `cprop_BB_wrk`.
* ir_opt.c: concatenate_irsbs: maybe de-dup w.r.t. maybe_unroll_loop_BB.
* remove option `guest_chase_cond` from VexControl (?). It was never used.
* convert option `guest_chase_thresh` from VexControl (?) into a Bool, since
the revised code here only cares about the 0-vs-nonzero distinction now.
Necessary changes to support nanoMIPS on Linux.
Part 3/4 - Coregrind and tools changes
Patch by Aleksandar Rikalo, Dimitrije Nikolic, Tamara Vlahovic,
Nikola Milutinovic and Aleksandra Karadzic.
Related KDE issue: #400872.
The commit 3a803036f7 (Allow the user to change a set of command line options
during execution) removed by mistake the code handling the option
--px-file-backed.
Add it back, and modify a trivialleak.vgtest to use the 'VEX registers'
options setting (and their synonym) to do a minimal verification that
the options and synonyms are accepted.
The options are specifying the default values, they should not influence
the result of the test.
Bug (and its origin) reported by Julian.
The addressability of the first byte of a new line was used for the first
byte of the previous line.
Fix by first outputting the line, then getting the addressability of
the first byte of the new line.
This patch changes the option parsing framework to allow a set of
core or tool (currently only memcheck) options to be changed dynamically.
Here is a summary of the new functionality (extracted from NEWS):
* It is now possible to dynamically change the value of many command
line options while your program (or its children) are running under
Valgrind.
To have the list of dynamically changeable options, run
valgrind --help-dyn-options
You can change the options from the shell by using vgdb to launch
the monitor command "v.clo <clo option>...".
The same monitor command can be used from a gdb connected
to the valgrind gdbserver.
Your program can also change the dynamically changeable options using
the client request VALGRIND_CLO_CHANGE(option).
Here is a brief description of the code changes.
* the command line options parsing macros are now checking a 'parsing' mode
to decide if the given option must be handled or not.
(more about the parsing mode below).
* the 'main' command option parsing code has been split in a function
'process_option' that can be called now by:
- early_process_cmd_line_options
(looping over args, calling process_option in mode "Early")
- main_process_cmd_line_options
(looping over args, calling process_option in mode "Processing")
- the new function VG_(process_dynamic_option) called from
gdbserver or from VALGRIND_CLO_CHANGE (calling
process_option in mode "Dynamic" or "Help")
* So, now, during startup, process_option is called twice for each arg:
- once during Early phase
- once during normal Processing
Then process_option can then be called again during execution.
So, the parsing mode is defined so that the option parsing code
behaves differently (e.g. allows or not to handle the option)
depending on the mode.
// Command line option parsing happens in the following modes:
// cloE : Early processing, used by coregrind m_main.c to parse the
// command line options that must be handled early on.
// cloP : Processing, used by coregrind and tools during startup, when
// doing command line options Processing.
// clodD : Dynamic, used to dynamically change options after startup.
// A subset of the command line options can be changed dynamically
// after startup.
// cloH : Help, special mode to produce the list of dynamically changeable
// options for --help-dyn-options.
typedef
enum {
cloE = 1,
cloP = 2,
cloD = 4,
cloH = 8
} Clo_Mode;
The option parsing macros in pub_tool_options.h have now all a new variant
*_CLOM with the mode(s) in which the given option is accepted.
The old variant is kept and calls the new variant with mode cloP.
The function VG_(check_clom) in the macro compares the current mode
with the modes allowed for the option, and returns True if qq_arg
should be further processed.
For example:
// String argument, eg. --foo=yes or --foo=no
(VG_(check_clom) \
(qq_mode, qq_arg, qq_option, \
VG_STREQN(VG_(strlen)(qq_option)+1, qq_arg, qq_option"=")) && \
({const HChar* val = &(qq_arg)[ VG_(strlen)(qq_option)+1 ]; \
if VG_STREQ(val, "yes") (qq_var) = True; \
else if VG_STREQ(val, "no") (qq_var) = False; \
else VG_(fmsg_bad_option)(qq_arg, "Invalid boolean value '%s'" \
" (should be 'yes' or 'no')\n", val); \
True; }))
VG_BOOL_CLOM(cloP, qq_arg, qq_option, qq_var)
To make an option dynamically excutable, it is typically enough to replace
VG_BOOL_CLO(...)
by
VG_BOOL_CLOM(cloPD, ...)
For example:
- else if VG_BOOL_CLO(arg, "--show-possibly-lost", tmp_show) {
+ else if VG_BOOL_CLOM(cloPD, arg, "--show-possibly-lost", tmp_show) {
cloPD means the option value is set/changed during the main command
Processing (P) and Dynamically during execution (D).
Note that the 'body/further processing' of a command is only executed when
the option is recognised and the current parsing mode is ok for this option.
check_preadv2_pwritev2.c: In function ‘main’:
check_preadv2_pwritev2.c:12:12: error: ‘__NR_preadv2’ undeclared (first use in this function)
syscall(__NR_preadv2, 0, NULL, 0, 0, 0);
^
check_preadv2_pwritev2.c:12:12: note: each undeclared identifier is reported only once for each function it appears in
check_preadv2_pwritev2.c:15:12: error: ‘__NR_pwritev2’ undeclared (first use in this function)
syscall(__NR_pwritev2, 0, NULL, 0, 0, 0);
Support for amd64, x86 - 64 and 32 bit, arm64, ppc64, ppc64le,
s390x, mips64. This should work identically on all
arches, tested on x86 32bit and 64bit one, but enabled on all.
Refactor the code to be reusable between old/new syscalls. Resolve TODO
items in the code. Add the testcase for the preadv2/pwritev2 and also
add the (similar) testcase for the older preadv/pwritev syscalls.
Trying to test handling an uninitialized flag argument for the v2 syscalls
does not work because the flag always comes out as defined zero.
Turns out glibc does this deliberately on 64bit architectures because
the kernel does actually have a low_offset and high_offset argument, but
ignores the high_offset/assumes it is zero.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=601cc11d054ae4b5e9b5babec3d8e4667a2cb9b5https://bugs.kde.org/408414
Update expected file for 32-bit platforms.
This is a follow up to:
commit bc758374a2
Author: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date: Sat May 18 12:10:40 2019 +0200
Add a test for the new gdbserver adddress[length] syntax.
Add Iop_Exp2_32Fx4 to VEX/pub/libvex_ir.h to support the 2^x instruction.
Enable the existing test support for the two instructions in
none/tests/ppc64/subnormal_test.c and none/tests/ppc64/jm-insns.c.
https://bugs.kde.org/show_bug.cgi?id=407340
Sync VEX/LICENSE.GPL with top-level COPYING file. We used 3 different
addresses for writing to the FSF to receive a copy of the GPL. Replace
all different variants with an URL <http://www.gnu.org/licenses/>.
The following files might still have some slightly different (L)GPL
copyright notice because they were derived from other programs:
- files under coregrind/m_demangle which come from libiberty:
cplus-dem.c, d-demangle.c, demangle.h, rust-demangle.c,
safe-ctype.c and safe-ctype.h
- coregrind/m_demangle/dyn-string.[hc] derived from GCC.
- coregrind/m_demangle/ansidecl.h derived from glibc.
- VEX files for FMA detived from glibc:
host_generic_maddf.h and host_generic_maddf.c
- files under coregrin/m_debuginfo derived from LZO:
lzoconf.h, lzodefs.h, minilzo-inl.c and minilzo.h
- files under coregrind/m_gdbserver detived from GDB:
gdb/signals.h, inferiors.c, regcache.c, regcache.h,
regdef.h, remote-utils.c, server.c, server.h, signals.c,
target.c, target.h and utils.c
Plus the following test files:
- none/tests/ppc32/testVMX.c derived from testVMX.
- ppc tests derived from QEMU: jm-insns.c, ppc64_helpers.h
and test_isa_3_0.c
- tests derived from bzip2 (with embedded GPL text in code):
hackedbz2.c, origin5-bz2.c, varinfo6.c
- tests detived from glibc: str_tester.c, pth_atfork1.c
- test detived from GCC libgomp: tc17_sembar.c
- performance tests derived from bzip2 or tinycc (with embedded GPL
text in code): bz2.c, test_input_for_tinycc.c and tinycc.c
sys-copy_file_range.c calls open with O_CREAT flag and so must provide
a mode argument. valgrind memcheck actually caught this ommission on
some arches (fedora rawhide i686 specifically).
This is a small additional fixup for
https://bugs.kde.org/show_bug.cgi?id=407218
Running the testsuite as root isn't really recommended.
But lets not make tests fail unnecessarily when running as root.
Similar to the arm64-linux/scalar fixes. Plus 32bit variants that
don't exist on arm64.
Pass really invalid arguments to setuid[32], setgid[32], acct, fchown[32].
Make setresgid[32], setresuid[32], setregid[32], setreuid[32] always succeed.
Running the testsuite as root isn't really recommended.
But lets not make tests fail unnecessarily when running as root.
Pass really invalid arguments to setuid, setgid, acct and fchown.
Make setresgid, setresuid, setregid and setreuid always succeed.
On some systems an extra warning could occur when a variable in
the memcheck/tests/linux/sys-copy_file_range testcase was undefined,
but (accidentially) pointed to known bad memory. Fix by defining the
variable as 0, but then marking it explicitly undefined using memcheck
VALGRIND_MAKE_MEM_UNDEFINED.
Followup for https://bugs.kde.org/show_bug.cgi?id=407218
memcheck/tests/leak_cpp_interior might fail because depending on compiler
and arch used the suppression file used doesn't catch some allocations
through the DT_INIT_ARRAY functions.
That memory doesn't actually leak, it gets freed by the
__gnu_cxx::__freeres hook. But since this testcase shows leaks while
running, not after the program ends, it needs to suppress these allocations.
https://bugs.kde.org/show_bug.cgi?id=406360
On arm64-linux memcheck/tests/leak-segv-jmp would fail because there
was no do_syscall_WRK implementation for that architecture. Implement
and adjust line numbers in .exp file to make it PASS.
GCC 7 instroduced -Wimplicit-fallthrough
https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/
It caught a couple of bugs, but it does need a bit of extra comments to
explain when a switch case statement fall-through is deliberate. Luckily
with -Wimplicit-fallthrough=2 various existing comments already do that.
I have fixed the bugs, but adding explicit break statements where
necessary and added comments where the fall-through was correct.
https://bugs.kde.org/show_bug.cgi?id=405430
Bug 398870 - Please add support for instruction vcvtps2ph
Bug 353370 - RDRAND amd64->IR: unhandled instruction bytes: 0x48 0xF 0xC7 0xF0
This commit implements:
* amd64 RDRAND instruction, on hosts that have it.
* amd64 VCVTPH2PS and VCVTPS2PH, on hosts that have it.
The presence/absence of these on the host is now reflected in the CPUID
results returned to the guest. So code that tests for these features in
CPUID and acts accordingly should "just work".
* New test cases, none/tests/amd64/rdrand and none/tests/amd64/f16c. These
are built if the host's assembler can handle them, in the usual way.
The previous commit 6b16f0e2a0 dated
Sat Jan 26 17:38:01 2019 by Julian Seward <jseward@acm.org> renamed some of
the int<->fp conversion Iops to add a trailing _DEP. The patch missed
renaming two of the Iops. This patch renames the missed Iops.
Secondary architectures on macOS are generally x86, which requires additional
LDFLAGS to be set to avoid linker errors.
apple clang (clang-800.0.42.1) error:
ld: illegal text-relocation to '___stderrp' in /usr/lib/libSystem.dylib from '_main'
in vbit_test_sec-main.o for architecture i386
Fixes: 49ca185 ("Also test memcheck/tests/vbit-test on any secondary arch.")
Signed-off-by: Rhys Kidd <rhyskidd@gmail.com>
2018-Dec-27: some of int<->fp conversion operations have been renamed so as to
have a trailing _DEP, meaning "deprecated". This is because they don't
specify a rounding mode to be used for the conversion and so are
underspecified. Their use should be replaced with equivalents that do specify
a rounding mode, either as a first argument or using a suffix on the name,
that indicates the rounding mode to use.
A few .exp files (not tested on amd64) have to be changed to
have the messages in the new order:
Use --track-origins=yes to see where uninitialised values come from
For lists of detected and suppressed errors, rerun with: -s
Each tool producing errors had identical code to produce this msg.
Factorize the production of the message in m_main.c
This prepares the work to have a specific option to show the list
of detected errors and the count of suppressed errors.
This has a (small) visible effect on the output of memcheck:
Instead of producing
For counts of detected and suppressed errors, rerun with: -v
Use --track-origins=yes to see where uninitialised values come from
memcheck now produces:
Use --track-origins=yes to see where uninitialised values come from
For counts of detected and suppressed errors, rerun with: -v
i.e. the track origin and counts of errors msg are inverted.
pshufb mm/xmm/ymm rearranges byte lanes in vector registers. It's fairly
widely used, but we generated terrible code for it. With this patch, we just
generate, at the back end, pshufb plus a bit of masking, which is a great
improvement.
* changes set_AV_CR6 so that it does scalar comparisons against zero,
rather than sometimes against an all-ones word. This is something
that Memcheck can instrument exactly.
* in Memcheck, requests expensive instrumentation of Iop_Cmp{EQ,NE}64
by default on ppc64le.
https://bugs.kde.org/show_bug.cgi?id=386945#c62
On powerpc partial unaligned loads of vectors from partially invalid
addresses are OK and could be generated by our translation of lxvd2x.
Adjust partial_load memcheck tests to allow partial loads of 16 byte
vectors on powerpc64.
Part of resolving bug #386945.
On powerpc partial unaligned loads of words from partially invalid
addresses are OK and could be generated by our translation of ldbrx.
Adjust partial_load memcheck tests to allow partial loads of words
on powerpc64.
Part of resolving bug #386945.
This happens when processing openssl aes_v8_set_encrypt_key
(aesv8-armx.S:133). The noteTmpUsesIn () function is new since
PR387664 Memcheck: make expensive-definedness-checks be the default.
It didn't handle Iex_VECRET which is used in the arm64 crypto
instruction dirty handlers.
glibc 2.28 filters out some bad signal numbers and returns
Invalid argument instead of passing such bad signal numbers
the kernel sigaction syscall. So we won't see such bad signal
numbers and won't print "bad signal number" ourselves.
Add a new memcheck/tests/sigkill.stderr.exp-glibc-2.28 to catch
this case.
- The option --xtree-leak=yes (to output leak result in xtree format)
automatically activates the option --show-leak-kinds=all,
as xtree visualisation tools such as kcachegrind can in any case
select what kind of leak to visualise.
memcheck/mc_translate.c:
Add mkRight{32,64} as right-travelling analogues to mkLeft{32,64}.
doCmpORD: for the cases of a signed comparison against zero, compute
definedness of the 3 result bits (lt,gt,eq) separately, and, for the lt and eq
bits, do it exactly accurately.
expensiveCountTrailingZeroes: no functional change. Re-analyse/verify and add
comments.
expensiveCountLeadingZeroes: add. Very similar to
expensiveCountTrailingZeroes.
Add some comments to mark unary ops which are self-shadowing.
Route Iop_Ctz{,Nat}{32,64} through expensiveCountTrailingZeroes.
Route Iop_Clz{,Nat}{32,64} through expensiveCountLeadingZeroes.
Add instrumentation for Iop_PopCount{32,64} and Iop_Reverse8sIn32_x1.
memcheck/tests/vbit-test/irops.c
Add dummy new entries for all new IROps, just enough to make it compile and
run.
This adds z/Architecture vector integer and string instruction support.
The main author of this patch is Vadim Barkov <vbrkov@gmail.com>. Some
fixes were provided by Andreas Arnez <arnez@linux.ibm.com>.
When the clone syscall was refactored to work across all linux arches
the arguments were checked in a different order. Fix the arm64-linux
scalar.stderr.exp to match the same order for the (invalid) clone
arguments.
This makes memcheck/tests/arm64-linux/scalar.vgtest pass again.
memcheck/tests/ppc64/power_ISA2_0[57] could spuriously fail when
some internal glibc function would allocate and free some memory.
To get the expected output run the tests with -q and clear stderr.exp.
* coregrind/m_redir.c: whitespace changes only
* memcheck/mc_main.c:
- change 6 guards of the form "defined (VGABI_N32)" to
"defined(VGA_mips64) && defined(VGABI_N32)"
- Fix up poor indentation
