Skip to content

Conversation

@asudarsa
Copy link
Owner

@asudarsa asudarsa commented Mar 26, 2025

This PR has the following changes:

  1. Replace llvm-link with calls to linkInModule to link device files
  2. Add -print-linked-module option to dump linked module for testing
  3. Added a test to verify that linking is working as expected.

We will eventually move to using thin LTO for linking device inputs.

Thanks

Signed-off-by: Arvind Sudarsanam <[email protected]>
Signed-off-by: Arvind Sudarsanam <[email protected]>
Signed-off-by: Arvind Sudarsanam <[email protected]>
@asudarsa
Copy link
Owner Author

Hi @bader and @jzc

Thanks for your kind feedback. I am refactoring my changes to provide an update. Please find my answers to your comments in place (adding them now).

Thanks

@asudarsa
Copy link
Owner Author

Hi @bader and @jzc

I have tried to address most of the comments in my recent commit. Please take a look when convenient. Sorry for major refactoring. I realized I was trying to 'unnecessarily' complicate things.

Thanks

Comment on lines 211 to 213
// Handle cases where input file is a fat object containing LLVM IR bitcode
// SYCL device libraries are provided as fat objects containing LLVM IR
// bitcode
Copy link

Choose a reason for hiding this comment

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

Why SYCL device libraries are passed to the clang-sycl-linker via command line arguments? Doesn't the tool itself know the default location and default library names to link? I'm okay to let the user to customize these parameters via options, but I see that clang-sycl-linker can't link device libraries unless they are configured from outside.

If they are configured from outside, I would argue that device libraries must be LLVM bitcode format, not fat objects. AFAIK, they are distributed in the LLVM bitcode format. @mdtoguchi, am I right?

Anyway, processing two different sets of files in one function doesn't look like a good approach in this case. User inputs are processed by the code at the beginning of the function. Basically, we have two separate functions glued together into one. Instead of separating these two functions, I suggest using LLVM bitcode format for the device libraries.

Copy link
Owner Author

Choose a reason for hiding this comment

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

This function gives us the ability to handle both bitcode files and bitcode files embedded in fat objects. I would like to keep it like this to give some flexibility on how inputs are sent into clang-sycl-linker. We can do a cleanup once all pieces are put together.

Hope that's alright. I looked into the /install directory and I see .o .spv and .bc files. With wait for @mdtoguchi to chime in here about this.

Thanks

Copy link

Choose a reason for hiding this comment

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

This function gives us the ability to handle both bitcode files and bitcode files embedded in fat objects. I would like to keep it like this to give some flexibility on how inputs are sent into clang-sycl-linker. We can do a cleanup once all pieces are put together.

This doesn't make sense to me. User inputs should never be fat objects.

Copy link
Collaborator

Choose a reason for hiding this comment

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

AFAIK, there was agreement that the device libraries should be known by the tools and not required to be passed in from the driver or from the user calling clang-linker-wrapper directly. I would also share Alexey's view that the device libraries should be .bc files. We have full control over the device libraries so any potential flexibility given shouldn't be needed.

One interesting note though, current usage of --offload-new-driver will pass in fat objects for the device libraries instead of .bc files like we do for the old model. I recall that @asudarsa made this distinction for some ease of use in the tool.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ok. I think we can use .bc files in new offload model as well. For now, I will continue to rely on the options for the device libraries to be passed in via options. Further discussions may be warranted on how we want to upstream the device library sources and build them in the upstream compiler.

Thanks

@asudarsa
Copy link
Owner Author

One more round of changes:

  1. Removed early exit in linkDeviceCode for --dry-run
  2. Removed reading module from fat binary
  3. Added test to check linked output
  4. Other minor fixes to get everything to work as expected.

Thanks again

Signed-off-by: Arvind Sudarsanam <[email protected]>
Signed-off-by: Arvind Sudarsanam <[email protected]>
Signed-off-by: Arvind Sudarsanam <[email protected]>
Copy link

@bader bader left a comment

Choose a reason for hiding this comment

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

@asudarsa, the code looks good in general. I have just a few style comments.
I would also make new tests smaller and simpler.

Comment on lines 204 to 207
/// Handle cases where input file is a LLVM IR bitcode file
/// When clang-sycl-linker is called via clang-linker-wrapper tool, input files
/// are LLVM IR bitcode files
// TODO: Support SPIR-V IR files
Copy link

