#144648 was reverted because it failed the new sanitizer test
`munmap_clear_shadow.c` in IOS's CI.
That issue could be fixed by disabling the test on some platforms, due
to the incompatibility of the test on these platforms.
In detail, we should disable the test in FreeBSD, Apple, NetBSD,
Solaris, and Haiku, where `ReleaseMemoryPagesToOS` executes
`madvise(beg, end, MADV_FREE)`, which tags the relevant pages as 'FREE'
and does not release them immediately.
Reapply "[NFC][DebugInfo][DWARF] Create new low-level dwarf library (#…
(#145959)
This reapplies cbf781f0bd, with fixes for
the shared-library build and the unconventional sanitizer-runtime build.
Original Description:
This is the culmination of a series of changes described in [1].
Although somewhat large by line count, it is almost entirely mechanical,
creating a new library in DebugInfo/DWARF/LowLevel. This new library has
very minimal dependencies, allowing it to be used from more places than
the normal DebugInfo/DWARF library--in particular from MC.
1.
https://discourse.llvm.org/t/rfc-debuginfo-dwarf-refactor-into-to-lower-and-higher-level-libraries/86665/2
Most pacbti instructions are a nop when the target does not have pacbti,
and thus safe to execute, but bxaut is an undefined instruction. When we
don't have pacbti (e.g. if we're compiling compiler-rt with
-mbranch-protection=standard in order to be forward-compatible with
pacbti while still working on targets without it) then we need to use
separate aut and bx instructions.
In TSan, every `k` bytes of application memory (where `k = 8`) maps to a
single shadow/meta cell. This design leads to two distinct outcomes when
calculating the end of a shadow range using `MemToShadow(addr_end)`,
depending on the alignment of `addr_end`:
- **Exclusive End:** If `addr_end` is aligned (`addr_end % k == 0`),
`MemToShadow(addr_end)` points to the first shadow cell *past* the
intended range. This address is an exclusive boundary marker, not a cell
to be operated on.
- **Inclusive End:** If `addr_end` is not aligned (`addr_end % k != 0`),
`MemToShadow(addr_end)` points to the last shadow cell that *is* part of
the range (i.e., the same cell as `MemToShadow(addr_end - 1)`).
Different TSan functions have different expectations for whether the
shadow end should be inclusive or exclusive. However, these expectations
are not always explicitly enforced, which can lead to subtle bugs or
reliance on unstated invariants.
The core of this patch is to ensure that functions ONLY requiring an
**exclusive shadow end** behave correctly.
1. Enforcing Existing Invariants:
For functions like `MetaMap::MoveMemory` and `MapShadow`, the assumption
is that the end address is always `k`-aligned. While this holds true in
the current codebase (e.g., due to some external implicit conditions),
this invariant is not guaranteed by the function's internal context. We
add explicit assertions to make this requirement clear and to catch any
future changes that might violate this assumption.
2. Fixing Latent Bugs:
In other cases, unaligned end addresses are possible, representing a
latent bug. This was the case in `UnmapShadow`. The `size` of a memory
region being unmapped is not always a multiple of `k`. When this
happens, `UnmapShadow` would fail to clear the final (tail) portion of
the shadow memory.
This patch fixes `UnmapShadow` by rounding up the `size` to the next
multiple of `k` before clearing the shadow memory. This is safe because
the underlying OS `unmap` operation is page-granular, and the page size
is guaranteed to be a multiple of `k`.
Notably, this fix makes `UnmapShadow` consistent with its inverse
operation, `MemoryRangeImitateWriteOrResetRange`, which already performs
a similar size round-up.
In summary, this PR:
- **Adds assertions** to `MetaMap::MoveMemory` and `MapShadow` to
enforce their implicit requirement for k-aligned end addresses.
- **Fixes a latent bug** in `UnmapShadow` by rounding up the size to
ensure the entire shadow range is cleared. Two new test cases have been
added to cover this scenario.
- Removes a redundant assertion in `__tsan_java_move`.
- Fixes an incorrect shadow end calculation introduced in commit
4052de6. The previous logic, while fixing an overestimation issue, did
not properly account for `kShadowCell` alignment and could lead to
underestimation.
This reverts commit 5eb5f0d876 i.e.,
relands 1b71ea411a.
Test case was failing on aarch64 because the long double type is
implemented differently on x86 vs aarch64. This reland restricts the
test to x86.
----
Original CL description:
A commonly used aid for debugging MSan reports is
`__msan_print_shadow()`, which requires manual app code annotations
(typically of the variable in the UUM report or nearby). This is in
contrast to ASan, which automatically prints out the shadow map when a
check fails.
This patch changes MSan to print the shadow that failed an outlined
check (checks are outlined per function after the
`-msan-instrumentation-with-call-threshold` is exceeded) if verbosity >=
1. Note that we do not print out the shadow map of "neighboring"
variables because this is technically infeasible; see "Caveat" below.
This patch can be easier to use than `__msan_print_shadow()` because
this does not require manual app code annotations. Additionally, due to
optimizations, `__msan_print_shadow()` calls can sometimes spuriously
affect whether a variable is initialized.
As a side effect, this patch also enables outlined checks for
arbitrary-sized shadows (vs. the current hardcoded handlers for
{1,2,4,8}-byte shadows).
Caveat: the shadow does not necessarily correspond to an individual user
variable, because MSan instrumentation may combine and/or truncate
multiple shadows prior to emitting a check that the mangled shadow is
zero (e.g., `convertShadowToScalar()`,
`handleSSEVectorConvertIntrinsic()`, `materializeInstructionChecks()`).
OTOH it is arguably a strength that this feature emit the shadow that
directly matters for the MSan check, but which cannot be obtained using
the MSan API.
A commonly used aid for debugging MSan reports is `__msan_print_shadow()`, which requires manual app code annotations (typically of the variable in the UUM report or nearby). This is in contrast to ASan, which automatically prints out the shadow map when a check fails.
This patch changes MSan to print the shadow that failed an outlined check (checks are outlined per function after the `-msan-instrumentation-with-call-threshold` is exceeded) if verbosity >= 1. Note that we do not print out the shadow map of "neighboring" variables because this is technically infeasible; see "Caveat" below.
This patch can be easier to use than `__msan_print_shadow()` because this does not require manual app code annotations. Additionally, due to optimizations, `__msan_print_shadow()` calls can sometimes spuriously affect whether a variable is initialized.
As a side effect, this patch also enables outlined checks for arbitrary-sized shadows (vs. the current hardcoded handlers for {1,2,4,8}-byte shadows).
Caveat: the shadow does not necessarily correspond to an individual user variable, because MSan instrumentation may combine and/or truncate multiple shadows prior to emitting a check that the mangled shadow is zero (e.g., `convertShadowToScalar()`, `handleSSEVectorConvertIntrinsic()`, `materializeInstructionChecks()`). OTOH it is arguably a strength that this feature emit the shadow that directly matters for the MSan check, but which cannot be obtained using the MSan API.
Correct the interval desc of ReleaseMemoryPagesToOS from [beg, end] to
[beg, end), as it actually does.
The previous incorrect description of [beg, end] might cause an
incorrect invoke as follows: `ReleaseMemoryPagesToOS(0, kPageSize - 1);`
This commit changes the interval shadow/meta address check from
inclusive-inclusive ( $[\mathrm{start}, \mathrm{end}]$ ) to
inclusive-exclusive ( $[\mathrm{start}, \mathrm{end})$ ), to resolve the
ambiguity of the end point address. This also aligns the logic with the
check for `isAppMem` (i.e., inclusive-exclusive), ensuring consistent
behavior across all memory classifications.
1. The `isShadowMem` and `isMetaMem` checks previously used an
inclusive-inclusive interval, i.e., $[\mathrm{start}, \mathrm{end}]$,
which could lead to a boundary address being incorrectly classified as
both Shadow and Meta memory, e.g., 0x3000_0000_0000 in
`Mapping48AddressSpace`.
- What's more, even when Shadow doesn't border Meta, `ShadowMem::end`
cannot be considered a legal shadow address, as TSan protects the gap,
i.e., `ProtectRange(ShadowEnd(), MetaShadowBeg());`
2. `ShadowMem`/`MetaMem` addresses are derived from `AppMem` using an
affine-like transformation (`* factor + bias`). This transformation
includes two extra modifications: high- and low-order bits are masked
out, and for Shadow Memory, an optional XOR operation may be applied to
prevent conflicts with certain AppMem regions.
- Given that all AppMem regions are defined as inclusive-exclusive
intervals, $[\mathrm{start}, \mathrm{end})$, the resulting Shadow/Meta
regions should logically also be inclusive-exclusive.
Note: This change is purely for improving code consistency and should
have no functional impact. In practice, the exact endpoint addresses of
the Shadow/Meta regions are generally not reached.
Fix for #144495 by 6f4add3 broke sanitizer-aarch64-linux buildbot.
compiler-rt/lib/fuzzer/tests build failed because the linker was
looking gcc_s without '-l' appended.
The CMake script was adding the library name without the required
'-l' prefix. This patch adds the -l prefix changing gcc_s to -lgcc_s
and gcc to -lgcc.
https://lab.llvm.org/buildbot/#/builders/51/builds/18170
Mark as many of the reportXX functions that take pointers const. This
avoid the need to use const_cast when calling these functions on an
already const pointer.
Fix reportHeaderCorruption calls where an argument was passed into an
append call that didn't use them.
compiler-rt/lib/fuzzer/tests build was failing on armv7, with undefined
references to unwinder symbols, such as __aeabi_unwind_cpp_pr0.
This occurs because the test is built with `-nostdlib++` but `libunwind`
is not explicitly linked to the final test executable.
This patch resolves the issue by adding CMake logic to explicitly link
the required unwinder to the fuzzer tests, inspired by the same solution
used to fix Scudo build failures by https://reviews.llvm.org/D142888.
MSan should unpoison the parameters of extended signal handlers.
However, MSan unpoisoned the second parameter with the wrong size
`sizeof(__sanitizer_sigaction)`, inconsistent with its real type
`siginfo_t`.
This commit fixes this issue by correcting the size to
`sizeof(__sanitizer_siginfo)`.
Currently, `ENABLE_BAREMETAL_AARCH64_FMV` is added to builtin defines
for all baremetal targets though it is only needed for aarch64. This
patch fixes this by adding it only for aarch64 target.
Adds support to LSan for `free_sized` and `free_aligned_sized` from C23.
Other sanitizers will be handled with their own separate PRs.
For #144435
Signed-off-by: Justin King <jcking@google.com>
AAPCS64 reserves any of X9-X15 for a compiler to choose to use for this
purpose, and says not to use X16 or X18 like GCC (and the previous
implementation) chose to use. The X18 register may need to get used by
the kernel in some circumstances, as specified by the platform ABI, so
it is generally an unwise choice. Simply choosing a different register
fixes the problem of this being broken on any platform that actually
follows the platform ABI (which is all of them except EABI, if I am
reading this linux kernel bug correctly
https://lkml2.uits.iu.edu/hypermail/linux/kernel/2001.2/01502.html). As
a side benefit, also generate slightly better code and avoids needing
the compiler-rt to be present. I did that by following the XCore
implementation instead of PPC (although in hindsight, following the
RISCV might have been slightly more readable). That X18 is wrong to use
for this purpose has been known for many years (e.g.
https://www.mail-archive.com/gcc@gcc.gnu.org/msg76934.html) and also
known that fixing this to use one of the correct registers is not an ABI
break, since this only appears inside of a translation unit. Some of the
other temporary registers (e.g. X9) are already reserved inside llvm for
internal use as a generic temporary register in the prologue before
saving registers, while X15 was already used in rare cases as a scratch
register in the prologue as well, so I felt that seemed the most logical
choice to choose here.
Some of the aeabi functions were missing PACBTI support. The lack of it
resulted in exceptions at runtime if the running environment had PAC
and/or BTI enabled.
This patch adds this support. This involves the addition of PACBTI
instructions, depending on whether each of these features is enabled,
plus the saving and restoring of the PAC code that resides in r12. Some
of the common code has been put in preprocessor macros to reduce
duplication, but not all, especially since some register saving and
restoring is very specific to each context.
When a CHECK() fails during TSan initialization, it may segfault (e.g., https://github.com/google/sanitizers/issues/837#issuecomment-2939267531). This is because a failed CHECK() will attempt to print a symbolized stack trace, which requires dl_iterate_phdr, but the interceptor may not yet be set up.
This patch fixes the issue by not symbolizing the stack trace if the dl_iterate_phdr interceptor is not ready.
Commit 75c3ff8c0b inadvertently removed
some files from the build related to the SME ABI routines.
This patch fixes the issue by reintroducing the files to the build in
CMake.
`sanitizer_procmaps_solaris.cpp` currently uses `#undef
_FILE_OFFSET_BITS` to hack around the fact that old versions of Solaris
`<procfs.h>` don't work in a largefile environment:
```
/usr/include/sys/procfs.h:42:2: error: #error "Cannot use procfs in the large file compilation environment"
42 | #error "Cannot use procfs in the large file compilation environment"
| ^~~~~
```
However, this is no longer an issue on either Solaris 11.4 or Illumos.
The workaround only existed for the benefit of Solaris 11.3. While that
had never been supported by LLVM, the sanitizer runtime libs were
imported into GCC's `libsanitzer`. With the removal of Solaris 11.3
support in GCC 15, this is no longer an issue and the workaround can be
removed.
Tested on `amd64-pc-solaris2.11` and `sparcv9-sun-solaris2.11`.
Currently, if TSan needs to disable ASLR but is unable to do so, it
aborts with a cryptic message:
```
ThreadSanitizer: CHECK failed: tsan_platform_linux.cpp:290 "((personality(old_personality | ADDR_NO_RANDOMIZE))) != ((-1))"
```
and a segfault
(https://github.com/google/sanitizers/issues/837#issuecomment-2939267531).
This patch replaces the CHECK with more user-friendly diagnostics and
suggestions via printf, followed by Die().
The existing implementations, written in assembly, make use of unaligned
accesses for performance reasons. They are not compatible with strict
aligned configurations, i.e. with `-mno-unaligned-access`.
If the functions are used in this scenario, an exception is raised due
to unaligned memory accesses.
This patch reintroduces vanilla implementations for these functions to
be used in strictly aligned configurations. The actual code is largely
based on the code from https://github.com/llvm/llvm-project/pull/77496
This changes the save/restore procedures to save/restore registers one
by one - to match the stack alignment for the ILP32E/LP64E ABIs, rather
than the larger batches of the conventional ABIs. The implementations of
the save routines are not tail-shared, to reduce the number of
instructions. I think this also helps code size but I need to check this
again.
I would expect (but haven't measured) that the majority of functions
compiled for the ILP32E/LP64E ABIs will in fact use both callee-saved
registers, and therefore there are still savings to be had, but I think
those can come later, with more data (especially if those changes are
just to the instruction sequences we use to save the registers, rather
than the number and alignment of how this is done).
This is a potential break for all of the ILP32E/LP64E ABI - we may
instead have to teach the compiler to emit the CFI information correctly
for the grouping we already have implemented (because that grouping
matches GCC). It depends on how intentional we think the grouping is in
the original ILP32E/LP64E save/restore implementation was, and whether
we think we can fix that now.
Created for Wine's memset by clang or mingw-gcc,
the latter places it quite at the start of the function:
```
0x00006ffffb67e210 <memset+0>: 0f b6 d2 movzbl %dl,%edx
0x00006ffffb67e213 <memset+3>: 48 b8 01 01 01 01 01 01 01 01 movabs $0x101010101010101,%rax
```
`3200 uint64_t v = 0x101010101010101ull * (unsigned char)c;`
290fd532ee/dlls/msvcrt/string.c (L3200)
Support `%` patterns in sanitizer report paths, similar to the patterns
supported in IRPGO
4bf67cdf02/compiler-rt/lib/profile/InstrProfilingFile.c (L999-L1017)
* `%%` becomes `%`
* `%H` expands to the environment variable `HOME`
* `%t` expands to the environment variable `TMPDIR`
* `%p` expands to the process ID (PID)
In particular, the `%H` pattern is useful to resolve the home directory
at runtime, which is not possible before this PR.
Also, avoid using `Report()` before the report path has been set.
Inside `getCurFilename`, there is this code snippit
```
if (!lprofCurFilename.FilenamePat || !lprofCurFilename.FilenamePat[0])
return 0;
```
If this is true, we return `"\0"`, but would leak the memory in
`FilenameBuf`.
This pull request adds a free before then to properly free the memory.
There was already a check that we allocated memory, so there is no need
to worry about freeing unallocated memory.
Previously we would add any ELF that contained a build id regardless
whether the ELF contained symbols or not. This works for Android since
soong will strip the symbols into a new directory. However other
build systems, like BUCK, will write the stripped file in the same
directory as the unstripped file. This would cause the hwasan_symbolize
script sometimes add then stripped ELF to its index and ignore the
symbolized ELF. The logic has now been changed to only add ELFs that
contain symbols to the index. If two symbolized ELFs are encountered
with the same build id, we now exit out with an error.
Fixes#135966
---------
Co-authored-by: Stefan Bossbaly <sboss@meta.com>
This PR fixes the bug reported in #134358.
In the current implementation of the tsan posix interceptors, the signal
set does not get restored to the correct original set, if a signal
handler gets called, while already inside of a signal handler. This
leads to the wrong signal set being set for the thread in which the
signal handler was called.
To fix this I introduced a stack of `__sanitizer_sigset_t` to keep all
the correct old signal sets and restore them in the correct order.
There was also already an existing test that tested nested / recursive
signal handlers, but it was disabled.
I therefore reenabled it, made it more robust by waiting for the second
thread to have been properly started and added checks for the signal
sets.
This test then failed before the introduction of the interceptor fix and
didn't fail with the fix.
@dvyukov What are your thoughts?
