Skip to content

Conversation

@dmpots
Copy link
Collaborator

@dmpots dmpots commented Jul 1, 2025

Draft PR to discuss issues.

clayborg and others added 30 commits March 19, 2025 17:02
This patch implements that changes needed to allow a GPU plug-in to
detect it is loadable and return a connection URL back to LLDB. LLDB
will attach to the GPU process automatically by having lldb-server
listen on port 0, figure out the real port and send it back to LLDB.
ProcessGDBRemote will create a new target and attach to it
automatically.
This patch formalizes the plug-ins and moves the code into the LLDB
library lldbPluginProcessGDBRemote. This allows the sever objects
GDBRemoteCommunicationServerLLGS to own a list of GPU plug-ins.

Removed the process stopped callback and we now iterate over all of the
installed GPU plug-ins in the GDBRemoteCommunicationServerLLGS class
itself. This will allow more functionality to be implemented for GPU
plugins.
GPU plugins can now intialize a GPUPluginInfo info structure in
LLDBServerPlugin::InitializePluginInfo() by overriding this function.
This info will be sent to LLDB. LLDB will now set any breakpoints that
are requested in this structure and the plugins can now specify a
breakpoint by name with a shared library basename and specify any
symbols that are desired when the breakpoint gets hit. When the
breakpoints get hit, the function:
void LLDBServerPlugin::BreakpointWasHit(GPUPluginBreakpointHitArgs &args)
will get called with any symbol values that were requested.
The response to jGPUPluginBreakpointHit is now JSON and can:
- disable the breakpoint if disable_bp is set to true
- set new breakpoints if there is an array of new breakpoints to set
- return a connect URL to allow reverse connection

The functionality for disabling the breakpoint, setting new breakpoints
and connecting to the URL is still TODO
Many GDB remote packets send JSON. This patch add new Put methods to
StreamGDBRemote to facilitate sendings objects that are converted to
JSON and sent as escaped JSON text.
- Added the ability to encode JSON as hex ascii so JSON can be used in
  stop reply packets
- Added exe_path to GPUPluginConnectionInfo
- Added StringExtractorGDBRemote::GetFromJSONText to decode objects from
  JSON text.
- Added StringExtractorGDBRemote::GetFromJSONHexASCII to decode objects from
  JSON HEX ASCII.
- Change GDBRemoteCommunicationClient::GPUBreakpointHit to return an
  optional GPUPluginBreakpointHitResponse and moved TODO work over into
  caller function where it makes more sense.
- Change stop reply GPU connections to use JSON encoding of a
  GPUPluginConnectionInfo object.
- Remove LLDBServerPlugin::GetConnectionURL() and replace with a
  function that is designed to always create a connection.
- Added LLDBServerPlugin::NativeProcessIsStopping() to let the GPU
  plug-in know that the native process is stopping in case the plug-in
  wants to start a connection on stop. The Mock GPU plug-in isn't using
  this feature anymore.
- Handle disabling the breakpoint in ProcessGDBRemote::GPUBreakpointHit
  and also setting any new breakpoints and connecting to LLDB if the
  breakpoint hit response struct GPUPluginBreakpointHitResponse asks for
  it.
- Rename "gpu-url" in the stop reply packet to "gpu-connection" and
  convert it to use hex ASCII encoded JSON version of GPUPluginConnectionInfo
- Added ProcessGDBRemote::HandleGPUBreakpoints() to handle any
  breakpoints requested by the GPU plug-ins since those requests can now
  be done by the plugin info or a breakpoint hit response
- Added ProcessGDBRemote::HandleConnectionRequest() to handle reverse
  connecting from stop reply or from breakpoint hit response.
- Fix the size of RegisterContextMockGPU::g_gpr_regnums and g_vec_regnums
This current patch allows GPU plug-ins to return GPUActions structs at
various times:
- Each plugin must return a GPUActions to response to the function
  GPUActions LLDBServerPlugin::GetInitializeActions() which replaces
  the old and deprecated void LLDBServerPlugin::InitializePluginInfo()
- Optionally to a LLDBServerPlugin::NativeProcessIsStopping() call each
  time the process stops
- In a breakpoint was hit response.

