[DLCov 1/5] Add CMake option to enable enhanced line number coverage tracking #107278

SLTozer · 2024-09-04T18:04:12Z

This is part of a series of patches that tries to improve DILocation bug detection in Debugify; see below for more details. This first patch adds the necessary CMake flag to LLVM and a variable defined by that flag to LLVM's config header, allowing the next patch to track information without affecting normal builds.

This series of patches adds a "DebugLoc coverage tracking" feature, that inserts conditionally-compiled tracking information into DebugLocs (and by extension, to Instructions), which is used by Debugify to provide more accurate and detailed coverage reports. When enabled, this features tracks whether and why we have intentionally dropped a DebugLoc, allowing Debugify to ignore false positives. An optional additional feature allows also storing a stack trace of the point where a DebugLoc was unintentionally dropped/not generated, which is used to make fixing detected errors significantly easier. The goal of these features is to provide useful tools for developers to fix existing DebugLoc errors and allow reliable detection of regressions by either manual inspection or an automated script.

Next: #107279

llvmbot · 2024-09-04T18:04:41Z

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-debuginfo

Author: Stephen Tozer (SLTozer)

Changes

This is part of a series of patches that tries to improve DILocation bug detection in Debugify; see below for more details. This first patch adds the necessary CMake flag to LLVM and a variable defined by that flag to LLVM's config header, allowing the next patch to track information without affecting normal builds.

This series of patches adds a "DebugLoc coverage tracking" feature, that inserts conditionally-compiled tracking information into DebugLocs (and by extension, to Instructions), which is used by Debugify to provide more accurate and detailed coverage reports. When enabled, this features tracks whether and why we have intentionally dropped a DebugLoc, allowing Debugify to ignore false positives. An optional additional feature allows also storing a stack trace of the point where a DebugLoc was unintentionally dropped/not generated, which is used to make fixing detected errors significantly easier. The goal of these features is to provide useful tools for developers to fix existing DebugLoc errors and allow reliable detection of regressions by either manual inspection or an automated script.

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

4 Files Affected:

(modified) llvm/CMakeLists.txt (+4)
(modified) llvm/cmake/modules/HandleLLVMOptions.cmake (+12)
(modified) llvm/docs/CMake.rst (+11)
(modified) llvm/include/llvm/Config/config.h.cmake (+4)

diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 12618966c4adfd..3e2e90f5adad2e 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -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
diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 5ca580fbb59c59..a4b11c149da9de 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -196,6 +196,18 @@ 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" )
+  message(FATAL_ERROR "\"COVERAGE_AND_ORIGIN\" setting for LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING currently unimplemented.")
+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()
diff --git a/llvm/docs/CMake.rst b/llvm/docs/CMake.rst
index 2a80813999ea1e..304e22759770d9 100644
--- a/llvm/docs/CMake.rst
+++ b/llvm/docs/CMake.rst
@@ -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.
diff --git a/llvm/include/llvm/Config/config.h.cmake b/llvm/include/llvm/Config/config.h.cmake
index ff30741c8f360a..388ce1e8f74e3e 100644
--- a/llvm/include/llvm/Config/config.h.cmake
+++ b/llvm/include/llvm/Config/config.h.cmake
@@ -19,6 +19,10 @@
 /* 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 prefer forward slashes on Windows, and to 0 prefer
    backslashes. */
 #cmakedefine01 LLVM_WINDOWS_PREFER_FORWARD_SLASH

djtodoro · 2024-09-11T11:41:54Z

@SLTozer Thanks a lot for this! Very interesting.

I will review the implementation, but I am curious whether we only modified the metadata/DebugLoc. To be more precise, my main concern is memory usage increase, and whether we have affected llvm::Instruction in any way other than metadata.

SLTozer · 2024-09-11T12:12:07Z

I will review the implementation, but I am curious whether we only modified the metadata/DebugLoc. To be more precise, my main concern is memory usage increase, and whether we have affected llvm::Instruction in any way other than metadata.

There will be increased memory usage - more exactly, we increase the size of DebugLoc, not !DILocation, which means the extra storage is part of the Instruction class. The intent here is that we conditionally compile this extra information in for a specific build configuration designed to test the compiler itself with debugify; in ordinary builds, including debug builds, the extra information is not stored, so run time and memory usage should be unaffected.

jryans

This part looks good so far, just a few editorial notes. 😄

llvm/CMakeLists.txt

llvm/docs/CMake.rst

jryans · 2024-09-27T11:37:33Z

llvm/test/Transforms/InstCombine/reduced.ll

@@ -0,0 +1,69 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+;; Tests that we do not always overwrite 
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s


Hmm, did you mean to add this test here...? It seems unrelated to this PR.

I did not, my fault for git adding a folder without checking my tree is clean...

SLTozer · 2024-10-01T17:08:15Z

@jryans Does this part look good to you now?

jryans

This part looks good to me. Thanks for working on this! 😄

llvm-ci · 2024-10-09T01:52:30Z

LLVM Buildbot has detected a new failure on builder lld-x86_64-win running on as-worker-93 while building llvm at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/1339

Here is the relevant piece of the build log for the reference

Step 7 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM-Unit :: Support/./SupportTests.exe/35/87' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe-LLVM-Unit-15828-35-87.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=87 GTEST_SHARD_INDEX=35 C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe
--

Script:
--
C:\a\lld-x86_64-win\build\unittests\Support\.\SupportTests.exe --gtest_filter=ProgramEnvTest.CreateProcessLongPath
--
C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(160): error: Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp(163): error: fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied



C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:160
Expected equality of these values:
  0
  RC
    Which is: -2

C:\a\lld-x86_64-win\llvm-project\llvm\unittests\Support\ProgramTest.cpp:163
fs::remove(Twine(LongPath)): did not return errc::success.
error number: 13
error message: permission denied




********************

It has been introduced in #107278 but it was passing "DISABLED" of LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING to cmakedefine01. cmakadefine01 treats non-false-like strings as 1. "DISABLED" is replaced with 1.

chapuni

Looks like ENABLE_DEBUGLOC_COVERAGE_TRACKING in llvm-config.h is always 1. Fixed in 1bc0b08.

chapuni · 2025-06-14T13:05:48Z

llvm/cmake/modules/HandleLLVMOptions.cmake

+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 )


IMO it is not a good practice to override a cached variable with a non-cached variable that has different semantics.

chapuni · 2025-06-14T13:07:18Z

llvm/cmake/modules/HandleLLVMOptions.cmake

+if( uppercase_LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING STREQUAL "COVERAGE" )
+  set( ENABLE_DEBUGLOC_COVERAGE_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.


It leaves LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING as "DISABLED".

chapuni · 2025-06-14T13:09:13Z

llvm/include/llvm/Config/config.h.cmake

@@ -19,6 +19,10 @@
 /* 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


#cmakedefine01 handles "DISABLED" as 1 since it is not false-like string.

It has been introduced in llvm#107278 but it was passing "DISABLED" of LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING to cmakedefine01. cmakadefine01 treats non-false-like strings as 1. "DISABLED" is replaced with 1.

jmorse · 2025-06-16T12:16:03Z

/me squints -- it looks like these issues were latent and then we exposed them with #143590. Thanks @chapuni for clearing this up!

Change in #107278 modified the CMake CACHE variable with values that are not supported for it as documented. This patch renames the derived vars so that they do not conflict with the CACHE variable.

…144391) Change in llvm#107278 modified the CMake CACHE variable with values that are not supported for it as documented. This patch renames the derived vars so that they do not conflict with the CACHE variable.

It has been introduced in llvm#107278 but it was passing "DISABLED" of LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING to cmakedefine01. cmakadefine01 treats non-false-like strings as 1. "DISABLED" is replaced with 1.

…144391) Change in llvm#107278 modified the CMake CACHE variable with values that are not supported for it as documented. This patch renames the derived vars so that they do not conflict with the CACHE variable.

SLTozer added the debuginfo label Sep 4, 2024

SLTozer requested review from chandlerc, jmorse, adrian-prantl, dwblaikie and pogo59 September 4, 2024 18:04

SLTozer self-assigned this Sep 4, 2024

llvmbot added the cmake Build system in general and CMake in particular label Sep 4, 2024

SLTozer mentioned this pull request Sep 4, 2024

[DLCov 2/5] Implement DebugLoc coverage tracking #107279

Merged

jryans reviewed Sep 23, 2024

View reviewed changes

llvm/CMakeLists.txt Outdated Show resolved Hide resolved

llvm/docs/CMake.rst Show resolved Hide resolved

llvm/docs/CMake.rst Outdated Show resolved Hide resolved

llvm/docs/CMake.rst Outdated Show resolved Hide resolved

llvmbot added the llvm:transforms label Sep 26, 2024

jryans reviewed Sep 27, 2024

View reviewed changes

jryans approved these changes Oct 1, 2024

View reviewed changes

SLTozer added 4 commits October 4, 2024 18:32

Add CMake option to enable expensive line number origin tracking

1efbd38

Update comments

0348ab9

Expand comment, remove unwanted test

84054b5

Remove references to COVERAGE_AND_ORIGIN option

7380c0d

SLTozer force-pushed the dl-coverage-1-cmake branch from 2928f48 to 7380c0d Compare October 4, 2024 19:48

SLTozer merged commit 84088d3 into llvm:main Oct 8, 2024
10 checks passed

chapuni reviewed Jun 14, 2025

View reviewed changes

chapuni mentioned this pull request Jun 14, 2025

[bazel] Update llvm-config.h and disable DebugLoc tracking #144125

Merged

jmorse mentioned this pull request Jun 16, 2025

[DLCov] Origin-Tracking: Add config options #143590

Merged

chapuni mentioned this pull request Jun 16, 2025

CMake: Fix LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING in reentrancy situa… #144331

Closed

vzakhari mentioned this pull request Jun 16, 2025

[build] Fixed LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING handling. #144391

Merged

[DLCov 1/5] Add CMake option to enable enhanced line number coverage tracking #107278

[DLCov 1/5] Add CMake option to enable enhanced line number coverage tracking #107278

Uh oh!

Conversation

SLTozer commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

djtodoro commented Sep 11, 2024

Uh oh!

SLTozer commented Sep 11, 2024

Uh oh!

jryans left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jryans Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

SLTozer Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

SLTozer commented Oct 1, 2024

Uh oh!

jryans left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvm-ci commented Oct 9, 2024

Uh oh!

chapuni left a comment

Choose a reason for hiding this comment

Uh oh!

chapuni Jun 14, 2025

Choose a reason for hiding this comment

Uh oh!

chapuni Jun 14, 2025

Choose a reason for hiding this comment

Uh oh!

chapuni Jun 14, 2025

Choose a reason for hiding this comment

Uh oh!

jmorse commented Jun 16, 2025

Uh oh!

Uh oh!

SLTozer commented Sep 4, 2024 •

edited

Loading

llvmbot commented Sep 4, 2024 •

edited

Loading