[llvm] [cmake] Fix the dead VSINSTALLDIR check in FindDIASDK (PR #217071)
Larry Meadows via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 19 03:01:36 PDT 2026
https://github.com/lfmeadow updated https://github.com/llvm/llvm-project/pull/217071
>From dda455e5bb87e4154922f8062dfc78c0794f8634 Mon Sep 17 00:00:00 2001
From: Larry Meadows <Lawrence.Meadows at amd.com>
Date: Tue, 18 Aug 2026 11:21:57 -0500
Subject: [PATCH 1/2] [cmake] Fix the dead VSINSTALLDIR check in FindDIASDK
`elseif($ENV{VSINSTALLDIR})` expands the environment variable's value into the
condition, where CMake reads it as the name of an undefined variable, so the
branch never runs and the DIA SDK is never located from a developer command
prompt. Test whether the variable is defined instead.
This was fixed once already, in #208524, and reverted in #209137 because it
broke the Windows LLDB bots. The reason is one step further on: config-ix.cmake
defaults LLVM_ENABLE_DIA_SDK to DIASDK_FOUND, so repairing detection turned DIA
on for every build whose environment happened to contain an SDK, and LLDB then
ran its DIA tests. Those need msdia140.dll, which LLVM loads by name through
NoRegCoCreate, and a developer environment does not put it on PATH.
Finding the SDK and choosing to use it are separate questions, so answer them
separately. The module now reports whether it inferred the location from the
environment or was given one through LLVM_WINSYSROOT or MSVC_DIA_SDK_DIR, and
only a location that was given enables DIA by default. A build that merely has
an SDK in its environment is unaffected, which is the case the bots are in.
What this buys is that DIA can be asked for. Today, from a developer command
prompt, -DLLVM_ENABLE_DIA_SDK=ON does not merely fail to help, it trips the
option's own consistency check:
FATAL_ERROR "DIA SDK not found. If you have both VS 2012 and 2013 installed..."
so the SDK has to be named by hand even when it sits where CMake was about to
look.
CMake.md claims the option defaults to ON. It has defaulted to whether the SDK
was found for as long as the option has existed; describe what it does now.
I have no Windows machine, so this is verified by running the module against a
fake SDK tree with the MSVC-only pieces stubbed: with LLVM_WINSYSROOT or
MSVC_DIA_SDK_DIR the SDK is found and the option defaults ON as before; with
only VSINSTALLDIR it is now found (it was not before) and the option defaults
OFF; with neither, the module reports it could not infer a location. Confirmation
on a real Windows build, and on the two bots that llvm#209137 mentions, would be
welcome.
Co-authored-by: Cursor <cursoragent at cursor.com>
---
llvm/cmake/config-ix.cmake | 15 ++++++++++++++-
llvm/cmake/modules/FindDIASDK.cmake | 17 ++++++++++++-----
llvm/docs/CMake.md | 5 ++++-
3 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/llvm/cmake/config-ix.cmake b/llvm/cmake/config-ix.cmake
index 5a7a2abec9dcf..606db13a04f80 100644
--- a/llvm/cmake/config-ix.cmake
+++ b/llvm/cmake/config-ix.cmake
@@ -631,8 +631,21 @@ if( MSVC )
"If set, argument to clang-cl's /winsysroot")
find_package(DIASDK)
+
+ # An SDK that happens to sit in a Visual Studio installation is not by itself
+ # a request to use it: LLVM loads msdia140.dll by name at run time, and a
+ # developer environment does not put it on PATH, so enabling DIA on the
+ # strength of the environment alone yields a PDB reader that cannot start.
+ # Enable it by default only where the SDK was pointed at deliberately; it can
+ # always be asked for with -DLLVM_ENABLE_DIA_SDK=ON.
+ if(DIASDK_FOUND AND NOT DIASDK_LOCATION_INFERRED)
+ set(LLVM_ENABLE_DIA_SDK_DEFAULT ON)
+ else()
+ set(LLVM_ENABLE_DIA_SDK_DEFAULT OFF)
+ endif()
+
option(LLVM_ENABLE_DIA_SDK "Use MSVC DIA SDK for debugging if available."
- ${DIASDK_FOUND})
+ ${LLVM_ENABLE_DIA_SDK_DEFAULT})
if(LLVM_ENABLE_DIA_SDK AND NOT DIASDK_FOUND)
message(FATAL_ERROR "DIA SDK not found. If you have both VS 2012 and 2013 installed, you may need to uninstall the former and re-install the latter afterwards.")
diff --git a/llvm/cmake/modules/FindDIASDK.cmake b/llvm/cmake/modules/FindDIASDK.cmake
index f3133d29de3b2..6f5efe32a2967 100644
--- a/llvm/cmake/modules/FindDIASDK.cmake
+++ b/llvm/cmake/modules/FindDIASDK.cmake
@@ -9,6 +9,8 @@
# DIASDK_FOUND
# DIASDK_INCLUDE_DIR
# DIASDK_LIBRARIES
+# DIASDK_LOCATION_INFERRED, true if the SDK was located from the environment
+# rather than from LLVM_WINSYSROOT or MSVC_DIA_SDK_DIR
#
# Additionally, the following import target will be defined:
# DIASDK::Diaguids
@@ -18,15 +20,20 @@ if(NOT WIN32)
return()
endif()
+set(DIASDK_LOCATION_INFERRED FALSE)
+
if(LLVM_WINSYSROOT)
set(MSVC_DIA_SDK_DIR "${LLVM_WINSYSROOT}/DIA SDK" CACHE PATH
"Path to the DIA SDK")
-elseif($ENV{VSINSTALLDIR})
- set(MSVC_DIA_SDK_DIR "$ENV{VSINSTALLDIR}DIA SDK" CACHE PATH
- "Path to the DIA SDK")
elseif(NOT DEFINED MSVC_DIA_SDK_DIR)
- message(STATUS "MSVC_DIA_SDK_DIR not set, and could not be inferred. DIA SDK "
- "may not be found.")
+ if(DEFINED ENV{VSINSTALLDIR})
+ set(MSVC_DIA_SDK_DIR "$ENV{VSINSTALLDIR}DIA SDK" CACHE PATH
+ "Path to the DIA SDK")
+ set(DIASDK_LOCATION_INFERRED TRUE)
+ else()
+ message(STATUS "MSVC_DIA_SDK_DIR not set, and could not be inferred. DIA "
+ "SDK may not be found.")
+ endif()
endif()
find_path(DIASDK_INCLUDE_DIR
diff --git a/llvm/docs/CMake.md b/llvm/docs/CMake.md
index c64b419cd06ba..2aa480967bde8 100644
--- a/llvm/docs/CMake.md
+++ b/llvm/docs/CMake.md
@@ -520,7 +520,10 @@ sub-projects. Nearly all of these variable names begin with `LLVM_`.
**LLVM_ENABLE_DIA_SDK**:BOOL
: Enable building with MSVC DIA SDK for PDB debugging support. Available only
- with MSVC. Defaults to ON.
+ with MSVC. Defaults to ON when the SDK is found at a location given by
+ `LLVM_WINSYSROOT` or `MSVC_DIA_SDK_DIR`, and to OFF when it is only found
+ through the `VSINSTALLDIR` environment variable, since using it also
+ requires `msdia140.dll` to be loadable at run time.
**LLVM_ENABLE_DOXYGEN**:BOOL
>From 04381954386693f599b11293535651ed516e774a Mon Sep 17 00:00:00 2001
From: Larry Meadows <Lawrence.Meadows at amd.com>
Date: Wed, 19 Aug 2026 05:00:44 -0500
Subject: [PATCH 2/2] Keep DIASDK_LOCATION_INFERRED across reconfigures
Once MSVC_DIA_SDK_DIR is in the cache the branches that set the flag no longer
run, so a plain variable read FALSE on every reconfigure and reported a location
inferred from the environment as one that was chosen deliberately.
On a plain reconfigure that was masked, because option() leaves an existing cache
entry alone and LLVM_ENABLE_DIA_SDK keeps the OFF from the first configure. It is
reachable where the option is absent from the cache but the path is not, such as
a configure that failed after find_package(DIASDK), and there it enabled DIA for
an environment-inferred SDK, which is what #209137 reverted.
Cache the flag as INTERNAL next to the path it describes.
Co-authored-by: Cursor <cursoragent at cursor.com>
---
llvm/cmake/modules/FindDIASDK.cmake | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/llvm/cmake/modules/FindDIASDK.cmake b/llvm/cmake/modules/FindDIASDK.cmake
index 6f5efe32a2967..e385444a34b70 100644
--- a/llvm/cmake/modules/FindDIASDK.cmake
+++ b/llvm/cmake/modules/FindDIASDK.cmake
@@ -20,16 +20,25 @@ if(NOT WIN32)
return()
endif()
-set(DIASDK_LOCATION_INFERRED FALSE)
+# Cached alongside MSVC_DIA_SDK_DIR: once the path is in the cache the branches
+# below no longer run, so a plain variable would read FALSE on every reconfigure
+# and describe a location nobody chose as one that was chosen deliberately.
+if(NOT DEFINED DIASDK_LOCATION_INFERRED)
+ set(DIASDK_LOCATION_INFERRED FALSE CACHE INTERNAL
+ "Whether MSVC_DIA_SDK_DIR was inferred from the environment")
+endif()
if(LLVM_WINSYSROOT)
set(MSVC_DIA_SDK_DIR "${LLVM_WINSYSROOT}/DIA SDK" CACHE PATH
"Path to the DIA SDK")
+ set(DIASDK_LOCATION_INFERRED FALSE CACHE INTERNAL
+ "Whether MSVC_DIA_SDK_DIR was inferred from the environment")
elseif(NOT DEFINED MSVC_DIA_SDK_DIR)
if(DEFINED ENV{VSINSTALLDIR})
set(MSVC_DIA_SDK_DIR "$ENV{VSINSTALLDIR}DIA SDK" CACHE PATH
"Path to the DIA SDK")
- set(DIASDK_LOCATION_INFERRED TRUE)
+ set(DIASDK_LOCATION_INFERRED TRUE CACHE INTERNAL
+ "Whether MSVC_DIA_SDK_DIR was inferred from the environment")
else()
message(STATUS "MSVC_DIA_SDK_DIR not set, and could not be inferred. DIA "
"SDK may not be found.")
More information about the llvm-commits
mailing list