This allows many ways to set breakpoints at various times. The
GPUActions struct also allows a connection to be made at all three of
the above times for GPUActions.

Changes:
- The SymbolValue class now has its "value" as an optional uint64_t. If
the symbol value that is sent to the GPU plugin during a breakpoint was
hit method call doesn't have a value, then the symbol is not avaiable or
can't be resolved to a load address.
- Removed the GPUPluginInfo class, plugins now return a GPUActions
  object.
- Added a GPUActions object that has the plug-in name, any breakpoints
  that need to be set, and an optional connection info struct. This
  allows plug-ins to return a GPUActions in response to the initializing
  of the plugin, breakpoint was hit or each time the native process stops.
- Added GDBRemoteCommunicationServerLLGS::GetCurrentProcess() to allow
  plug-ins to access the current NativeProcessProtocol.
- Removed LLDBServerPlugin::GetPluginInfo() and
  LLDBServerPlugin::InitializePluginInfo(). Clients now must implement
  the GPUActions LLDBServerPlugin::GetInitializeActions()
- Removed the virtual CreateConnection() from LLDBServerPlugin.
- Updated the Mock GPU to set a new breakpoint on the third stop to test
  that we can return a GPUActions from a native process stop.
Cleaned up the register context code to auto generate the register
offset in the RegisterContext class. Also rename m_regs_valid to
m_reg_value_is_valid to be more clear that this represents if the
value of the register is valid in the data structures.
Added a "jGPUPluginGetDynamicLoaderLibraryInfo" packet that gets GPU
dynamic loader information from a GPU. It does this when the stop reason
for a thread is set to eStopReasonDynammicLoader. The
NativeProcessProtocol::GetGPUDynamicLoaderLibraryInfos() will get called
with the arguments to get the full list of shared libraries or a partial
list.

The shared libraries can be specified in many ways:
- path to a library on disk for self contains object files
- path to a file on disk that contains an object file at a file offset
  and file size
- native process memory location where the object file is loaded and
  should be read from

Shared libraries can then specify how to get loaded by:
- Specify a load address where all sections will be slid evenly
- Specify each section within an ELF file and exactly where they should
  be loaded, this can include subsections being loaded
Dymamic loader for GPUs is implemented by the NativeProcessProtocol
function:

std::optional<GPUDynamicLoaderResponse>
NativeProcessProtocol::GetGPUDynamicLoaderLibraryInfos(const GPUDynamicLoaderArgs &args);

Most of the functionality is here, still need to make the sections load
correctly.
Changed the definition of GPUSectionInfo to not have children, it now
can have an array of section names that define the hiearchy to use when
finding the section by names. If there is only one section name, then we
find it regardless of the hiearchy.

Hooked up the loading of sections and the loading of the entire file.
This patch makes it so that a breakpoint in the native process can cause
shared libraries to be loaded in the GPU process. When GPU plug-ins
respond to the LLDBServerPlugin method:

  GPUPluginBreakpointHitResponse
  LLDBServerPlugin::BreakpointWasHit(GPUPluginBreakpointHitArgs &args);

They can now set the GPUPluginBreakpointHitResponse.load_libraries to
true. The GPU plug-in should already hvae notified LLDB that it is
stopped prior to sending this to ensure that no progress is made on the
GPU while shared libraries are loaded and breakpoints get resolved.

Code was added to Target.h that allows a target to know about all of the
installed GPU plug-ins. This allows a native process to make calls on
the GPU process, and also for the GPU process to get the native target.
This will allow code to resume both targets when one gets resumed if
that is the preferred methodology for the native process and GPU. It
also allows us to stop the native target from the GPU target and vice
versa.

