Summary:
There is no need to write out gcdas when forking because we can just reset the counters in the parent process.
Let say a counter is N before the fork, then fork and this counter is set to 0 in the child process.
In the parent process, the counter is incremented by P and in the child process it's incremented by C.
When dump is ran at exit, parent process will dump N+P for the given counter and the child process will dump 0+C, so when the gcdas are merged the resulting counter will be N+P+C.
About exec** functions, since the current process is replaced by an another one there is no need to reset the counters but just write out the gcdas since the counters are definitely lost.
To avoid to have lists in a bad state, we just lock them during the fork and the flush (if called explicitely) and lock them when an element is added.
Reviewers: marco-c
Reviewed By: marco-c
Subscribers: hiraditya, cfe-commits, #sanitizers, llvm-commits, sylvestre.ledru
Tags: #clang, #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74953
Summary:
The number of "inputs have the Data Flow Trace" cannot be greater than
the number of inputs touching the focus function. The existing message is rather
confusing as the same log would mention a greater total number of traces a few
lines above.
Reviewers: kcc, metzman
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74779
The removed loop clears reused for entries at the tail of a SyncClock.
The loop is redundant since those entries were already cleared by the
immediately preceding loop, which iterates over all entries in the
SyncClock (including the tail entries).
Summary:
Most of our larger data is dynamically allocated (via `map`) but it
became an hindrance with regard to init time, for a cost to benefit
ratio that is not great. So change the `TSD`s, `RegionInfo`, `ByteMap`
to be static.
Additionally, for reclaiming, we used mapped & unmapped a buffer each
time, which is costly. It turns out that we can have a static buffer,
and that there isn't much contention on it.
One of the other things changed here, is that we hard set the number
of TSDs on Android to the maximum number, as there could be a
situation where cores are put to sleep and we could miss some.
Subscribers: mgorny, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74696
Summary:
Add a method to set the release to OS value as the system runs,
and allow this to be set differently in the primary and the secondary.
Also, add a default value to use for primary and secondary. This
allows Android to have a default that is different for
primary/secondary.
Update mallopt to support setting the release to OS value.
Reviewers: pcc, cryptoad
Reviewed By: cryptoad
Subscribers: cryptoad, jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74448
Summary: It complains about reaching the end of a non-void returning function.
Reviewers: eugenis, hctim, morehouse
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74578
Summary:
Due to Unity, we had to reduce our region sizes, but in some rare
situations, some programs (mostly tests AFAICT) manage to fill up
a region for a given size class.
So this adds a workaround for that attempts to allocate the block
from the immediately larger size class, wasting some memory but
allowing the application to keep going.
Reviewers: pcc, eugenis, cferris, hctim, morehouse
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74567
The interceptor uses thread-local variables, which (until very recently)
are emu-tls. An access to such variable may call malloc which can
deadlock the runtime library.
For CleanseCrashInput, discards stdout output anyway since it is not used.
These changes are to defend against aggressive PID recycle on windows to reduce the chance of contention on files.
Using pipe instead of file also workaround the problem that when the
process is spawned by llvm-lit, the aborted process keeps a handle to the
output file such that the output file can not be removed. This will
cause random test failures.
https://devblogs.microsoft.com/oldnewthing/20110107-00/?p=11803
Reviewers: kcc, vitalybuka
Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D73329
Add an optional table lookup after the existing logarithm computation
for MidSize < Size <= MaxSize during size -> class lookups. The lookup is
O(1) due to indexing a precomputed (via constexpr) table based on a size
table. Switch to this approach for the Android size class maps.
Other approaches considered:
- Binary search was found to have an unacceptable (~30%) performance cost.
- An approach using NEON instructions (see older version of D73824) was found
to be slightly slower than this approach on newer SoCs but significantly
slower on older ones.
By selecting the values in the size tables to minimize wastage (for example,
by passing the malloc_info output of a target program to the included
compute_size_class_config program), we can increase the density of allocations
at a small (~0.5% on bionic malloc_sql_trace as measured using an identity
table) performance cost.
Reduces RSS on specific Android processes as follows (KB):
Before After
zygote (median of 50 runs) 26836 26792 (-0.2%)
zygote64 (median of 50 runs) 30384 30076 (-1.0%)
dex2oat (median of 3 runs) 375792 372952 (-0.8%)
I also measured the amount of whole-system idle dirty heap on Android by
rebooting the system and then running the following script repeatedly until
the results were stable:
for i in $(seq 1 50); do grep -A5 scudo: /proc/*/smaps | grep Pss: | cut -d: -f2 | awk '{s+=$1} END {print s}' ; sleep 1; done
I did this 3 times both before and after this change and the results were:
Before: 365650, 356795, 372663
After: 344521, 356328, 342589
These results are noisy so it is hard to make a definite conclusion, but
there does appear to be a significant effect.
On other platforms, increase the sizes of all size classes by a fixed offset
equal to the size of the allocation header. This has also been found to improve
density, since it is likely for allocation sizes to be a power of 2, which
would otherwise waste space by pushing the allocation into the next size class.
Differential Revision: https://reviews.llvm.org/D73824
This lets us remove two pointer indirections (one by removing the pointer,
and another by making the AllocatorPtr declaration hidden) in the C++ wrappers.
Differential Revision: https://reviews.llvm.org/D74356
Summary:
Instead of hand-crafting an offset into the structure returned by
dlopen(3) to get at the link map, use the documented API. This is
described in dlinfo(3): by calling it with `RTLD_DI_LINKMAP`, the
dynamic linker ensures the right address is returned.
This is a recommit of 92e267a94d, with
dlinfo(3) expliclity being referenced only for FreeBSD, non-Android
Linux, NetBSD and Solaris. Other OSes will have to add their own
implementation.
Reviewers: devnexen, emaste, MaskRay, krytarowski
Reviewed By: krytarowski
Subscribers: krytarowski, vitalybuka, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73990
Summary:
The refactoring has caused a failure in
http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/29265
The idea of failing the symbolization when the symbolizer bufer is too small
was incorrect. The symbolizer can be invoked for other frames that may fit into
the buffer and get symbolized.
Reviewers: vitalybuka, eugenis
Subscribers: dberris, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74343
Summary:
Instead of hand-crafting an offset into the structure returned by
dlopen(3) to get at the link map, use the documented API. This is
described in dlinfo(3): by calling it with `RTLD_DI_LINKMAP`, the
dynamic linker ensures the right address is returned.
Reviewers: devnexen, emaste, MaskRay, krytarowski
Reviewed By: krytarowski
Subscribers: krytarowski, vitalybuka, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73990
Summary:
Nothing critical, just a few potential improvements I've noticed while reading
the code:
- return `false` when symbolizer buffer is too small to read all data
- invert some conditions to reduce indentation
- prefer `nullptr` over `0` for pointers; init some pointers on stack;
- remove minor code duplication
Reviewers: eugenis, vitalybuka
Subscribers: dberris, #sanitizers, llvm-commits, kcc
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74137
Summary:
This tweaks some behaviors of the allocator wrt 32-bit, notably
tailoring the size-class map.
I had to remove a `printStats` from `__scudo_print_stats` since when
within Bionic they share the same slot so they can't coexist at the
same time. I have to find a solution for that later, but right now we
are not using the Svelte configuration.
Reviewers: rengolin
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74178
Summary:
New shard out of the Check() function for GWP-ASan uses
android_set_abort_message. This is happily present on bionic Android,
but not Android for glibc host x86. Fix up to use the weak definition
always, so we don't have to worry.
Reviewers: eugenis
Reviewed By: eugenis
Subscribers: #sanitizers, llvm-commits, pcc, cferris
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74142
Summary:
GWP-ASan's PRNG didn't use Initial-Exec TLS. Fix that to ensure that we don't
have infinite recursion, and also that we don't allocate a DTV on Android when
GWP-ASan is touched.
Test coverage ensuring that the sample counter is UINT32_MAX for an
uninitialised GWP-ASan is provided by gwp_asan/tests/late_init.cpp.
Reviewers: pcc, cferris
Reviewed By: pcc
Subscribers: #sanitizers, llvm-commits, rprichard, eugenis
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74135
The class is only used in SizeClassAllocator32 in 64-bit mode, but we don't
use that class in 64-bit mode.
Differential Revision: https://reviews.llvm.org/D74099
Summary:
Forewarning: This patch looks big in #LOC changed. I promise it's not that bad, it just moves a lot of content from one file to another. I've gone ahead and left inline comments on Phabricator for sections where this has happened.
This patch:
1. Introduces the crash handler API (crash_handler_api.h).
2. Moves information required for out-of-process crash handling into an AllocatorState. This is a trivially-copied POD struct that designed to be recovered from a deceased process, and used by the crash handler to create a GWP-ASan report (along with the other trivially-copied Metadata struct).
3. Implements the crash handler API using the AllocatorState and Metadata.
4. Adds tests for the crash handler.
5. Reimplements the (now optionally linked by the supporting allocator) in-process crash handler (i.e. the segv handler) using the new crash handler API.
6. Minor updates Scudo & Scudo Standalone to fix compatibility.
7. Changed capitalisation of errors (e.g. /s/Use after free/Use After Free).
Reviewers: cryptoad, eugenis, jfb
Reviewed By: eugenis
Subscribers: merge_guards_bot, pcc, jfb, dexonsmith, mgorny, cryptoad, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73557
Summary:
I tried to move the `madvise` calls outside of one of the secondary
mutexes, but this backfired. There is situation when a low release
interval is set combined with secondary pressure that leads to a race:
a thread can get a block from the cache, while another thread is
`madvise`'ing that block, resulting in a null header.
I changed the secondary race test so that this situation would be
triggered, and moved the release into the cache mutex scope.
Reviewers: cferris, pcc, eugenis, hctim, morehouse
Subscribers: jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74072
Summary:
`sigaltstack` interception is implemented D73816. This updated OpenBSD and Solaris bits to fix the build errors.
Reviewers: eugenis
Reviewed By: eugenis
Subscribers: fedor.sergeev, krytarowski, emaste, eugenis, dberris, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Patch by Igor Sugak.
Differential Revision: https://reviews.llvm.org/D73976
By subtracting 1 from Size at the beginning we can simplify the
subsequent calculations. This also saves 4 instructions on aarch64
and 9 instructions on x86_64, but seems to be perf neutral.
Differential Revision: https://reviews.llvm.org/D73936
Summary:
Fix parsing of mangled stack trace lines where the address has been
replaced with "0x", literally.
Reviewers: vitalybuka
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73823
Summary:
An implementation for `sigaltstack` to make its side effect be visible to MSAN.
```
ninja check-msan
```
Reviewers: vitalybuka, eugenis
Reviewed By: eugenis
Subscribers: dberris, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73816
Patch by Igor Sugak.
As a result of recent changes to the Android size classes, the malloc_free_loop
benchmark started exhausting the 8192 size class at 32768 iterations. To avoid
this problem (and to make the test more realistic), change the benchmark to
use a variety of size classes.
Differential Revision: https://reviews.llvm.org/D73918
Summary:
This patch allows for late initialisation of the GWP-ASan allocator. Previously, if late initialisation occurred, the sample counter was never updated, meaning we would end up having to wait for 2^32 allocations before getting a sampled allocation.
Now, we initialise the sampling mechanism in init() as well. We require init() to be called single-threaded, so this isn't a problem.
Reviewers: eugenis
Reviewed By: eugenis
Subscribers: merge_guards_bot, mgorny, #sanitizers, llvm-commits, cferris
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73896