Emit debug info with original source location for tokens from macros … #163190

SergejSalnikov · 2025-10-13T12:16:39Z

Use source location for macro arguments when generating debug info.

github-actions · 2025-10-13T12:16:57Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-10-13T12:17:51Z

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: SKill (SergejSalnikov)

Changes

Use source location for macro arguments when generating debug info.

fixes #160667

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

1 Files Affected:

(modified) clang/lib/CodeGen/CGDebugInfo.cpp (+23-6)

diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 9fe9a13610296..b5216806c3e83 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -110,6 +110,20 @@ static bool IsArtificial(VarDecl const *VD) {
                               cast<Decl>(VD->getDeclContext())->isImplicit());
 }
 
+/// Given a SourceLocation object, return the spelling location referenced by
+/// the ID.
+///
+/// The key difference from SourceManager::getPresumedLoc that a presumed
+/// location of macro arguments is based on the spelling location.
+static PresumedLoc getPresumedLoc(SourceManager &SM, SourceLocation Loc) {
+  // If the location is a macro argument expansion, get the spelling location
+  // instead.
+  if (SM.isMacroArgExpansion(Loc, nullptr)) {
+    Loc = SM.getSpellingLoc(Loc);
+  }
+  return SM.getPresumedLoc(Loc);
+}
+
 CGDebugInfo::CGDebugInfo(CodeGenModule &CGM)
     : CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()),
       DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs),