The LLDBServerPluginMockGPU now tests that breakpoints by address work
by setting a brekapoint by address from the "gdb_shlib_load" symbol that
is requested by the "gpu_initialize" breakpoint.
The idea behind this change is a GDB server can return a "reason:dyld;"
by having its NativeThreadProtocol class return a stop reason of
lldb::eStopReasonDynammicLoader. This allows the GPU to halt its process
and return this stop reason during a LLDBServerPlugin::BreakpointWasHit()
call. When the GPU reports this stop the shared libraries will be
requested and a stop StopReasonDyld() will be created in LLDB that will
auto resume the GPU process.
Adding the this pointer to each log line for sending and receiving
packets allows us to see the log for the native process and GPU plug-in
so we can tell which communication is being used.
Only allow the native process linux to claim it has GPU plug-ins. Prior
to this change the GPU GDB server connection was claiming to have GPU
plug-ins as well.
Anytime a GPUActions is returned, clients can now set
GPUActions.resume_gpu_process = true if they want to resume the GPU
process from the native process.
This patch enables a ModuleSpec to specify the file offset and size
with:

void SBModuleSpec::SetObjectOffset(uint64_t offset);
void SBModuleSpec::SetObjectSize(uint64_t size);

And allows LLDB to load the right file for AMD.
If the GPU plug-ins need to halt the process so that the GPU plug-in
will receive a LLDBServerPlugin::NativeProcessIsStopping(...) call, then
it can call this function. This allows synchronization with the native
process and the GPU plug-in can return GPUActions to perform.
resume.

GPUActions now has a new "bool wait_for_gpu_process_to_resume" member
variable. If this is set then when the native process gets GPUActions,
they will wait for the GPU process to resume.
…the logs

This allows differenciating GPU and CPU gdb-remote logs emitted by the
server.

E.g.
```
1749485814.359431028 [754988/755010] nvidia-gpu.server <  19> read packet: $QStartNoAckMode#b0
1749485814.359459400 [754988/755010] nvidia-gpu.server <   1> send packet: +

1749485813.658699989 [754988/754988] gdb-server <  29> read packet: $qXfer:auxv:read::0,131071#d7
1749485813.658788681 [754988/754988] gdb-server < 341> send packet: $l!
```
walter-erquinigo and others added 10 commits June 9, 2025 17:07
This allows differenciating GPU and CPU gdb-remote logs emitted by the server.

E.g.

```
1749485814.359431028 [754988/755010] nvidia-gpu.server <  19> read packet: $QStartNoAckMode#b0
1749485814.359459400 [754988/755010] nvidia-gpu.server <   1> send packet: +

1749485813.658699989 [754988/754988] gdb-server <  29> read packet: $qXfer:auxv:read::0,131071#d7
1749485813.658788681 [754988/754988] gdb-server < 341> send packet: $l!
```
* Add some constructors that receive the plugin name as argument. This comes in handy because the plugin_name is required.
* Do some code formatting
Server errors were being disposed right after parsing in the client. Instead, now they are displayed to the user. This helps a lot with development.
I'm using Debugger::Report error for this because they are non-blocking issues, as the CPU target can keep being debugged, but the user needs to know that the GPU plugin couldn't be initialized and they can report the issue if they want it.
Check object offset in ModuleSpec matching
@clayborg clayborg force-pushed the llvm-server-plugins branch from 2368a38 to f24f12e Compare July 1, 2025 18:08
@dmpots dmpots closed this Jul 1, 2025
pveras pushed a commit to pveras/llvm-project that referenced this pull request Oct 10, 2025
A recent change adding a new sanitizer kind (via Sanitizers.def) was
reverted in c74fa20 ("Revert "[Clang][CodeGen] Introduce the
AllocToken SanitizerKind" (llvm#162413)"). The reason was this ASan report,
when running the test cases in
clang/test/Preprocessor/print-header-json.c:

