#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.
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.
Once part of PR #144648, follow the reviewer's advice and split into
this separate PR.
`unmap` works at page granularity, but supports an arbitrary non-zero
size as an argument, which results in possible shadow undercleaning in
the existing TSan implementation when `size % kShadowCell != 0`.
This change introduces two test cases to verify the shadow cleaning
effect in `unmap`.
- java_heap_init2.cpp: Imitating java_heap_init cpp, verify the
incomplete cleaning of meta
- munmap_clear_shadow.c: verify the incomplete cleaning of shadow
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.
In the current gcov implementation, all lines within a basic block are
attributed to the source file of the block's containing function. This
is inaccurate when a block contains lines from other files (e.g., via
#include "foo.inc").
Commit
[406e81b](406e81b79d)
attempted to address this by filtering lines based on debug info types,
but this approach has two limitations:
* **Over-filtering**: Some valid lines belonging to the function are
incorrectly excluded.
* **Under-counting**: Lines not belonging to the function are filtered
out and omitted from coverage statistics.
**GCC Reference Behavior**
GCC's gcov implementation handles this case correctly.This change aligns
the LLVM behavior with GCC.
**Proposed Solution**
1. **GCNO Generation**:
* **Current**: Each block stores a single GCOVLines record (filename +
lines).
* **New**: Dynamically create new GCOVLines records whenever consecutive
lines in a block originate from different source files. Group subsequent
lines from the same file under one record.
2. **GCNO Parsing**:
* **Current**: Lines are directly attributed to the function's source
file.
* **New**: Introduce a GCOVLocation type to track filename/line mappings
within blocks. Statistics will reflect the actual source file for each
line.
Today, the `uncaught-exception.test` fuzzer test checks for the string
"libFuzzer: deadly signal" in the program output as the result of an
uncaught exception.
Although this is correct for `clang`, `msvc` reports a different error
message: "libFuzzer: uncaught C++ exception". Since `msvc` reuses the
`libFuzzer` infrastructure for ASan regression testing, it would help us
greatly if the test handled the `msvc` divergence more gracefully.
**This PR:** augments this test so check for a different string (namely
"libFuzzer: uncaught C++ exception") if the compiler target matches the
`msvc` naming scheme.
I understand if this is outside the scope of support for LLVM as well,
and I'm also open for different approaches here. Thanks!
In https://github.com/llvm/llvm-project/pull/143183 was mistakenly added
a default value to `python_root_dir` in lit tests of compiler-rt.
This is unused by the lit tests of compiler-rt, as it was meant to be
used by `lldb`.
This patch removes this change.
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.
The runtimes build expects libclang_rt.tysan.a to be available, but the
check-tysan target does not actually depend on it when built using a
runtimes build with LLVM_ENABLE_RUNTIMES pointing at ./llvm. This means
we get test failures when running check-compiler-rt due to the missing
static archive.
This patch also makes check-tysan depend on tysan when we are using the
runtimes build.
This is causing premerge failures currently since we recently migrated
to the runtimes build.
When testing LLDB, we want to make sure to use the same Python as the
one we used to build it.
This patch used the CMake variable `Python3_ROOT_DIR` to set the
`PYTHONHOME` env variable in LLDB lit tests, in order to ensure of this.
Please see https://github.com/swiftlang/swift/pull/82063 for the
original issue.
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.
The XRay interface header uses no C++ specific features aside from using
the `std` namespace and including the C++ variant of C headers. Yet,
these changes prevent using `xray_interface.h` in external tools relying
on C for different reasons. Make this header C compliant by using C
headers, removing the `std` namespace from `std::size_t` and guard
`extern "C"`.
To make sure that further changes to not break the interface
accidentally, port one test from C++ to C. This requires the C23
standard to officially support the attribute syntax used in this test
case.
Note that this only resolves this issue for `xray_interface.h`.
`xray_records.h` is also not C compliant, but requires more work to
port.
Fixes#139902
Signed-off-by: Jan André Reuter <j.reuter@fz-juelich.de>
While the current test exercised the deadlock behavior prior to #131756,
deadlock still can occur intermittently. Since this results in test
flakes in CI, we disable this test until the locking behavior can be
fixed in the runtime. See #140646 for details.
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?
While investigating an issue with code coverage reporting around
exceptions it was useful to have a baseline of what works today.
This change adds end-to-end testing to validate code coverage behavior
that is currently working with regards to exception handling.
The current region mapping for do-while loops that contain statements
such as break or continue results in inaccurate line coverage reports
for the line following the loop.
This change handles terminating statements the same way that other loop
constructs do, correcting the region mapping for accurate reports. It
also fixes a fragile test relying on exact line numbers.
Fixes#139122
Currently, `Posix/sanitizer_set_report_path_test.cpp` contains the
following check: `// CHECK: ERROR: Can't create directory:
{{.*}}Posix/Output/sanitizer_set_report_path_test.cpp.tmp`. This makes
an assumption that the test file resides in `Posix/Output`, however when
testing on a remote device, an alternative temporary directory path is
used. This patch instead checks that the path in the error message
matches the requested path dynamically.
10s looks not enough. With highly parallel test
execution on VMs it's very possible that Asan
report will have no enough time to produce output.
I can reproduce locally 1s is not always enough,
but likely my workstation is faster then buildbot.
Additionally, don't use puts/CHECK to validate
timeout. We can exit with 0 and it should violate
"not" expectation.
Follow up to #131756.
