[codeview] omit debug locations for nested exprs unless column info enabled

[inglorion]

Summary

Microsoft Visual Studio expects debug locations to correspond to
statements. We used to emit locations for expressions nested inside statements.
This would confuse the debugger, causing it to stop multiple times on the
same line and breaking the "step into specific" feature. This change inhibits
the emission of debug locations for nested expressions when emitting CodeView
debug information, unless column information is enabled.

Fixes PR34312.

[inglorion]

removed accidentally left in include and reformatted mangled comment

[zturner]

clang/lib/CodeGen/CGDebugInfo.h

66

Can you move this line up to put it next to another bool? Not a huge deal, but might as well pack the class members.

clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

45

Can you make a function called int foo() and make this int a = bar(foo(), y) + ...

[rnk]

clang/lib/CodeGen/CGStmt.cpp

38

"Stop point" is a hold-over from the llvm.dbg.stoppoint, which doesn't exist anymore:
http://releases.llvm.org/2.6/docs/SourceLevelDebugging.html#format_common_stoppoint

@echristo renamed CGDebugInfo::EmitStopPoint to EmitLocation long ago, and we should do the same for CodeGenFunction::EmitStopPoint. I'd like to have something like EmitStmtLocation and EmitExprLocation.

45

Does MSVC accept this? I think it will emit the copy ctor call in an -O0 build.

154

Doesn't this end up recursing? Won't InhibitDebugLocation prevent us from applying the inner statement locations?

clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

13

This is pretty painful to test. :(

If we let ourselves do an asm test, .cv_loc is printed with some nice assembly comments that make this testing easy.

49

I'd like to see some compound statement tests: for, while, if, regular braces, etc.

[inglorion]

clang/lib/CodeGen/CGStmt.cpp

45

I wrote this thinking that the right thing would happen under copy elision (there is only one object, move constructor isn't called, and the destructor only runs once) and without copy elision (there are two objects, move constructor is called, destructor is run for both objects but is a no-op for the moved-from object). If that's not the case, how would you rewrite this to do the right thing?

154

Yeah, this looks wrong. Let me get back to you with fixed code or an explanation of why it actually does the right thing.

clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

13

Would you like me to write an asm test in addition to / instead of this one?

45

Yes. Why? To test an additional level of nesting?

[rnk]

clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

13

Oops, I thought I wrote this comment and deleted it before hitting send, but apparently not. No, let's stick with the IR test.

[zturner]

clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

45

Yes. for that matter, even better would be if this call to foo() spans multiple lines. Right here you've got a single statement which spans multiple lines, but no individual sub-expression spans multiple lines. And FWICT there is no nesting at all, since + is just a builtin operator.

[inglorion]

clang/lib/CodeGen/CGStmt.cpp

45

FWIW, the test passes with MSVC in a Debug build, too.

[inglorion]

clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

45

The nesting here is the calls to bar, baz, and qux inside the declaration of a. The old behavior would emit separate locations for each of the calls, the new behavior annotates each of the calls with the same location as the declaration. This causes the debugger to stop only once for the entire statement when using step over, and makes step into specific work.

[zturner]

clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

45

Sure, but does that execute the same codepath as:

int a = bar(
                  baz());

a = bar(baz());

for (const auto &X : foo(bar()))

for (int x = foo(); x < bar(); baz(x))

if (foo() && bar())

if (createObject().func())

etc? And what if one or more of these functions get inlined? For example, what if you have:

int a = foo(bar(baz()), bar(buzz()));

[inglorion]

I limited the change to only calls, returns, and declarations. I also
updated the test case to include a multi-variable declaration, a while
loop, a for loop, and an if statement (after verifying the behavior in
the debugger, compared to MSVC). I discovered that there is a
difference between the generated info for DWARF with or without
-dwarf-column-info, so I included that in the test, too. I also made a
couple of minor changes that were suggested.

[inglorion]

rnk and I talked about a different approach. The idea is to explicitly emit locations in some cases (e.g. inside compound statements, the braces of for loops, ...), and otherwise emit locations only when emitting column info or emitting non-codeview debug info. That may lead to more elegant code. I'll give it a try later.

[inglorion]

Following conversation with @rnk, I managed to whittle this down to a very small change that seems to do what we need. Step into specific works and single stepping through the program behaves similarly whether compiled with clang-cl or cl.

[inglorion]

@zturner: I am still thinking about your comment about other cases to test. My concern is that there are very many possible combinations.

I'm actually not too concerned about not exactly matching cl's behavior in every single case. The difference in behavior here is us emitting a debug location for an expression that doesn't get its own debug location from cl. In general, I think having more fine-grained information is good, so I don't think differing in this way is a problem. That is, unless we end up breaking functionality in the debugger (Visual Studio). The behavior I know of we can end up breaking this way is step into specific, which appears to require multiple calls that are associated with a single debug location.

I think, at a minimum, the test case should cover a scenario where we would normally like to emit some debug locations, but need to elide them if we want to be compatible with Visual Studio. I also think it makes sense to include some cases where we want the behavior to be the same whether or not we're targeting MS compatibility. That way, we can verify that we aren't throwing away too much information. Beyond that, I feel there are diminishing returns. To avoid going too far down that path, I would like to start with a relatively small test case (as I've done), fix the bug that prompted me to write this code, and then add additional tests if we find out there are other cases where people care strongly about the granularity of the debug locations we emit. Does that sound reasonable?

[inglorion]

removed compound statement; that was only in there to double check that the debugger stops on the first child statement, but that's true with or without this change

[zturner]

@inglorion : Since we're already in this code anyway, should we at least do due diligence and check the basics, even if only manually? As in, at least we should check in MSVC if any of those other types of examples that I came up with present a problem. Otherwise we're going to be back here fixing this again in a few weeks. And it doesn't seem like it would be that hard to whip up some sample code with 10 or 12 cases and just do a sanity check that they all work. If they don't, we can then decide whether it should be fixed in this patch, or a bug can be filed to track it.

[inglorion]

added examples suggested by @zturner, verified step over and step into specific behavior matches MSVC, and added tests for them

[inglorion]

Of course, ApplyDebugLocation is also a perfectly legitimate way to add a debug location to nodes that are not nested inside nodes that already have a location. I updated the diff so that we do end up applying the location in such cases.

[rnk]

lgtm

clang/lib/CodeGen/CodeGenModule.h

517

I'd drop the "nested" part of this name. Statements are also nested, in a sense. A for loop is a statement, and it has child statements. This is really about expression locations vs. statement expressions.

[majnemer]

clang/lib/CodeGen/CodeGenModule.h

32

Any reason to do this? I'd just keep getNestedExpressionLocationsEnabled in the .cpp file.

[rnk]

clang/lib/CodeGen/CodeGenModule.h

32

+1, I think we can continue to expect that ApplyDebugLocation will be used for expression emission and CodeGenFunction::EmitStopPoint (or whatever we rename it to) will be called for statements in CGStmt.

[inglorion]

renamed get{Nested,}ExpressionLocationsEnabled and moved it into CodeGetModule.cpp

clang/lib/CodeGen/CGDebugInfo.h

 /// responsible for emitting to llvm globals or pass directly to the
 /// backend.
 class CGDebugInfo {
   friend class ApplyDebugLocation;
   friend class SaveAndRestoreLocation;
   CodeGenModule &CGM;
   const codegenoptions::DebugInfoKind DebugKind;
   bool DebugTypeExtRefs;
+  bool LocationEnabled = true;
   llvm::DIBuilder DBuilder;
   llvm::DICompileUnit *TheCU = nullptr;
   ModuleMap *ClangModuleMap = nullptr;
   ExternalASTSource::ASTSourceDescriptor PCHDescriptor;
   SourceLocation CurLoc;
   llvm::MDNode *CurInlinedAt = nullptr;
   llvm::DIType *VTablePtrType = nullptr;
   llvm::DIType *ClassTy = nullptr;
   llvm::DICompositeType *ObjTy = nullptr;
   llvm::DIType *SelTy = nullptr;
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
   llvm::DIType *SingletonId = nullptr;
 #include "clang/Basic/OpenCLImageTypes.def"
   llvm::DIType *OCLSamplerDITy = nullptr;
   llvm::DIType *OCLEventDITy = nullptr;
public:
 
   void finalize();
 
   /// Module debugging: Support for building PCMs.
   /// @{
   /// Set the main CU's DwoId field to \p Signature.
   void setDwoId(uint64_t Signature);
 
+  bool getLocationEnabled() const { return LocationEnabled; }
+
+  /// Used to enable/disable location tracking. This is used by
+  /// ApplyDebugLocation to inhibit the emission of location information
+  /// for nested calls.
+  void setLocationEnabled(bool Enabled) { LocationEnabled = Enabled; }
+
   /// When generating debug information for a clang module or
   /// precompiled header, this module map will be used to determine
   /// the module of origin of each Decl.
   void setModuleMap(ModuleMap &MMap) { ClangModuleMap = &MMap; }
 
   /// When generating debug information for a clang module or
   /// precompiled header, this module map will be used to determine
   /// the module of origin of each Decl.
public:
   /// Set up the CodeGenFunction's DebugInfo to produce inline locations for the
   /// function \p InlinedFn. The current debug location becomes the inlined call
   /// site of the inlined function.
   ApplyInlineDebugLocation(CodeGenFunction &CGF, GlobalDecl InlinedFn);
   /// Restore everything back to the orginial state.
   ~ApplyInlineDebugLocation();
 };
 
+/// Scoped helper to inhibit location emission.
+class InhibitDebugLocation {
+public:
+  InhibitDebugLocation() : DI(nullptr), ReEnable(false) {}
+  explicit InhibitDebugLocation(CGDebugInfo *DI)
+      : DI(DI), ReEnable(DI && DI->getLocationEnabled()) {
+    if (DI)
+      DI->setLocationEnabled(false);
+  }
+
+  InhibitDebugLocation(const InhibitDebugLocation &Other) = delete;
+  InhibitDebugLocation(InhibitDebugLocation &&Other) {
+    ReEnable = false;
+    *this = std::move(Other);
+  }
+
+  InhibitDebugLocation &operator=(const InhibitDebugLocation &Other) = delete;
+  InhibitDebugLocation &operator=(InhibitDebugLocation &&Other) {
+    DI = Other.DI;
+    ReEnable = Other.ReEnable;
+    Other.ReEnable = false;
+    return *this;
+  }
+  ~InhibitDebugLocation() {
+    if (ReEnable)
+      DI->setLocationEnabled(true);
+  }
+
+private:
+  CGDebugInfo *DI;
+  bool ReEnable;
+};
+
 } // namespace CodeGen
 } // namespace clang
 
 #endif // LLVM_CLANG_LIB_CODEGEN_CGDEBUGINFO_H

clang/lib/CodeGen/CGDebugInfo.cpp

ApplyInlineDebugLocation::~ApplyInlineDebugLocation() {
   if (!CGF)
     return;
   auto &DI = *CGF->getDebugInfo();
   DI.EmitInlineFunctionEnd(CGF->Builder);
   DI.EmitLocation(CGF->Builder, SavedLocation);
 }
 
 void CGDebugInfo::setLocation(SourceLocation Loc) {
-  // If the new location isn't valid return.
-  if (Loc.isInvalid())
+  if (!LocationEnabled || Loc.isInvalid())
     return;
 
   CurLoc = CGM.getContext().getSourceManager().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
   // from the one in the scope.
   if (LexicalBlockStack.empty())

clang/lib/CodeGen/CGException.cpp

void CodeGenFunction::ExitSEHTryStmt(const SEHTryStmt &S) {
   EmitBlock(ContBB);
 }
 
 void CodeGenFunction::EmitSEHLeaveStmt(const SEHLeaveStmt &S) {
   // If this code is reachable then emit a stop point (if generating
   // debug info). We have to do this ourselves because we are on the
   // "simple" statement path.
   if (HaveInsertPoint())
-    EmitStopPoint(&S);
+    EmitExprLocation(&S);
 
   // This must be a __leave from a __finally block, which we warn on and is UB.
   // Just emit unreachable.
   if (!isSEHTryScope()) {
     Builder.CreateUnreachable();
     Builder.ClearInsertionPoint();
     return;
   }
 
   EmitBranchThroughCleanup(*SEHTryEpilogueStack.back());
 }

clang/lib/CodeGen/CGStmt.cpp

 
 using namespace clang;
 using namespace CodeGen;
 
 //===----------------------------------------------------------------------===//
 //                              Statement Emission
 //===----------------------------------------------------------------------===//
 
-void CodeGenFunction::EmitStopPoint(const Stmt *S) {
-  if (CGDebugInfo *DI = getDebugInfo()) {
+InhibitDebugLocation CodeGenFunction::EmitStmtLocation(const Stmt *S) {
+  InhibitDebugLocation IDL;
+  if (HaveInsertPoint()) {
+    EmitExprLocation(S);
+    if (!CGM.getNestedExpressionLocationsEnabled())
+      IDL = InhibitDebugLocation(getDebugInfo());
+  }
+  return IDL;
+}
+
+void CodeGenFunction::EmitExprLocation(const Stmt *S) {
+  CGDebugInfo *DI = getDebugInfo();
+  if (DI && DI->getLocationEnabled()) {
     SourceLocation Loc;
     Loc = S->getLocStart();
     DI->EmitLocation(Builder, Loc);
 
-    LastStopPoint = Loc;
+    LastDebugLocation = Loc;
   }
 }
 
 void CodeGenFunction::EmitStmt(const Stmt *S) {
   assert(S && "Null statement?");
   PGO.setCurrentStmt(S);
 
   // These statements have their own debug info handling.
if (!ContainsLabel(S)) {
       return;
     }
 
     // Otherwise, make a new block to hold the code.
     EnsureInsertPoint();
   }
 
   // Generate a stoppoint if we are emitting debug info.
-  EmitStopPoint(S);
+  InhibitDebugLocation IDL;
+  switch (S->getStmtClass()) {
+  case Stmt::CallExprClass:
+  case Stmt::DeclStmtClass:
+  case Stmt::ReturnStmtClass:
+    IDL = EmitStmtLocation(S);
+    break;
+  default:
+    EmitExprLocation(S);
+  }
 
   switch (S->getStmtClass()) {
   case Stmt::NoStmtClass:
   case Stmt::CXXCatchStmtClass:
   case Stmt::SEHExceptStmtClass:
   case Stmt::SEHFinallyStmtClass:
   case Stmt::MSDependentExistsStmtClass:
     llvm_unreachable("invalid statement class to emit generically");
if (incoming != outgoing && outgoing->use_empty()) {
       Builder.ClearInsertionPoint();
     }
     break;
   }
 
   case Stmt::IndirectGotoStmtClass:
     EmitIndirectGotoStmt(cast<IndirectGotoStmt>(*S)); break;
 
   case Stmt::IfStmtClass:       EmitIfStmt(cast<IfStmt>(*S));             break;
   case Stmt::WhileStmtClass:    EmitWhileStmt(cast<WhileStmt>(*S));       break;
   case Stmt::DoStmtClass:       EmitDoStmt(cast<DoStmt>(*S));             break;
   case Stmt::ForStmtClass:      EmitForStmt(cast<ForStmt>(*S));           break;
 
   case Stmt::ReturnStmtClass:   EmitReturnStmt(cast<ReturnStmt>(*S));     break;
 
   case Stmt::SwitchStmtClass:   EmitSwitchStmt(cast<SwitchStmt>(*S));     break;
   case Stmt::GCCAsmStmtClass:   // Intentional fall-through.
void CodeGenFunction::EmitAttributedStmt(const AttributedStmt &S) {
   }
 }
 
 void CodeGenFunction::EmitGotoStmt(const GotoStmt &S) {
   // If this code is reachable then emit a stop point (if generating
   // debug info). We have to do this ourselves because we are on the
   // "simple" statement path.
   if (HaveInsertPoint())
-    EmitStopPoint(&S);
+    EmitExprLocation(&S);
 
   EmitBranchThroughCleanup(getJumpDestForLabel(S.getLabel()));
 }
 
 
 void CodeGenFunction::EmitIndirectGotoStmt(const IndirectGotoStmt &S) {
   if (const LabelDecl *Target = S.getConstantTarget()) {
     EmitBranchThroughCleanup(getJumpDestForLabel(Target));
// because it might be a singleton DeclStmt.
     EmitStmt(S.getBody());
   }
 
   BreakContinueStack.pop_back();
 
   // Immediately force cleanup.
   ConditionScope.ForceCleanup();
 
-  EmitStopPoint(&S);
+  EmitExprLocation(&S);
   // Branch to the loop header again.
   EmitBranch(LoopHeader.getBlock());
 
   LoopStack.pop();
 
   // Emit the exit block.
   EmitBlock(LoopExit.getBlock(), true);
 
if (S.getInc()) {
     EmitBlock(Continue.getBlock());
     EmitStmt(S.getInc());
   }
 
   BreakContinueStack.pop_back();
 
   ConditionScope.ForceCleanup();
 
-  EmitStopPoint(&S);
+  EmitExprLocation(&S);
   EmitBranch(CondBlock);
 
   ForScope.ForceCleanup();
 
   LoopStack.pop();
 
   // Emit the fall-through block.
   EmitBlock(LoopExit.getBlock(), true);
CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
 
   {
     // Create a separate cleanup scope for the loop variable and body.
     LexicalScope BodyScope(*this, S.getSourceRange());
     EmitStmt(S.getLoopVarStmt());
     EmitStmt(S.getBody());
   }
 
-  EmitStopPoint(&S);
+  EmitExprLocation(&S);
   // If there is an increment, emit it next.
   EmitBlock(Continue.getBlock());
   EmitStmt(S.getInc());
 
   BreakContinueStack.pop_back();
 
   EmitBranch(CondBlock);
 
void CodeGenFunction::EmitReturnStmt(const ReturnStmt &S) {
 
   cleanupScope.ForceCleanup();
   EmitBranchThroughCleanup(ReturnBlock);
 }
 
 void CodeGenFunction::EmitDeclStmt(const DeclStmt &S) {
   // As long as debug info is modeled with instructions, we have to ensure we
   // have a place to insert here and write the stop point here.
-  if (HaveInsertPoint())
-    EmitStopPoint(&S);
+  InhibitDebugLocation IDL = EmitStmtLocation(&S);
 
   for (const auto *I : S.decls())
     EmitDecl(*I);
 }
 
 void CodeGenFunction::EmitBreakStmt(const BreakStmt &S) {
   assert(!BreakContinueStack.empty() && "break stmt not in a loop or switch!");
 
   // If this code is reachable then emit a stop point (if generating
   // debug info). We have to do this ourselves because we are on the
   // "simple" statement path.
   if (HaveInsertPoint())
-    EmitStopPoint(&S);
+    EmitExprLocation(&S);
 
   EmitBranchThroughCleanup(BreakContinueStack.back().BreakBlock);
 }
 
 void CodeGenFunction::EmitContinueStmt(const ContinueStmt &S) {
   assert(!BreakContinueStack.empty() && "continue stmt not in a loop!");
 
   // If this code is reachable then emit a stop point (if generating
   // debug info). We have to do this ourselves because we are on the
   // "simple" statement path.
   if (HaveInsertPoint())
-    EmitStopPoint(&S);
+    EmitExprLocation(&S);
 
   EmitBranchThroughCleanup(BreakContinueStack.back().ContinueBlock);
 }
 
 /// EmitCaseStmtRange - If case statement range is not too big then
 /// add multiple cases to switch instruction, one for each value within
 /// the range. If range is too big then emit "if" condition check.
 void CodeGenFunction::EmitCaseStmtRange(const CaseStmt &S) {

clang/lib/CodeGen/CGStmtOpenMP.cpp

void CodeGenFunction::EmitOMPSimdFinal(
   if (DoneBB)
     EmitBlock(DoneBB, /*IsFinished=*/true);
 }
 
 static void emitOMPLoopBodyWithStopPoint(CodeGenFunction &CGF,
                                          const OMPLoopDirective &S,
                                          CodeGenFunction::JumpDest LoopExit) {
   CGF.EmitOMPLoopBody(S, LoopExit);
-  CGF.EmitStopPoint(&S);
+  CGF.EmitExprLocation(&S);
 }
 
 void CodeGenFunction::EmitOMPSimdDirective(const OMPSimdDirective &S) {
   auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &) {
     OMPLoopScope PreInitScope(CGF, S);
     // if (PreCond) {
     //   for (IV in 0..LastIteration) BODY;
     //   <Final counter/linear vars updates>;
(void)CGF.EmitOMPLinearClauseInit(S);
       CGF.EmitOMPReductionClauseInit(S, LoopScope);
       bool HasLastprivateClause =
           CGF.EmitOMPLastprivateClauseInit(S, LoopScope);
       (void)LoopScope.Privatize();
       CGF.EmitOMPInnerLoop(S, LoopScope.requiresCleanups(), S.getCond(),
                            S.getInc(),
                            [&S](CodeGenFunction &CGF) {
                              CGF.EmitOMPLoopBody(S, JumpDest());
-                             CGF.EmitStopPoint(&S);
+                             CGF.EmitExprLocation(&S);
                            },
                            [](CodeGenFunction &) {});
       CGF.EmitOMPSimdFinal(
           S, [](CodeGenFunction &) -> llvm::Value * { return nullptr; });
       // Emit final copy of the lastprivate variables at the end of loops.
       if (HasLastprivateClause)
         CGF.EmitOMPLastprivateClauseFinal(S, /*NoFinals=*/true);
       CGF.EmitOMPReductionClauseFinal(S, /*ReductionKind=*/OMPD_simd);
// Emit 'then' code.
         EmitIgnoredExpr(S.getEnsureUpperBound());
         // IV = LB;
         EmitIgnoredExpr(S.getInit());
         // while (idx <= UB) { BODY; ++idx; }
         EmitOMPInnerLoop(S, LoopScope.requiresCleanups(), S.getCond(),
                          S.getInc(),
                          [&S, LoopExit](CodeGenFunction &CGF) {
                            CGF.EmitOMPLoopBody(S, LoopExit);
-                           CGF.EmitStopPoint(&S);
+                           CGF.EmitExprLocation(&S);
                          },
                          [](CodeGenFunction &) {});
         EmitBlock(LoopExit.getBlock());
         // Tell the runtime we are done.
         auto &&CodeGen = [&S](CodeGenFunction &CGF) {
           CGF.CGM.getOpenMPRuntime().emitForStaticFinish(CGF, S.getLocEnd());
         };
         OMPCancelStack.emitExit(*this, S.getDirectiveKind(), CodeGen);
for (const auto *C : Compound->body()) {
       if (const auto *EWC = dyn_cast<ExprWithCleanups>(C)) {
         enterFullExpression(EWC);
       }
     }
   }
 
   auto &&CodeGen = [&S, Kind, IsSeqCst, CS](CodeGenFunction &CGF,
                                             PrePostActionTy &) {
-    CGF.EmitStopPoint(CS);
+    CGF.EmitExprLocation(CS);
     EmitOMPAtomicExpr(CGF, Kind, IsSeqCst, S.isPostfixUpdate(), S.getX(),
                       S.getV(), S.getExpr(), S.getUpdateExpr(),
                       S.isXLHSInRHSPart(), S.getLocStart());
   };
   OMPLexicalScope Scope(*this, S, /*AsInlined=*/true);
   CGM.getOpenMPRuntime().emitInlinedDirective(*this, OMPD_atomic, CodeGen);
 }
 
if (auto LIExpr = dyn_cast<DeclRefExpr>(S.getLastIteration())) {
       // Emit calculation of the iterations count.
       CGF.EmitIgnoredExpr(S.getCalcLastIteration());
     }
 
     CGF.EmitOMPInnerLoop(S, LoopScope.requiresCleanups(), S.getCond(),
                          S.getInc(),
                          [&S](CodeGenFunction &CGF) {
                            CGF.EmitOMPLoopBody(S, JumpDest());
-                           CGF.EmitStopPoint(&S);
+                           CGF.EmitExprLocation(&S);
                          },
                          [](CodeGenFunction &) {});
     // Emit: if (PreCond) - end.
     if (ContBlock) {
       CGF.EmitBranch(ContBlock);
       CGF.EmitBlock(ContBlock, true);
     }
     // Emit final copy of the lastprivate variables if IsLastIter != 0.

clang/lib/CodeGen/CodeGenFunction.h

 class CGFunctionInfo;
 class CGRecordLayout;
 class CGBlockInfo;
 class CGCXXABI;
 class BlockByrefHelpers;
 class BlockByrefInfo;
 class BlockFlags;
 class BlockFieldFlags;
+class InhibitDebugLocation;
 class RegionCodeGenTy;
 class TargetCodeGenInfo;
 struct OMPTaskDataTy;
 struct CGCoroData;
 
 /// The kind of evaluation to perform on values of a particular
 /// type.  Basically, is the code in CGExprScalar, CGExprComplex, or
 /// CGExprAgg?
private:
 
   /// Counts of the number return expressions in the function.
   unsigned NumReturnExprs;
 
   /// Count the number of simple (constant) return expressions in the function.
   unsigned NumSimpleReturnExprs;
 
   /// The last regular (non-return) debug location (breakpoint) in the function.
-  SourceLocation LastStopPoint;
+  SourceLocation LastDebugLocation;
 
 public:
   /// A scope within which we are constructing the fields of an object which
   /// might use a CXXDefaultInitExpr. This stashes away a 'this' value to use
   /// if we need to evaluate a CXXDefaultInitExpr within the evaluation.
   class FieldConstructionScope {
   public:
     FieldConstructionScope(CodeGenFunction &CGF, Address This)
void EmitAlignmentAssumption(llvm::Value *PtrValue, llvm::Value *Alignment,
     Builder.CreateAlignmentAssumption(CGM.getDataLayout(), PtrValue, Alignment,
                                       OffsetValue);
   }
 
   //===--------------------------------------------------------------------===//
   //                             Statement Emission
   //===--------------------------------------------------------------------===//
 
-  /// EmitStopPoint - Emit a debug stoppoint if we are emitting debug info.
-  void EmitStopPoint(const Stmt *S);
+  /// Emit a debug location for a statement if we are emitting debug info. If
+  /// getNestedExpressionLocationsEnabled() returns false, this also inhibits
+  /// the emission of debug location information until the returned
+  /// InhibitDebugLocation object is destroyed.
+  InhibitDebugLocation EmitStmtLocation(const Stmt *S);
+
+  /// Emit a debug location for an expression if we are emitting debug info.
+  void EmitExprLocation(const Stmt *S);
 
   /// EmitStmt - Emit the code for the statement \arg S. It is legal to call
   /// this function even if there is no current insertion point.
   ///
   /// This function may clear the current insertion point; callers should use
   /// EnsureInsertPoint if they wish to subsequently generate code without first
   /// calling EmitBlock, EmitBranch, or EmitStmt.
   void EmitStmt(const Stmt *S);

clang/lib/CodeGen/CodeGenFunction.cpp

void CodeGenFunction::FinishFunction(SourceLocation EndLoc) {
   // the return statement.  Otherwise the cleanup code should be at the
   // end of the function's lexical scope.
   //
   // If there are multiple branches to the return block, the branch
   // instructions will get the location of the return statements and
   // all will be fine.
   if (CGDebugInfo *DI = getDebugInfo()) {
     if (OnlySimpleReturnStmts)
-      DI->EmitLocation(Builder, LastStopPoint);
+      DI->EmitLocation(Builder, LastDebugLocation);
     else
       DI->EmitLocation(Builder, EndLoc);
   }
 
   // Pop any cleanups that might have been associated with the
   // parameters.  Do this in whatever block we're currently in; it's
   // important to do this before we enter the return block or return
   // edges will be *really* confused.

clang/lib/CodeGen/CodeGenModule.h

 #include "clang/AST/DeclOpenMP.h"
 #include "clang/AST/GlobalDecl.h"
 #include "clang/AST/Mangle.h"
 #include "clang/Basic/ABI.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SanitizerBlacklist.h"
 #include "clang/Basic/XRayLists.h"
+#include "clang/Frontend/CodeGenOptions.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Transforms/Utils/SanitizerStats.h"
 

 class Expr;
 class Stmt;
 class InitListExpr;
 class StringLiteral;
 class NamedDecl;
 class ValueDecl;
 class VarDecl;
 class LangOptions;
-class CodeGenOptions;
 class HeaderSearchOptions;
 class PreprocessorOptions;
 class DiagnosticsEngine;
 class AnnotateAttr;
 class CXXDestructorDecl;
 class Module;
 class CoverageSourceInfo;
 
public:
 
   ~CodeGenModule();
 
   void clear();
 
   /// Finalize LLVM code generation.
   void Release();
 
+  /// Return true if we should emit location information for nested expressions.
+  bool getNestedExpressionLocationsEnabled() const {
+    return !CodeGenOpts.EmitCodeView || CodeGenOpts.DebugColumnInfo;
+  }
+
   /// Return a reference to the configured Objective-C runtime.
   CGObjCRuntime &getObjCRuntime() {
     if (!ObjCRuntime) createObjCRuntime();
     return *ObjCRuntime;
   }
 
   /// Return true iff an Objective-C runtime has been configured.
   bool hasObjCRuntime() { return !!ObjCRuntime; }

clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:    -gcodeview -emit-llvm -o - %s | FileCheck -check-prefix=NONEST %s
+// RUN: %clang_cc1 -triple=x86_64-pc-windows-msvc -debug-info-kind=limited \
+// RUN:    -gcodeview -dwarf-column-info -emit-llvm -o - %s \
+// RUN:    | FileCheck -check-prefix=COLUMNS %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:    -emit-llvm -o - %s | FileCheck -check-prefix=NESTED %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -debug-info-kind=limited \
+// RUN:    -dwarf-column-info -emit-llvm -o - %s \
+// RUN:    | FileCheck -check-prefix=COLUMNS %s
+
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NONEST: call i32 @{{.*}}baz{{.*}}, !dbg ![[LOC]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 {{.*}}, !dbg ![[LOC]]
+// NONEST: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NONEST: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NONEST: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NONEST: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NONEST: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NONEST: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NONEST: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ![[WHILE1]] = !DILocation(
+// NONEST: ![[WHILE2]] = !DILocation(
+// NONEST: ![[FOR1]] = !DILocation(
+// NONEST: ![[FOR2]] = !DILocation(
+// NONEST: ![[IF1]] = !DILocation(
+// NONEST: ![[IF2]] = !DILocation(
+// NONEST: ![[IF3]] = !DILocation(
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// NESTED: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// NESTED: store i32 0, i32* %b,{{.*}} !dbg ![[ILOC]]
+// NESTED: store i32 0, i32* %c,{{.*}} !dbg ![[ILOC]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// NESTED: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED: ![[BAR]] = !DILocation(
+// NESTED: ![[BAZ]] = !DILocation(
+// NESTED: ![[QUX]] = !DILocation(
+// NESTED: ![[RETSUB]] = !DILocation(
+// NESTED: ![[RETMUL]] = !DILocation(
+
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[BAR:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}baz{{.*}}, !dbg ![[BAZ:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[QUX:[0-9]+]]
+// COLUMNS: store i32 1, i32* %i,{{.*}} !dbg ![[ILOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %b,{{.*}} !dbg ![[BLOC:[0-9]+]]
+// COLUMNS: store i32 0, i32* %c,{{.*}} !dbg ![[CLOC:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[WHILE1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %i,{{.*}} !dbg ![[WHILE2:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}bar{{.*}}, !dbg ![[FOR1:[0-9]+]]
+// COLUMNS: call i32 @{{.*}}qux{{.*}}, !dbg ![[FOR2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %t,{{.*}} !dbg ![[IF1:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %a,{{.*}} !dbg ![[IF2:[0-9]+]]
+// COLUMNS: store i32 %{{[^,]+}}, i32* %b,{{.*}} !dbg ![[IF3:[0-9]+]]
+// COLUMNS: mul nsw i32 {{.*}}, !dbg ![[RETMUL:[0-9]+]]
+// COLUMNS: sub nsw i32 {{.*}}, !dbg ![[RETSUB:[0-9]+]]
+// COLUMNS: ret i32 {{.*}}, !dbg !
+// COLUMNS: ![[BAR]] = !DILocation(
+// COLUMNS: ![[BAZ]] = !DILocation(
+// COLUMNS: ![[QUX]] = !DILocation(
+// COLUMNS: ![[ILOC]] = !DILocation(
+// COLUMNS: ![[BLOC]] = !DILocation(
+// COLUMNS: ![[CLOC]] = !DILocation(
+// COLUNMS: ![[RETSUB]] = !DILocation(
+// COLUMNS: ![[RETMUL]] = !DILocation(
+
+int bar(int x, int y);
+int baz(int x, int y);
+int qux(int x, int y);
+
+int foo(int x, int y, int z) {
+  int a = bar(x, y) +
+          baz(x, z) +
+          qux(y, z);
+
+  int i = 1, b = 0, c = 0;
+  while (i > 0) {
+    b = bar(a, b);
+    --i;
+  }
+  for (i = 0; i < 1; i++) {
+    b = bar(a, b);
+    c = qux(a, c);
+  }
+  if (a < b) {
+    int t = a;
+    a = b;
+    b = t;
+  }
+
+  return a -
+         (b * z);
+}