```
==clang==483265==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7d82b97e8b58 at pc 0x562cd432231f bp 0x7fff3fad0850 sp 0x7fff3fad0848
READ of size 16 at 0x7d82b97e8b58 thread T0
    #0 0x562cd432231e in __copy_non_overlapping_range<const unsigned long *, const unsigned long *> zorg-test/libcxx_install_asan_ubsan/include/c++/v1/string:2144:38
    clayborg#1 0x562cd432231e in void std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>::__init_with_size[abi:nn220000]<unsigned long const*, unsigned long const*>(unsigned long const*, unsigned long const*, unsigned long) zorg-test/libcxx_install_asan_ubsan/include/c++/v1/string:2685:18
    clayborg#2 0x562cd41e2797 in __init<const unsigned long *, 0> zorg-test/libcxx_install_asan_ubsan/include/c++/v1/string:2673:3
    clayborg#3 0x562cd41e2797 in basic_string<const unsigned long *, 0> zorg-test/libcxx_install_asan_ubsan/include/c++/v1/string:1174:5
    clayborg#4 0x562cd41e2797 in clang::ASTReader::ReadString(llvm::SmallVectorImpl<unsigned long> const&, unsigned int&) clang/lib/Serialization/ASTReader.cpp:10171:15
    clayborg#5 0x562cd41fd89a in clang::ASTReader::ParseLanguageOptions(llvm::SmallVector<unsigned long, 64u> const&, llvm::StringRef, bool, clang::ASTReaderListener&, bool) clang/lib/Serialization/ASTReader.cpp:6475:28
    clayborg#6 0x562cd41eea53 in clang::ASTReader::ReadOptionsBlock(llvm::BitstreamCursor&, llvm::StringRef, unsigned int, bool, clang::ASTReaderListener&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&) clang/lib/Serialization/ASTReader.cpp:3069:11
    clayborg#7 0x562cd4204ab8 in clang::ASTReader::ReadControlBlock(clang::serialization::ModuleFile&, llvm::SmallVectorImpl<clang::ASTReader::ImportedModule>&, clang::serialization::ModuleFile const*, unsigned int) clang/lib/Serialization/ASTReader.cpp:3249:15
    clayborg#8 0x562cd42097d2 in clang::ASTReader::ReadASTCore(llvm::StringRef, clang::serialization::ModuleKind, clang::SourceLocation, clang::serialization::ModuleFile*, llvm::SmallVectorImpl<clang::ASTReader::ImportedModule>&, long, long, clang::ASTFileSignature, unsigned int) clang/lib/Serialization/ASTReader.cpp:5182:15
    clayborg#9 0x562cd421ec77 in clang::ASTReader::ReadAST(llvm::StringRef, clang::serialization::ModuleKind, clang::SourceLocation, unsigned int, clang::serialization::ModuleFile**) clang/lib/Serialization/ASTReader.cpp:4828:11
    clayborg#10 0x562cd3d07b74 in clang::CompilerInstance::findOrCompileModuleAndReadAST(llvm::StringRef, clang::SourceLocation, clang::SourceLocation, bool) clang/lib/Frontend/CompilerInstance.cpp:1805:27
    clayborg#11 0x562cd3d0b2ef in clang::CompilerInstance::loadModule(clang::SourceLocation, llvm::ArrayRef<clang::IdentifierLoc>, clang::Module::NameVisibilityKind, bool) clang/lib/Frontend/CompilerInstance.cpp:1956:31
    clayborg#12 0x562cdb04eb1c in clang::Preprocessor::HandleHeaderIncludeOrImport(clang::SourceLocation, clang::Token&, clang::Token&, clang::SourceLocation, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) clang/lib/Lex/PPDirectives.cpp:2423:49
    clayborg#13 0x562cdb042222 in clang::Preprocessor::HandleIncludeDirective(clang::SourceLocation, clang::Token&, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) clang/lib/Lex/PPDirectives.cpp:2101:17
    clayborg#14 0x562cdb043366 in clang::Preprocessor::HandleDirective(clang::Token&) clang/lib/Lex/PPDirectives.cpp:1338:14
    clayborg#15 0x562cdafa84bc in clang::Lexer::LexTokenInternal(clang::Token&, bool) clang/lib/Lex/Lexer.cpp:4512:7
    clayborg#16 0x562cdaf9f20b in clang::Lexer::Lex(clang::Token&) clang/lib/Lex/Lexer.cpp:3729:24
    clayborg#17 0x562cdb0d4ffa in clang::Preprocessor::Lex(clang::Token&) clang/lib/Lex/Preprocessor.cpp:896:11
    clayborg#18 0x562cd77da950 in clang::ParseAST(clang::Sema&, bool, bool) clang/lib/Parse/ParseAST.cpp:163:7
    [...]

0x7d82b97e8b58 is located 0 bytes after 3288-byte region [0x7d82b97e7e80,0x7d82b97e8b58)
allocated by thread T0 here:
    #0 0x562cca76f604 in malloc zorg-test/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:67:3
    clayborg#1 0x562cd1cce452 in safe_malloc llvm/include/llvm/Support/MemAlloc.h:26:18
    clayborg#2 0x562cd1cce452 in llvm::SmallVectorBase<unsigned int>::grow_pod(void*, unsigned long, unsigned long) llvm/lib/Support/SmallVector.cpp:151:15
    clayborg#3 0x562cdbe1768b in grow_pod llvm/include/llvm/ADT/SmallVector.h:139:11
    clayborg#4 0x562cdbe1768b in grow llvm/include/llvm/ADT/SmallVector.h:525:41
    clayborg#5 0x562cdbe1768b in reserve llvm/include/llvm/ADT/SmallVector.h:665:13
    clayborg#6 0x562cdbe1768b in llvm::BitstreamCursor::readRecord(unsigned int, llvm::SmallVectorImpl<unsigned long>&, llvm::StringRef*) llvm/lib/Bitstream/Reader/BitstreamReader.cpp:230:10
    clayborg#7 0x562cd41ee8ab in clang::ASTReader::ReadOptionsBlock(llvm::BitstreamCursor&, llvm::StringRef, unsigned int, bool, clang::ASTReaderListener&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&) clang/lib/Serialization/ASTReader.cpp:3060:49
    clayborg#8 0x562cd4204ab8 in clang::ASTReader::ReadControlBlock(clang::serialization::ModuleFile&, llvm::SmallVectorImpl<clang::ASTReader::ImportedModule>&, clang::serialization::ModuleFile const*, unsigned int) clang/lib/Serialization/ASTReader.cpp:3249:15
    clayborg#9 0x562cd42097d2 in clang::ASTReader::ReadASTCore(llvm::StringRef, clang::serialization::ModuleKind, clang::SourceLocation, clang::serialization::ModuleFile*, llvm::SmallVectorImpl<clang::ASTReader::ImportedModule>&, long, long, clang::ASTFileSignature, unsigned int) clang/lib/Serialization/ASTReader.cpp:5182:15
    clayborg#10 0x562cd421ec77 in clang::ASTReader::ReadAST(llvm::StringRef, clang::serialization::ModuleKind, clang::SourceLocation, unsigned int, clang::serialization::ModuleFile**) clang/lib/Serialization/ASTReader.cpp:4828:11
    clayborg#11 0x562cd3d07b74 in clang::CompilerInstance::findOrCompileModuleAndReadAST(llvm::StringRef, clang::SourceLocation, clang::SourceLocation, bool) clang/lib/Frontend/CompilerInstance.cpp:1805:27
    clayborg#12 0x562cd3d0b2ef in clang::CompilerInstance::loadModule(clang::SourceLocation, llvm::ArrayRef<clang::IdentifierLoc>, clang::Module::NameVisibilityKind, bool) clang/lib/Frontend/CompilerInstance.cpp:1956:31
    clayborg#13 0x562cdb04eb1c in clang::Preprocessor::HandleHeaderIncludeOrImport(clang::SourceLocation, clang::Token&, clang::Token&, clang::SourceLocation, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) clang/lib/Lex/PPDirectives.cpp:2423:49
    clayborg#14 0x562cdb042222 in clang::Preprocessor::HandleIncludeDirective(clang::SourceLocation, clang::Token&, clang::detail::SearchDirIteratorImpl<true>, clang::FileEntry const*) clang/lib/Lex/PPDirectives.cpp:2101:17
    clayborg#15 0x562cdb043366 in clang::Preprocessor::HandleDirective(clang::Token&) clang/lib/Lex/PPDirectives.cpp:1338:14
    clayborg#16 0x562cdafa84bc in clang::Lexer::LexTokenInternal(clang::Token&, bool) clang/lib/Lex/Lexer.cpp:4512:7
    clayborg#17 0x562cdaf9f20b in clang::Lexer::Lex(clang::Token&) clang/lib/Lex/Lexer.cpp:3729:24
    clayborg#18 0x562cdb0d4ffa in clang::Preprocessor::Lex(clang::Token&) clang/lib/Lex/Preprocessor.cpp:896:11
    clayborg#19 0x562cd77da950 in clang::ParseAST(clang::Sema&, bool, bool) clang/lib/Parse/ParseAST.cpp:163:7
    [...]

SUMMARY: AddressSanitizer: heap-buffer-overflow clang/lib/Serialization/ASTReader.cpp:10171:15 in clang::ASTReader::ReadString(llvm::SmallVectorImpl<unsigned long> const&, unsigned int&)
```