@@ -318,7 +332,11 @@ void CGDebugInfo::setLocation(SourceLocation Loc) {
   if (Loc.isInvalid())
     return;
 
-  CurLoc = CGM.getContext().getSourceManager().getExpansionLoc(Loc);
+  SourceManager &SM = CGM.getContext().getSourceManager();
+  if (SM.isMacroArgExpansion(Loc)) {
+    Loc = SM.getSpellingLoc(Loc);
+  }
+  CurLoc = SM.getExpansionLoc(Loc);
 
   // If we've changed files in the middle of a lexical scope go ahead
   // and create a new lexical scope with file node if it's different
@@ -326,7 +344,6 @@ void CGDebugInfo::setLocation(SourceLocation Loc) {
   if (LexicalBlockStack.empty())
     return;
 
-  SourceManager &SM = CGM.getContext().getSourceManager();
   auto *Scope = cast<llvm::DIScope>(LexicalBlockStack.back());
   PresumedLoc PCLoc = SM.getPresumedLoc(CurLoc);
   if (PCLoc.isInvalid() || Scope->getFile() == getOrCreateFile(CurLoc))
@@ -545,7 +562,7 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation Loc) {
     FileName = TheCU->getFile()->getFilename();
     CSInfo = TheCU->getFile()->getChecksum();
   } else {
-    PresumedLoc PLoc = SM.getPresumedLoc(Loc);
+    PresumedLoc PLoc = getPresumedLoc(SM, Loc);
     FileName = PLoc.getFilename();
 
     if (FileName.empty()) {
@@ -627,7 +644,7 @@ unsigned CGDebugInfo::getLineNumber(SourceLocation Loc) {
   if (Loc.isInvalid())
     return 0;
   SourceManager &SM = CGM.getContext().getSourceManager();
-  return SM.getPresumedLoc(Loc).getLine();
+  return getPresumedLoc(SM, Loc).getLine();
 }
 
 unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) {
@@ -639,7 +656,7 @@ unsigned CGDebugInfo::getColumnNumber(SourceLocation Loc, bool Force) {
   if (Loc.isInvalid() && CurLoc.isInvalid())
     return 0;
   SourceManager &SM = CGM.getContext().getSourceManager();
-  PresumedLoc PLoc = SM.getPresumedLoc(Loc.isValid() ? Loc : CurLoc);
+  PresumedLoc PLoc = getPresumedLoc(SM, Loc.isValid() ? Loc : CurLoc);
   return PLoc.isValid() ? PLoc.getColumn() : 0;
 }
 
@@ -6183,7 +6200,7 @@ void CGDebugInfo::EmitGlobalAlias(const llvm::GlobalValue *GV,
 void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
                                             const StringLiteral *S) {
   SourceLocation Loc = S->getStrTokenLoc(0);
-  PresumedLoc PLoc = CGM.getContext().getSourceManager().getPresumedLoc(Loc);
+  PresumedLoc PLoc = getPresumedLoc(CGM.getContext().getSourceManager(), Loc);
   if (!PLoc.isValid())
     return;

github-actions · 2025-10-13T15:53:38Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff origin/main HEAD --extensions cpp,h,c -- clang/test/DebugInfo/Generic/macro-info.c clang/include/clang/Basic/SourceManager.h clang/lib/Basic/SourceManager.cpp clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CGDebugInfo.h --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.

diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h
index 5e8ca172a..48ddfd9d7 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -1261,7 +1261,8 @@ public:
   SourceLocation getRefinedSpellingLoc(SourceLocation Loc) const {
     // Handle the non-mapped case inline, defer to out of line code to handle
     // expansions.
-    if (Loc.isFileID()) return Loc;
+    if (Loc.isFileID())
+      return Loc;
     return getRefinedSpellingLocSlowCase(Loc);
   }
 
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 835a9538f..913fb7da3 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -5469,8 +5469,7 @@ void CGDebugInfo::EmitDeclareOfBlockDeclRefVariable(
       Ty = CreateSelfType(VD->getType(), Ty);
 
   // Get location information.
-  const unsigned Line =
-      getLineNumber(Loc.isValid() ? Loc : CurLoc);
+  const unsigned Line = getLineNumber(Loc.isValid() ? Loc : CurLoc);
   unsigned Column = getColumnNumber(Loc);
 
   const llvm::DataLayout &target = CGM.getDataLayout();
@@ -5579,7 +5578,8 @@ void CGDebugInfo::EmitDeclareOfBlockLiteralArgVariable(const CGBlockInfo &block,
   const BlockDecl *blockDecl = block.getBlockDecl();
 
   // Collect some general information about the block's location.
-  SourceLocation loc = getRefinedSpellingLocation(blockDecl->getCaretLocation());
+  SourceLocation loc =
+      getRefinedSpellingLocation(blockDecl->getCaretLocation());
   llvm::DIFile *tunit = getOrCreateFile(loc);
   unsigned line = getLineNumber(loc);
   unsigned column = getColumnNumber(loc);
@@ -6111,8 +6111,8 @@ void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue &Init) {
     }
 
   GV.reset(DBuilder.createGlobalVariableExpression(
-      DContext, Name, StringRef(), Unit, getLineNumber(Loc), Ty,
-      true, true, InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VarD),
+      DContext, Name, StringRef(), Unit, getLineNumber(Loc), Ty, true, true,
+      InitExpr, getOrCreateStaticDataMemberDeclarationOrNull(VarD),
       TemplateParameters, Align));
 }
 
@@ -6131,8 +6131,8 @@ void CGDebugInfo::EmitExternalVariable(llvm::GlobalVariable *Var,
   llvm::DIScope *DContext = getDeclContextDescriptor(D);
   llvm::DIGlobalVariableExpression *GVE =
       DBuilder.createGlobalVariableExpression(
-          DContext, Name, StringRef(), Unit, getLineNumber(Loc),
-          Ty, false, false, nullptr, nullptr, nullptr, Align);
+          DContext, Name, StringRef(), Unit, getLineNumber(Loc), Ty, false,
+          false, nullptr, nullptr, nullptr, Align);
   Var->addDebugInfo(GVE);
 }
 
@@ -6228,8 +6228,8 @@ void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
   llvm::DIFile *File = getOrCreateFile(Loc);
   llvm::DIGlobalVariableExpression *Debug =
       DBuilder.createGlobalVariableExpression(
-          nullptr, StringRef(), StringRef(), File,
-          getLineNumber(Loc), getOrCreateType(S->getType(), File), true);
+          nullptr, StringRef(), StringRef(), File, getLineNumber(Loc),
+          getOrCreateType(S->getType(), File), true);
   GV->addDebugInfo(Debug);
 }

