inglorion created this revision.
Herald added a subscriber: aprantl.

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.


https://reviews.llvm.org/D37529

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCXX/debug-info-nested-exprs.cpp

Index: clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-nested-exprs.cpp
@@ -0,0 +1,48 @@
+// 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=NESTED %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=NESTED %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: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NONEST: sub nsw i32 {{.*}}, !dbg ![[RETLOC]]
+// NONEST: ret i32 {{.*}}, !dbg ![[RETLOC]]
+
+// NESTED: call i32 @{{.*}}bar{{.*}}, !dbg ![[LOC:[0-9]+]]
+// NESTED: call i32 @{{.*}}baz{{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: call i32 @{{.*}}qux{{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: store i32 {{.*}}, !dbg !
+// NESTED-NOT: [[LOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: mul nsw i32 {{.*}}, !dbg ![[RETLOC:[0-9]+]]
+// NESTED: sub nsw i32 {{.*}}, !dbg !
+// NESTED-NOT: [[RETLOC]]
+// NESTED-SAME: {{[0-9]+}}
+// NESTED: ret i32 {{.*}}, !dbg !
+// NESTED-NOT: [[RETLOC]]
+// NESTED-SAME: {{[0-9]+}}
+
+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);
+  return a -
+         (y * z);
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===================================================================
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -29,6 +29,7 @@
 #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"
@@ -70,7 +71,6 @@
 class ValueDecl;
 class VarDecl;
 class LangOptions;
-class CodeGenOptions;
 class HeaderSearchOptions;
 class PreprocessorOptions;
 class DiagnosticsEngine;
@@ -513,6 +513,11 @@
   /// 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();
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -87,6 +87,7 @@
 class BlockByrefInfo;
 class BlockFlags;
 class BlockFieldFlags;
+class InhibitDebugLocation;
 class RegionCodeGenTy;
 class TargetCodeGenInfo;
 struct OMPTaskDataTy;
@@ -2525,6 +2526,14 @@
   //                             Statement Emission
   //===--------------------------------------------------------------------===//
 
+  /// EmitStmtStopPoint - Emit a statement stoppoint 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 EmitStmtStopPoint(const Stmt *S);
+
   /// EmitStopPoint - Emit a debug stoppoint if we are emitting debug info.
   void EmitStopPoint(const Stmt *S);
 
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -35,8 +35,19 @@
 //                              Statement Emission
 //===----------------------------------------------------------------------===//
 
+InhibitDebugLocation CodeGenFunction::EmitStmtStopPoint(const Stmt *S) {
+  InhibitDebugLocation IDL;
+  if (HaveInsertPoint()) {
+    EmitStopPoint(S);
+    if (!CGM.getNestedExpressionLocationsEnabled())
+      IDL = InhibitDebugLocation(getDebugInfo());
+  }
+  return IDL;
+}
+
 void CodeGenFunction::EmitStopPoint(const Stmt *S) {
-  if (CGDebugInfo *DI = getDebugInfo()) {
+  CGDebugInfo *DI = getDebugInfo();
+  if (DI && DI->getLocationEnabled()) {
     SourceLocation Loc;
     Loc = S->getLocStart();
     DI->EmitLocation(Builder, Loc);
@@ -72,7 +83,7 @@
   }
 
   // Generate a stoppoint if we are emitting debug info.
-  EmitStopPoint(S);
+  InhibitDebugLocation IDL = EmitStmtStopPoint(S);
 
   switch (S->getStmtClass()) {
   case Stmt::NoStmtClass:
@@ -1110,8 +1121,7 @@
 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 = EmitStmtStopPoint(&S);
 
   for (const auto *I : S.decls())
     EmitDecl(*I);
Index: clang/lib/CodeGen/CGDebugInfo.h
===================================================================
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -29,6 +29,8 @@
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Support/Allocator.h"
 
+#include "CodeGenModule.h"
+
 namespace llvm {
 class MDNode;
 }
@@ -62,6 +64,7 @@
   ExternalASTSource::ASTSourceDescriptor PCHDescriptor;
   SourceLocation CurLoc;
   llvm::MDNode *CurInlinedAt = nullptr;
+  bool LocationEnabled = true;
   llvm::DIType *VTablePtrType = nullptr;
   llvm::DIType *ClassTy = nullptr;
   llvm::DICompositeType *ObjTy = nullptr;
@@ -314,6 +317,13 @@
   /// 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.
@@ -685,6 +695,39 @@
   ~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
 
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -161,8 +161,7 @@
 }
 
 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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to