Choose a reason for hiding this comment

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

Suggested change
/// Handle cases where input file is a LLVM IR bitcode file
/// When clang-sycl-linker is called via clang-linker-wrapper tool, input files
/// are LLVM IR bitcode files
// TODO: Support SPIR-V IR files
/// Handle cases where input file is a LLVM IR bitcode file.
/// When clang-sycl-linker is called via clang-linker-wrapper tool, input files
/// are LLVM IR bitcode files.
// TODO: Support SPIR-V IR files.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks. Will add this as part of my upcoming commit.

Comment on lines 259 to 270
// Create a new file to write the linked device file to
auto BitcodeOutput =
createTempFile(Args, sys::path::filename(OutputFile), "bc");
if (!BitcodeOutput)
return BitcodeOutput.takeError();

// Get all SYCL device library files, if any
auto SYCLDeviceLibFiles = getSYCLDeviceLibs(Args);
if (!SYCLDeviceLibFiles)
return SYCLDeviceLibFiles.takeError();
if ((*SYCLDeviceLibFiles).empty())
return InputFile;

Expected<std::string> LLVMLinkPath =
findProgram(Args, "llvm-link", {getMainExecutable("llvm-link")});
if (!LLVMLinkPath)
return LLVMLinkPath.takeError();
if (Verbose) {
Copy link

Choose a reason for hiding this comment

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

This code is still organized for "early exit in dry-run mode". Let's align it with the function comment at the top to make it easier to read.

  1. Move if (Verbose) to the end of the function just before the return statement.
  2. Move BitcodeOutput declaration (lines 259-263) to line 317 (i.e. before writing the output to the file).
  3. Move SYCLDeviceLibFiles declaration (lines 265-268) to line 301. I recommend using generic name for this variable like DeviceLibs because OpenMP offload will use them.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Great reorg. Thanks.

}

if (Args.hasArg(OPT_print_linked_module))
errs() << *LinkerOutput;
Copy link

Choose a reason for hiding this comment

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

I don't understand the reason to print module to the error stream. LIT will fail is tests writes to the error stream. Redirecting the error stream to the output stream each for each test is annoying. Usually, you want your error stream to be empty. Non-empty error stream should signal about errors. I think other tools like clang driver print out verbose non-error output to the output stream.
We should discuss clang-linker-wrapper choice in the upstream and if clang-sycl-linker should follow the same approach.

Suggested change
errs() << *LinkerOutput;
outs() << *LinkerOutput;

Copy link
Owner Author

Choose a reason for hiding this comment

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

Interesting. Will change it to outs(). Thanks

Copy link

Choose a reason for hiding this comment

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

Let's put new files to clang/test/Driver/Inputs/SYCL/* directory.

Comment on lines 7 to 11
define spir_func noundef range(i32 -2147483643, -2147483648) i32 @_Z9lib_func1i(i32 noundef %a) local_unnamed_addr #0 {
entry:
%add = add nsw i32 %a, 5
ret i32 %add
}
Copy link

Choose a reason for hiding this comment

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

I would prefer to have simpler code for the library (and tests):

  1. Unmingled names.
  2. No unnecessary attributes.
  3. Less code. Do we need 4 library functions? I would leave just one:
define spir_func i32 addFive(i32 %x) {
  %res = add i32 %x, 5
  ret i32 %res
}

It's easier to maintain and read.

Comment on lines 1 to 4
; ModuleID = 'libsycl.cpp'
source_filename = "libsycl.cpp"
target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1"
target triple = "spirv64"
Copy link

Choose a reason for hiding this comment

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

Suggested change
; ModuleID = 'libsycl.cpp'
source_filename = "libsycl.cpp"
target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1"
target triple = "spirv64"
target triple = "spirv64"

Please, remove from the tests as well.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done as part of upcoming commit. Thanks

target triple = "spirv64"

; Function Attrs: mustprogress noinline
define spir_func noundef i32 @_Z9foo_func1ii(i32 noundef %a, i32 noundef %b) local_unnamed_addr #0 {
Copy link

Choose a reason for hiding this comment

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

Please, use one notation for function prefixes and file names.
Either rename test1.ll to foo.ll and test2.ll to bar.ll or use test1_ and test2_ function name prefixes.

}

; Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(none)
define spir_func noundef i32 @_Z9bar_func2iii(i32 noundef %c, i32 noundef %d, i32 noundef %e) local_unnamed_addr #1 {
Copy link

Choose a reason for hiding this comment

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

Let's remove func2 and keep only one function in this test.

; CHECK-DEVICE-LIB: define {{.*}}foo_func1{{.*}}
; CHECK-DEVICE-LIB: define {{.*}}foo_func2{{.*}}
; CHECK-DEVICE-LIB: define {{.*}}bar_func1{{.*}}
; CHECK-DEVICE-LIB: define {{.*}}bar_func2{{.*}}
Copy link

Choose a reason for hiding this comment

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

CHECK-DEVICE-LIB doesn't check that device libraries are linked.
I would also check that CHECK-SIMPLE doesn't define library functions.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Updating the tests now.
Thanks

Signed-off-by: Arvind Sudarsanam <[email protected]>
Signed-off-by: Arvind Sudarsanam <[email protected]>
Copy link

@bader bader left a comment

Choose a reason for hiding this comment

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

LGTM.

a.spv Outdated
Copy link

Choose a reason for hiding this comment

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

Please, remove this file.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah. I deleted that specifically before merge

Signed-off-by: Arvind Sudarsanam <[email protected]>
@asudarsa
Copy link
Owner Author

asudarsa commented Mar 29, 2025

Removed the file. Thanks @bader for the great feedback. I learnt a lot and will try my incorporate these learnings in upcoming patches.

Will prepare and submit an upstream patch in a bit.

asudarsa pushed a commit that referenced this pull request Apr 10, 2025
…ctor-bits=128." (llvm#134997)

Reverts llvm#134068

Caused a stage 2 build failure:
https://lab.llvm.org/buildbot/#/builders/41/builds/6016