The reason is this particular RUN line:
```
// RUN: env CC_PRINT_HEADERS_FORMAT=json CC_PRINT_HEADERS_FILTERING=direct-per-file CC_PRINT_HEADERS_FILE=%t.txt %clang -fsyntax-only -I %S/Inputs/print-header-json -isystem %S/Inputs/print-header-json/system -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -o /dev/null
```

which was added in 8df194f ("[Clang] Support includes translated to
module imports in -header-include-filtering=direct-per-file (llvm#156756)").

The problem is caused by an incremental build reusing stale cached
module files (.pcm) that are no longer binary-compatible with the
updated compiler. Adding a new sanitizer option altered the implicit
binary layout of the serialized LangOptions data structure. The build +
test system is oblivious to such changes. When the new compiler
attempted to read the old module file (from the previous test
invocation), it misinterpreted the data due to the layout mismatch,
resulting in a heap-buffer-overflow. Unfortunately Clang's PCM format
does not encode nor detect version mismatches here; a more graceful
failure mode would be preferable.

For now, fix the test to be more robust with incremental build + test.
clayborg pushed a commit that referenced this pull request Oct 24, 2025
**Mitigation for:** google/sanitizers#749

**Disclosure:** I'm not an ASan compiler expert yet (I'm trying to
learn!), I primarily work in the runtime. Some of this PR was developed
with the help of AI tools (primarily as a "fuzzy `grep` engine"), but
I've manually refined and tested the output, and can speak for every
line. In general, I used it only to orient myself and for
"rubberducking".

**Context:**

The msvc ASan team (👋 ) has received an internal request to improve
clang's exception handling under ASan for Windows. Namely, we're
interested in **mitigating** this bug:
google/sanitizers#749

To summarize, today, clang + ASan produces a false-positive error for
this program:

```C++
#include <cstdio>
#include <exception>
int main()
{
	try	{
		throw std::exception("test");
	}catch (const std::exception& ex){
		puts(ex.what());
	}
	return 0;
}
```

The error reads as such:


```
C:\Users\dajusto\source\repros\upstream>type main.cpp
#include <cstdio>
#include <exception>
int main()
{
        try     {
                throw std::exception("test");
        }catch (const std::exception& ex){
                puts(ex.what());
        }
        return 0;
}
C:\Users\dajusto\source\repros\upstream>"C:\Users\dajusto\source\repos\llvm-project\build.runtimes\bin\clang.exe" -fsanitize=address -g -O0 main.cpp

C:\Users\dajusto\source\repros\upstream>a.exe
=================================================================
==19112==ERROR: AddressSanitizer: access-violation on unknown address 0x000000000000 (pc 0x7ff72c7c11d9 bp 0x0080000ff960 sp 0x0080000fcf50 T0)
==19112==The signal is caused by a READ memory access.
==19112==Hint: address points to the zero page.
    #0 0x7ff72c7c11d8 in main C:\Users\dajusto\source\repros\upstream\main.cpp:8
    #1 0x7ff72c7d479f in _CallSettingFrame C:\repos\msvc\src\vctools\crt\vcruntime\src\eh\amd64\handlers.asm:49
    #2 0x7ff72c7c8944 in __FrameHandler3::CxxCallCatchBlock(struct _EXCEPTION_RECORD *) C:\repos\msvc\src\vctools\crt\vcruntime\src\eh\frame.cpp:1567
    #3 0x7ffb4a90e3e5  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18012e3e5)
    #4 0x7ff72c7c1128 in main C:\Users\dajusto\source\repros\upstream\main.cpp:6
    #5 0x7ff72c7c33db in invoke_main C:\repos\msvc\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
    #6 0x7ff72c7c33db in __scrt_common_main_seh C:\repos\msvc\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #7 0x7ffb49b05c06  (C:\WINDOWS\System32\KERNEL32.DLL+0x180035c06)
    #8 0x7ffb4a8455ef  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x1800655ef)

==19112==Register values:
rax = 0  rbx = 80000ff8e0  rcx = 27d76d00000  rdx = 80000ff8e0
rdi = 80000fdd50  rsi = 80000ff6a0  rbp = 80000ff960  rsp = 80000fcf50
r8  = 100  r9  = 19930520  r10 = 8000503a90  r11 = 80000fd540
r12 = 80000fd020  r13 = 0  r14 = 80000fdeb8  r15 = 0
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: access-violation C:\Users\dajusto\source\repros\upstream\main.cpp:8 in main
==19112==ABORTING
```

The root of the issue _appears to be_ that ASan's instrumentation is
incompatible with Window's assumptions for instantiating `catch`-block's
parameters (`ex` in the snippet above).

The nitty gritty details are lost on me, but I understand that to make
this work without loss of ASan coverage, a "serious" refactoring is
needed. In the meantime, users risk false positive errors when pairing
ASan + catch-block parameters on Windows.

**To mitigate this** I think we should avoid instrumenting catch-block
parameters on Windows. It appears to me this is as "simple" as marking
catch block parameters as "uninteresting" in
`AddressSanitizer::isInterestingAlloca`. My manual tests seem to confirm
this.

I believe this is strictly better than today's status quo, where the
runtime generates false positives. Although we're now explicitly
choosing to instrument less, the benefit is that now more programs can
run with ASan without _funky_ macros that disable ASan on exception
blocks.

**This PR:** implements the mitigation above, and creates a simple new
test for it.

_Thanks!_

---------

Co-authored-by: Antonio Frighetto <me@antoniofrighetto.com>
clayborg pushed a commit that referenced this pull request Nov 14, 2025
…am (llvm#167724)

This got exposed by `09262656f32ab3f2e1d82e5342ba37eecac52522`.

The underlying stream of `m_os` is referenced by the `TextDiagnostic`
member of `TextDiagnosticPrinter`. It got turned into a
`llvm::formatted_raw_ostream` in the commit above. When
`~TextDiagnosticPrinter` (and thus `~TextDiagnostic`) is invoked, we now
call `~formatted_raw_ostream`, which tries to access the underlying
stream. But `m_os` was already deleted because it is earlier in the
order of destruction in `TextDiagnosticPrinter`. Move the `m_os` member
before the `TextDiagnosticPrinter` to avoid a use-after-free.

Drive-by:
* Also move the `m_output` member which the `m_os` holds a reference to.
The fact it's a reference indicates the expectation is most likely that
the string outlives the stream.

The ASAN macOS bot is currently failing with this:
```
08:15:39  =================================================================
08:15:39  ==61103==ERROR: AddressSanitizer: heap-use-after-free on address 0x60600012cf40 at pc 0x00012140d304 bp 0x00016eecc850 sp 0x00016eecc848
08:15:39  READ of size 8 at 0x60600012cf40 thread T0
08:15:39      #0 0x00012140d300 in llvm::formatted_raw_ostream::releaseStream() FormattedStream.h:205
08:15:39      #1 0x00012140d3a4 in llvm::formatted_raw_ostream::~formatted_raw_ostream() FormattedStream.h:145
08:15:39      #2 0x00012604abf8 in clang::TextDiagnostic::~TextDiagnostic() TextDiagnostic.cpp:721
08:15:39      #3 0x00012605dc80 in clang::TextDiagnosticPrinter::~TextDiagnosticPrinter() TextDiagnosticPrinter.cpp:30
08:15:39      #4 0x00012605dd5c in clang::TextDiagnosticPrinter::~TextDiagnosticPrinter() TextDiagnosticPrinter.cpp:27
08:15:39      #5 0x0001231fb210 in (anonymous namespace)::StoringDiagnosticConsumer::~StoringDiagnosticConsumer() ClangModulesDeclVendor.cpp:47
08:15:39      #6 0x0001231fb3bc in (anonymous namespace)::StoringDiagnosticConsumer::~StoringDiagnosticConsumer() ClangModulesDeclVendor.cpp:47
08:15:39      #7 0x000129aa9d70 in clang::DiagnosticsEngine::~DiagnosticsEngine() Diagnostic.cpp:91
08:15:39      #8 0x0001230436b8 in llvm::RefCountedBase<clang::DiagnosticsEngine>::Release() const IntrusiveRefCntPtr.h:103
08:15:39      #9 0x0001231fe6c8 in (anonymous namespace)::ClangModulesDeclVendorImpl::~ClangModulesDeclVendorImpl() ClangModulesDeclVendor.cpp:93
08:15:39      #10 0x0001231fe858 in (anonymous namespace)::ClangModulesDeclVendorImpl::~ClangModulesDeclVendorImpl() ClangModulesDeclVendor.cpp:93
...
08:15:39
08:15:39  0x60600012cf40 is located 32 bytes inside of 56-byte region [0x60600012cf20,0x60600012cf58)
08:15:39  freed by thread T0 here:
08:15:39      #0 0x0001018abb88 in _ZdlPv+0x74 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x4bb88)
08:15:39      #1 0x0001231fb1c0 in (anonymous namespace)::StoringDiagnosticConsumer::~StoringDiagnosticConsumer() ClangModulesDeclVendor.cpp:47
08:15:39      #2 0x0001231fb3bc in (anonymous namespace)::StoringDiagnosticConsumer::~StoringDiagnosticConsumer() ClangModulesDeclVendor.cpp:47
08:15:39      #3 0x000129aa9d70 in clang::DiagnosticsEngine::~DiagnosticsEngine() Diagnostic.cpp:91
08:15:39      #4 0x0001230436b8 in llvm::RefCountedBase<clang::DiagnosticsEngine>::Release() const IntrusiveRefCntPtr.h:103
08:15:39      #5 0x0001231fe6c8 in (anonymous namespace)::ClangModulesDeclVendorImpl::~ClangModulesDeclVendorImpl() ClangModulesDeclVendor.cpp:93
08:15:39      #6 0x0001231fe858 in (anonymous namespace)::ClangModulesDeclVendorImpl::~ClangModulesDeclVendorImpl() ClangModulesDeclVendor.cpp:93
...
08:15:39
08:15:39  previously allocated by thread T0 here:
08:15:39      #0 0x0001018ab760 in _Znwm+0x74 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x4b760)
08:15:39      #1 0x0001231f8dec in lldb_private::ClangModulesDeclVendor::Create(lldb_private::Target&) ClangModulesDeclVendor.cpp:732
08:15:39      #2 0x00012320af58 in lldb_private::ClangPersistentVariables::GetClangModulesDeclVendor() ClangPersistentVariables.cpp:124
08:15:39      #3 0x0001232111f0 in lldb_private::ClangUserExpression::PrepareForParsing(lldb_private::DiagnosticManager&, lldb_private::ExecutionContext&, bool) ClangUserExpression.cpp:536
08:15:39      #4 0x000123213790 in lldb_private::ClangUserExpression::Parse(lldb_private::DiagnosticManager&, lldb_private::ExecutionContext&, lldb_private::ExecutionPolicy, bool, bool) ClangUserExpression.cpp:647
08:15:39      #5 0x00012032b258 in lldb_private::UserExpression::Evaluate(lldb_private::ExecutionContext&, lldb_private::EvaluateExpressionOptions const&, llvm::StringRef, llvm::StringRef, std::__1::shared_ptr<lldb_private::ValueObject>&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>*, lldb_private::ValueObject*) UserExpression.cpp:280
08:15:39      #6 0x000120724010 in lldb_private::Target::EvaluateExpression(llvm::StringRef, lldb_private::ExecutionContextScope*, std::__1::shared_ptr<lldb_private::ValueObject>&, lldb_private::EvaluateExpressionOptions const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>*, lldb_private::ValueObject*) Target.cpp:2905
08:15:39      #7 0x00011fc7bde0 in lldb::SBTarget::EvaluateExpression(char const*, lldb::SBExpressionOptions const&) SBTarget.cpp:2305
08:15:39  ==61103==ABORTING
...
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants