Skip to content

[DLCov 3/5] Implement DebugLoc origin-tracking #107369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions clang/lib/CodeGen/BackendUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,22 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
Debugify.setOrigDIVerifyBugsReportFilePath(
CodeGenOpts.DIBugsReportFilePath);
Debugify.registerCallbacks(PIC, MAM);

#if ENABLE_DEBUGLOC_COVERAGE_TRACKING
// If we're using debug location coverage tracking, mark all the
// instructions coming out of the frontend without a DebugLoc as being
// intentional line-zero locations, to prevent both those instructions and
// new instructions that inherit their location from being treated as
// incorrectly empty locations.
for (Function &F : *TheModule) {
if (!F.getSubprogram())
continue;
for (BasicBlock &BB : F)
for (Instruction &I : BB)
if (!I.getDebugLoc())
I.setDebugLoc(DebugLoc::getLineZero());
}
#endif
}
// Attempt to load pass plugins and register their callbacks with PB.
for (auto &PluginFN : CodeGenOpts.PassPlugins) {
Expand Down
4 changes: 4 additions & 0 deletions llvm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,10 @@ endif()

option(LLVM_ENABLE_CRASH_DUMPS "Turn on memory dumps on crashes. Currently only implemented on Windows." OFF)

set(LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING "DISABLED" CACHE STRING
"Enhance debugify's line number coverage tracking; enabling this is abi-breaking. Can be DISABLED, COVERAGE, or COVERAGE_AND_ORIGIN.")
set_property(CACHE LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING PROPERTY STRINGS DISABLED COVERAGE COVERAGE_AND_ORIGIN)

set(WINDOWS_PREFER_FORWARD_SLASH_DEFAULT OFF)
if (MINGW)
# Cygwin doesn't identify itself as Windows, and thus gets path::Style::posix
Expand Down
13 changes: 13 additions & 0 deletions llvm/cmake/modules/HandleLLVMOptions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,19 @@ else()
message(FATAL_ERROR "Unknown value for LLVM_ABI_BREAKING_CHECKS: \"${LLVM_ABI_BREAKING_CHECKS}\"!")
endif()

string(TOUPPER "${LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING}" uppercase_LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING)

if( uppercase_LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING STREQUAL "COVERAGE" )
set( ENABLE_DEBUGLOC_COVERAGE_TRACKING 1 )
elseif( uppercase_LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING STREQUAL "COVERAGE_AND_ORIGIN" )
set( ENABLE_DEBUGLOC_COVERAGE_TRACKING 1 )
set( ENABLE_DEBUGLOC_ORIGIN_TRACKING 1 )
elseif( uppercase_LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING STREQUAL "DISABLED" OR NOT DEFINED LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING )
# The DISABLED setting is default and requires no additional defines.
else()
message(FATAL_ERROR "Unknown value for LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING: \"${LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING}\"!")
endif()

if( LLVM_REVERSE_ITERATION )
set( LLVM_ENABLE_REVERSE_ITERATION 1 )
endif()
Expand Down
11 changes: 11 additions & 0 deletions llvm/docs/CMake.rst
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,17 @@ enabled sub-projects. Nearly all of these variable names begin with
**LLVM_ENABLE_BINDINGS**:BOOL
If disabled, do not try to build the OCaml bindings.

**LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING**:STRING
Enhances Debugify's ability to detect line number errors by storing extra
information inside Instructions, removing false positives from Debugify's
results at the cost of performance. Allowed values are `DISABLED` (default),
`COVERAGE`, and `COVERAGE_AND_ORIGIN`. `COVERAGE` tracks whether and why a
line number was intentionally dropped or not generated for an instruction,
allowing Debugify to avoid reporting these as errors. `COVERAGE_AND_ORIGIN`
additionally stores a stacktrace of the point where each DebugLoc is
unintentionally dropped, allowing for much easier bug triaging at the cost of
a ~10x performance slowdown.

**LLVM_ENABLE_DIA_SDK**:BOOL
Enable building with MSVC DIA SDK for PDB debugging support. Available
only with MSVC. Defaults to ON.
Expand Down
8 changes: 8 additions & 0 deletions llvm/include/llvm/Config/config.h.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
/* Define to 1 to enable crash memory dumps, and to 0 otherwise. */
#cmakedefine01 LLVM_ENABLE_CRASH_DUMPS

/* Define to 1 to enable expensive checks for debug location coverage checking,
and to 0 otherwise. */
#cmakedefine01 ENABLE_DEBUGLOC_COVERAGE_TRACKING

/* Define to 1 to enable expensive tracking of the origin of debug location
coverage bugs, and to 0 otherwise. */
#cmakedefine01 ENABLE_DEBUGLOC_ORIGIN_TRACKING

/* Define to 1 to prefer forward slashes on Windows, and to 0 prefer
backslashes. */
#cmakedefine01 LLVM_WINDOWS_PREFER_FORWARD_SLASH
Expand Down
114 changes: 113 additions & 1 deletion llvm/include/llvm/IR/DebugLoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#ifndef LLVM_IR_DEBUGLOC_H
#define LLVM_IR_DEBUGLOC_H

#include "llvm/Config/config.h"
#include "llvm/IR/TrackingMDRef.h"
#include "llvm/Support/DataTypes.h"

Expand All @@ -22,6 +23,90 @@ namespace llvm {
class LLVMContext;
class raw_ostream;
class DILocation;
class Function;

#if ENABLE_DEBUGLOC_COVERAGE_TRACKING
#if ENABLE_DEBUGLOC_ORIGIN_TRACKING
struct DbgLocOrigin {
static constexpr unsigned long MaxDepth = 16;
using StackTracesTy =
SmallVector<std::pair<int, std::array<void *, MaxDepth>>, 0>;
StackTracesTy StackTraces;
DbgLocOrigin(bool ShouldCollectTrace);
void addTrace();
const StackTracesTy &getOriginStackTraces() const { return StackTraces; };
};
#else
struct DbgLocOrigin {
DbgLocOrigin(bool) {}
};
#endif

// Used to represent different "kinds" of DebugLoc, expressing that a DebugLoc
// is either ordinary, containing a valid DILocation, or otherwise describing
// the reason why the DebugLoc does not contain a valid DILocation.
enum class DebugLocKind : uint8_t {
// DebugLoc is expected to contain a valid DILocation.
Normal,
// DebugLoc intentionally does not have a valid DILocation; may be for a
// compiler-generated instruction, or an explicitly dropped location.
LineZero,
// DebugLoc does not have a known or currently knowable source location,
// e.g. the attribution is ambiguous in a way that can't be represented, or
// determining the correct location is complicated and requires future
// developer effort.
Unknown,
// DebugLoc is attached to an instruction that we don't expect to be
// emitted, and so can omit a valid DILocation; we don't expect to ever try
// and emit these into the line table, and trying to do so is a sign that
// something has gone wrong (most likely a DebugLoc leaking from a transient
// compiler-generated instruction).
Temporary
};

// Extends TrackingMDNodeRef to also store a DebugLocKind and Origin,
// allowing Debugify to ignore intentionally-empty DebugLocs and display the
// code responsible for generating unintentionally-empty DebugLocs.
// Currently we only need to track the Origin of this DILoc when using a
// DebugLoc that is not annotated (i.e. has DebugLocKind::Normal) and has a
// null DILocation, so only collect the origin stacktrace in those cases.
class DILocAndCoverageTracking : public TrackingMDNodeRef,
public DbgLocOrigin {
public:
DebugLocKind Kind;
// Default constructor for empty DebugLocs.
DILocAndCoverageTracking()
: TrackingMDNodeRef(nullptr), DbgLocOrigin(true),
Kind(DebugLocKind::Normal) {}
// Valid or nullptr MDNode*, normal DebugLocKind
DILocAndCoverageTracking(const MDNode *Loc)
: TrackingMDNodeRef(const_cast<MDNode *>(Loc)), DbgLocOrigin(!Loc),
Kind(DebugLocKind::Normal) {}
DILocAndCoverageTracking(const DILocation *Loc);
// Always nullptr MDNode*, any DebugLocKind
DILocAndCoverageTracking(DebugLocKind Kind)
: TrackingMDNodeRef(nullptr),
DbgLocOrigin(Kind == DebugLocKind::Normal), Kind(Kind) {}
};
template <> struct simplify_type<DILocAndCoverageTracking> {
using SimpleType = MDNode *;

static MDNode *getSimplifiedValue(DILocAndCoverageTracking &MD) {
return MD.get();
}
};
template <> struct simplify_type<const DILocAndCoverageTracking> {
using SimpleType = MDNode *;

static MDNode *getSimplifiedValue(const DILocAndCoverageTracking &MD) {
return MD.get();
}
};

using DebugLocTrackingRef = DILocAndCoverageTracking;
#else
using DebugLocTrackingRef = TrackingMDNodeRef;
#endif // ENABLE_DEBUGLOC_COVERAGE_TRACKING

/// A debug info location.
///
Expand All @@ -31,7 +116,8 @@ namespace llvm {
/// To avoid extra includes, \a DebugLoc doubles the \a DILocation API with a
/// one based on relatively opaque \a MDNode pointers.
class DebugLoc {
TrackingMDNodeRef Loc;

DebugLocTrackingRef Loc;

public:
DebugLoc() = default;
Expand All @@ -47,6 +133,32 @@ namespace llvm {
/// IR.
explicit DebugLoc(const MDNode *N);

#if ENABLE_DEBUGLOC_COVERAGE_TRACKING
DebugLoc(DebugLocKind Kind) : Loc(Kind) {}
DebugLocKind getKind() const { return Loc.Kind; }
#endif

#if ENABLE_DEBUGLOC_ORIGIN_TRACKING
#if !ENABLE_DEBUGLOC_COVERAGE_TRACKING
#error Cannot enable DebugLoc origin-tracking without coverage-tracking!
#endif
Comment on lines +142 to +144
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how critical checking this case is... IIUC, CMake options won't let you set such anything anyway, so perhaps remove this?

If keeping it, perhaps remove the blank line after this #endif here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured it'd be worth keeping something in the source just in case of some weird scenario with leaky environment variables; it's not likely to happen, but this will give a much cleaner error if it does.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, how are environment variables ending up with up confused compiler defines...? Anyway, it's fine to keep, even if highly unlikely to be hit.


const DbgLocOrigin::StackTracesTy &getOriginStackTraces() const {
return Loc.getOriginStackTraces();
}
DebugLoc getCopied() const {
DebugLoc NewDL = *this;
NewDL.Loc.addTrace();
return NewDL;
}
#else
DebugLoc getCopied() const { return *this; }
#endif

static DebugLoc getTemporary();
static DebugLoc getUnknown();
static DebugLoc getLineZero();

/// Get the underlying \a DILocation.
///
/// \pre !*this or \c isa<DILocation>(getAsMDNode()).
Expand Down
40 changes: 40 additions & 0 deletions llvm/include/llvm/Support/Signals.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,31 @@
#ifndef LLVM_SUPPORT_SIGNALS_H
#define LLVM_SUPPORT_SIGNALS_H

#include "llvm/Config/config.h"
#include <array>
#include <cstdint>
#include <string>

namespace llvm {
class StringRef;
class raw_ostream;

#if ENABLE_DEBUGLOC_ORIGIN_TRACKING
// Typedefs that are convenient but only used by the stack-trace-collection code
// added if DebugLoc origin-tracking is enabled.
template <typename T, typename Enable> struct DenseMapInfo;
template <typename ValueT, typename ValueInfoT> class DenseSet;
namespace detail {
template <typename KeyT, typename ValueT> struct DenseMapPair;
}
template <typename KeyT, typename ValueT, typename KeyInfoT, typename BucketT>
class DenseMap;
using AddressSet = DenseSet<void *, DenseMapInfo<void *, void>>;
using SymbolizedAddressMap =
DenseMap<void *, std::string, DenseMapInfo<void *, void>,
detail::DenseMapPair<void *, std::string>>;
#endif

namespace sys {

/// This function runs all the registered interrupt handlers, including the
Expand Down Expand Up @@ -55,6 +73,28 @@ namespace sys {
/// specified, the entire frame is printed.
void PrintStackTrace(raw_ostream &OS, int Depth = 0);

#if ENABLE_DEBUGLOC_ORIGIN_TRACKING
#ifdef NDEBUG
#error DebugLoc origin-tracking should not be enabled in Release builds.
#endif
/// Populates the given array with a stack trace of the current program, up to
/// MaxDepth frames. Returns the number of frames returned, which will be
/// inserted into \p StackTrace from index 0. All entries after the returned
/// depth will be unmodified. NB: This is only intended to be used for
/// introspection of LLVM by Debugify, will not be enabled in release builds,
/// and should not be relied on for other purposes.
template <unsigned long MaxDepth>
int getStackTrace(std::array<void *, MaxDepth> &StackTrace);

/// Takes a set of \p Addresses, symbolizes them and stores the result in the
/// provided \p SymbolizedAddresses map.
/// NB: This is only intended to be used for introspection of LLVM by
/// Debugify, will not be enabled in release builds, and should not be relied
/// on for other purposes.
void symbolizeAddresses(AddressSet &Addresses,
SymbolizedAddressMap &SymbolizedAddresses);
#endif

// Run all registered signal handlers.
void RunSignalHandlers();

Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "llvm/CodeGen/TargetLowering.h"
#include "llvm/CodeGen/TargetRegisterInfo.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/Config/config.h"
#include "llvm/DebugInfo/DWARF/DWARFDataExtractor.h"
#include "llvm/DebugInfo/DWARF/DWARFExpression.h"
#include "llvm/IR/Constants.h"
Expand Down Expand Up @@ -2080,6 +2081,10 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {
}

if (!DL) {
// FIXME: We could assert that `DL.getKind() != DebugLocKind::Temporary`
// here, or otherwise record any temporary DebugLocs seen to ensure that
// transient compiler-generated instructions aren't leaking their DLs to
// other instructions.
// We have an unspecified location, which might want to be line 0.
// If we have already emitted a line-0 record, don't repeat it.
if (LastAsmLine == 0)
Expand Down
7 changes: 7 additions & 0 deletions llvm/lib/CodeGen/BranchFolding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "llvm/CodeGen/TargetPassConfig.h"
#include "llvm/CodeGen/TargetRegisterInfo.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/Config/config.h"
#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/DebugLoc.h"
#include "llvm/IR/Function.h"
Expand Down Expand Up @@ -910,7 +911,13 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,

// Sort by hash value so that blocks with identical end sequences sort
// together.
#if ENABLE_DEBUGLOC_ORIGIN_TRACKING
// If origin-tracking is enabled then MergePotentialElt is no longer a POD
// type, so we need std::sort instead.
std::sort(MergePotentials.begin(), MergePotentials.end());
#else
array_pod_sort(MergePotentials.begin(), MergePotentials.end());
#endif

// Walk through equivalence sets looking for actual exact matches.
while (MergePotentials.size() > 1) {
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/IR/DebugInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ void Instruction::dropLocation() {
}

if (!MayLowerToCall) {
setDebugLoc(DebugLoc());
setDebugLoc(DebugLoc::getLineZero());
return;
}

Expand All @@ -998,7 +998,7 @@ void Instruction::dropLocation() {
//
// One alternative is to set a line 0 location with the existing scope and
// inlinedAt info. The location might be sensitive to when inlining occurs.
setDebugLoc(DebugLoc());
setDebugLoc(DebugLoc::getLineZero());
}

//===----------------------------------------------------------------------===//
Expand Down
36 changes: 36 additions & 0 deletions llvm/lib/IR/DebugLoc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,44 @@
#include "llvm/IR/DebugLoc.h"
#include "llvm/Config/llvm-config.h"
#include "llvm/IR/DebugInfo.h"

#if ENABLE_DEBUGLOC_ORIGIN_TRACKING
#include "llvm/Support/Signals.h"

namespace llvm {
DbgLocOrigin::DbgLocOrigin(bool ShouldCollectTrace) {
if (ShouldCollectTrace) {
auto &[Depth, StackTrace] = StackTraces.emplace_back();
Depth = sys::getStackTrace(StackTrace);
}
}
void DbgLocOrigin::addTrace() {
if (StackTraces.empty())
return;
auto &[Depth, StackTrace] = StackTraces.emplace_back();
Depth = sys::getStackTrace(StackTrace);
}
} // namespace llvm
#endif

using namespace llvm;

#if ENABLE_DEBUGLOC_COVERAGE_TRACKING
DILocAndCoverageTracking::DILocAndCoverageTracking(const DILocation *L)
: TrackingMDNodeRef(const_cast<DILocation *>(L)), DbgLocOrigin(!L),
Kind(DebugLocKind::Normal) {}

DebugLoc DebugLoc::getTemporary() { return DebugLoc(DebugLocKind::Temporary); }
DebugLoc DebugLoc::getUnknown() { return DebugLoc(DebugLocKind::Unknown); }
DebugLoc DebugLoc::getLineZero() { return DebugLoc(DebugLocKind::LineZero); }

#else

DebugLoc DebugLoc::getTemporary() { return DebugLoc(); }
DebugLoc DebugLoc::getUnknown() { return DebugLoc(); }
DebugLoc DebugLoc::getLineZero() { return DebugLoc(); }
#endif // ENABLE_DEBUGLOC_COVERAGE_TRACKING

//===----------------------------------------------------------------------===//
// DebugLoc Implementation
//===----------------------------------------------------------------------===//
Expand Down
Loading
Loading