```
FAILED: lib/Support/CMakeFiles/LLVMSupport.dir/Caching.cpp.o 
/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/stage1.install/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/stage2/lib/Support -I/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/llvm/llvm/lib/Support -I/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/stage2/include -I/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/llvm/llvm/include -mcpu=neoverse-512tvb -mllvm -scalable-vectorization=preferred -mllvm -treat-scalable-fixed-error-as-warning=false -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Werror=global-constructors -O3 -DNDEBUG -std=c++17 -UNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -MD -MT lib/Support/CMakeFiles/LLVMSupport.dir/Caching.cpp.o -MF lib/Support/CMakeFiles/LLVMSupport.dir/Caching.cpp.o.d -o lib/Support/CMakeFiles/LLVMSupport.dir/Caching.cpp.o -c /home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/llvm/llvm/lib/Support/Caching.cpp
Opcode has unknown scale!
UNREACHABLE executed at ../llvm/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:4530!
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.	Program arguments: /home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/stage1.install/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/stage2/lib/Support -I/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/llvm/llvm/lib/Support -I/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/stage2/include -I/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/llvm/llvm/include -mcpu=neoverse-512tvb -mllvm -scalable-vectorization=preferred -mllvm -treat-scalable-fixed-error-as-warning=false -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Werror=global-constructors -O3 -DNDEBUG -std=c++17 -UNDEBUG -fno-exceptions -funwind-tables -fno-rtti -MD -MT lib/Support/CMakeFiles/LLVMSupport.dir/Caching.cpp.o -MF lib/Support/CMakeFiles/LLVMSupport.dir/Caching.cpp.o.d -o lib/Support/CMakeFiles/LLVMSupport.dir/Caching.cpp.o -c /home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/llvm/llvm/lib/Support/Caching.cpp
1.	<eof> parser at end of file
2.	Code generation
3.	Running pass 'Function Pass Manager' on module '/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/llvm/llvm/lib/Support/Caching.cpp'.
4.	Running pass 'AArch64 load / store optimization pass' on function '@"_ZNSt17_Function_handlerIFN4llvm8ExpectedISt8functionIFNS1_ISt10unique_ptrINS0_16CachedFileStreamESt14default_deleteIS4_EEEEjRKNS0_5TwineEEEEEjNS0_9StringRefESB_EZNS0_10localCacheESB_SB_SB_S2_IFvjSB_S3_INS0_12MemoryBufferES5_ISH_EEEEE3$_0E9_M_invokeERKSt9_Any_dataOjOSF_SB_"'
 #0 0x0000b6eae9b67bf0 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/stage1.install/bin/clang+++0x81c7bf0)
 #1 0x0000b6eae9b65aec llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/stage1.install/bin/clang+++0x81c5aec)
 #2 0x0000b6eae9acd5f4 CrashRecoverySignalHandler(int) CrashRecoveryContext.cpp:0:0
 #3 0x0000f16c1aff28f8 (linux-vdso.so.1+0x8f8)
 #4 0x0000f16c1aacf1f0 __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
 #5 0x0000f16c1aa8a67c gsignal ./signal/../sysdeps/posix/raise.c:27:6
 #6 0x0000f16c1aa77130 abort ./stdlib/abort.c:81:7
 llvm#7 0x0000b6eae9ad6628 (/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/stage1.install/bin/clang+++0x8136628)
 llvm#8 0x0000b6eae72e95a8 (/home/tcwg-buildbot/worker/clang-aarch64-sve-vla-2stage/stage1.install/bin/clang+++0x59495a8)
 llvm#9 0x0000b6eae74ca9a8 (anonymous namespace)::AArch64LoadStoreOpt::findMatchingInsn(llvm::MachineInstrBundleIterator<llvm::MachineInstr, false>, (anonymous namespace)::LdStPairFlags&, unsigned int, bool) AArch64LoadStoreOptimizer.cpp:0:0
llvm#10 0x0000b6eae74c85a8 (anonymous namespace)::AArch64LoadStoreOpt::tryToPairLdStInst(llvm::MachineInstrBundleIterator<llvm::MachineInstr, false>&) AArch64LoadStoreOptimizer.cpp:0:0
llvm#11 0x0000b6eae74c624c (anonymous namespace)::AArch64LoadStoreOpt::optimizeBlock(llvm::MachineBasicBlock&, bool) AArch64LoadStoreOptimizer.cpp:0:0
llvm#12 0x0000b6eae74c429c (anonymous namespace)::AArch64LoadStoreOpt::runOnMachineFunction(llvm::MachineFunction&) AArch64LoadStoreOptimizer.cpp:0:0
```
asudarsa pushed a commit that referenced this pull request Apr 14, 2025
…vailable (llvm#135343)

When a frame is inlined, LLDB will display its name in backtraces as
follows:
```
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.3
  * frame #0: 0x0000000100000398 a.out`func() [inlined] baz(x=10) at inline.cpp:1:42
    frame #1: 0x0000000100000398 a.out`func() [inlined] bar() at inline.cpp:2:37
    frame #2: 0x0000000100000398 a.out`func() at inline.cpp:4:15
    frame #3: 0x00000001000003c0 a.out`main at inline.cpp:7:5
    frame #4: 0x000000026eb29ab8 dyld`start + 6812
```
The longer the names get the more confusing this gets because the first
function name that appears is the parent frame. My assumption (which may
need some more surveying) is that for the majority of cases we only care
about the actual frame name (not the parent). So this patch removes all
the special logic that prints the parent frame.

Another quirk of the current format is that the inlined frame name does
not abide by the `${function.name-XXX}` format variables. We always just
print the raw demangled name. With this patch, we would format the
inlined frame name according to the `frame-format` setting (see the
test-cases).

If we really want to have the `parentFrame [inlined] inlinedFrame`
format, we could expose it through a new `frame-format` variable (e..g.,
`${function.inlined-at-name}` and let the user decide where to place
things.
asudarsa pushed a commit that referenced this pull request Apr 16, 2025
…reporting (llvm#131756)

### Description  
This PR resolves a deadlock between AddressSanitizer (ASan) and
LeakSanitizer (LSan)
that occurs when both sanitizers attempt to acquire locks in conflicting
orders across
threads. The fix ensures safe lock acquisition ordering by preloading
module information
before error reporting.  

---  

### Issue Details  
**Reproducer**
```cpp  
// Thread 1: ASan error path 
int arr[1] = {0};
std::thread t([&]() {  
  arr[1] = 1; // Triggers ASan OOB error  
});  

// Thread 2: LSan check path  
__lsan_do_leak_check();   
```  

**Lock Order Conflict**:  

- Thread 1 (ASan error reporting):  
  1. Acquires ASan thread registry lock (B)  
  1. Attempts to acquire libdl lock (A) via `dl_iterate_phdr`

- Thread 2 (LSan leak check):  
  1. Acquires libdl lock (A) via `dl_iterate_phdr`
  1. Attempts to acquire ASan thread registry lock (B)  
  

This creates a circular wait condition (A -> B -> A) meeting all four
Coffman deadlock criteria.

---  

### Fix Strategy  
The root cause lies in ASan's error reporting path needing
`dl_iterate_phdr` (requiring lock A)
while already holding its thread registry lock (B). The solution:  

1. **Preload Modules Early**: Force module list initialization _before_
acquiring ASan's thread lock
2. **Avoid Nested Locking**: Ensure symbolization (via dl_iterate_phdr)
completes before error reporting locks

Key code change:  
```cpp  
// Before acquiring ASan's thread registry lock:  
Symbolizer::GetOrInit()->GetRefreshedListOfModules();  
```  

This guarantees module information is cached before lock acquisition,
eliminating
the need for `dl_iterate_phdr` calls during error reporting.  

---  

### Testing  
Added **asan_lsan_deadlock.cpp** test case:  
- Reproduces deadlock reliably without fix **under idle system
conditions**
   - Uses watchdog thread to detect hangs  
   - Verifies ASan error reports correctly without deadlock  

**Note**: Due to the inherent non-determinism of thread scheduling and
lock acquisition timing,
this test may not reliably reproduce the deadlock on busy systems (e.g.,
during parallel
   `ninja check-asan` runs).



---  

### Impact  
- Fixes rare but severe deadlocks in mixed ASan+LSan environments  
- Maintains thread safety guarantees for both sanitizers  
- No user-visible behavior changes except deadlock elimination  

---

### Relevant Buggy Code
- Code in ASan's asan_report.cpp
```cpp
  explicit ScopedInErrorReport(bool fatal = false)
      : halt_on_error_(fatal || flags()->halt_on_error) {
    // Acquire lock B
    asanThreadRegistry().Lock();
  }
  ~ScopedInErrorReport() {
    ...
    // Try to acquire lock A under holding lock B via the following path
    // #4  0x000071a353d83e93 in __GI___dl_iterate_phdr (
    //     callback=0x5d1a07a39580 <__sanitizer::dl_iterate_phdr_cb(dl_phdr_info*, unsigned long, void*)>, 
    //     data=0x6da3510fd3f0) at ./elf/dl-iteratephdr.c:39
    // #5  0x00005d1a07a39574 in __sanitizer::ListOfModules::init (this=0x71a353ebc080)
    //     at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp:784
    // #6  0x00005d1a07a429e3 in __sanitizer::Symbolizer::RefreshModules (this=0x71a353ebc058)
    //     at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp:188
    // llvm#7  __sanitizer::Symbolizer::FindModuleForAddress (this=this@entry=0x71a353ebc058, 
    //     address=address@entry=102366378805727)
    //     at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp:214
    // llvm#8  0x00005d1a07a4291b in __sanitizer::Symbolizer::SymbolizePC (this=0x71a353ebc058, addr=102366378805727)
    //     at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cpp:88
    // llvm#9  0x00005d1a07a40df7 in __sanitizer::(anonymous namespace)::StackTraceTextPrinter::ProcessAddressFrames (
    //     this=this@entry=0x6da3510fd520, pc=102366378805727)
    //     at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp:37
    // llvm#10 0x00005d1a07a40d27 in __sanitizer::StackTrace::PrintTo (this=this@entry=0x6da3510fd5e8, 
    //     output=output@entry=0x6da3510fd588)
    //     at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp:110
    // llvm#11 0x00005d1a07a410a1 in __sanitizer::StackTrace::Print (this=0x6da3510fd5e8)
    //     at llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_libcdep.cpp:133
    // llvm#12 0x00005d1a0798758d in __asan::ErrorGeneric::Print (
    //     this=0x5d1a07aa4e08 <__asan::ScopedInErrorReport::current_error_+8>)
    //     at llvm-project/compiler-rt/lib/asan/asan_errors.cpp:617    
    current_error_.Print();
    ... 
  }
```

- Code in LSan's lsan_common_linux.cpp
```cpp
void LockStuffAndStopTheWorld(StopTheWorldCallback callback,
                              CheckForLeaksParam *argument) {
  // Acquire lock A
  dl_iterate_phdr(LockStuffAndStopTheWorldCallback, &param);
}

static int LockStuffAndStopTheWorldCallback(struct dl_phdr_info *info,
                                            size_t size, void *data) {
  // Try to acquire lock B under holding lock A via the following path
  // #3  0x000055555562b34a in __sanitizer::ThreadRegistry::Lock (this=<optimized out>)
  //     at llvm-project/compiler-rt/lib/asan/../sanitizer_common/sanitizer_thread_registry.h:99
  // #4  __lsan::LockThreads () at llvm-project/compiler-rt/lib/asan/asan_thread.cpp:484
  // #5  0x0000555555652629 in __lsan::ScopedStopTheWorldLock::ScopedStopTheWorldLock (this=<optimized out>)
  //     at llvm-project/compiler-rt/lib/lsan/lsan_common.h:164
  // #6  __lsan::LockStuffAndStopTheWorldCallback (info=<optimized out>, size=<optimized out>, data=0x0, 
  //     data@entry=0x7fffffffd158) at llvm-project/compiler-rt/lib/lsan/lsan_common_linux.cpp:120
  ScopedStopTheWorldLock lock;
  DoStopTheWorldParam *param = reinterpret_cast<DoStopTheWorldParam *>(data);
  StopTheWorld(param->callback, param->argument);
  return 1;
}
```
asudarsa pushed a commit that referenced this pull request Jun 6, 2025
Fixes llvm#123300

What is seen 
```
clang-repl> int x = 42;
clang-repl> auto capture = [&]() { return x * 2; };
In file included from <<< inputs >>>:1:
input_line_4:1:17: error: non-local lambda expression cannot have a capture-default
    1 | auto capture = [&]() { return x * 2; };
      |                 ^
zsh: segmentation fault  clang-repl --Xcc="-v"

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x8)
  * frame #0: 0x0000000107b4f8b8 libclang-cpp.19.1.dylib`clang::IncrementalParser::CleanUpPTU(clang::PartialTranslationUnit&) + 988
    frame #1: 0x0000000107b4f1b4 libclang-cpp.19.1.dylib`clang::IncrementalParser::ParseOrWrapTopLevelDecl() + 416
    frame #2: 0x0000000107b4fb94 libclang-cpp.19.1.dylib`clang::IncrementalParser::Parse(llvm::StringRef) + 612
    frame #3: 0x0000000107b52fec libclang-cpp.19.1.dylib`clang::Interpreter::ParseAndExecute(llvm::StringRef, clang::Value*) + 180
    frame #4: 0x0000000100003498 clang-repl`main + 3560
    frame #5: 0x000000018d39a0e0 dyld`start + 2360