dwblaikie

Some test coverage will be needed - take a look in clang/test/DebugInfo/... and see about adding a test that demonstrates the change in locations provided by this patch?

clang/lib/CodeGen/CGDebugInfo.cpp

…arguments.

SergejSalnikov · 2025-10-13T19:19:49Z

clang/lib/CodeGen/CGDebugInfo.cpp

+    return ParentLoc;
+  }
+  return SM.getSpellingLoc(Loc);
+}


Wouldn't mind hearing from @AaronBallman whether this is the right/efficient way to make this determination.

Sorry, I'm new to github forced pushed the changelist and made the previous comment obsolete.

This implementation is garbage. I'll update the code tomorrow with a better version.

See SourceManager.getRefinedSpellingLocSlowCase.

The method preserves source locations for macro arguments.

* Add unit test.

SergejSalnikov · 2025-10-14T13:26:33Z

I've found that the same location expansion logic was sometime executed multiple times for the same location, so I've optmized the code to reduce number of location adjustments. I believe new code should be even faster than the old one.

Also I've added the test.

Michael137 · 2025-10-15T10:12:07Z

Ran CI on this PR. I expect some tests to fail. Fixing those up would help show what the new expected behaviour is for a wider range of macros. Also, providing a couple of examples (before/after) in the PR description would be useful. Particularly for debug locations within macro bodies. Don't think you added a test for that?

Michael137 · 2025-10-15T10:14:07Z

CC @AaronBallman for the SourceManager changes

SergejSalnikov · 2025-10-15T11:02:23Z

@Michael137, I've run the test and they all pass. There were some issues with code formatting, which I've fixed.
The newly added test can be found here.
The demo can be found here.

dwblaikie

Looks about right to me.

Are there any uses of "unrefined" locations in CGDebugInfo with this patch? If there are, what's the distinction/how was it chosen which would be refined, and which would not?
The slow/fast path through getRefinedSpellingLoc probably isn't worth it - probably make the whole thing outofline?

Some questions for other reviewers, etc:

getRefinedSpellingLoc - I'm not sure "refined" carries enough information (is it a reference to some other existing use of the term?) - but I don't have any great suggestions for a name. Perhaps "immediate" or "local" spelling location?
it's probably not practical to test all the modified code paths - any thoughts on what the right testing tradeoff is here? That various codepaths /inside/ getRefinedSpellingLoc are tested, from perhaps a variety of call sites/ways that manifests in the resulting IR metadata without being exhaustive, seems OK to me? (so perhaps in the test case at least an instruction location and a type location could be tested?)

dwblaikie · 2025-10-15T17:49:00Z

clang/test/DebugInfo/Generic/macro-info.c

+// CHECK: DIGlobalVariable(name: "global3",{{.*}} line: [[@LINE+6]]
+// CHECK: DIGlobalVariable(name: "global2",{{.*}} line: [[@LINE+2]]


Could these comments (including those elsewhere in the test) be moved closer (within the macro argument) to the relevant line - ideally immediately prior (so it's @LINE+1, or even in a trailing comment on the same line using @LINE)

The order in which variables are defined is important. As you can see the global3 precedes global2 despite decreasing line number.

I think that placing CHECKs right before the marco is good for this test.

SergejSalnikov · 2025-10-15T18:14:40Z

Are there any uses of "unrefined" locations in CGDebugInfo with this patch? If there are, what's the distinction/how was it chosen which would be refined, and which would not? The slow/fast path through getRefinedSpellingLoc probably isn't worth it - probably make the whole thing outofline?

I'm converting all locations that came from public entry points into refined locations. The only non-converted locations are ones that located in private methods and it's guaranteed that the called already passes a refined location.

Keep in mind that majority of tokens are not related to macro so having a fast path seems reasonable.

SergejSalnikov marked this pull request as ready for review October 13, 2025 12:17

llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. debuginfo labels Oct 13, 2025

SergejSalnikov force-pushed the main branch 4 times, most recently from 3249d8f to 16030d8 Compare October 13, 2025 15:22

AaronBallman requested review from adrian-prantl, dwblaikie and echristo October 13, 2025 15:49

SergejSalnikov force-pushed the main branch 2 times, most recently from de48195 to f0734d4 Compare October 13, 2025 16:30

dwblaikie reviewed Oct 13, 2025

View reviewed changes

clang/lib/CodeGen/CGDebugInfo.cpp Outdated Show resolved Hide resolved

clang/lib/CodeGen/CGDebugInfo.cpp Outdated Show resolved Hide resolved

clang/lib/CodeGen/CGDebugInfo.cpp Show resolved Hide resolved

SergejSalnikov force-pushed the main branch 2 times, most recently from 8e0373b to 6ef1441 Compare October 13, 2025 16:49

Emit debug info with original source location for tokens from macros …

aabe21c

…arguments.

SergejSalnikov force-pushed the main branch from 6ef1441 to aabe21c Compare October 13, 2025 16:57

SergejSalnikov commented Oct 13, 2025

View reviewed changes

Michael137 self-requested a review October 14, 2025 10:00

Add getRefinedSpellingLoc to SourceManager.

cf8a150

The method preserves source locations for macro arguments.

llvmbot added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Oct 14, 2025

SergejSalnikov added 2 commits October 14, 2025 14:55

Minimize the amount of location expansions inside CGDebugInfo.

3afd854

* Add unit test.

Add more cases to macro-info test.

731f671

SergejSalnikov and others added 2 commits October 14, 2025 15:52

Merge branch 'llvm:main' into main

dbcc184

Add comment for getRefinedSpellingLocation.

252e537

SergejSalnikov and others added 6 commits October 14, 2025 16:45

Optimize implementation of getRefinedSpellingLocSlowCase.

2855939

Rename some variables.

37bae8c

Merge branch 'main' into main

fd8a6a8

Update CGDebugInfo.h

b7f9236

Move method comment to header file.

e065eff

Merge branch 'main' into main

ee6de87

Michael137 requested a review from AaronBallman October 15, 2025 10:12

Fix code formatting.

456ab3e

SergejSalnikov requested a review from dwblaikie October 15, 2025 13:05

dwblaikie reviewed Oct 15, 2025

View reviewed changes

		// CHECK: DIGlobalVariable(name: "global3",{{.*}} line: [[@LINE+6]]
		// CHECK: DIGlobalVariable(name: "global2",{{.*}} line: [[@LINE+2]]

Emit debug info with original source location for tokens from macros … #163190

Are you sure you want to change the base?

Emit debug info with original source location for tokens from macros … #163190

Conversation

SergejSalnikov commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 13, 2025

Uh oh!

llvmbot commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dwblaikie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SergejSalnikov Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

SergejSalnikov Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

SergejSalnikov commented Oct 14, 2025

Uh oh!

Michael137 commented Oct 15, 2025

Uh oh!

Michael137 commented Oct 15, 2025

Uh oh!

SergejSalnikov commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dwblaikie left a comment

Choose a reason for hiding this comment

Uh oh!

dwblaikie Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

SergejSalnikov Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SergejSalnikov commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SergejSalnikov commented Oct 13, 2025 •

edited

Loading

llvmbot commented Oct 13, 2025 •

edited

Loading

github-actions bot commented Oct 13, 2025 •

edited

Loading

SergejSalnikov commented Oct 15, 2025 •

edited

Loading

SergejSalnikov Oct 15, 2025 •

edited

Loading

SergejSalnikov commented Oct 15, 2025 •

edited

Loading