[LLVM][rtsan] Add LLVM attribute sanitize_realtime_unsafe #34
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ectorsCombine. (llvm#104774) UZP2 requires both operands to match the result type but the combine tries to replace a truncate by passing the pre-truncated operands directly to an UZP2 with the truncated result type. This patch nop-casts the operands to keep the DAG consistent. There should be no changes to the generated code, which is fine as it. This patch also enables more target specific getNode() validation for fixed length vector types.
…ecks (llvm#104478) The CMake docs state that `check_c_source_compiles()` checks whether the supplied code "can be compiled as a C source file and linked as an executable (so it must contain at least a `main()` function)." https://cmake.org/cmake/help/v3.30/module/CheckCSourceCompiles.html In practice, this command is a wrapper around `try_compile()`: - https://gitlab.kitware.com/cmake/cmake/blob/2904ce00d2ed6ad5dac6d3459af62d8223e06ce0/Modules/CheckCSourceCompiles.cmake#L54 - https://gitlab.kitware.com/cmake/cmake/blob/2904ce00d2ed6ad5dac6d3459af62d8223e06ce0/Modules/Internal/CheckSourceCompiles.cmake#L101 When `CMAKE_SOURCE_DIR` is compiler-rt/lib/builtins/, `CMAKE_TRY_COMPILE_TARGET_TYPE` is set to `STATIC_LIBRARY`, so the checks for `float16` and `bfloat16` support work as intended in a Clang + compiler-rt runtime build for example, as it runs CMake recursively from that directory. However, when using llvm/ or compiler-rt/ as CMake source directory, as `CMAKE_TRY_COMPILE_TARGET_TYPE` defaults to `EXECUTABLE`, these checks will indeed fail if the code doesn't have a `main()` function. This results in LLVM using x86 SIMD registers when generating calls to builtins that, with Arch Linux's compiler-rt package for example, actually use a GPR for their argument or return value as they use `uint16_t` instead of `_Float16`. This had been caught in post-commit review: https://reviews.llvm.org/D145237#4521152. Use of the internal `CMAKE_C_COMPILER_WORKS` variable is not what hides the issue, however. PR llvm#69842 tried to fix this by unconditionally setting `CMAKE_TRY_COMPILE_TARGET_TYPE` to `STATIC_LIBRARY`, but it apparently caused other issues, so it was reverted. This PR just adds a `main()` function in the checks, as per the CMake docs.
…lvm#106707) * Revert "Fix MSVC "not all control paths return a value" warning. NFC." Dep to revert c9b6e01 * Revert "[AMDGPU] Graph-based Module Splitting Rewrite (llvm#104763)" Breaks tests.
Make dsymutil return a non-zero exit code when crashing during linking.
Need to use FinalShuffle function for all vectorized results to correctly produce vectorized value. Fixes llvm#106655
By choosing an initial value whose mask is only used by the blend we can remove the need for the mask entirely.
Recursion here causes stack overflow on large inputs. Fixing by unrolling via a stack.
…intrinsics to support llvm#106584
Code lowering always generates fir.if else blocks for source level if statements, whether needed or not. Change this to only generate else blocks that are needed.
Trivially extend dd0cf23 ([LICM] Reassociate & hoist sub expressions) to handle unsigned predicates as well. Alive2 proofs: https://alive2.llvm.org/ce/z/GdDBtT.
Ever since 6859685 (or, precisely, 84428da) relative jumps emitted by the AVR codegen are off by two bytes - this pull request fixes it. ## Abstract As compared to absolute jumps, relative jumps - such as rjmp, rcall or brsh - have an implied `pc+2` behavior; that is, `jmp 100` is `pc = 100`, but `rjmp 100` gets understood as `pc = pc + 100 + 2`. This is not reflected in the AVR codegen: https://github.com/llvm/llvm-project/blob/f95026dbf66e353128a3a3d7b55f3e52d5985535/llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp#L89 ... which always emits relative jumps that are two bytes too far - or rather it _would_ emit such jumps if not for this check: https://github.com/llvm/llvm-project/blob/f95026dbf66e353128a3a3d7b55f3e52d5985535/llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp#L517 ... which causes most of the relative jumps to be actually resolved late, by the linker, which applies the offsetting logic on its own, hiding the issue within LLVM. [Some time ago](llvm@697a162) we've had a similar "jumps are off" problem that got solved by touching `shouldForceRelocation()`, but I think that has worked only by accident. It's exploited the fact that absolute vs relative jumps in the parsed assembly can be distinguished through a "side channel" check relying on the existence of labels (i.e. absolute jumps happen to named labels, but relative jumps are anonymous, so to say). This was an alright idea back then, but it got broken by 6859685. I propose a different approach: - when emitting relative jumps, offset them by `-2` (well, `-1`, strictly speaking, because those instructions rely on right-shifted offset), - when parsing relative jumps, treat `.` as `+2` and read `rjmp .+1234` as `rjmp (1234 + 2)`. This approach seems to be sound and now we generate the same assembly as avr-gcc, which can be confirmed with: ```cpp // avr-gcc test.c -O3 && avr-objdump -d a.out int main() { asm( " foo:\n\t" " rjmp .+2\n\t" " rjmp .-2\n\t" " rjmp foo\n\t" " rjmp .+8\n\t" " rjmp end\n\t" " rjmp .+0\n\t" " end:\n\t" " rjmp .-4\n\t" " rjmp .-6\n\t" " x:\n\t" " rjmp x\n\t" " .short 0xc00f\n\t" ); } ``` avr-gcc is also how I got the opcodes for all new tests like `inst-brbc.s`, so we should be good.
…san_disable (llvm#106727) This better matches lsan_enable and disable, which we are trying to emulate.
Fixes issue found here llvm#106691 (comment) The issue wasn't in the code change itself, just the unittest; the trailing marker wasn't properly cleaned up.
llvm#100692 changes clang template deduction, and an error was now emitted when building flang with top of the tree clang when mapping std::pow in intrinsics-library.cpp for constant folding `error: address of overloaded function 'pow' is ambiguous` See https://lab.llvm.org/buildbot/#/builders/4/builds/1670 I I am not expert enough to understand if the new error is justified or not here, but it is easy to help the compiler here with explicit wrappers to fix the builds.
… of for loop. NFC.
Update the documentation to direct new users to the Github instead of the discontinued Phabricator archive. Also details more ways and information regarding clang-query usage. Partially resolves/disclaims llvm#106656 and llvm#106663 as per discussion in https://discourse.llvm.org/t/inconsistency-between-hasdescendant-in-clang-query-and-clang-libtooling-matchers/80799/. Also updates the out-of-tree guide. For context, I recently went through the Contributing guide while writing llvm#102299, and many of these updates were from my experience trying to follow the guide. e.g. I was trying to link the shared library of an out-of-tree check as SHARED in CMake and encountered duplicate symbols like _ZTIN5clang4tidy14ClangTidyCheckE. It wasn't until I saw llvm@84f137a that I found out I had to use MODULE. I also encountered the clang-query difference which was a surprise as the documentation said the two matchers were "virtually identical". Also, the -header-filter thing tripped me out until I found llvm#25590 and llvm#91400. Usually, when people say restrict and filter, they mean filter out (since -header-filter instead includes/filters in said headers).
…tures (llvm#106625) The commit ff3f3a5 made it possible to enable transitively implied features when parsing assembler directives. For example enabling sve2 also enables sve. This patch allows disabling features which depend on each other. For example disabling sve also disables sve2.
…6708) This removes the need for the long list of platforms in strong_order_long_double_verify.
To support detecting MD5 checksum mismatches, store a SupportFile rather than a plain FileSpec in SourceManager::File.
cjappl
reviewed
Aug 30, 2024
There was a problem hiding this comment.
Overall this looks good.
you are missing a test to confirm that these attributes are not compatible:
llvm/test/Verifier/rtsan-attrs.ll
and the verifier code to enforce they are not compatible:
llvm/lib/IR/Verifier.cpp
Also, this code will be somewhat affected when I revert the nosanitize attribute
I would also make sure you run git-clang-tidy main, to ensure everything is clean. That is os one of the things that gets me most often.
|
Super, thanks @cjappl - will update all of these when the nosanitize_realtime attribute is removed :) |
Install scan-build-py modules into the plain `lib` directory, without LLVM_LIBDIR_SUFFIX appended, to match the path expected by `intercept-build` executable. This fixes the program being unable to find its modules. Using unsuffixed path makes sense here, since Python modules are not subject to multilib. This change effectively reverts 1334e12. The commit in question changed the path without a clear justification ("does not respect the given prefix") and the Python code was never modified to actually work with the change. Fixes llvm#106608
…7762) When we encounter a bitcast from an integer type we can use the information from `KnownBits` to glean some information about the fpclass: - If the sign bit is known, we can transfer this information over. - If the float is IEEE format and enough of the bits are known, we may be able to prove or rule out some fpclasses such as NaN, Zero, or Inf.
There is no documentation or description of the different apps in the LLVM benchmark test-suite and this is a first attempt to document this for the MultiSource apps.
…5447)" (llvm#106743) This reverts commit 178fc47. This attribute was not needed now that we are using the lsan style ScopedDisabler for disabling this sanitizer See llvm#106736 llvm#106125 For more discussion
Since we don't generate relocations for those, it doesn't make sense to assert them here; fallout of llvm#106722.
davidtrevelyan
force-pushed
the
rtsan/blocking-1-llvm-attr
branch
from
ef470bb to
04c350b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
22 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.