Skip to content

Conversation

@fmayer
Copy link
Contributor

@fmayer fmayer commented Jul 17, 2025

No description provided.

Created using spr 1.3.4
@fmayer fmayer requested a review from thurstond July 17, 2025 03:01
@fmayer fmayer marked this pull request as ready for review July 17, 2025 03:01
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. debuginfo labels Jul 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 17, 2025

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-clang

Author: Florian Mayer (fmayer)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/149237.diff

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+6-3)
  • (added) clang/test/CodeGen/null-sanitizer-debug-info-regression.cpp (+5)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 75ee08a2bcfa6..f1a3ae2e057b2 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -6480,20 +6480,23 @@ SanitizerOrdinalToCheckLabel(SanitizerKind::SanitizerOrdinal Ordinal) {
 llvm::DILocation *CodeGenFunction::SanitizerAnnotateDebugInfo(
     ArrayRef<SanitizerKind::SanitizerOrdinal> Ordinals,
     SanitizerHandler Handler) {
+  llvm::DILocation *CheckDI = Builder.getCurrentDebugLocation();
+  auto *DI = getDebugInfo();
+  if (!DI)
+    return CheckDI;
+
   std::string Label;
   if (Ordinals.size() == 1)
     Label = SanitizerOrdinalToCheckLabel(Ordinals[0]);
   else
     Label = SanitizerHandlerToCheckLabel(Handler);
 
-  llvm::DILocation *CheckDI = Builder.getCurrentDebugLocation();
-
   for (auto Ord : Ordinals) {
     // TODO: deprecate ClArrayBoundsPseudoFn
     if (((ClArrayBoundsPseudoFn && Ord == SanitizerKind::SO_ArrayBounds) ||
          CGM.getCodeGenOpts().SanitizeAnnotateDebugInfo.has(Ord)) &&
         CheckDI) {
-      return getDebugInfo()->CreateSyntheticInlineAt(CheckDI, Label);
+      return DI->CreateSyntheticInlineAt(CheckDI, Label);
     }
   }
 
diff --git a/clang/test/CodeGen/null-sanitizer-debug-info-regression.cpp b/clang/test/CodeGen/null-sanitizer-debug-info-regression.cpp
new file mode 100644
index 0000000000000..0b62f24177bbd
--- /dev/null
+++ b/clang/test/CodeGen/null-sanitizer-debug-info-regression.cpp
@@ -0,0 +1,5 @@
+// RUN: %clangxx -g -fsanitize=null -fsanitize-trap=all -fsanitize-annotate-debug-info=all -O2 -std=c++17 -c -o /dev/null %s
+
+struct foo {
+  foo(int, long, const int & = int());
+} foo(0, 0);

@llvmbot
Copy link
Member

llvmbot commented Jul 17, 2025

@llvm/pr-subscribers-clang-codegen

Author: Florian Mayer (fmayer)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/149237.diff

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+6-3)
  • (added) clang/test/CodeGen/null-sanitizer-debug-info-regression.cpp (+5)
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 75ee08a2bcfa6..f1a3ae2e057b2 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -6480,20 +6480,23 @@ SanitizerOrdinalToCheckLabel(SanitizerKind::SanitizerOrdinal Ordinal) {
 llvm::DILocation *CodeGenFunction::SanitizerAnnotateDebugInfo(
     ArrayRef<SanitizerKind::SanitizerOrdinal> Ordinals,
     SanitizerHandler Handler) {
+  llvm::DILocation *CheckDI = Builder.getCurrentDebugLocation();
+  auto *DI = getDebugInfo();
+  if (!DI)
+    return CheckDI;
+
   std::string Label;
   if (Ordinals.size() == 1)
     Label = SanitizerOrdinalToCheckLabel(Ordinals[0]);
   else
     Label = SanitizerHandlerToCheckLabel(Handler);
 
-  llvm::DILocation *CheckDI = Builder.getCurrentDebugLocation();
-
   for (auto Ord : Ordinals) {
     // TODO: deprecate ClArrayBoundsPseudoFn
     if (((ClArrayBoundsPseudoFn && Ord == SanitizerKind::SO_ArrayBounds) ||
          CGM.getCodeGenOpts().SanitizeAnnotateDebugInfo.has(Ord)) &&
         CheckDI) {
-      return getDebugInfo()->CreateSyntheticInlineAt(CheckDI, Label);
+      return DI->CreateSyntheticInlineAt(CheckDI, Label);
     }
   }
 
diff --git a/clang/test/CodeGen/null-sanitizer-debug-info-regression.cpp b/clang/test/CodeGen/null-sanitizer-debug-info-regression.cpp
new file mode 100644
index 0000000000000..0b62f24177bbd
--- /dev/null
+++ b/clang/test/CodeGen/null-sanitizer-debug-info-regression.cpp
@@ -0,0 +1,5 @@
+// RUN: %clangxx -g -fsanitize=null -fsanitize-trap=all -fsanitize-annotate-debug-info=all -O2 -std=c++17 -c -o /dev/null %s
+
+struct foo {
+  foo(int, long, const int & = int());
+} foo(0, 0);

llvm::DILocation *CodeGenFunction::SanitizerAnnotateDebugInfo(
ArrayRef<SanitizerKind::SanitizerOrdinal> Ordinals,
SanitizerHandler Handler) {
llvm::DILocation *CheckDI = Builder.getCurrentDebugLocation();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: variable naming - "CheckDI" is a DILocation, not DebugInfo like the similarly named "DI".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

// TODO: deprecate ClArrayBoundsPseudoFn
if (((ClArrayBoundsPseudoFn && Ord == SanitizerKind::SO_ArrayBounds) ||
CGM.getCodeGenOpts().SanitizeAnnotateDebugInfo.has(Ord)) &&
CheckDI) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: does the DI check above provide any guarantees that CheckDI is non-null? (i.e., can && CheckDI be safely elided or does it require further working group approval?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we need to check both.

Created using spr 1.3.4
@fmayer fmayer requested a review from thurstond July 17, 2025 03:16
@fmayer fmayer merged commit 0f09f2c into main Jul 17, 2025
9 checks passed
@fmayer fmayer deleted the users/fmayer/spr/sanitize-fix-crash-in-fsanitize-annotate-debug-info branch July 17, 2025 04:28
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

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

Next time can you add more details to the summary, these are especially helpful for downstream folks but also for reviewers. Something along the lines of "Perform a null check in SanitizerAnnotateDebugInfo for the result of getCurrentDebugLocation".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category debuginfo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants