-
Notifications
You must be signed in to change notification settings - Fork 0
COFF ImgRel32 jump tables #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
625258d
to
d2d593b
Compare
`clang-repl --cuda` was previously crashing with a segmentation fault, instead of reporting a clean error ``` (base) anutosh491@Anutoshs-MacBook-Air bin % ./clang-repl --cuda #0 0x0000000111da4fbc llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/local/libexec/llvm-20/lib/libLLVM.dylib+0x150fbc) #1 0x0000000111da31dc llvm::sys::RunSignalHandlers() (/opt/local/libexec/llvm-20/lib/libLLVM.dylib+0x14f1dc) #2 0x0000000111da5628 SignalHandler(int) (/opt/local/libexec/llvm-20/lib/libLLVM.dylib+0x151628) llvm#3 0x000000019b242de4 (/usr/lib/system/libsystem_platform.dylib+0x180482de4) llvm#4 0x0000000107f638d0 clang::IncrementalCUDADeviceParser::IncrementalCUDADeviceParser(std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>, clang::CompilerInstance&, llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>, llvm::Error&, std::__1::list<clang::PartialTranslationUnit, std::__1::allocator<clang::PartialTranslationUnit>> const&) (/opt/local/libexec/llvm-20/lib/libclang-cpp.dylib+0x216b8d0) llvm#5 0x0000000107f638d0 clang::IncrementalCUDADeviceParser::IncrementalCUDADeviceParser(std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>, clang::CompilerInstance&, llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>, llvm::Error&, std::__1::list<clang::PartialTranslationUnit, std::__1::allocator<clang::PartialTranslationUnit>> const&) (/opt/local/libexec/llvm-20/lib/libclang-cpp.dylib+0x216b8d0) llvm#6 0x0000000107f6bac8 clang::Interpreter::createWithCUDA(std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>, std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>) (/opt/local/libexec/llvm-20/lib/libclang-cpp.dylib+0x2173ac8) llvm#7 0x000000010206f8a8 main (/opt/local/libexec/llvm-20/bin/clang-repl+0x1000038a8) llvm#8 0x000000019ae8c274 Segmentation fault: 11 ``` The underlying issue was that the `DeviceCompilerInstance` (used for device-side CUDA compilation) was never initialized with a `Sema`, which is required before constructing the `IncrementalCUDADeviceParser`. https://github.com/llvm/llvm-project/blob/89687e6f383b742a3c6542dc673a84d9f82d02de/clang/lib/Interpreter/DeviceOffload.cpp#L32 https://github.com/llvm/llvm-project/blob/89687e6f383b742a3c6542dc673a84d9f82d02de/clang/lib/Interpreter/IncrementalParser.cpp#L31 Unlike the host-side `CompilerInstance` which runs `ExecuteAction` inside the Interpreter constructor (thereby setting up Sema), the device-side CI was passed into the parser uninitialized, leading to an assertion or crash when accessing its internals. To fix this, I refactored the `Interpreter::create` method to include an optional `DeviceCI` parameter. If provided, we know we need to take care of this instance too. Only then do we construct the `IncrementalCUDADeviceParser`. (cherry picked from commit 21fb19f)
llvm#138091) Check this error for more context (https://github.com/compiler-research/CppInterOp/actions/runs/14749797085/job/41407625681?pr=491#step:10:531) This fails with ``` * thread #1, name = 'CppInterOpTests', stop reason = signal SIGSEGV: address not mapped to object (fault address: 0x55500356d6d3) * frame #0: 0x00007fffee41cfe3 libclangCppInterOp.so.21.0gitclang::PragmaNamespace::~PragmaNamespace() + 99 frame #1: 0x00007fffee435666 libclangCppInterOp.so.21.0gitclang::Preprocessor::~Preprocessor() + 3830 frame #2: 0x00007fffee20917a libclangCppInterOp.so.21.0gitstd::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() + 58 frame llvm#3: 0x00007fffee224796 libclangCppInterOp.so.21.0gitclang::CompilerInstance::~CompilerInstance() + 838 frame llvm#4: 0x00007fffee22494d libclangCppInterOp.so.21.0gitclang::CompilerInstance::~CompilerInstance() + 13 frame llvm#5: 0x00007fffed95ec62 libclangCppInterOp.so.21.0gitclang::IncrementalCUDADeviceParser::~IncrementalCUDADeviceParser() + 98 frame llvm#6: 0x00007fffed9551b6 libclangCppInterOp.so.21.0gitclang::Interpreter::~Interpreter() + 102 frame llvm#7: 0x00007fffed95598d libclangCppInterOp.so.21.0gitclang::Interpreter::~Interpreter() + 13 frame llvm#8: 0x00007fffed9181e7 libclangCppInterOp.so.21.0gitcompat::createClangInterpreter(std::vector<char const*, std::allocator<char const*>>&) + 2919 ``` Problem : 1) The destructor currently handles no clearance for the DeviceParser and the DeviceAct. We currently only have this https://github.com/llvm/llvm-project/blob/976493822443c52a71ed3c67aaca9a555b20c55d/clang/lib/Interpreter/Interpreter.cpp#L416-L419 2) The ownership for DeviceCI currently is present in IncrementalCudaDeviceParser. But this should be similar to how the combination for hostCI, hostAction and hostParser are managed by the Interpreter. As on master the DeviceAct and DeviceParser are managed by the Interpreter but not DeviceCI. This is problematic because : IncrementalParser holds a Sema& which points into the DeviceCI. On master, DeviceCI is destroyed before the base class ~IncrementalParser() runs, causing Parser::reset() to access a dangling Sema (and as Sema holds a reference to Preprocessor which owns PragmaNamespace) we see this ``` * frame #0: 0x00007fffee41cfe3 libclangCppInterOp.so.21.0gitclang::PragmaNamespace::~PragmaNamespace() + 99 frame #1: 0x00007fffee435666 libclangCppInterOp.so.21.0gitclang::Preprocessor::~Preprocessor() + 3830 ``` (cherry picked from commit 529b6fc)
/// should use signed extension, false if zero extension (unsigned) | ||
bool MachineJumpTableInfo::getEntryIsSigned() const { | ||
switch (getEntryKind()) { | ||
case MachineJumpTableInfo::EK_BlockAddress: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is BlockAddress really signed? Same goes for the GP Rel entries. I know that's the existing behavior, but it seems odd to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's odd to me, too, but I'm being cautious here and not changing the existing behavior. I'll make sure to point that out during the upstream PR and see what people think.
@@ -1,6 +1,6 @@ | |||
; RUN: llc < %s -relocation-model=static | FileCheck %s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need the STATIC
check prefix?
The function already exposes a work list to avoid deep recursion, this commit starts utilizing it in a helper that could also lead to a deep recursion. We have observed this crash on `clang/test/C/C99/n590.c` with our internal builds that enable aggressive optimizations and hit the limit earlier than default release builds of Clang. See the added test for an example with a deeper recursion that used to crash in upstream Clang before this change with the following stack trace: ``` #0 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/Unix/Signals.inc:804:13 #1 llvm::sys::RunSignalHandlers() /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/Signals.cpp:106:18 #2 SignalHandler(int, siginfo_t*, void*) /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/Unix/Signals.inc:0:3 llvm#3 (/lib/x86_64-linux-gnu/libc.so.6+0x3fdf0) llvm#4 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12772:0 llvm#5 CheckCommaOperand /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:0:3 llvm#6 AnalyzeImplicitConversions /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12644:7 llvm#7 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12776:5 llvm#8 CheckCommaOperand /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:0:3 llvm#9 AnalyzeImplicitConversions /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12644:7 llvm#10 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12776:5 llvm#11 CheckCommaOperand /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:0:3 llvm#12 AnalyzeImplicitConversions /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12644:7 llvm#13 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12776:5 llvm#14 CheckCommaOperand /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:0:3 llvm#15 AnalyzeImplicitConversions /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12644:7 llvm#16 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12776:5 llvm#17 CheckCommaOperand /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:0:3 llvm#18 AnalyzeImplicitConversions /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12644:7 llvm#19 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12776:5 ... 700+ more stack frames. ```
- Use -passes="" so its correctly handed in DOS batch scripts - Move CHECKs inside tests to reduce regeneration diff when the update script can properly handle -asm-verbose=0
…6767) Rename `getXYZInstructionsByEnumValue()` to just `getXYZInstructions` and drop the `ByEnumValue` in the name.
Currently the default exception handling type is scattered across the backends in MCAsmInfo constructors. Allow this to be computed from the triple so the IR can centrally determine the set of ABI calls. Manually submitting, closes llvm#147225
The subheading for a destructor contained only the identifier. The tilde must also be included as it is necessary to differentiate the destructor from any constructors present. rdar://129587608
…lvm#147219) Fixes llvm#144775 --- This patch addresses a false-positive `-Wdocumentation` warning on `@tparam` comments attached to _variable template partial specializations_
C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\llvm\include\llvm/SandboxIR/Value.h(172,16): warning: unqualified friend declaration referring to type outside of the nearest enclosing namespace is a Microsoft extension; add a nested name specifier [-Wmicrosoft-unqualified-friend] Clang suggests adding ::llvm::, but: * Region is in ::llvm::sandboxir * Region is not defined at this point So forward declare it.
The 'cache' construct is an interesting one, in that it doesn't take any clauses, and is exclusively a collection of variables. Lowering wise, these just get added to the associated acc.loop. This did require some work to ensure that the cache doesn't have 'vars' that aren't inside of the loop, but Sema is taking care of that with a warning. Otherwise this is just a fairly simple amount of lowering, where each 'var' in the list creates an acc.cache, which is added to the acc.loop.
Implement functional cast to ComplexType llvm#141365
llvm#146766) This removes the `{BENIGN,COMPATIBLE}{,_ENUM,_VALUE}_LANGOPT` X macros controlling `LangOptions`. These are permutations of the base `LANGOPT`, `ENUM_LANGOPT` and `VALUE_LANGOPT` X macros that also carry the information of their effect on AST (and therefore module compatibility). Their functionality is now implemented by passing `Benign`, `Compatible` or `NotCompatible` argument to the base X macros and using C++17 `if constexpr` in the clients to achieve the same codegen. This PR solves this FIXME: ``` // FIXME: Clients should be able to more easily select whether they want // different levels of compatibility versus how to handle different kinds // of option. ``` The base X macros are preserved, since they are used in `LangOptions.h` to generate different kinds of field and function declarations for flags, values and enums, which can't be achieved with `if constexpr`. The new syntax also forces developers to think about compatibility when adding new language option, hopefully reducing the number of new options that are affecting by default even though they are benign or compatible. Note that the `BENIGN_` macros used to forward to their `COMPATIBLE_` counterparts. I don't think this ever kicked in, since there are no clients of the `.def` file that define `COMPATIBLE_` without also defining `BENIGN_`. However, this might be something downstream forks need to take care of by doing `if constexpr (CK::Compatibility == CK::Benign || CK::Compatibility == CK::Compatible)` in place of `#define COMPATIBLE_`.
… arg (llvm#146910) This is the `CodeGenOptions` counterpart to llvm#146766.
This revision adds DeviceMaskingAttrInterface and extends DeviceMappingArrayAttr to accept a union of DeviceMappingAttrInterface and DeviceMaskingAttrInterface. Support is added to GPUTransformOps to take advantage of this information and lower to block/warpgroup/warp/thread specialization when mapped to linear ids. The revision also connects to scf::ForallOp and uses the new attribute to implement warp specialization. The implementation is in the form of a GPUMappingMaskAttr, which can be additionally passed to the scf.forall.mapping attribute to specify a mask on compute resources that should be active. In the first implementation the masking is a bitfield that specifies for each processing unit whether it is active or not. In the future, we may want to implement this as a symbol to refer to dynamically defined values. Extending op semantics with an operand is deemed too intrusive at this time. --------- Co-authored-by: Oleksandr "Alex" Zinenko <[email protected]>
Change undef branch conditions to the values that loop-simplify gives them, and handle other undef values by using extra arguments. I'm making this change because of an upcoming loop strength reduction change that results in instsimplify removing more instructions due to them using undef, causing the test checks to fail.
Fixes `llvm#146802` llvm#146582 started using the `Reserved` Frame Pointer kind for Arm64 Windows, but this revealed a bug in Flang where it copied the `-mframe-pointer=reserved` flag from Clang, but didn't correctly handle it in its own command line parser and subsequent compilation pipeline. This change adds support for `-mframe-pointer=reserved` and adds a test to make sure that functions are correctly marked when the flag is set.
…r vector floating point operations (llvm#146792) Fixes llvm#146428 _fltused reference generated for vector floating point operations Currently references to _fltused are incorrectly emitted due to the presence of vector floating point operations. This causes spurious references to _fltused in vector floating point system code where the CRT is not used. This issue is limited to the X86 MSVC environment. As described in the bug: * _fltused should only be emitted for floating point operations as the reference is used to initialize some parts of FP CRT support. * Vector floating point operations on their own don't require that CRT support. Have confirmed intended behavior with MSVC team. Fix alters usesMSVCFloatingPoint() to look for floating point instructions/operands rather than floating point or vector floating point.
Co-authored-by: Son Tuan Vu <[email protected]>
…lvm#146916) This was an oversight in llvm#128942 where I forgot to add the configured page size to the `WasmLimits` in the import we emit when importing a memory. Fixes: llvm#146713
) Implement AbstractConditionalOperator support for ComplexType llvm#141365
…c (NFC) (llvm#146784) Change the name of `ValueObjectSyntheticFilter.{h,cpp}` to match the main type they declare and define: `ValueObjectSynthetic`.
llvm#147354) Re-land llvm#146582 now that the Flang bugs have been fixed. There is no way in Arm64 Windows to indicate that a given function has used the Frame Pointer as a General Purpose Register, as such stack walks will always assume that the frame chain is valid and will follow whatever value has been saved for the Frame Pointer (even if it is pointing to data, etc.). This change makes the Frame Pointer always reserved when building for Arm64 Windows to avoid this issue. We will be updating the official Windows ABI documentation to reflect this requirement, and I will provide a link once it's available.
When complete record support was initially added, the parsing support was left incomplete. This change adds the necessary parsing.
…ts (llvm#147566) Forked from llvm/test/CodeGen/X86.
) Fixes llvm#146973 When an object with alignment requirements is placed on the stack, this causes a stack realignment which causes AArch64 to use x19 to refer to objects on the stack as there may be a gap between local variables and the Stack Pointer. This causes issues with the MSVC C++ exception personality as the offset to the catch object recorded in the handler table no longer matches the object being used in the catch block itself. The fix for this is to place catch objects into the fixed object area.
…_truncf to amdgpu (llvm#146372) - add conversion from arith.scaling_extf to amdgpu.scaled_ext_packed - add conversion from arith.scaling_truncf to amdgpu.packed_scaled_trunc
D is already of CXXMethodDecl *.
AfterCSRPopSize is already of int64_t.
I is already of int64_t.
These are identified by misc-include-cleaner. I've filtered out those that break builds. Also, I'm staying away from llvm-config.h, config.h, and Compiler.h, which likely cause platform- or compiler-specific build failures.
Co-authored-by: Mekhanoshin, Stanislav <[email protected]>
This should improve synchronizing the MainLoopWindows monitor thread with the main loop state. This uses the `m_ready` and `m_event` event handles to manage when the Monitor thread continues and adds new tests to cover additional use cases. I believe this should fix llvm#147291 but it is hard to ensure a race condition is fixed without running the CI on multiple machines/configurations. --------- Co-authored-by: Pavel Labath <[email protected]>
… like matchers (llvm#147071) Fixes llvm#147083
getOrCreateResultFromMemIntrinsic can modify the current function by inserting new instructions without EarlyCSE keeping track of the changes. Introduce a new CanCreate argument, and update the function to only create new instructions when CanCreate = true. Use it when appropriate. Fixes llvm#145183
…immediate. (llvm#147458) This applies the same check as llvm.vector.splice which checks that the immediate is in the range [-VL, VL-1] where VL is the minimum vector length. If vscale_range is available, the lower bound is used to increase the known minimum vector length for this check. This ensures the immediate is in range for any possible value of vscale that satisfies the vscale_range.
This patch fixes: third-party/unittest/googletest/include/gtest/gtest.h:1379:11: error: comparison of integers of different signs: 'const unsigned long' and 'const int' [-Werror,-Wsign-compare]
Co-authored-by: Mirko Brkušanin <[email protected]>
…46966) Update tests to fix silently failing test and handle when a module is removed.
…ion is lower than 1.6 (llvm#147432) Fixes llvm#147394 References DXC for the implementation logic: https://github.com/microsoft/DirectXShaderCompiler/blob/d751c827ed3b61e87fdf57d0f424cb2d7af30cd0/lib/HLSL/DxilPreparePasses.cpp#L693-L699 If DXIL Version < 1.6 then replace lifetime intrinsics with stores - For validator version >= 1.6, store an undef - For validator version < 1.6, store zeros else keep the lifetime intrinsics in the DXIL. After this PR, the number of DML shaders failing validation due to llvm#146974 is reduced from 157 to 50.
…vm#144970) 443377a handled simple variable definitions, but it didn't handle uninitialized variables with a consteval constructor, and it didn't handle template instantiation. Fixes llvm#135281 .
This patch fixes: lldb/tools/lldb-dap/ProtocolUtils.cpp:77:22: error: implicit instantiation of undefined template 'std::basic_ostringstream<char>'
…rget divergency. (llvm#147560) This is the next attempt to upstream this: llvm#144947 The las one caused build errors in AArch64. Issue was resolved.
Same as llvm#146696 but for llvm#145357. --------- Signed-off-by: Justin King <[email protected]>
Working in the Breakpoint library is a minefield if you have your editor configured to trim trailing whitespace. Remove it and format the affected lines.
…vm#147011) Commands that take an address expression/address through the OptionArgParser::ToAddress method, which has filtered this user-specified address through one of the Process Fix methods to clear non-addressable bits (MTE, PAC, top byte ignore, etc). We don't know what class of address this is, IMEM or DMEM, but this method is passing the addresses through Process::FixCodeAddress, and on at least one target, FixCodeAddress clears low bits which are invalid for instructions. Correct this to use FixAnyAddress, which doesn't make alignment assumptions. The actual issue found was by people debugging on a 32-bit ARM Cortex-M part, who tried to do a memory read from an odd address, and lldb returned results starting at the next lower even address. rdar://154885727
d2d593b
to
e9aeef6
Compare
e9aeef6
to
e717cd0
Compare
Fix unnecessary conversion of C-String to StringRef in the `Cmp` lambda inside `lookupLLVMIntrinsicByName`. This both fixes an ASAN error in the code that happens when the `Name` StringRef passed in is not a Null terminated StringRef, and additionally can potentially speed up the code as well by eliminating the unnecessary computation of string length every time a C String is converted to StringRef in this code (It seems practically this computation is eliminated in optimized builds, but this will avoid it in O0 builds as well). Added a unit test that demonstrates this issue by building LLVM with these options: ``` CMAKE_BUILD_TYPE=Debug LLVM_USE_SANITIZER=Address LLVM_OPTIMIZE_SANITIZED_BUILDS=OFF ``` The error reported is as follows: ``` ==462665==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x5030000391a2 at pc 0x56525cc30bbf bp 0x7fff9e4ccc60 sp 0x7fff9e4cc428 READ of size 19 at 0x5030000391a2 thread T0 #0 0x56525cc30bbe in strlen (upstream-llvm-second/llvm-project/build/unittests/IR/IRTests+0x713bbe) (BuildId: 0651acf1e582a4d2) #1 0x7f8ff22ad334 in std::char_traits<char>::length(char const*) /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/char_traits.h:399:9 #2 0x7f8ff22a34a0 in llvm::StringRef::StringRef(char const*) /home/rjoshi/upstream-llvm-second/llvm-project/llvm/include/llvm/ADT/StringRef.h:96:33 llvm#3 0x7f8ff28ca184 in _ZZL25lookupLLVMIntrinsicByNameN4llvm8ArrayRefIjEENS_9StringRefES2_ENK3$_0clIjPKcEEDaT_T0_ upstream-llvm-second/llvm-project/llvm/lib/IR/Intrinsics.cpp:673:18 ```
On AMD64 and ARM64, use ImageBase-relative jump table offsets