```

Though the error is justified, we shouldn't be interested in exiting
through a segfault in such cases.

The issue is that empty named decls weren't being taken care of
resulting into this assert


https://github.com/llvm/llvm-project/blob/c1a229252617ed58f943bf3f4698bd8204ee0f04/clang/include/clang/AST/DeclarationName.h#L503

Can also be seen when the example is attempted through xeus-cpp-lite.


![image](https://github.com/user-attachments/assets/9b0e6ead-138e-4b06-9ad9-fcb9f8d5bf6e)
asudarsa pushed a commit that referenced this pull request Jun 6, 2025
…142952)

This was removed in llvm#135343 in
favour of making it a format variable, which we do here. This follows
the precedent of the `[opt]` and `[artificial]` markers.

Before:
```
 thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.2
 * frame #0: 0x000000010000037c a.out`inlined1() at inline.cpp:4:3
   frame #1: 0x000000010000037c a.out`regular() at inline.cpp:6:17
   frame #2: 0x00000001000003b8 a.out`inlined2() at inline.cpp:7:43
   frame #3: 0x00000001000003b4 a.out`main at inline.cpp:10:3
   frame #4: 0x0000000186345be4 dyld`start + 7040
```

After (note the `[inlined]` markers):
```
thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.2
* frame #0: 0x000000010000037c a.out`inlined1() at inline.cpp:4:3 [inlined]
  frame #1: 0x000000010000037c a.out`regular() at inline.cpp:6:17
  frame #2: 0x00000001000003b8 a.out`inlined2() at inline.cpp:7:43 [inlined]
  frame #3: 0x00000001000003b4 a.out`main at inline.cpp:10:3
  frame #4: 0x0000000186345be4 dyld`start + 7040
```

rdar://152642178
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