Skip to content

[llvm-debuginfo-analyzer] Fix a couple of unhandled DWARF situations leading to a crash #1

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

Conversation

jalopezg-git
Copy link

@jalopezg-git jalopezg-git commented Apr 24, 2025

This pull request fixes a couple of unhandled situations in DWARF input leading to a crash. Specifically,

  • If the DWARF input contains a declaration of a C variadic function (where ... translates to DW_TAG_unspecified_parameters), which is then followed by a definition, llvm_unreachable() is hit in LVScope::addMissingElements().
    This is only visible in Debug builds (see stack trace below), but still. test-dwarf-clang-unspec-params.elf triggers this condition.
Invalid symbol kind.
UNREACHABLE executed at /home/jalopezg/repos/llvm-project/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp:345!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: _build/Debug/bin/llvm-debuginfo-analyzer --print=all --attribute=all /tmp/test-dwarf-clang-unspec-params.elf
 #0 0x00005577295666f6 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/jalopezg/repos/llvm-project/llvm/lib/Support/Unix/Signals.inc:804:22
 #1 0x0000557729566b09 PrintStackTraceSignalHandler(void*) /home/jalopezg/repos/llvm-project/llvm/lib/Support/Unix/Signals.inc:880:1
 #2 0x0000557729563f1f llvm::sys::RunSignalHandlers() /home/jalopezg/repos/llvm-project/llvm/lib/Support/Signals.cpp:105:20
 #3 0x0000557729565fb4 SignalHandler(int, siginfo_t*, void*) /home/jalopezg/repos/llvm-project/llvm/lib/Support/Unix/Signals.inc:418:13
 #4 0x00007fc5ff23e710 (/usr/lib/libc.so.6+0x3e710)
 #5 0x00007fc5ff28e83c (/usr/lib/libc.so.6+0x8e83c)
 #6 0x00007fc5ff23e668 gsignal (/usr/lib/libc.so.6+0x3e668)
 #7 0x00007fc5ff2264b8 abort (/usr/lib/libc.so.6+0x264b8)
 #8 0x00005577294ad073 bindingsErrorHandler(void*, char const*, bool) /home/jalopezg/repos/llvm-project/llvm/lib/Support/ErrorHandling.cpp:223:55
 #9 0x0000557728f56b0c llvm::logicalview::LVScope::addMissingElements(llvm::logicalview::LVScope*) /home/jalopezg/repos/llvm-project/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp:322:5
#10 0x0000557728f5f4b7 llvm::logicalview::LVScopeFunction::resolveReferences() /home/jalopezg/repos/llvm-project/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp:1773:9
  • Parsing of instructions in LVBinaryReader::createInstructions() does not check whether Offset lies within the Bytes ArrayRef. A specially crafted DWARF input can lead to this condition.

__
NOTE: this PR is a backport of upstream llvm#137221 to the zmpr-b-llvmorg-19.1.1-patches branch. Such branch is forked from upstream tag llvmorg-19.1.1.

FYI @peledins-zimperium.

@jalopezg-git jalopezg-git added the bug Something isn't working label Apr 24, 2025
@jalopezg-git jalopezg-git force-pushed the zmpr-b-llvmorg-19.1.1-logicalview-fixes branch from 7275173 to f5bbbef Compare April 25, 2025 10:34
Copy link
Author

@jalopezg-git jalopezg-git left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for review 👍; replied to your comments; PTAL.

Copy link

@peledins-zimperium peledins-zimperium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jalopezg-git jalopezg-git force-pushed the zmpr-b-llvmorg-19.1.1-logicalview-fixes branch from f5bbbef to 74d31a8 Compare April 25, 2025 12:12
@jalopezg-git jalopezg-git merged commit b842f92 into Zimperium:zmpr-b-llvmorg-19.1.1-patches Apr 25, 2025
2 checks passed
@jalopezg-git jalopezg-git deleted the zmpr-b-llvmorg-19.1.1-logicalview-fixes branch April 25, 2025 13:13
peledins-zimperium pushed a commit that referenced this pull request May 22, 2025
…ible (llvm#123752)

This patch adds a new option `-aarch64-enable-zpr-predicate-spills`
(which is disabled by default), this option replaces predicate spills
with vector spills in streaming[-compatible] functions.

For example:

```
str	p8, [sp, llvm#7, mul vl]            // 2-byte Folded Spill
// ...
ldr	p8, [sp, llvm#7, mul vl]            // 2-byte Folded Reload
```

Becomes:

```
mov	z0.b, p8/z, #1
str	z0, [sp]                        // 16-byte Folded Spill
// ...
ldr	z0, [sp]                        // 16-byte Folded Reload
ptrue	p4.b
cmpne	p8.b, p4/z, z0.b, #0
```

This is done to avoid streaming memory hazards between FPR/vector and
predicate spills, which currently occupy the same stack area even when
the `-aarch64-stack-hazard-size` flag is set.

This is implemented with two new pseudos SPILL_PPR_TO_ZPR_SLOT_PSEUDO
and FILL_PPR_FROM_ZPR_SLOT_PSEUDO. The expansion of these pseudos
handles scavenging the required registers (z0 in the above example) and,
in the worst case spilling a register to an emergency stack slot in the
expansion. The condition flags are also preserved around the `cmpne` in
case they are live at the expansion point.
peledins-zimperium pushed a commit that referenced this pull request May 22, 2025
`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)
#3 0x000000019b242de4 (/usr/lib/system/libsystem_platform.dylib+0x180482de4)
#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)
#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)
peledins-zimperium pushed a commit that referenced this pull request May 22, 2025
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 #3: 0x00007fffee224796 libclangCppInterOp.so.21.0gitclang::CompilerInstance::~CompilerInstance() + 838
    frame #4: 0x00007fffee22494d libclangCppInterOp.so.21.0gitclang::CompilerInstance::~CompilerInstance() + 13
    frame #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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants