Summary:
A script to symbolize hwasan reports after the fact using unstripped
binaries. Supports stack-based reports. Requires llvm-symbolizer
(addr2line is not an option).
Reviewers: pcc, hctim
Subscribers: mgorny, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D71148
Summary:
GWP-ASan test currently fail on 32-bit platforms, as some of the pointers are
larger than `uintptr_t` on 32-bit platforms. Fix up all those instances.
Also add an uncompress varint test where the result is an underflow.
Furthermore, allow multi-init for testing. Each gtest when running
`check-gwp_asan` apparently runs in its own instance, but when integrating
these tests into Android, this behaviour isn't the same. We remove the
global multi-init check here, to allow for testing to work elsewhere, and we're
not really worried about multi-init anyway as it's part of our contract with
the allocator.
Reviewers: eugenis, vlad.tsyrklevich
Reviewed By: eugenis
Subscribers: #sanitizers, llvm-commits, pcc
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D71121
Summary:
Counters can be flushed in a multi-threaded context for example when the process is forked in different threads (https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp#L632-L663).
In order to avoid pretty bad things, a critical section is needed around the flush.
We had a lot of crashes in this code in Firefox CI when we switched to clang for linux ccov builds and those crashes disappeared with this patch.
Reviewers: marco-c, froydnj, dmajor, davidxl
Reviewed By: marco-c, dmajor
Subscribers: froydnj, dmajor, dberris, jfb, #sanitizers, llvm-commits, sylvestre.ledru
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70910
The test ScudoWrappersCTest.Realloc expects realloc of memalign to work on
Android, but this relies on dealloc_type_mismatch being set to false. Commit
0d3d4d3b0 caused us to start setting it to true in the C wrapper tests,
which broke the test. Set it to the correct value on Android.
Differential Revision: https://reviews.llvm.org/D71078
The Android headers don't provide a declaration of valloc or pvalloc, so we
need to declare them ourselves.
Differential Revision: https://reviews.llvm.org/D71077
Summary:
In order to be compliant with tcmalloc's extension ownership
determination function, we have to expose a function that will
say if a chunk was allocated by us.
As to whether or not this has security consequences: someone
able to call this function repeatedly could use it to determine
secrets (cookie) or craft a valid header. So this should not be
exposed directly to untrusted user input.
Add related tests.
Additionally clang-format caught a few things to change.
Reviewers: hctim, pcc, cferris, eugenis, vitalybuka
Subscribers: JDevlieghere, jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70908
This test was previously effectively doing:
P = malloc(X); write X bytes to P; P = realloc(P, X - Y); P = realloc(P, X)
and expecting that all X bytes stored to P would still be identical after
the final realloc.
This happens to be true for the current scudo implementation of realloc,
but is not guaranteed to be true by the C standard ("Any bytes in the new
object beyond the size of the old object have indeterminate values.").
This implementation detail will change with the new memory tagging support,
which unconditionally zeros newly allocated granules when memory tagging
is enabled. Fix this by limiting the number of bytes that we test to the
minimum size that we realloc the allocation to.
Differential Revision: https://reviews.llvm.org/D70761
The macros INLINE and COMPILER_CHECK always expand to the same thing (inline
and static_assert respectively). Both expansions are standards compliant C++
and are used consistently in the rest of LLVM, so let's improve consistency
with the rest of LLVM by replacing them with the expansions.
Differential Revision: https://reviews.llvm.org/D70793
Otherwise, we will hit a use-after-free when testing multiple instances of
the same allocator on the same thread. This only recently became a problem
with D70552 which caused us to run both ScudoCombinedTest.BasicCombined and
ScudoCombinedTest.ReleaseToOS on the unit tests' main thread.
Differential Revision: https://reviews.llvm.org/D70760
Shadow memory (and short granules) are not prepended with memory
address and arrow at the end of line is removed.
Differential Revision: https://reviews.llvm.org/D70707
Summary:
This CL makes unit tests compatible with Fuchsia's zxtest. This
required a few changes here and there, but also unearthed some
incompatibilities that had to be addressed.
A header is introduced to allow to account for the zxtest/gtest
differences, some `#if SCUDO_FUCHSIA` are used to disable incompatible
code (the 32-bit primary, or the exclusive TSD).
It also brought to my attention that I was using
`__scudo_default_options` in different tests, which ended up in a
single binary, and I am not sure how that ever worked. So move
this to the main cpp.
Additionally fully disable the secondary freelist on Fuchsia as we do
not track VMOs for secondary allocations, so no release possible.
With some modifications to Scudo's BUILD.gn in Fuchsia:
```
[==========] 79 tests from 23 test cases ran (10280 ms total).
[ PASSED ] 79 tests
```
Reviewers: mcgrathr, phosek, hctim, pcc, eugenis, cferris
Subscribers: srhines, jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70682
See PR43425:
https://bugs.llvm.org/show_bug.cgi?id=43425
When writing profile data on Windows we were opening profile file with
exclusive read/write access.
In case we are trying to write to the file from multiple processes
simultaneously, subsequent calls to CreateFileA would return
INVALID_HANDLE_VALUE.
To fix this, I changed to open without exclusive access and then take a
lock.
Patch by Michael Holman!
Differential revision: https://reviews.llvm.org/D70330
Summary:
As mentioned in D69104, glibc changed ABI recently with the [[ https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=2f959dfe849e0646e27403f2e4091536496ac0f0| 2f959dfe ]] change.
D69104 dealt with just 32-bit ARM, but that is just one of the many affected architectures.
E.g. x86_64, i?86, riscv64, sparc 32-bit, s390 31-bit are affected too (and various others).
This patch instead of adding a long list of further architectures that wouldn't be checked ever next to arm 32-bit changes the structures to match the 2.31 layout and performs the checking on Linux for ipc_perm mode position/size only on non-Linux or on Linux with glibc 2.31 or later. I think this matches what is done for aarch64 already.
If needed, we could list architectures that haven't changed ABI (e.g. powerpc), so that they would be checked even with older glibcs. AFAIK sanitizers don't actually use ipc_perm.mode and
so all they care about is the size and alignment of the whole structure.
Note, s390 31-bit and arm 32-bit big-endian changed ABI even further, there will now be shmctl with old symbol version and shmctl@@GLIBC_2.31 which will be incompatible. I'm afraid this isn't really solvable unless the sanitizer libraries are symbol versioned and use matching symbol versions to glibc symbols for stuff they intercept, plus use dlvsym.
This patch doesn't try to address that.
Patch by Jakub Jelinek.
Reviewers: kcc, eugenis, dvyukov
Reviewed By: eugenis
Subscribers: jyknight, kristof.beyls, fedor.sergeev, simoncook, PkmX, s.egerton, steven.zhang, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70662
Implements __fixtfti builtin for PowerPC. This builtin converts a
long double (IBM double-double) to a signed int128. The conversion relies on
the unsigned conversion of the absolute value of the long double.
Tests included for both positive and negative long doubles.
Patch By: Baptiste Saleil
Differential Revision: https://reviews.llvm.org/D69730
Summary:
When platforms use their own `LIKELY()` definitions, it can be quite
troublesome to ensure they don't conflict with the GWP-ASan internal
definitions. Just force the GWP_ASAN_ prefix to help this issue.
Reviewers: eugenis
Reviewed By: eugenis
Subscribers: #sanitizers, llvm-commits, cferris, pcc
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70683
Summary:
cferris@ found an issue where calling `releaseToOS` prior to any other
heap operation would lead to a crash, due to the allocator not being
properly initialized (it was discovered via `mallopt`).
The fix is to call `initThreadMaybe` prior to calling `releaseToOS` for
the Primary.
Add a test that crashes prior to fix.
Reviewers: hctim, cferris, pcc, eugenis
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70552
This header fragment is useful on its own for any consumer that wants
to use custom instruction profile runtime with the LLVM instrumentation.
The concrete use case is in Fuchsia's kernel where we want to use
instruction profile instrumentation, but we cannot use the compiler-rt
runtime because it's not designed for use in the kernel environment.
This change allows installing this header as part of compiler-rt.
Differential Revision: https://reviews.llvm.org/D64532
This header fragment is useful on its own for any consumer that wants
to use custom instruction profile runtime with the LLVM instrumentation.
The concrete use case is in Fuchsia's kernel where we want to use
instruction profile instrumentation, but we cannot use the compiler-rt
runtime because it's not designed for use in the kernel environment.
This change allows installing this header as part of compiler-rt.
Differential Revision: https://reviews.llvm.org/D64532
Summary:
This commit moves the `DiscardOutput` function in FuzzerIO to
FuzzerUtil, so fuchsia can have its own specialized version.
In fuchsia, accessing `/dev/null` is not supported, and there's nothing
similar to a file that discards everything that is written to it. The
way of doing something similar in fuchsia is by using `fdio_null_create`
and binding that to a file descriptor with `fdio_bind_to_fd`.
This change should fix one of the issues with the `-close_fd_mask` flag
in libfuzzer, in which closing stdout was not working due to
`fopen("/dev/null", "w")` returning `NULL`.
Reviewers: kcc, aarongreen
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69593
Summary:
This commit fixes part of the issues with stack unwinding in fuchsia for
arm64 and x86_64. It consists of multiple fixes:
(1) The cfa_offset calculation was wrong, instead of pointing to the
previous stack pointer, it was pointing to the current one. It worked in
most of the cases because the crashing functions already had a
prologue and had their cfa information relative to another register. The
fix consists on adding a constant that can be used to calculate the
crashing function's stack pointer, and base all the cfi information
relative to that offset.
(2) (arm64) Due to errors with the syntax for the dwarf information, most
of the `OP_NUM` macros were not working. The problem was that they were
referred to as `r##NUM` (like `r14`), when it should have been `x##num`
(like `x14`), or even without the x.
(3) (arm64) The link register was being considered a part of the main
registers (`r30`), when in the real struct it has its own field. Given
that the link register is in the same spot in the struct as r[30] would be,
and that C++ doesn't care about anything, the calculation was still correct.
(4) (x86_64) The stack doesn't need to be aligned to 16 bytes when we
jump to the trampoline function, but it needs to be before performing
call instructions. Encoding that logic in cfi information was tricky, so
we decided to make the cfa information relative to `rbp` and align `rsp`.
Note that this could have been done using another register directly,
but it seems cleaner to make a new fake stack frame.
There are some other minor changes like adding a `brk 1` instruction in
arm64 to make sure that we never return to the crash trampoline (similar to
what we do in x86_64).
Sadly this commit does not fix unwinding for all use cases for arm64.
Crashing functions that do not add information related to the return column in
their cfi information will fail to unwind due to a bug in libunwinder.
Reviewers: mcgrathr, jakehehrlich, phosek, kcc, aarongreen
Subscribers: aprantl, kristof.beyls, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69579
Summary:
This commit fixes the check in the return value from the `DuplicateFile`
function, which returns a new file descriptor. `DuplicateFile` can
return 0 if that file descriptor is available (for example, if stdin has
already been closed).
In particular, this could cause a bug with the `-close_fd_mask` flag in
some platforms: just call the fuzzer with stdin closed and the
`-close_fd_mask=2` flag, and stderr will not be muted.
Example fuzzer:
```
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* Data, size_t Size) {
fprintf(stderr, "STDERR\n");
fprintf(stdout, "STDOUT\n");
return 0;
}
```
Invocation (muting both stderr and stdout):
```
./test -close_fd_mask=3 -runs=1 0<&-
INFO: Seed: 1155116940
INFO: Loaded 1 modules (1 inline 8-bit counters): 1 [0x48b020, 0x48b021),
INFO: Loaded 1 PC tables (1 PCs): 1 [0x478dc8,0x478dd8),
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
STDERR
INFO: A corpus is not provided, starting from an empty corpus
STDERR
Done 2 runs in 0 second(s)
```
Reviewers: mcgrathr, jakehehrlich, phosek, kcc, aarongreen
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D68775
Summary:
A few small improvements and optimizations:
- when refilling the free list, push back the last batch and return
the front one: this allows to keep the allocations towards the front
of the region;
- instead of using 48 entries in the shuffle array, use a multiple of
`MaxNumCached`;
- make the maximum number of batches to create on refil a constant;
ultimately it should be configurable, but that's for later;
- `initCache` doesn't need to zero out the cache, it's already done.
- it turns out that when using `||` or `&&`, the compiler is adamant
on adding a short circuit for every part of the expression. Which
ends up making somewhat annoying asm with lots of test and
conditional jump. I am changing that to bitwise `|` or `&` in two
place so that the generated code looks better. Added comments since
it might feel weird to people.
This yields to some small performance gains overall, nothing drastic
though.
Reviewers: hctim, morehouse, cferris, eugenis
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70452
Darwin lazily saves the AVX512 context on first use [1]: instead of checking
that it already does to figure out if the OS supports AVX512, trust that
the kernel will do the right thing and always assume the context save
support is available.
[1] https://github.com/apple/darwin-xnu/blob/xnu-4903.221.2/osfmk/i386/fpu.c#L174
Reviewers: ab, RKSimon, craig.topper
Reviewed By: craig.topper
Subscribers: dberris, JDevlieghere, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70454
When we're not building libc's allocator, just use a regular TLS variable. This
lets the unit tests pass on Android devices whose libc uses Scudo. Otherwise
libc's copy of Scudo and the unit tests' copy will both try to use the same
TLS slot, in likely incompatible ways.
This requires using ELF TLS, so start passing -fno-emulated-tls when building
the library and the unit tests on Android.
Differential Revision: https://reviews.llvm.org/D70472
Continuous mode is not yet supported on Fuchsia/Windows, however an
error should not be reported unless the user attempted to actually
enable continuous mode.
Summary:
The sanitizer symbolizers support printing the function offset
(difference between pc and function start) of a stackframe using the
`%q` format specifier.
Unfortunately this didn't actually work because neither the atos
or dladdr symbolizer set the `AddressInfo::function_offset` field.
This patch teaches both symbolizers to try to compute the function
offset. In the case of the atos symbolizer, atos might not report the
function offset (e.g. it reports a source location instead) so in this
case it fallsback to using `dladdr()` to compute the function offset.
Two test cases are included.
rdar://problem/56695185
Reviewers: kubamracek, yln
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69549
Make it possible to use online profile merging ("%m" mode) with
continuous sync ("%c" mode).
To implement this, the merged profile is locked in the runtime
initialization step and either a) filled out for the first time or b)
checked for compatibility. Then, the profile can simply be mmap()'d with
MAP_SHARED set. With the mmap() in place, counter updates from every
process which uses an image are mapped onto the same set of physical
pages assigned by the filesystem cache. After the mmap() is set up, the
profile is unlocked.
Differential Revision: https://reviews.llvm.org/D69586
Summary:
`SCUDO_DEBUG` was not enabled for unit tests, meaning the `DCHECK`s
were never tripped. While turning this on, I discovered that a few
of those not-exercised checks were actually wrong. This CL addresses
those incorrect checks.
Not that to work in tests `CHECK_IMPL` has to explicitely use the
`scudo` namespace. Also changes a C cast to a C++ cast.
Reviewers: hctim, pcc, cferris, eugenis, vitalybuka
Subscribers: mgorny, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70276
Summary:
Previously it wasn't obvious what the default value of various sanitizer
options were. A very close approximation of the "default values" for the
options are the current value of the options at the time of printing the
help output.
In the case that no other options are provided then the current values
are the default values (apart from `help`).
```
ASAN_OPTIONS=help=1 ./program
```
This patch causes the current option values to be printed when the
`help` output is enabled. The original intention for this patch was to append
`(Default: <value>)` to an option's help text. However because this
is technically wrong (and misleading) I've opted to append
`(Current Value: <value>)` instead.
When trying to implement a way of displaying the default value of the
options I tried another solution where the default value used in `*.inc` files
were used to create compile time strings that where used when printing
the help output. This solution was not satisfactory for several reasons:
* Stringifying the default values with the preprocessor did not work very
well in several cases. Some options contain boolean operators which no
amount of macro expansion can get rid of.
* It was much more invasive than this patch. Every sanitizer had to be changed.
* The settings of `__<sanitizer>_default_options()` are ignored.
For those reasons I opted for the solution in this patch.
rdar://problem/42567204
Reviewers: kubamracek, yln, kcc, dvyukov, vitalybuka, cryptoad, eugenis, samsonov
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69546
Split out the logic to get the size of a merged profile and to do a
compatibility check. This can be shared with both the continuous+merging
mode implementation, as well as the runtime-allocated counters
implementation planned for Fuchsia.
Lifted out of D69586.
Differential Revision: https://reviews.llvm.org/D70135
A few years back a similar change was made for getenv since neither function is supported on the PS4 platform.
Recently, commit d889d1e added a call to setenv in compiler-rt which was causing linking errors because the symbol was not found. This fixes that issue by putting in a shim similar to how we previously dealt with the lack of getenv.
Differential Revision: https://reviews.llvm.org/D70033
Summary:
This patch fixes two problems with the crtbegin.c as written:
1. In do_init, register_frame_info is not guarded by a #define, but in
do_fini, deregister_frame_info is guarded by #ifndef
CRT_HAS_INITFINI_ARRAY. Thus when CRT_HAS_INITFINI_ARRAY is not
defined, frames are registered but then never deregistered.
The frame registry mechanism builds a linked-list from the .so's
static variable do_init.object, and when the .so is unloaded, this
memory becomes invalid and should be deregistered.
Further, libgcc's crtbegin treats the frame registry as independent
from the initfini array mechanism.
This patch fixes this by adding a new #define,
"EH_USE_FRAME_INFO_REGISTRY", which is set by the cmake option
COMPILER_RT_CRT_USE_EH_FRAME_REGISTRY Currently, do_init calls
register_frame_info, and then calls the binary's constructors. This
allows constructors to safely use libunwind. However, do_fini calls
deregister_frame_info and then calls the binary's destructors. This
prevents destructors from safely using libunwind.
This patch also switches that ordering, so that destructors can safely
use libunwind. As it happens, this is a fairly common scenario for
thread sanitizer.
Summary: Bionic was modified to have all function names consistent. Modify the code and get rid of the special case for bionic since it's no longer needed.
Reviewers: cryptoad
Reviewed By: cryptoad
Subscribers: srhines, llvm-commits, #sanitizers
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70031
__fixunstfti converts a long double (IBM double-double) to an unsigned 128 bit
integer. This patch enables it to handle a previously unhandled case in which
a negative low double may impact the result of the conversion.
Collaborated with @masoud.ataei and @renenkel.
Patch By: Baptiste Saleil
Differential Revision: https://reviews.llvm.org/D69193
https://bugs.llvm.org/show_bug.cgi?id=43722
GWP-ASan didn't include SANITIZER_COMMON_CFLAGS, and thus would produce
LLVM bitcode files, when compiler-rt is generally built without LTO.
This was an experiment made possible by a non-standard feature of the
Android dynamic loader.
It required introducing a flag to tell the compiler which ABI was being
targeted.
This flag is no longer needed, since the generated code now works for
both ABI's.
We leave that flag untouched for backwards compatibility. This also
means that if we need to distinguish between targeted ABI's again
we can do that without disturbing any existing workflows.
We leave a comment in the source code and mention in the help text to
explain this for any confused person reading the code in the future.
Patch by Matthew Malcomson
Differential Revision: https://reviews.llvm.org/D69574
This commit added use of a Windows API in InstrProfilingPort.h.
When _MSC_VER is defined (for MSVC), windows.h is already included
earlier in the same header (for atomics), but MinGW, the gcc
atomics builtins are used instead. Therefore explicitly include
windows.h here, where the API is used.
Use the printf macros from inttypes.h to sidestep -Wformat issues:
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/lib/profile/InstrProfilingFile.c:425:14: error: format specifies type 'long long' but the argument has type 'off_t' (aka 'long') [-Werror,-Wformat]
CurrentFileOffset, PageSize);
^~~~~~~~~~~~~~~~~
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/lib/profile/InstrProfilingPort.h:114:50: note: expanded from macro 'PROF_ERR'
fprintf(stderr, "LLVM Profile Error: " Format, __VA_ARGS__);
~~~~~~ ^~~~~~~~~~~
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/lib/profile/InstrProfilingFile.c:461:41: error: format specifies type 'unsigned long long' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
strerror(errno), CountersBegin, PageAlignedCountersLength, Fileno,
^~~~~~~~~~~~~~~~~~~~~~~~~
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/lib/profile/InstrProfilingPort.h:114:50: note: expanded from macro 'PROF_ERR'
fprintf(stderr, "LLVM Profile Error: " Format, __VA_ARGS__);
~~~~~~ ^~~~~~~~~~~
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/lib/profile/InstrProfilingFile.c:462:9: error: format specifies type 'unsigned long long' but the argument has type 'uint64_t' (aka 'unsigned long') [-Werror,-Wformat]
FileOffsetToCounters);
^~~~~~~~~~~~~~~~~~~~
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/lib/profile/InstrProfilingPort.h:114:50: note: expanded from macro 'PROF_ERR'
fprintf(stderr, "LLVM Profile Error: " Format, __VA_ARGS__);
VLAs in C appear to not work on Windows, so use COMPILER_RT_ALLOCA:
C:\b\slave\sanitizer-windows\llvm-project\compiler-rt\lib\profile\InstrProfilingWriter.c(264): error C2057: expected constant expression
C:\b\slave\sanitizer-windows\llvm-project\compiler-rt\lib\profile\InstrProfilingWriter.c(264): error C2466: cannot allocate an array of constant size 0
C:\b\slave\sanitizer-windows\llvm-project\compiler-rt\lib\profile\InstrProfilingWriter.c(264): error C2133: 'Zeroes': unknown size
Add support for continuously syncing profile counter updates to a file.
The motivation for this is that programs do not always exit cleanly. On
iOS, for example, programs are usually killed via a signal from the OS.
Running atexit() handlers after catching a signal is unreliable, so some
method for progressively writing out profile data is necessary.
The approach taken here is to mmap() the `__llvm_prf_cnts` section onto
a raw profile. To do this, the linker must page-align the counter and
data sections, and the runtime must ensure that counters are mapped to a
page-aligned offset within a raw profile.
Continuous mode is (for the moment) incompatible with the online merging
mode. This limitation is lifted in https://reviews.llvm.org/D69586.
Continuous mode is also (for the moment) incompatible with value
profiling, as I'm not sure whether there is interest in this and the
implementation may be tricky.
As I have not been able to test extensively on non-Darwin platforms,
only Darwin support is included for the moment. However, continuous mode
may "just work" without modification on Linux and some UNIX-likes. AIUI
the default value for the GNU linker's `--section-alignment` flag is set
to the page size on many systems. This appears to be true for LLD as
well, as its `no_nmagic` option is on by default. Continuous mode will
not "just work" on Fuchsia or Windows, as it's not possible to mmap() a
section on these platforms. There is a proposal to add a layer of
indirection to the profile instrumentation to support these platforms.
rdar://54210980
Differential Revision: https://reviews.llvm.org/D68351
Summary:
cferris@ found an issue due to the new Secondary free list behavior
and unfortunately it's completely my fault. The issue is twofold:
- I lost track of the (major) fact that the Combined assumes that
all chunks returned by the Secondary are zero'd out apprioriately
when dealing with `ZeroContents`. With the introduction of the
freelist, it's no longer the case as there can be a small portion
of memory between the header and the next page boundary that is
left untouched (the rest is zero'd via release). So the next time
that block is returned, it's not fully zero'd out.
- There was no test that would exercise that behavior :(
There are several ways to fix this, the one I chose makes the most
sense to me: we pass `ZeroContents` to the Secondary's `allocate`
and it zero's out the block if requested and it's coming from the
freelist. The prevents an extraneous `memset` in case the block
comes from `map`. Another possbility could have been to `memset`
in `deallocate`, but it's probably overzealous as all secondary
blocks don't need to be zero'd out.
Add a test that would have found the issue prior to fix.
Reviewers: morehouse, hctim, cferris, pcc, eugenis, vitalybuka
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69675
Summary:
Sometimes an allocation stack trace is not very informative. Provide a
way to replace it with a stack trace of the user's choice.
Reviewers: pcc, kcc
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69208
Summary:
Previously the CMake code looked for filepaths of the form
`<arch>/<filename>` as an indication that `<arch>/<filename>` provided a
specialization of a top-level file `<filename>`. For powerpc there was a
bug because the powerpc specialized implementations lived in `ppc/` but
the architectures were `powerpc64` and `powerpc64le` which meant that
CMake was looking for files at `powerpc64/<filename>` and
`powerpc64le/<filename>`.
The result of this is that for powerpc the builtins library contained a
duplicate symbol for `divtc3` because it had the generic implementation
and the specialized version in the built static library.
Although we could just add similar code to what there is for arm (i.e.
compute `${_arch}`) to fix this, this is extremely error prone (until
r375150 no error was raised). Instead this patch takes a different
approach that removes looking for the architecture name entirely.
Instead this patch uses the convention that a source file in a
sub-directory might be a specialization of a generic implementation and
if a source file of the same name (ignoring extension) exists at the
top-level then it is the corresponding generic implementation. This
approach is much simpler because it doesn't require keeping track of
different architecture names.
This convention already existed in repository but previously it was
implicit. This change makes it explicit.
This patch is motivated by wanting to revert r375162 which worked around
the powerpc bug found when r375150 landed.
Once it lands we should revert r375162.
Reviewers: phosek, beanz, compnerd, shiva0217, amyk, rupprecht, kongyi, mstorsjo, t.p.northover, weimingz, jroelofs, joerg, sidneym
Subscribers: nemanjai, mgorny, kristof.beyls, jsji, shchenz, steven.zhang, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D69189
Summary:
The flag allows the user to specify a maximum allocation size that the
sanitizers will honor. Any larger allocations will return nullptr or
crash depending on allocator_may_return_null.
Reviewers: kcc, eugenis
Reviewed By: kcc, eugenis
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69576
Summary:
The secondary allocator is slow, because we map and unmap each block
on allocation and deallocation.
While I really like the security benefits of such a behavior, this
yields very disappointing performance numbers on Android for larger
allocation benchmarks.
So this change adds a free list to the secondary, that will hold
recently deallocated chunks, and (currently) release the extraneous
memory. This allows to save on some memory mapping operations on
allocation and deallocation. I do not think that this lowers the
security of the secondary, but can increase the memory footprint a
little bit (RSS & VA).
The maximum number of blocks the free list can hold is templatable,
`0U` meaning that we fallback to the old behavior. The higher that
number, the higher the extra memory footprint.
I added default configurations for all our platforms, but they are
likely to change in the near future based on needs and feedback.
Reviewers: hctim, morehouse, cferris, pcc, eugenis, vitalybuka
Subscribers: mgorny, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69570
Summary:
The hwasan interceptor ABI doesn't have interceptors for longjmp and setjmp.
This patch introduces them.
We require the size of the jmp_buf on the platform to be at least as large as
the jmp_buf in our implementation. To enforce this we compile
hwasan_type_test.cpp that ensures a compile time failure if this is not true.
Tested on both GCC and clang using an AArch64 virtual machine.
Reviewers: eugenis, kcc, pcc, Sanatizers
Reviewed By: eugenis, Sanatizers
Tags: #sanatizers, #llvm
Differential Revision: https://reviews.llvm.org/D69045
Patch By: Matthew Malcomson <matthew.malcomson@arm.com>
Summary:
When the ABI namespace isn't a reserved identifier, we were issuing a
warning, but this should have been an error since the beginning. This
commit enforces that the ABI namespace is a reserved identifier, and
changes the ABI namespace used by LibFuzzer.
Reviewers: phosek, EricWF
Subscribers: mgorny, christof, jkorous, dexonsmith, #sanitizers, libcxx-commits, llvm-commits
Tags: #sanitizers, #libc, #llvm
Differential Revision: https://reviews.llvm.org/D69408
Summary:
Apparently during the review of D69265, and my flailing around with
git, a somewhat important line disappeared.
On top of that, there was no test exercising that code path, and
while writing the follow up patch I intended to write, some `CHECK`s
were failing.
Re-add the missing line, and add a test that fails without said line.
Reviewers: hctim, morehouse, pcc, cferris
Reviewed By: hctim
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69529
Summary:
This is a clean patch using the last diff of D69265, but using git
instead of svn, since svn went ro and arc was making my life harded
than it needed to be.
I was going to introduce a couple more lists and realized that our
lists are currently a bit all over the place. While we have a singly
linked list type relatively well defined, we are using doubly linked
lists defined on the fly for the stats and for the secondary blocks.
This CL adds a doubly linked list object, reorganizing the singly list
one to extract as much of the common code as possible. We use this
new type in the stats and the secondary. We also reorganize the list
tests to benefit from this consolidation.
There are a few side effect changes such as using for iterator loops
that are, in my opinion, cleaner in a couple of places.
Reviewers: hctim, morehouse, pcc, cferris
Reviewed By: hctim
Subscribers: jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69516
Use new control bits CTR_EL0.DIC and CTR_EL0.IDC to discover the d-cache
cleaning and i-cache invalidation requirements for instruction-to-data
coherence. This matches the behavior in the latest libgcc.
Author: Shaokun Zhang <zhangshaokun@hisilicon.com>
Reviewed By: peter.smith
Differential Revision: https://reviews.llvm.org/D69247
Summary:
GCC would like to emit a function call to report a tag mismatch
rather than hard-code the `brk` instruction directly.
__hwasan_tag_mismatch_stub contains most of the functionality to do
this already, but requires exposure in the dynamic library.
This patch moves __hwasan_tag_mismatch_stub outside of the anonymous
namespace that it was defined in and declares it in
hwasan_interface_internal.h.
We also add the ability to pass sizes larger than 16 bytes to this
reporting function by providing a fourth parameter that is only looked
at when the size provided is not in the original accepted range.
This does not change the behaviour where it is already being called,
since the previous definition only accepted sizes up to 16 bytes and
hence the change in behaviour is not seen by existing users.
The change in declaration does not matter, since the only existing use
is in the __hwasan_tag_mismatch function written in assembly.
Reviewers: eugenis, kcc, pcc, #sanitizers
Reviewed By: eugenis, #sanitizers
Subscribers: kristof.beyls, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69113
Patch by Matthew Malcomson <matthew.malcomson@arm.com>
Summary:
GCC would like to emit a function call to report a tag mismatch
rather than hard-code the `brk` instruction directly.
__hwasan_tag_mismatch_stub contains most of the functionality to do
this already, but requires exposure in the dynamic library.
This patch moves __hwasan_tag_mismatch_stub outside of the anonymous
namespace that it was defined in and declares it in
hwasan_interface_internal.h.
We also add the ability to pass sizes larger than 16 bytes to this
reporting function by providing a fourth parameter that is only looked
at when the size provided is not in the original accepted range.
This does not change the behaviour where it is already being called,
since the previous definition only accepted sizes up to 16 bytes and
hence the change in behaviour is not seen by existing users.
The change in declaration does not matter, since the only existing use
is in the __hwasan_tag_mismatch function written in assembly.
Tested with gcc and clang on an AArch64 vm.
Reviewers: eugenis, kcc, pcc, #sanitizers
Reviewed By: eugenis, #sanitizers
Subscribers: kristof.beyls, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69113
When the %m filename pattern is used, the filename is unique to each
image, so the cached value is wrong.
It struck me that the full filename isn't something that's recomputed
often, so perhaps it doesn't need to be cached at all. David Li pointed
out we can go further and just hide lprofCurFilename. This may regress
workflows that depend on using the set-filename API to change filenames
across all loaded DSOs, but this is expected to be very rare.
rdar://55137071
Differential Revision: https://reviews.llvm.org/D69137
llvm-svn: 375301
Summary:
This has been an experiment with late malloc interposition, made
possible by a non-standard feature of the Android dynamic loader.
Reviewers: pcc, mmalcomson
Subscribers: srhines, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69199
llvm-svn: 375296
Android now allocates only 8 fixed TLS slots. Somehow we were getting away
with using a non-existent slot until now, but in some cases the TLS slots
were being placed at the end of a page, which led to a segfault at startup.
Differential Revision: https://reviews.llvm.org/D69191
llvm-svn: 375276
Glibc has recently introduced changed to the mode field in ipc_perm in commit
2f959dfe849e0646e27403f2e4091536496ac0f0. For Arm this means that the mode
field no longer has the same size.
This causes an assert failure against libsanitizer's internal copy of ipc_perm.
Since this change can't be easily detected I am adding arm to the list of
targets that are excluded from this check.
Patch by: Tamar Christina
Differential Revision: https://reviews.llvm.org/D69104
llvm-svn: 375220
Summary:
Until now AArch64 development has been on patched kernels that have an always
on relaxed syscall ABI where tagged pointers are accepted.
The patches that have gone into the mainline kernel rely on each process opting
in to this relaxed ABI.
This commit adds code to choose that ABI into __hwasan_init.
The idea has already been agreed with one of the hwasan developers
(http://lists.llvm.org/pipermail/llvm-dev/2019-September/135328.html).
The patch ignores failures of `EINVAL` for Android, since there are older versions of the Android kernel that don't require this `prctl` or even have the relevant values. Avoiding EINVAL will let the library run on them.
I've tested this on an AArch64 VM running a kernel that requires this
prctl, having compiled both with clang and gcc.
Patch by Matthew Malcomson.
Reviewers: eugenis, kcc, pcc
Reviewed By: eugenis
Subscribers: srhines, kristof.beyls, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D68794
llvm-svn: 375166
This #define is in the non-Go ppc64le build but not in the Go build.
Reviewed-in: https://reviews.llvm.org/D68046
Author: randall77 (Keith Randall)
llvm-svn: 374868
The ExecuteCommand function in fuchsia used to prefix the
getOutputFile for each command run with the artifact_prefix flag if
it was available, because fuchsia components don't have a writable working
directory. However, if a file with a global path is provided, fuchsia
should honor that.
An example of this is using the global /tmp directory to store stuff.
In fuchsia it ended up being translated to data///tmp, whereas we want
to make sure it is using /tmp (which is available to components using the
isolated-temp feature).
To test this I made the change, compiled fuchsia with this toolchain and
ran a fuzzer with the -fork=1 flag (that mode makes use of the /tmp
directory). I also tested that normal fuzzing workflow was not affected
by this.
Author: charco (Marco Vanotti)
Differential Revision: https://reviews.llvm.org/D68774
llvm-svn: 374612
Updated: Removed offending TODO comment.
Dereferences with addresses above the 48-bit hardware addressable range
produce "invalid instruction" (instead of "invalid access") hardware
exceptions (there is no hardware address decoding logic for those bits),
and the address provided by this exception is the address of the
instruction (not the faulting address). The kernel maps the "invalid
instruction" to SEGV, but fails to provide the real fault address.
Because of this ASan lies and says that those cases are null
dereferences. This downgrades the severity of a found bug in terms of
security. In the ASan signal handler, we can not provide the real
faulting address, but at least we can try not to lie.
rdar://50366151
Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D68676
> llvm-svn: 374265
llvm-svn: 374384
- Available from 12.x branch, by the time it lands next year in FreeBSD tree, the 11.x's might be EOL.
- Intentionally changed the getrandom test to C code as with 12.0 (might be fixed in CURRENT since), there is a linkage issue in C++ context.
Reviewers: emaste, dim, vitalybuka
Reviewed-By: vitalybuka
Differential Revision: https://reviews.llvm.org/D68451
llvm-svn: 374315
Summary:
Quote from http://eel.is/c++draft/expr.add#4:
```
4 When an expression J that has integral type is added to or subtracted
from an expression P of pointer type, the result has the type of P.
(4.1) If P evaluates to a null pointer value and J evaluates to 0,
the result is a null pointer value.
(4.2) Otherwise, if P points to an array element i of an array object x with n
elements ([dcl.array]), the expressions P + J and J + P
(where J has the value j) point to the (possibly-hypothetical) array
element i+j of x if 0≤i+j≤n and the expression P - J points to the
(possibly-hypothetical) array element i−j of x if 0≤i−j≤n.
(4.3) Otherwise, the behavior is undefined.
```
Therefore, as per the standard, applying non-zero offset to `nullptr`
(or making non-`nullptr` a `nullptr`, by subtracting pointer's integral value
from the pointer itself) is undefined behavior. (*if* `nullptr` is not defined,
i.e. e.g. `-fno-delete-null-pointer-checks` was *not* specified.)
To make things more fun, in C (6.5.6p8), applying *any* offset to null pointer
is undefined, although Clang front-end pessimizes the code by not lowering
that info, so this UB is "harmless".
Since rL369789 (D66608 `[InstCombine] icmp eq/ne (gep inbounds P, Idx..), null -> icmp eq/ne P, null`)
LLVM middle-end uses those guarantees for transformations.
If the source contains such UB's, said code may now be miscompiled.
Such miscompilations were already observed:
* https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20190826/687838.html
* https://github.com/google/filament/pull/1566
Surprisingly, UBSan does not catch those issues
... until now. This diff teaches UBSan about these UB's.
`getelementpointer inbounds` is a pretty frequent instruction,
so this does have a measurable impact on performance;
I've addressed most of the obvious missing folds (and thus decreased the performance impact by ~5%),
and then re-performed some performance measurements using my [[ https://github.com/darktable-org/rawspeed | RawSpeed ]] benchmark:
(all measurements done with LLVM ToT, the sanitizer never fired.)
* no sanitization vs. existing check: average `+21.62%` slowdown
* existing check vs. check after this patch: average `22.04%` slowdown
* no sanitization vs. this patch: average `48.42%` slowdown
Reviewers: vsk, filcab, rsmith, aaron.ballman, vitalybuka, rjmccall, #sanitizers
Reviewed By: rsmith
Subscribers: kristof.beyls, nickdesaulniers, nikic, ychen, dtzWill, xbolva00, dberris, arphaman, rupprecht, reames, regehr, llvm-commits, cfe-commits
Tags: #clang, #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D67122
llvm-svn: 374293
Dereferences with addresses above the 48-bit hardware addressable range
produce "invalid instruction" (instead of "invalid access") hardware
exceptions (there is no hardware address decoding logic for those bits),
and the address provided by this exception is the address of the
instruction (not the faulting address). The kernel maps the "invalid
instruction" to SEGV, but fails to provide the real fault address.
Because of this ASan lies and says that those cases are null
dereferences. This downgrades the severity of a found bug in terms of
security. In the ASan signal handler, we can not provide the real
faulting address, but at least we can try not to lie.
rdar://50366151
Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D68676
llvm-svn: 374265
Evgenii Stepanov