From 1680a5821979d23d7446b170d226d66dc56ac755 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Tue, 3 Jun 2025 22:01:37 +0100 Subject: [PATCH 01/20] [DTLTO][LLD][ELF] Add support for Integrated Distributed ThinLTO This patch introduces support for Integrated Distributed ThinLTO (DTLTO) in ELF LLD. DTLTO enables the distribution of ThinLTO backend compilations via external distribution systems, such as Incredibuild, during the traditional link step: https://llvm.org/docs/DTLTO.html. It is expected that users will invoke DTLTO through the compiler driver (e.g., Clang) rather than calling LLD directly. A Clang-side interface for DTLTO will be added in a follow-up patch. Note: Bitcode members of non-thin archives are not currently supported. This will be addressed in a future change. Testing: - ELF LLD `lit` test coverage has been added, using a mock distributor to avoid requiring Clang. - Cross-project `lit` tests cover integration with Clang. For the design discussion of the DTLTO feature, see: https://github.com/llvm/llvm-project/issues/126654 --- cross-project-tests/CMakeLists.txt | 12 ++- cross-project-tests/dtlto/README.md | 3 + cross-project-tests/dtlto/archive-thin.test | 65 ++++++++++++++++ cross-project-tests/dtlto/dtlto.c | 35 +++++++++ cross-project-tests/dtlto/lit.local.cfg | 2 + cross-project-tests/lit.cfg.py | 4 +- lld/ELF/Config.h | 4 + lld/ELF/Driver.cpp | 5 ++ lld/ELF/InputFiles.cpp | 39 +++++++++- lld/ELF/LTO.cpp | 7 ++ lld/ELF/Options.td | 12 ++- lld/docs/DTLTO.rst | 42 +++++++++++ lld/docs/index.rst | 1 + lld/test/ELF/dtlto/archive-thin.test | 82 +++++++++++++++++++++ lld/test/ELF/dtlto/empty.test | 44 +++++++++++ lld/test/ELF/dtlto/imports.test | 53 +++++++++++++ lld/test/ELF/dtlto/index.test | 44 +++++++++++ lld/test/ELF/dtlto/options.test | 41 +++++++++++ lld/test/ELF/dtlto/partitions.test | 61 +++++++++++++++ lld/test/ELF/dtlto/save-temps.test | 39 ++++++++++ lld/test/lit.cfg.py | 1 + 21 files changed, 588 insertions(+), 8 deletions(-) create mode 100644 cross-project-tests/dtlto/README.md create mode 100644 cross-project-tests/dtlto/archive-thin.test create mode 100644 cross-project-tests/dtlto/dtlto.c create mode 100644 cross-project-tests/dtlto/lit.local.cfg create mode 100644 lld/docs/DTLTO.rst create mode 100644 lld/test/ELF/dtlto/archive-thin.test create mode 100644 lld/test/ELF/dtlto/empty.test create mode 100644 lld/test/ELF/dtlto/imports.test create mode 100644 lld/test/ELF/dtlto/index.test create mode 100644 lld/test/ELF/dtlto/options.test create mode 100644 lld/test/ELF/dtlto/partitions.test create mode 100644 lld/test/ELF/dtlto/save-temps.test diff --git a/cross-project-tests/CMakeLists.txt b/cross-project-tests/CMakeLists.txt index 7f2fee48fda77..192db87043177 100644 --- a/cross-project-tests/CMakeLists.txt +++ b/cross-project-tests/CMakeLists.txt @@ -19,11 +19,12 @@ set(CROSS_PROJECT_TEST_DEPS FileCheck check-gdb-llvm-support count - llvm-dwarfdump + llvm-ar llvm-config + llvm-dwarfdump llvm-objdump - split-file not + split-file ) if ("clang" IN_LIST LLVM_ENABLE_PROJECTS) @@ -94,6 +95,13 @@ add_lit_testsuite(check-cross-amdgpu "Running AMDGPU cross-project tests" DEPENDS clang ) +# DTLTO tests. +add_lit_testsuite(check-cross-dtlto "Running DTLTO cross-project tests" + ${CMAKE_CURRENT_BINARY_DIR}/dtlto + EXCLUDE_FROM_CHECK_ALL + DEPENDS ${CROSS_PROJECT_TEST_DEPS} + ) + # Add check-cross-project-* targets. add_lit_testsuites(CROSS_PROJECT ${CMAKE_CURRENT_SOURCE_DIR} DEPENDS ${CROSS_PROJECT_TEST_DEPS} diff --git a/cross-project-tests/dtlto/README.md b/cross-project-tests/dtlto/README.md new file mode 100644 index 0000000000000..12f9aa19b0d9b --- /dev/null +++ b/cross-project-tests/dtlto/README.md @@ -0,0 +1,3 @@ +Tests for DTLTO (Integrated Distributed ThinLTO) functionality. + +These are integration tests as DTLTO invokes `clang` for code-generation. \ No newline at end of file diff --git a/cross-project-tests/dtlto/archive-thin.test b/cross-project-tests/dtlto/archive-thin.test new file mode 100644 index 0000000000000..2d6b7f1cb7e37 --- /dev/null +++ b/cross-project-tests/dtlto/archive-thin.test @@ -0,0 +1,65 @@ +# REQUIRES: x86-registered-target,ld.lld,llvm-ar + +# Test that a DTLTO link succeeds and outputs the expected set of files +# correctly when thin archives are present. + +RUN: rm -rf %t && split-file %s %t && cd %t +RUN: %clang --target=x86_64-linux-gnu -flto=thin -c \ +RUN: foo.c bar.c dog.c cat.c _start.c + +RUN: llvm-ar rcs foo.a foo.o --thin +# Create this bitcode thin archive in a subdirectory to test the expansion of +# the path to a bitcode file that is referenced using "..", e.g., in this case +# "../bar.o". +RUN: mkdir lib +RUN: llvm-ar rcs lib/bar.a bar.o --thin +# Create this bitcode thin archive with an absolute path entry containing "..". +RUN: llvm-ar rcs dog.a %t/lib/../dog.o --thin +RUN: llvm-ar rcs cat.a cat.o --thin +RUN: llvm-ar rcs _start.a _start.o --thin + +RUN: mkdir %t/out && cd %t/out + +RUN: ld.lld %t/foo.a %t/lib/bar.a ../_start.a %t/cat.a \ +RUN: --whole-archive ../dog.a \ +RUN: --thinlto-distributor=%python \ +RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \ +RUN: --thinlto-remote-compiler=%clang \ +RUN: --save-temps + +# Check that the required output files have been created. +RUN: ls | FileCheck %s --check-prefix=OUTPUTS \ +RUN: --implicit-check-not=cat --implicit-check-not=foo + +# JSON jobs description. +OUTPUTS-DAG: a.[[PID:[a-zA-Z0-9_]+]].dist-file.json + +# Individual summary index files. +OUTPUTS-DAG: dog.1.[[PID]].native.o.thinlto.bc{{$}} +OUTPUTS-DAG: _start.2.[[PID]].native.o.thinlto.bc{{$}} +OUTPUTS-DAG: bar.3.[[PID]].native.o.thinlto.bc{{$}} + +# Native output object files. +OUTPUTS-DAG: dog.1.[[PID]].native.o{{$}} +OUTPUTS-DAG: _start.2.[[PID]].native.o{{$}} +OUTPUTS-DAG: bar.3.[[PID]].native.o{{$}} + +#--- foo.c +__attribute__((retain)) void foo() {} + +#--- bar.c +extern void foo(); +__attribute__((retain)) void bar() { foo(); } + +#--- dog.c +__attribute__((retain)) void dog() {} + +#--- cat.c +__attribute__((retain)) void cat() {} + +#--- _start.c +extern void bar(); +__attribute__((retain)) void _start() { + bar(); +} + diff --git a/cross-project-tests/dtlto/dtlto.c b/cross-project-tests/dtlto/dtlto.c new file mode 100644 index 0000000000000..e967ede35b09b --- /dev/null +++ b/cross-project-tests/dtlto/dtlto.c @@ -0,0 +1,35 @@ +// REQUIRES: x86-registered-target,ld.lld + +/// Simple test that DTLTO works with a single input bitcode file and that +/// --save-temps can be applied to the remote compilation. +// RUN: rm -rf %t && mkdir %t && cd %t + +// RUN: %clang --target=x86_64-linux-gnu -c -flto=thin %s + +// RUN: ld.lld dtlto.o \ +// RUN: --thinlto-distributor=%python \ +// RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \ +// RUN: --thinlto-remote-compiler=%clang \ +// RUN: --thinlto-remote-compiler-arg=--save-temps + +/// Check that the required output files have been created. +// RUN: ls | count 10 +// RUN: ls | FileCheck %s + +/// Produced by the bitcode compilation. +// CHECK-DAG: {{^}}dtlto.o{{$}} + +/// Linked ELF. +// CHECK-DAG: {{^}}a.out{{$}} + +/// --save-temps output for the backend compilation. +// CHECK-DAG: {{^}}dtlto.s{{$}} +// CHECK-DAG: {{^}}dtlto.s.0.preopt.bc{{$}} +// CHECK-DAG: {{^}}dtlto.s.1.promote.bc{{$}} +// CHECK-DAG: {{^}}dtlto.s.2.internalize.bc{{$}} +// CHECK-DAG: {{^}}dtlto.s.3.import.bc{{$}} +// CHECK-DAG: {{^}}dtlto.s.4.opt.bc{{$}} +// CHECK-DAG: {{^}}dtlto.s.5.precodegen.bc{{$}} +// CHECK-DAG: {{^}}dtlto.s.resolution.txt{{$}} + +int _start() { return 0; } diff --git a/cross-project-tests/dtlto/lit.local.cfg b/cross-project-tests/dtlto/lit.local.cfg new file mode 100644 index 0000000000000..530f4c01646ff --- /dev/null +++ b/cross-project-tests/dtlto/lit.local.cfg @@ -0,0 +1,2 @@ +if "clang" not in config.available_features: + config.unsupported = True diff --git a/cross-project-tests/lit.cfg.py b/cross-project-tests/lit.cfg.py index b35c643ac898c..ac27753472646 100644 --- a/cross-project-tests/lit.cfg.py +++ b/cross-project-tests/lit.cfg.py @@ -19,7 +19,7 @@ config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell) # suffixes: A list of file extensions to treat as test files. -config.suffixes = [".c", ".cl", ".cpp", ".m"] +config.suffixes = [".c", ".cl", ".cpp", ".m", ".test"] # excludes: A list of directories to exclude from the testsuite. The 'Inputs' # subdirectories contain auxiliary inputs for various tests in their parent @@ -107,6 +107,8 @@ def get_required_attr(config, attr_name): if lldb_path is not None: config.available_features.add("lldb") +if llvm_config.use_llvm_tool("llvm-ar"): + config.available_features.add("llvm-ar") def configure_dexter_substitutions(): """Configure substitutions for host platform and return list of dependencies""" diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h index f0e9592d85dd6..5e0fb9c9b00ab 100644 --- a/lld/ELF/Config.h +++ b/lld/ELF/Config.h @@ -249,6 +249,10 @@ struct Config { llvm::SmallVector searchPaths; llvm::SmallVector symbolOrderingFile; llvm::SmallVector thinLTOModulesToCompile; + llvm::StringRef dtltoDistributor; + llvm::SmallVector dtltoDistributorArgs; + llvm::StringRef dtltoCompiler; + llvm::SmallVector dtltoCompilerArgs; llvm::SmallVector undefined; llvm::SmallVector dynamicList; llvm::SmallVector buildIdVector; diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index 6150fe072156f..e89eaef27d6bd 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -1341,6 +1341,11 @@ static void readConfigs(Ctx &ctx, opt::InputArgList &args) { args.hasFlag(OPT_dependent_libraries, OPT_no_dependent_libraries, true); ctx.arg.disableVerify = args.hasArg(OPT_disable_verify); ctx.arg.discard = getDiscard(args); + ctx.arg.dtltoDistributor = args.getLastArgValue(OPT_thinlto_distributor_eq); + ctx.arg.dtltoDistributorArgs = + args::getStrings(args, OPT_thinlto_distributor_arg); + ctx.arg.dtltoCompiler = args.getLastArgValue(OPT_thinlto_compiler_eq); + ctx.arg.dtltoCompilerArgs = args::getStrings(args, OPT_thinlto_compiler_arg); ctx.arg.dwoDir = args.getLastArgValue(OPT_plugin_opt_dwo_dir_eq); ctx.arg.dynamicLinker = getDynamicLinker(ctx, args); ctx.arg.ehFrameHdr = diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp index 12a77736aba7f..5523d929f6bfd 100644 --- a/lld/ELF/InputFiles.cpp +++ b/lld/ELF/InputFiles.cpp @@ -20,6 +20,7 @@ #include "llvm/ADT/CachedHashString.h" #include "llvm/ADT/STLExtras.h" #include "llvm/LTO/LTO.h" +#include "llvm/Object/Archive.h" #include "llvm/Object/IRObjectFile.h" #include "llvm/Support/ARMAttributeParser.h" #include "llvm/Support/ARMBuildAttributes.h" @@ -1739,6 +1740,33 @@ static uint8_t getOsAbi(const Triple &t) { } } +// For DTLTO, bitcode member names must be valid paths to files on disk. +// For thin archives, resolve `memberPath` relative to the archive's location. +// Returns true if adjusted; false otherwise. Non-thin archives are unsupported. +static bool dtltoAdjustMemberPathIfThinArchive(Ctx &ctx, StringRef archivePath, + std::string &memberPath) { + assert(!archivePath.empty() && !ctx.arg.dtltoDistributor.empty()); + + // Check if the archive file is a thin archive by reading its header. + auto bufferOrErr = + MemoryBuffer::getFileSlice(archivePath, sizeof(ThinArchiveMagic) - 1, 0); + if (std::error_code ec = bufferOrErr.getError()) { + ErrAlways(ctx) << "cannot open " << archivePath << ": " << ec.message(); + return false; + } + + if (!bufferOrErr->get()->getBuffer().starts_with(ThinArchiveMagic)) + return false; + + SmallString<64> resolvedPath; + if (path::is_relative(memberPath)) { + resolvedPath = path::parent_path(archivePath); + path::append(resolvedPath, memberPath); + memberPath = resolvedPath.str(); + } + return true; +} + BitcodeFile::BitcodeFile(Ctx &ctx, MemoryBufferRef mb, StringRef archiveName, uint64_t offsetInArchive, bool lazy) : InputFile(ctx, BitcodeKind, mb) { @@ -1756,10 +1784,13 @@ BitcodeFile::BitcodeFile(Ctx &ctx, MemoryBufferRef mb, StringRef archiveName, // symbols later in the link stage). So we append file offset to make // filename unique. StringSaver &ss = ctx.saver; - StringRef name = archiveName.empty() - ? ss.save(path) - : ss.save(archiveName + "(" + path::filename(path) + - " at " + utostr(offsetInArchive) + ")"); + StringRef name = + (archiveName.empty() || + (!ctx.arg.dtltoDistributor.empty() && + dtltoAdjustMemberPathIfThinArchive(ctx, archiveName, path))) + ? ss.save(path) + : ss.save(archiveName + "(" + path::filename(path) + " at " + + utostr(offsetInArchive) + ")"); MemoryBufferRef mbref(mb.getBuffer(), name); obj = CHECK2(lto::InputFile::create(mbref), this); diff --git a/lld/ELF/LTO.cpp b/lld/ELF/LTO.cpp index 82a7463446a94..8d4a6c9e3a81e 100644 --- a/lld/ELF/LTO.cpp +++ b/lld/ELF/LTO.cpp @@ -180,6 +180,13 @@ BitcodeCompiler::BitcodeCompiler(Ctx &ctx) : ctx(ctx) { std::string(ctx.arg.thinLTOPrefixReplaceNew), std::string(ctx.arg.thinLTOPrefixReplaceNativeObject), ctx.arg.thinLTOEmitImportsFiles, indexFile.get(), onIndexWrite); + } else if (!ctx.arg.dtltoDistributor.empty()) { + backend = lto::createOutOfProcessThinBackend( + llvm::hardware_concurrency(ctx.arg.thinLTOJobs), onIndexWrite, + ctx.arg.thinLTOEmitIndexFiles, ctx.arg.thinLTOEmitImportsFiles, + ctx.arg.outputFile, ctx.arg.dtltoDistributor, + ctx.arg.dtltoDistributorArgs, ctx.arg.dtltoCompiler, + ctx.arg.dtltoCompilerArgs, !ctx.arg.saveTempsArgs.empty()); } else { backend = lto::createInProcessThinBackend( llvm::heavyweight_hardware_concurrency(ctx.arg.thinLTOJobs), diff --git a/lld/ELF/Options.td b/lld/ELF/Options.td index c795147eb9662..6599691244fe8 100644 --- a/lld/ELF/Options.td +++ b/lld/ELF/Options.td @@ -710,7 +710,17 @@ def thinlto_object_suffix_replace_eq: JJ<"thinlto-object-suffix-replace=">; def thinlto_prefix_replace_eq: JJ<"thinlto-prefix-replace=">; def thinlto_single_module_eq: JJ<"thinlto-single-module=">, HelpText<"Specify a single module to compile in ThinLTO mode, for debugging only">; - +def thinlto_distributor_eq: JJ<"thinlto-distributor=">, + HelpText<"Distributor to use for ThinLTO backend compilations. If specified, " + "ThinLTO backend compilations will be distributed.">; +defm thinlto_distributor_arg: EEq<"thinlto-distributor-arg", "Arguments to " + "pass to the ThinLTO distributor">; +def thinlto_compiler_eq: JJ<"thinlto-remote-compiler=">, + HelpText<"Compiler for the ThinLTO distributor to invoke for ThinLTO backend " + "compilations">; +defm thinlto_compiler_arg: EEq<"thinlto-remote-compiler-arg", "Compiler " + "arguments for the ThinLTO distributor to pass for ThinLTO backend " + "compilations">; defm fat_lto_objects: BB<"fat-lto-objects", "Use the .llvm.lto section, which contains LLVM bitcode, in fat LTO object files to perform LTO.", "Ignore the .llvm.lto section in relocatable object files (default).">; diff --git a/lld/docs/DTLTO.rst b/lld/docs/DTLTO.rst new file mode 100644 index 0000000000000..985decf6c7db8 --- /dev/null +++ b/lld/docs/DTLTO.rst @@ -0,0 +1,42 @@ +Integrated Distributed ThinLTO (DTLTO) +====================================== + +Integrated Distributed ThinLTO (DTLTO) enables the distribution of backend +ThinLTO compilations via external distribution systems, such as Incredibuild, +during the traditional link step. + +The implementation is documented here: https://llvm.org/docs/DTLTO.html. + +Currently, DTLTO is only supported in ELF LLD. Support will be added to other +LLD flavours in the future. + +ELF LLD +------- + +The command-line interface is as follows: + +- ``--thinlto-distributor=`` + Specifies the file to execute as the distributor process. If specified, + ThinLTO backend compilations will be distributed. + +- ``--thinlto-remote-compiler=`` + Specifies the path to the compiler that the distributor process will use for + backend compilations. The compiler invoked must match the version of LLD. + +- ``--thinlto-distributor-arg=`` + Specifies ```` on the command line when invoking the distributor. + Can be specified multiple times. + +- ``--thinlto-remote-compiler-arg=`` + Appends ```` to the remote compiler's command line. + Can be specified multiple times. + + Options that introduce extra input/output files may cause miscompilation if + the distribution system does not automatically handle pushing/fetching them to + remote nodes. In such cases, configure the distributor - possibly using + ``--thinlto-distributor-arg=`` - to manage these dependencies. See the + distributor documentation for details. + +Some LLD LTO options (e.g., ``--lto-sample-profile=``) are supported. +Currently, other options are silently accepted but do not have the intended +effect. Support for such options will be expanded in the future. diff --git a/lld/docs/index.rst b/lld/docs/index.rst index 8260461c36905..69792e3b575be 100644 --- a/lld/docs/index.rst +++ b/lld/docs/index.rst @@ -147,3 +147,4 @@ document soon. ELF/start-stop-gc ELF/warn_backrefs MachO/index + DTLTO diff --git a/lld/test/ELF/dtlto/archive-thin.test b/lld/test/ELF/dtlto/archive-thin.test new file mode 100644 index 0000000000000..45f841be3fcf6 --- /dev/null +++ b/lld/test/ELF/dtlto/archive-thin.test @@ -0,0 +1,82 @@ +# REQUIRES: x86 + +# Test that a DTLTO link succeeds and outputs the expected set of files +# correctly when thin archives are present. + +RUN: rm -rf %t && split-file %s %t && cd %t + +# Generate ThinLTO bitcode files. +RUN: opt -thinlto-bc t1.ll -o t1.bc -O2 +RUN: opt -thinlto-bc t2.ll -o t2.bc -O2 +RUN: opt -thinlto-bc t3.ll -o t3.bc -O2 + +# Generate object files for mock.py to return. +RUN: llc t1.ll --filetype=obj -o t1.o --relocation-model=pic +RUN: llc t2.ll --filetype=obj -o t2.o --relocation-model=pic +RUN: llc t3.ll --filetype=obj -o t3.o --relocation-model=pic + +RUN: llvm-ar rcs t1.a t1.bc --thin +# Create this bitcode thin archive in a subdirectory to test the expansion of +# the path to a bitcode file that is referenced using "..", e.g., in this case +# "../t2.bc". +RUN: mkdir lib +RUN: llvm-ar rcs lib/t2.a t2.bc --thin +# Create this bitcode thin archive with an absolute path entry containing "..". +RUN: llvm-ar rcs t3.a %t/lib/../t3.bc --thin +RUN: llvm-ar rcs t4.a t1.bc --thin + +RUN: mkdir %t/out && cd %t/out + +# Note that mock.py does not do any compilation, instead it simply writes the +# contents of the object files supplied on the command line into the output +# object files in job order. +RUN: ld.lld --whole-archive %t/t1.a %t/lib/t2.a ../t3.a \ +RUN: --no-whole-archive %t/t4.a \ +RUN: --thinlto-distributor=%python \ +RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/mock.py \ +RUN: --thinlto-distributor-arg=../t1.o \ +RUN: --thinlto-distributor-arg=../t2.o \ +RUN: --thinlto-distributor-arg=../t3.o \ +RUN: --save-temps + +RUN: ls | FileCheck %s --implicit-check-not=4 + +# JSON jobs description. +CHECK-DAG: a.{{[0-9]+}}.dist-file.json + +# Individual summary index files. +CHECK-DAG: t1.{{[0-9]+}}.{{[0-9]+}}.native.o.thinlto.bc{{$}} +CHECK-DAG: t2.{{[0-9]+}}.{{[0-9]+}}.native.o.thinlto.bc{{$}} +CHECK-DAG: t3.{{[0-9]+}}.{{[0-9]+}}.native.o.thinlto.bc{{$}} + +# Native output object files. +CHECK-DAG: t1.{{[0-9]+}}.{{[0-9]+}}.native.o{{$}} +CHECK-DAG: t2.{{[0-9]+}}.{{[0-9]+}}.native.o{{$}} +CHECK-DAG: t3.{{[0-9]+}}.{{[0-9]+}}.native.o{{$}} + +#--- t1.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @t1() { +entry: + ret void +} + +#--- t2.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @t2() { +entry: + ret void +} + +#--- t3.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @t3() { +entry: + ret void +} diff --git a/lld/test/ELF/dtlto/empty.test b/lld/test/ELF/dtlto/empty.test new file mode 100644 index 0000000000000..5f69d90051220 --- /dev/null +++ b/lld/test/ELF/dtlto/empty.test @@ -0,0 +1,44 @@ +# REQUIRES: x86 + +# Test that DTLTO options are a no-op if no thinLTO is performed. + +RUN: rm -rf %t && split-file %s %t && cd %t + +RUN: opt t1.ll -o t1.o -O2 +RUN: opt t2.ll -o t2.o -O2 + +# Common command-line arguments. Note that mock.py does not do any compilation; +# instead, it simply writes the contents of the object files supplied on the +# command line into the output object files in job order. +RUN: echo "t1.o t2.o \ +RUN: --thinlto-distributor=%python \ +RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/mock.py \ +RUN: --thinlto-distributor-arg=no-exist1.o \ +RUN: --thinlto-distributor-arg=no-exist2.o" > l.rsp + +# Check linking succeeds when all input files are Full LTO. +RUN: ld.lld @l.rsp + +RUN: llc -filetype=obj t1.ll -o t1.o +RUN: llc -filetype=obj t2.ll -o t2.o + +# Check linking succeeds when all input files are ET_RELs. +RUN: ld.lld @l.rsp + +#--- t1.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @t1() { +entry: + ret void +} + +#--- t2.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @t2() { +entry: + ret void +} diff --git a/lld/test/ELF/dtlto/imports.test b/lld/test/ELF/dtlto/imports.test new file mode 100644 index 0000000000000..74d5223f04e24 --- /dev/null +++ b/lld/test/ELF/dtlto/imports.test @@ -0,0 +1,53 @@ +# REQUIRES: x86 + +# Check that DTLTO creates imports lists if requested. + +RUN: rm -rf %t && split-file %s %t && cd %t + +RUN: opt -thinlto-bc t1.ll -o t1.bc -O2 +RUN: opt -thinlto-bc t2.ll -o t2.bc -O2 + +# Generate object files for mock.py to return. +RUN: llc t1.ll --filetype=obj -o t1.o --relocation-model=pic +RUN: llc t2.ll --filetype=obj -o t2.o --relocation-model=pic + +# Common command-line arguments. Note that mock.py does not do any compilation; +# instead, it simply writes the contents of the object files supplied on the +# command line into the output object files in job order. +RUN: echo "t1.bc t2.bc \ +RUN: --thinlto-distributor=%python \ +RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/mock.py \ +RUN: --thinlto-distributor-arg=t1.o \ +RUN: --thinlto-distributor-arg=t2.o" > l.rsp + +# Check that imports files are not created normally. +RUN: ld.lld @l.rsp +RUN: ls | FileCheck %s --check-prefix=NOIMPORTSFILES +NOIMPORTSFILES-NOT: .imports + +# Check that imports files are created with --thinlto-emit-imports-files. +RUN: ld.lld @l.rsp --thinlto-emit-imports-files +RUN: ls | FileCheck %s --check-prefix=IMPORTSFILES +IMPORTSFILES: t1.bc.imports +IMPORTSFILES: t2.bc.imports + +#--- t1.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @t1() { +entry: + ret void +} + +#--- t2.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +declare void @t1(...) + +define void @t2() { +entry: + call void (...) @t1() + ret void +} diff --git a/lld/test/ELF/dtlto/index.test b/lld/test/ELF/dtlto/index.test new file mode 100644 index 0000000000000..4a869c65468fe --- /dev/null +++ b/lld/test/ELF/dtlto/index.test @@ -0,0 +1,44 @@ +# REQUIRES: x86 + +# Check that DTLTO creates individual summary index files if requested. + +RUN: rm -rf %t && split-file %s %t && cd %t + +# Generate ThinLTO bitcode files. +RUN: opt -thinlto-bc t1.ll -o t1.bc -O2 +RUN: opt -thinlto-bc t1.ll -o t2.bc -O2 + +# Generate object files for mock.py to return. +RUN: llc t1.ll --filetype=obj -o t1.o --relocation-model=pic + +# Use a response file for the common command-line arguments. +# Note that mock.py does not perform any compilation; instead, it copies the +# contents of the specified object files into the output object files, in the +# order the jobs are received. +# The "--start-lib/--end-lib" options are used to exercise the special case +# where unused lazy object inputs result in empty index files. +RUN: echo "t1.bc --start-lib t2.bc --end-lib \ +RUN: --thinlto-distributor=%python \ +RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/mock.py \ +RUN: --thinlto-distributor-arg=t1.o \ +RUN: --thinlto-distributor-arg=t2.o" > l.rsp + +# Check that index files are not created normally. +RUN: ld.lld @l.rsp +RUN: ls | FileCheck %s --check-prefix=NOINDEXFILES +NOINDEXFILES-NOT: .thinlto.bc + +# Check that index files are created with --thinlto-emit-index-files. +RUN: ld.lld @l.rsp --thinlto-emit-index-files +RUN: ls | FileCheck %s --check-prefix=INDEXFILES +INDEXFILES: t1.bc.thinlto.bc +INDEXFILES: t2.bc.thinlto.bc + +#--- t1.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @t1() { +entry: + ret void +} diff --git a/lld/test/ELF/dtlto/options.test b/lld/test/ELF/dtlto/options.test new file mode 100644 index 0000000000000..53f0eb41952d0 --- /dev/null +++ b/lld/test/ELF/dtlto/options.test @@ -0,0 +1,41 @@ +# REQUIRES: x86 + +# Test that DTLTO options are passed correctly to the distributor and +# remote compiler. + +RUN: rm -rf %t && split-file %s %t && cd %t + +RUN: opt -thinlto-bc foo.ll -o foo.o + +# Note: validate.py does not perform any compilation. Instead, it validates the +# received JSON, pretty-prints the JSON and the supplied arguments, and then +# exits with an error. This allows FileCheck directives to verify the +# distributor inputs. +RUN: not ld.lld foo.o \ +RUN: -o my.elf \ +RUN: --thinlto-distributor=%python \ +RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/validate.py \ +RUN: --thinlto-distributor-arg=darg1=10 \ +RUN: --thinlto-distributor-arg=darg2=20 \ +RUN: --thinlto-remote-compiler=my_clang.exe \ +RUN: --thinlto-remote-compiler-arg=carg1=20 \ +RUN: --thinlto-remote-compiler-arg=carg2=30 2>&1 | FileCheck %s + +CHECK: distributor_args=['darg1=10', 'darg2=20'] + +CHECK: "linker_output": "my.elf" + +CHECK: "my_clang.exe" +CHECK: "carg1=20" +CHECK: "carg2=30" + +CHECK: ld.lld: error: DTLTO backend compilation: cannot open native object file: + +#--- foo.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @foo() { +entry: + ret void +} diff --git a/lld/test/ELF/dtlto/partitions.test b/lld/test/ELF/dtlto/partitions.test new file mode 100644 index 0000000000000..306dbedcbb19c --- /dev/null +++ b/lld/test/ELF/dtlto/partitions.test @@ -0,0 +1,61 @@ +# REQUIRES: x86 + +# Test that DTLTO works with more than one LTO partition. + +RUN: rm -rf %t && split-file %s %t && cd %t + +# Compile bitcode. +RUN: llvm-as -o full.bc full.ll +RUN: opt -thinlto-bc thin1.ll -o thin1.bc +RUN: opt -thinlto-bc thin2.ll -o thin2.bc + +# Generate object files for mock.py to return. +RUN: llc thin1.ll --filetype=obj -o thin1.o --relocation-model=pic +RUN: llc thin2.ll --filetype=obj -o thin2.o --relocation-model=pic + +# Link with 3 LTO partitions. +RUN: ld.lld full.bc thin1.bc thin2.bc \ +RUN: --thinlto-distributor=%python \ +RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/mock.py \ +RUN: --thinlto-distributor-arg=thin1.o \ +RUN: --thinlto-distributor-arg=thin2.o \ +RUN: --save-temps \ +RUN: --lto-partitions=3 + +# DTLTO temporary object files include the task number and a PID component. The +# task number should incorporate the LTO partition number. +RUN: ls | FileCheck %s +CHECK: thin1.3.[[#]].native.o +CHECK: thin2.4.[[#]].native.o + +#--- full.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @foo() { + call void @bar() + ret void +} + +define void @bar() { + call void @foo() + ret void +} + +#--- thin1.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @baz() { +entry: + ret void +} + +#--- thin2.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @_start() { +entry: + ret void +} diff --git a/lld/test/ELF/dtlto/save-temps.test b/lld/test/ELF/dtlto/save-temps.test new file mode 100644 index 0000000000000..07e5b0f4d8e7d --- /dev/null +++ b/lld/test/ELF/dtlto/save-temps.test @@ -0,0 +1,39 @@ +# REQUIRES: x86 + +# Check that DTLTO responds to --save-temps. + +RUN: rm -rf %t && split-file %s %t && cd %t + +# Generate ThinLTO bitcode files. +RUN: opt -thinlto-bc t1.ll -o t1.bc -O2 + +# Generate object files for mock.py to return. +RUN: llc t1.ll --filetype=obj -o t1.o --relocation-model=pic + +# Use a response file for common command-line arguments. +# Note that mock.py does not perform any compilation; instead, it simply writes +# the contents of the object files supplied on the command line into the output +# object files in job order. +RUN: echo "t1.bc -o my.elf \ +RUN: --thinlto-distributor=%python \ +RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/mock.py \ +RUN: --thinlto-distributor-arg=t1.o" > l.rsp + +# Check that DTLTO temporary files are removed by default. +RUN: ld.lld @l.rsp +RUN: ls | FileCheck %s --check-prefix=REMOVED +REMOVED-NOT: .json + +# Check that DTLTO temporary files are retained with --save-temps. +RUN: ld.lld @l.rsp --save-temps +RUN: ls | FileCheck %s --check-prefix=RETAINED +RETAINED: my.[[#]].dist-file.json + +#--- t1.ll +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @t1() { +entry: + ret void +} diff --git a/lld/test/lit.cfg.py b/lld/test/lit.cfg.py index 9e6b0e839d9a8..10f556567cdc8 100644 --- a/lld/test/lit.cfg.py +++ b/lld/test/lit.cfg.py @@ -36,6 +36,7 @@ llvm_config.use_default_substitutions() llvm_config.use_lld() +config.substitutions.append(("%llvm_src_root", config.llvm_src_root)) tool_patterns = [ "llc", From d24e5311b4a1154c09fed454b5e39a9b3c1f2be2 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Tue, 17 Jun 2025 10:57:01 +0100 Subject: [PATCH 02/20] [DTLTO] Normalize Windows 8.3 short paths to long form 8.3-style short paths (e.g., C:\PROGRA~1) can cause inconsistencies or failures on remote machines during distributed linking. Such paths may not resolve correctly or may differ between systems. We anticipate that many/all distribution systems will have difficulties with such paths. This patch ensures that all Windows paths used for DTLTO in ELF LLD are converted to their long form using GetLongPathNameW. This avoids ambiguity and improves portability across most distribution systems. --- cross-project-tests/dtlto/path.test | 65 ++++++ lld/Common/Filesystem.cpp | 18 ++ lld/ELF/InputFiles.cpp | 9 + lld/include/lld/Common/Filesystem.h | 1 + .../llvm/Support/Windows/WindowsSupport.h | 8 + llvm/lib/Support/Windows/Path.inc | 113 ++++++++--- llvm/unittests/Support/Path.cpp | 186 ++++++++++++++++++ 7 files changed, 368 insertions(+), 32 deletions(-) create mode 100644 cross-project-tests/dtlto/path.test diff --git a/cross-project-tests/dtlto/path.test b/cross-project-tests/dtlto/path.test new file mode 100644 index 0000000000000..156bebce169c0 --- /dev/null +++ b/cross-project-tests/dtlto/path.test @@ -0,0 +1,65 @@ +# REQUIRES: x86-registered-target,ld.lld,llvm-ar + +# Check that any shortened "8.3" paths containing '~' characters are expanded, +# and their path seperators changed to Winodws style ones, prior to being passed +# to SN-DBS during DTLTO. + +RUN: rm -rf %t && split-file %s %t && cd %t + +# Create an archive to confirm unpacked member paths are expanded. +RUN: prospero-clang --target=x86_64-linux-gnu -c start.c f.c -flto=thin -O2 +RUN: prospero-llvm-ar rcs libf.a f.o --thin + +RUN: %python in-tilde-dir.py \ +RUN: ld.lld start.o libf.a -o start.elf \ +RUN: --thinlto-distributor=%python \ +RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \ +RUN: --thinlto-remote-compiler=%clang \ +RUN: --save-temps + +# Ensure that cross-module importing occurred. Cross-module importing is somewhat +# fragile - e.g. it requires amenable code and compilation with -O2 or above. +# It's important that importing happens for this test case, as the paths to +# imported-from modules are recorded in the ThinLTO metadata. +RUN: prospero-llvm-objdump -d start.elf | tee d.txt | FileCheck %s --check-prefix=IMPORT +IMPORT: <_start>: +IMPORT: jmp 0x{{[0-9A-F]+}} <_start> +IMPORT: : +IMPORT-NEXT: jmp 0x{{[0-9A-F]+}} + +RUN: FileCheck --input-file start.*.dist-file.json %s --check-prefix=TILDE +TILDE-NOT: ~ + +#--- f.c +int _start(); +int f() { return _start(); } + +#--- start.c +int f(); +int _start() { return f(); } + +#--- in-tilde-dir.py +import os, shutil, sys, uuid, subprocess +from pathlib import Path + +temp = Path(os.environ['TEMP']) +assert temp.is_dir() + +# Copy the CWD to a unique directory inside TEMP and determine its 8.3 form. +# TEMP is likely to be on a drive that supports long+short paths. +d = (temp / str(uuid.uuid4()) / 'veryverylong').resolve() +d83 = d.parent / (d.name[:6] + '~1') +d.parent.mkdir(parents=True) +try: + shutil.copytree(Path.cwd(), d) + + # Replace the arguments of the command that name files with equivalents in + # our temp directory, prefixed with the 8.3 form. + cmd = [a if not Path(a).is_file() else str(d83/a) \ + for a in sys.argv[1:]] + print(cmd) + + sys.exit(subprocess.run(cmd).returncode) + +finally: + shutil.rmtree(d.parent) diff --git a/lld/Common/Filesystem.cpp b/lld/Common/Filesystem.cpp index c2d3644191c9f..8cbe34a1d9d50 100644 --- a/lld/Common/Filesystem.cpp +++ b/lld/Common/Filesystem.cpp @@ -22,6 +22,12 @@ #include #endif #include +#if defined(_WIN32) +#include "llvm/Support/ConvertUTF.h" +#include "llvm/Support/Windows/WindowsSupport.h" +#include "llvm/Support/WindowsError.h" +#include +#endif using namespace llvm; using namespace lld; @@ -155,3 +161,15 @@ std::unique_ptr lld::openLTOOutputFile(StringRef file) { return fs; return openFile(file); } + +std::error_code lld::dtltoNormalizePath(std::string &PathOut) { +#if defined(_WIN32) + llvm::SmallString<128> Expanded; + if (auto EC = llvm::sys::windows::makeLong(PathOut, Expanded)) + return EC; + + PathOut.assign(Expanded.begin(), Expanded.end()); +#endif + (void)PathOut; + return std::error_code{}; +} diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp index 5523d929f6bfd..761648afaf77a 100644 --- a/lld/ELF/InputFiles.cpp +++ b/lld/ELF/InputFiles.cpp @@ -17,6 +17,7 @@ #include "SyntheticSections.h" #include "Target.h" #include "lld/Common/DWARF.h" +#include "lld/Common/Filesystem.h" #include "llvm/ADT/CachedHashString.h" #include "llvm/ADT/STLExtras.h" #include "llvm/LTO/LTO.h" @@ -1764,6 +1765,9 @@ static bool dtltoAdjustMemberPathIfThinArchive(Ctx &ctx, StringRef archivePath, path::append(resolvedPath, memberPath); memberPath = resolvedPath.str(); } + + lld::dtltoNormalizePath(memberPath); + return true; } @@ -1777,6 +1781,11 @@ BitcodeFile::BitcodeFile(Ctx &ctx, MemoryBufferRef mb, StringRef archiveName, if (ctx.arg.thinLTOIndexOnly) path = replaceThinLTOSuffix(ctx, mb.getBufferIdentifier()); + // SCE_PRIVATE: begin DTLTO + // TODO: Investigate if we can remove this code. + if (!ctx.arg.dtltoDistributor.empty()) + lld::dtltoNormalizePath(path); + // ThinLTO assumes that all MemoryBufferRefs given to it have a unique // name. If two archives define two members with the same name, this // causes a collision which result in only one of the objects being taken diff --git a/lld/include/lld/Common/Filesystem.h b/lld/include/lld/Common/Filesystem.h index 61b32eec2ee7d..f6db7693a140b 100644 --- a/lld/include/lld/Common/Filesystem.h +++ b/lld/include/lld/Common/Filesystem.h @@ -19,6 +19,7 @@ void unlinkAsync(StringRef path); std::error_code tryCreateFile(StringRef path); std::unique_ptr openFile(StringRef file); std::unique_ptr openLTOOutputFile(StringRef file); +std::error_code dtltoNormalizePath(std::string &PathOut); } // namespace lld #endif diff --git a/llvm/include/llvm/Support/Windows/WindowsSupport.h b/llvm/include/llvm/Support/Windows/WindowsSupport.h index ffc6fdf5b983c..ed60f66956506 100644 --- a/llvm/include/llvm/Support/Windows/WindowsSupport.h +++ b/llvm/include/llvm/Support/Windows/WindowsSupport.h @@ -245,6 +245,14 @@ LLVM_ABI std::error_code widenPath(const Twine &Path8, SmallVectorImpl &Path16, size_t MaxPathLen = MAX_PATH); +/// Convert UTF-8 path into a long form UTF-8 path. +LLVM_ABI std::error_code makeLong(const Twine &Path8, + llvm::SmallVectorImpl &Result); + +/// Retrieves the volume mount point from UTF-16 path as UTF-16 result. +std::error_code getVolumeRootFromPath(SmallVectorImpl &Path, + SmallVectorImpl &Result); + } // end namespace windows } // end namespace sys } // end namespace llvm. diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc index 0afd0141b525a..a6891043e1227 100644 --- a/llvm/lib/Support/Windows/Path.inc +++ b/llvm/lib/Support/Windows/Path.inc @@ -58,6 +58,19 @@ static bool is_separator(const wchar_t value) { } } +static void stripExtendedPrefix(wchar_t *&Data, DWORD &CountChars) { + if (CountChars >= 8 && ::memcmp(Data, L"\\\\?\\UNC\\", 16) == 0) { + // Convert \\?\UNC\foo\bar to \\foo\bar + CountChars -= 6; + Data += 6; + Data[0] = '\\'; + } else if (CountChars >= 4 && ::memcmp(Data, L"\\\\?\\", 8) == 0) { + // Convert \\?\C:\foo to C:\foo + CountChars -= 4; + Data += 4; + } +} + namespace llvm { namespace sys { namespace windows { @@ -125,6 +138,70 @@ std::error_code widenPath(const Twine &Path8, SmallVectorImpl &Path16, return UTF8ToUTF16(FullPath, Path16); } +std::error_code makeLong(const Twine &Path8, + llvm::SmallVectorImpl &Result8) { + constexpr StringLiteral LongPathPrefix = R"(\\?\)"; + + SmallString<128> PathStorage; + StringRef PathStr = Path8.toStringRef(PathStorage); + bool HadPrefix = PathStr.starts_with(LongPathPrefix); + + // Convert UTF-8 to UTF-16 + SmallVector Path16; + if (std::error_code EC = widenPath(PathStr, Path16)) + return EC; + + // Start with a buffer equal to input. + llvm::SmallVector Long16; + DWORD Len = static_cast(Path16.size()); + + do { + Long16.resize_for_overwrite(Len); // grow + + Len = ::GetLongPathNameW(Path16.data(), Long16.data(), Len); + + // A zero return value indicates a failure other than insufficient space. + if (Len == 0) + return mapWindowsError(::GetLastError()); + + // If there's insufficient space, the len returned is larger than the len + // given - in this case len includes the null-terminator. + } while (Len >= Long16.size()); + + // On success, GetLongPathNameW returns the number of characters not + // including the null-terminator. + Long16.truncate(Len); + // Strip \\?\ or \\?\UNC\ prefix if it wasn't part of the original path + wchar_t *Data = Long16.data(); + if (!HadPrefix) + stripExtendedPrefix(Data, Len); + + return sys::windows::UTF16ToUTF8(Data, Len, Result8); +} + +std::error_code getVolumeRootFromPath(SmallVectorImpl &Path, + SmallVectorImpl &VolumeRoot) { + size_t Len = 128; + while (true) { + VolumeRoot.resize(Len); + BOOL Success = + ::GetVolumePathNameW(Path.data(), VolumeRoot.data(), VolumeRoot.size()); + if (Success) + break; + + DWORD Err = ::GetLastError(); + if (Err != ERROR_INSUFFICIENT_BUFFER) + return mapWindowsError(Err); + + Len *= 2; + } + + // Ensure null-termination in edge case where buffer was exactly full + VolumeRoot.push_back(L'\0'); + VolumeRoot.truncate(wcslen(VolumeRoot.data())); + return std::error_code(); +} + } // end namespace windows namespace fs { @@ -309,29 +386,10 @@ std::error_code remove(const Twine &path, bool IgnoreNonExisting) { static std::error_code is_local_internal(SmallVectorImpl &Path, bool &Result) { SmallVector VolumePath; - size_t Len = 128; - while (true) { - VolumePath.resize(Len); - BOOL Success = - ::GetVolumePathNameW(Path.data(), VolumePath.data(), VolumePath.size()); - - if (Success) - break; - - DWORD Err = ::GetLastError(); - if (Err != ERROR_INSUFFICIENT_BUFFER) - return mapWindowsError(Err); + if (std::error_code EC = windows::getVolumeRootFromPath(Path, VolumePath)) + return EC; - Len *= 2; - } - // If the output buffer has exactly enough space for the path name, but not - // the null terminator, it will leave the output unterminated. Push a null - // terminator onto the end to ensure that this never happens. - VolumePath.push_back(L'\0'); - VolumePath.truncate(wcslen(VolumePath.data())); - const wchar_t *P = VolumePath.data(); - - UINT Type = ::GetDriveTypeW(P); + UINT Type = ::GetDriveTypeW(VolumePath.data()); switch (Type) { case DRIVE_FIXED: Result = true; @@ -392,16 +450,7 @@ static std::error_code realPathFromHandle(HANDLE H, // paths don't get canonicalized by file APIs. wchar_t *Data = Buffer.data(); DWORD CountChars = Buffer.size(); - if (CountChars >= 8 && ::memcmp(Data, L"\\\\?\\UNC\\", 16) == 0) { - // Convert \\?\UNC\foo\bar to \\foo\bar - CountChars -= 6; - Data += 6; - Data[0] = '\\'; - } else if (CountChars >= 4 && ::memcmp(Data, L"\\\\?\\", 8) == 0) { - // Convert \\?\c:\foo to c:\foo - CountChars -= 4; - Data += 4; - } + stripExtendedPrefix(Data, CountChars); // Convert the result from UTF-16 to UTF-8. if (std::error_code EC = UTF16ToUTF8(Data, CountChars, RealPath)) diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index 355aa6b9ade06..339792e5457a4 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -32,6 +32,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/Support/Chrono.h" #include "llvm/Support/Windows/WindowsSupport.h" +#include #include #include #endif @@ -2448,6 +2449,191 @@ TEST_F(FileSystemTest, widenPath) { #endif #ifdef _WIN32 + +TEST_F(FileSystemTest, getVolumeRootFromPath) { + SmallVector Path16; + + ASSERT_FALSE(sys::windows::UTF8ToUTF16(TestDirectory, Path16)); + + SmallVector VolumeRoot16; + ASSERT_FALSE(sys::windows::getVolumeRootFromPath(Path16, VolumeRoot16)); + + SmallString<128> VolumeRoot8; + ASSERT_FALSE(sys::windows::UTF16ToUTF8(VolumeRoot16.data(), + VolumeRoot16.size(), VolumeRoot8)); + + SmallString<128> ExpectedRoot(sys::path::root_name(TestDirectory)); + ExpectedRoot += "\\"; + + EXPECT_EQ(VolumeRoot8, ExpectedRoot); +} + +/// Resolves a UTF-16 path to its absolute form using GetFullPathNameW. +/// Returns true on success and stores the result in `Result`. +static bool getFullPath(llvm::SmallVectorImpl &Path16, + llvm::SmallVectorImpl &Result) { + // Call GetFullPathNameW to get the required buffer size. + DWORD Len = ::GetFullPathNameW(Path16.data(), 0, nullptr, nullptr); + if (Len == 0) + return false; + + // Call GetFullPathNameW to get the required buffer size. + Result.resize(Len); + DWORD FinalLen = + ::GetFullPathNameW(Path16.data(), Len, Result.data(), nullptr); + return FinalLen > 0 && FinalLen < Len; +} + +/// Checks whether short (8.3) names are enabled for the volume containing the +/// given UTF-8 path. +static bool isShortNameEnabledForPath(llvm::StringRef Path8) { + llvm::SmallVector Path16; + if (windows::widenPath(Path8, Path16)) + return false; + + SmallVector FullPath16; + if (!getFullPath(Path16, FullPath16)) + return false; + + // Get volume root. + llvm::SmallVector VolumeRoot16; + if (std::error_code EC = + windows::getVolumeRootFromPath(FullPath16, VolumeRoot16)) + return false; + + // Attempt to load AreShortNamesEnabled dynamically (it's avalible only on + // some versions of Windows). + using AreShortNamesEnabledFn = BOOL(WINAPI *)(HANDLE, BOOL *); + auto Kernel32 = ::GetModuleHandleW(L"kernel32.dll"); + if (!Kernel32) + return false; + + auto *FnPtr = + reinterpret_cast(reinterpret_cast( + ::GetProcAddress(Kernel32, "AreShortNamesEnabled"))); + if (!FnPtr) + return false; + + // Open the volume root for querying. + HANDLE VolumeHandle = ::CreateFileW( + VolumeRoot16.data(), GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, nullptr, + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, nullptr); + + if (VolumeHandle == INVALID_HANDLE_VALUE) + return false; + + BOOL Enabled = FALSE; + bool Success = FnPtr(static_cast(VolumeHandle), &Enabled); + ::CloseHandle(VolumeHandle); + + return Success && Enabled; +} + +/// Returns the 8.3 path for the given UTF-8 path, or an empty string +/// on failure. Uses Win32 GetShortPathNameW. +static std::string getShortPathUtf8(llvm::StringRef Path8) { + using namespace llvm; + + // Convert UTF-8 to UTF-16 + SmallVector Path16; + if (std::error_code EC = sys::windows::widenPath(Path8, Path16)) + return {}; + + // Get required buffer size for short path (includes null terminator) + DWORD Required = ::GetShortPathNameW(Path16.data(), nullptr, 0); + if (Required == 0) + return {}; + + SmallVector ShortPath; + ShortPath.resize_for_overwrite(Required); + + DWORD Written = + ::GetShortPathNameW(Path16.data(), ShortPath.data(), Required); + if (Written == 0 || Written >= Required) + return {}; + + ShortPath.truncate(Written); + + SmallString<128> Utf8Result; + if (std::error_code EC = sys::windows::UTF16ToUTF8( + ShortPath.data(), ShortPath.size(), Utf8Result)) + return {}; + + return std::string(Utf8Result); +} + +static std::string stripPrefix(llvm::StringRef P) { + if (P.starts_with(R"(\\?\UNC\)")) + return "\\" + P.drop_front(8).str(); + if (P.starts_with(R"(\\?\)")) + return P.drop_front(4).str(); + return P.str(); +} + +TEST_F(FileSystemTest, makeLong) { + if (!isShortNameEnabledForPath(TestDirectory.str())) + GTEST_SKIP() << "Short names not enabled on volume."; + + // Get a short-path version of the test directory + std::string Short8 = getShortPathUtf8(TestDirectory); + ASSERT_FALSE(Short8.empty()) + << "Expected short path form for test directory."; + + // Setup: Create a path where even if all components were reduced to short + // form (typically 8 characters e.g. 123456~1) the total length would + // exceed MAX_PATH. + constexpr const char *OneDir = "\\123456789"; // >8 chars + const size_t NLevels = (MAX_PATH / 8) + 1; + SmallString Long(TestDirectory); + for (size_t I = 0; I < NLevels; ++I) + Long.append(OneDir); + + ASSERT_NO_ERROR(fs::create_directories(Long)); + std::string LongShort8WithPrefix = getShortPathUtf8(Long); + ASSERT_TRUE(StringRef(LongShort8WithPrefix).starts_with(R"(\\?\)")) + << "Expected prefixed short path, got: " << LongShort8WithPrefix; + + std::string LongShort8 = stripPrefix(LongShort8WithPrefix); + + // === Case 1: Non-existent short path should fail === + SmallString<128> NoExist("No Existy~1 Path"); + SmallString<128> NoExistResult; + EXPECT_TRUE(windows::makeLong(NoExist, NoExistResult)); + EXPECT_TRUE(NoExistResult.empty()); + + // === Case 2: Short path that exists, no prefix === + SmallString<128> ShortResult; + ASSERT_FALSE(windows::makeLong(Short8, ShortResult)); + fs::UniqueID ShortID1, ShortID2; + ASSERT_NO_ERROR(fs::getUniqueID(Short8, ShortID1)); + ASSERT_NO_ERROR(fs::getUniqueID(ShortResult, ShortID2)); + EXPECT_EQ(ShortID1, ShortID2); + + // === Case 3: Long short path (no prefix) === + SmallString<128> LongResult; + ASSERT_FALSE(windows::makeLong(LongShort8, LongResult)); + fs::UniqueID LongID1, LongID2; + ASSERT_NO_ERROR(fs::getUniqueID(LongShort8, LongID1)); + ASSERT_NO_ERROR(fs::getUniqueID(LongResult, LongID2)); + EXPECT_EQ(LongID1, LongID2); + EXPECT_FALSE(StringRef(LongResult).starts_with(R"(\\?\)")) + << "Expected unprefixed result, got: " << LongResult; + + // === Case 4: Long short path with prefix === + SmallString<128> LongPrefixedResult; + ASSERT_FALSE(windows::makeLong(LongShort8WithPrefix, LongPrefixedResult)); + fs::UniqueID LongPrefixedID1, LongPrefixedID2; + ASSERT_NO_ERROR(fs::getUniqueID(LongShort8WithPrefix, LongPrefixedID1)); + ASSERT_NO_ERROR(fs::getUniqueID(LongPrefixedResult, LongPrefixedID2)); + EXPECT_EQ(LongPrefixedID1, LongPrefixedID2); + EXPECT_TRUE(StringRef(LongPrefixedResult).starts_with(R"(\\?\)")) + << "Expected prefixed result, got: " << LongPrefixedResult; + + // Cleanup + ASSERT_NO_ERROR(fs::remove(Long)); +} + // Windows refuses lock request if file region is already locked by the same // process. POSIX system in this case updates the existing lock. TEST_F(FileSystemTest, FileLocker) { From 2ca44d9b77f675b3bd93a0c89b3e85101e88f1f5 Mon Sep 17 00:00:00 2001 From: bd1976bris Date: Tue, 17 Jun 2025 14:42:54 +0100 Subject: [PATCH 03/20] Update cross-project-tests/dtlto/path.test Co-authored-by: Edd Dawson --- cross-project-tests/dtlto/path.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cross-project-tests/dtlto/path.test b/cross-project-tests/dtlto/path.test index 156bebce169c0..496a3b9ba2a5f 100644 --- a/cross-project-tests/dtlto/path.test +++ b/cross-project-tests/dtlto/path.test @@ -21,7 +21,7 @@ RUN: --save-temps # fragile - e.g. it requires amenable code and compilation with -O2 or above. # It's important that importing happens for this test case, as the paths to # imported-from modules are recorded in the ThinLTO metadata. -RUN: prospero-llvm-objdump -d start.elf | tee d.txt | FileCheck %s --check-prefix=IMPORT +RUN: prospero-llvm-objdump -d start.elf | FileCheck %s --check-prefix=IMPORT IMPORT: <_start>: IMPORT: jmp 0x{{[0-9A-F]+}} <_start> IMPORT: : From c2a3334364e2f03e04e3200a2dd1ac42adef291e Mon Sep 17 00:00:00 2001 From: bd1976bris Date: Tue, 17 Jun 2025 14:43:54 +0100 Subject: [PATCH 04/20] Update lld/ELF/InputFiles.cpp Co-authored-by: Edd Dawson --- lld/ELF/InputFiles.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp index 761648afaf77a..c03be1d67a865 100644 --- a/lld/ELF/InputFiles.cpp +++ b/lld/ELF/InputFiles.cpp @@ -1781,8 +1781,6 @@ BitcodeFile::BitcodeFile(Ctx &ctx, MemoryBufferRef mb, StringRef archiveName, if (ctx.arg.thinLTOIndexOnly) path = replaceThinLTOSuffix(ctx, mb.getBufferIdentifier()); - // SCE_PRIVATE: begin DTLTO - // TODO: Investigate if we can remove this code. if (!ctx.arg.dtltoDistributor.empty()) lld::dtltoNormalizePath(path); From f90d4fde544e7b47f8605cd2ddccb7ff743bdbd3 Mon Sep 17 00:00:00 2001 From: bd1976bris Date: Tue, 17 Jun 2025 16:18:18 +0100 Subject: [PATCH 05/20] Update llvm/lib/Support/Windows/Path.inc Co-authored-by: Edd Dawson --- llvm/lib/Support/Windows/Path.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc index a6891043e1227..08aecb8f7d69c 100644 --- a/llvm/lib/Support/Windows/Path.inc +++ b/llvm/lib/Support/Windows/Path.inc @@ -156,7 +156,7 @@ std::error_code makeLong(const Twine &Path8, DWORD Len = static_cast(Path16.size()); do { - Long16.resize_for_overwrite(Len); // grow + Long16.resize_for_overwrite(Len); Len = ::GetLongPathNameW(Path16.data(), Long16.data(), Len); From b96acc279fca4b8ac15641da43a53841f6f2ce81 Mon Sep 17 00:00:00 2001 From: bd1976bris Date: Tue, 17 Jun 2025 16:19:01 +0100 Subject: [PATCH 06/20] Update llvm/unittests/Support/Path.cpp Co-authored-by: Edd Dawson --- llvm/unittests/Support/Path.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index 339792e5457a4..e1ae3289b7d8d 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -2565,7 +2565,7 @@ static std::string getShortPathUtf8(llvm::StringRef Path8) { static std::string stripPrefix(llvm::StringRef P) { if (P.starts_with(R"(\\?\UNC\)")) - return "\\" + P.drop_front(8).str(); + return R"\\" + P.drop_front(8).str(); // or "\\" + P.drop_front(7).str(); if (P.starts_with(R"(\\?\)")) return P.drop_front(4).str(); return P.str(); From 9d4d009a8a925321390c038f5ef714dfb34c6b63 Mon Sep 17 00:00:00 2001 From: bd1976bris Date: Tue, 17 Jun 2025 16:19:16 +0100 Subject: [PATCH 07/20] Update cross-project-tests/dtlto/path.test Co-authored-by: Edd Dawson --- cross-project-tests/dtlto/path.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cross-project-tests/dtlto/path.test b/cross-project-tests/dtlto/path.test index 496a3b9ba2a5f..89f23148babbd 100644 --- a/cross-project-tests/dtlto/path.test +++ b/cross-project-tests/dtlto/path.test @@ -1,7 +1,7 @@ # REQUIRES: x86-registered-target,ld.lld,llvm-ar # Check that any shortened "8.3" paths containing '~' characters are expanded, -# and their path seperators changed to Winodws style ones, prior to being passed +# and their path seperators changed to Windows style ones, prior to being passed # to SN-DBS during DTLTO. RUN: rm -rf %t && split-file %s %t && cd %t From 0207d9f63300fb462f8fadfd1edfbf14bb19d973 Mon Sep 17 00:00:00 2001 From: bd1976bris Date: Tue, 17 Jun 2025 16:19:24 +0100 Subject: [PATCH 08/20] Update cross-project-tests/dtlto/path.test Co-authored-by: Edd Dawson --- cross-project-tests/dtlto/path.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cross-project-tests/dtlto/path.test b/cross-project-tests/dtlto/path.test index 89f23148babbd..f3f551d25da77 100644 --- a/cross-project-tests/dtlto/path.test +++ b/cross-project-tests/dtlto/path.test @@ -2,7 +2,7 @@ # Check that any shortened "8.3" paths containing '~' characters are expanded, # and their path seperators changed to Windows style ones, prior to being passed -# to SN-DBS during DTLTO. +# to the distributor. RUN: rm -rf %t && split-file %s %t && cd %t From 0a7560d4ece55657131853eadb8c0cd1e8aab3fa Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Tue, 17 Jun 2025 16:20:35 +0100 Subject: [PATCH 09/20] Mark LIT test as UNSUPPORTED where short paths are not available The cross-project `path.test` assumes that the Windows volume for the %TEMP% directories has short paths enabled, this is not guaranteed. This change allows tests cross-project-tests to `REQUIRE: shortpaths` and `path.test` test uses this feature. --- cross-project-tests/dtlto/path.test | 2 +- cross-project-tests/lit.site.cfg.py.in | 33 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/cross-project-tests/dtlto/path.test b/cross-project-tests/dtlto/path.test index f3f551d25da77..da1da4261a358 100644 --- a/cross-project-tests/dtlto/path.test +++ b/cross-project-tests/dtlto/path.test @@ -1,4 +1,4 @@ -# REQUIRES: x86-registered-target,ld.lld,llvm-ar +# REQUIRES: x86-registered-target,ld.lld,llvm-ar,system-windows,shortpaths # Check that any shortened "8.3" paths containing '~' characters are expanded, # and their path seperators changed to Windows style ones, prior to being passed diff --git a/cross-project-tests/lit.site.cfg.py.in b/cross-project-tests/lit.site.cfg.py.in index 39458dfc79afd..a097d500e50bc 100644 --- a/cross-project-tests/lit.site.cfg.py.in +++ b/cross-project-tests/lit.site.cfg.py.in @@ -26,3 +26,36 @@ lit.llvm.initialize(lit_config, config) # Let the main config do the real work. lit_config.load_config(config, "@CROSS_PROJECT_TESTS_SOURCE_DIR@/lit.cfg.py") + +def is_short_names_enabled(path_utf8: str) -> bool: + import os, ctypes; from ctypes import wintypes + + k32 = ctypes.WinDLL("kernel32", use_last_error=True) + try: + fn = k32.AreShortNamesEnabled + fn.restype = wintypes.BOOL + fn.argtypes = [wintypes.HANDLE, ctypes.POINTER(wintypes.BOOL)] + except AttributeError: + return False + + path_w = ctypes.create_unicode_buffer(os.path.abspath(path_utf8), 260) + vol_w = ctypes.create_unicode_buffer(260) + if not k32.GetVolumePathNameW(path_w, vol_w, 260): + return False + + # 0x80000000 = GENERIC_READ + # 0x7 = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE + # 3 = OPEN_EXISTING + # 0x02000000 = FILE_FLAG_BACKUP_SEMANTICS + handle = k32.CreateFileW( + vol_w, 0x80000000, 0x7, None, 3, 0x02000000, None) + if handle == ctypes.c_void_p(-1).value: + return False + + res = wintypes.BOOL() + ok = fn(handle, ctypes.byref(res)) + k32.CloseHandle(handle) + return bool(ok and res.value) + +if os.name == "nt" and is_short_names_enabled(config.environment.get('TEMP')): + config.available_features.add('shortpaths') From f858814c60bae5b1e542072ceb839e8f6250ba58 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Tue, 17 Jun 2025 17:09:07 +0100 Subject: [PATCH 10/20] Add LLVM_ABI and make the naming of function parameters more consistent with existing code. --- llvm/include/llvm/Support/Windows/WindowsSupport.h | 7 ++++--- llvm/lib/Support/Windows/Path.inc | 13 ++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/llvm/include/llvm/Support/Windows/WindowsSupport.h b/llvm/include/llvm/Support/Windows/WindowsSupport.h index ed60f66956506..18c3d8bbfa1ff 100644 --- a/llvm/include/llvm/Support/Windows/WindowsSupport.h +++ b/llvm/include/llvm/Support/Windows/WindowsSupport.h @@ -247,11 +247,12 @@ LLVM_ABI std::error_code widenPath(const Twine &Path8, /// Convert UTF-8 path into a long form UTF-8 path. LLVM_ABI std::error_code makeLong(const Twine &Path8, - llvm::SmallVectorImpl &Result); + llvm::SmallVectorImpl &Result8); /// Retrieves the volume mount point from UTF-16 path as UTF-16 result. -std::error_code getVolumeRootFromPath(SmallVectorImpl &Path, - SmallVectorImpl &Result); +LLVM_ABI std::error_code +getVolumeRootFromPath(SmallVectorImpl &Path16, + SmallVectorImpl &Result16); } // end namespace windows } // end namespace sys diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc index 08aecb8f7d69c..634cd1e1b9dc1 100644 --- a/llvm/lib/Support/Windows/Path.inc +++ b/llvm/lib/Support/Windows/Path.inc @@ -146,7 +146,6 @@ std::error_code makeLong(const Twine &Path8, StringRef PathStr = Path8.toStringRef(PathStorage); bool HadPrefix = PathStr.starts_with(LongPathPrefix); - // Convert UTF-8 to UTF-16 SmallVector Path16; if (std::error_code EC = widenPath(PathStr, Path16)) return EC; @@ -179,13 +178,13 @@ std::error_code makeLong(const Twine &Path8, return sys::windows::UTF16ToUTF8(Data, Len, Result8); } -std::error_code getVolumeRootFromPath(SmallVectorImpl &Path, - SmallVectorImpl &VolumeRoot) { +std::error_code getVolumeRootFromPath(SmallVectorImpl &Path16, + SmallVectorImpl &Result16) { size_t Len = 128; while (true) { - VolumeRoot.resize(Len); + Result16.resize(Len); BOOL Success = - ::GetVolumePathNameW(Path.data(), VolumeRoot.data(), VolumeRoot.size()); + ::GetVolumePathNameW(Path16.data(), Result16.data(), Result16.size()); if (Success) break; @@ -197,8 +196,8 @@ std::error_code getVolumeRootFromPath(SmallVectorImpl &Path, } // Ensure null-termination in edge case where buffer was exactly full - VolumeRoot.push_back(L'\0'); - VolumeRoot.truncate(wcslen(VolumeRoot.data())); + Result16.push_back(L'\0'); + Result16.truncate(wcslen(Result16.data())); return std::error_code(); } From ccd3e9253b7386819032a07ce095fd6ca8d28785 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Wed, 18 Jun 2025 09:43:20 +0100 Subject: [PATCH 11/20] Just use path::root_path and drop Win32 API wrapper functions we were using --- .../llvm/Support/Windows/WindowsSupport.h | 5 -- llvm/lib/Support/Windows/Path.inc | 48 +++++++++---------- llvm/unittests/Support/Path.cpp | 47 +----------------- 3 files changed, 24 insertions(+), 76 deletions(-) diff --git a/llvm/include/llvm/Support/Windows/WindowsSupport.h b/llvm/include/llvm/Support/Windows/WindowsSupport.h index 18c3d8bbfa1ff..4755f66267f3f 100644 --- a/llvm/include/llvm/Support/Windows/WindowsSupport.h +++ b/llvm/include/llvm/Support/Windows/WindowsSupport.h @@ -249,11 +249,6 @@ LLVM_ABI std::error_code widenPath(const Twine &Path8, LLVM_ABI std::error_code makeLong(const Twine &Path8, llvm::SmallVectorImpl &Result8); -/// Retrieves the volume mount point from UTF-16 path as UTF-16 result. -LLVM_ABI std::error_code -getVolumeRootFromPath(SmallVectorImpl &Path16, - SmallVectorImpl &Result16); - } // end namespace windows } // end namespace sys } // end namespace llvm. diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc index 634cd1e1b9dc1..7df04b1889408 100644 --- a/llvm/lib/Support/Windows/Path.inc +++ b/llvm/lib/Support/Windows/Path.inc @@ -178,29 +178,6 @@ std::error_code makeLong(const Twine &Path8, return sys::windows::UTF16ToUTF8(Data, Len, Result8); } -std::error_code getVolumeRootFromPath(SmallVectorImpl &Path16, - SmallVectorImpl &Result16) { - size_t Len = 128; - while (true) { - Result16.resize(Len); - BOOL Success = - ::GetVolumePathNameW(Path16.data(), Result16.data(), Result16.size()); - if (Success) - break; - - DWORD Err = ::GetLastError(); - if (Err != ERROR_INSUFFICIENT_BUFFER) - return mapWindowsError(Err); - - Len *= 2; - } - - // Ensure null-termination in edge case where buffer was exactly full - Result16.push_back(L'\0'); - Result16.truncate(wcslen(Result16.data())); - return std::error_code(); -} - } // end namespace windows namespace fs { @@ -385,10 +362,29 @@ std::error_code remove(const Twine &path, bool IgnoreNonExisting) { static std::error_code is_local_internal(SmallVectorImpl &Path, bool &Result) { SmallVector VolumePath; - if (std::error_code EC = windows::getVolumeRootFromPath(Path, VolumePath)) - return EC; + size_t Len = 128; + while (true) { + VolumePath.resize(Len); + BOOL Success = + ::GetVolumePathNameW(Path.data(), VolumePath.data(), VolumePath.size()); + + if (Success) + break; - UINT Type = ::GetDriveTypeW(VolumePath.data()); + DWORD Err = ::GetLastError(); + if (Err != ERROR_INSUFFICIENT_BUFFER) + return mapWindowsError(Err); + + Len *= 2; + } + // If the output buffer has exactly enough space for the path name, but not + // the null terminator, it will leave the output unterminated. Push a null + // terminator onto the end to ensure that this never happens. + VolumePath.push_back(L'\0'); + VolumePath.truncate(wcslen(VolumePath.data())); + const wchar_t *P = VolumePath.data(); + + UINT Type = ::GetDriveTypeW(P); switch (Type) { case DRIVE_FIXED: Result = true; diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index e1ae3289b7d8d..a8a4614c118f3 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -2450,55 +2450,12 @@ TEST_F(FileSystemTest, widenPath) { #ifdef _WIN32 -TEST_F(FileSystemTest, getVolumeRootFromPath) { - SmallVector Path16; - - ASSERT_FALSE(sys::windows::UTF8ToUTF16(TestDirectory, Path16)); - - SmallVector VolumeRoot16; - ASSERT_FALSE(sys::windows::getVolumeRootFromPath(Path16, VolumeRoot16)); - - SmallString<128> VolumeRoot8; - ASSERT_FALSE(sys::windows::UTF16ToUTF8(VolumeRoot16.data(), - VolumeRoot16.size(), VolumeRoot8)); - - SmallString<128> ExpectedRoot(sys::path::root_name(TestDirectory)); - ExpectedRoot += "\\"; - - EXPECT_EQ(VolumeRoot8, ExpectedRoot); -} - -/// Resolves a UTF-16 path to its absolute form using GetFullPathNameW. -/// Returns true on success and stores the result in `Result`. -static bool getFullPath(llvm::SmallVectorImpl &Path16, - llvm::SmallVectorImpl &Result) { - // Call GetFullPathNameW to get the required buffer size. - DWORD Len = ::GetFullPathNameW(Path16.data(), 0, nullptr, nullptr); - if (Len == 0) - return false; - - // Call GetFullPathNameW to get the required buffer size. - Result.resize(Len); - DWORD FinalLen = - ::GetFullPathNameW(Path16.data(), Len, Result.data(), nullptr); - return FinalLen > 0 && FinalLen < Len; -} - /// Checks whether short (8.3) names are enabled for the volume containing the /// given UTF-8 path. static bool isShortNameEnabledForPath(llvm::StringRef Path8) { - llvm::SmallVector Path16; - if (windows::widenPath(Path8, Path16)) - return false; - - SmallVector FullPath16; - if (!getFullPath(Path16, FullPath16)) - return false; - // Get volume root. llvm::SmallVector VolumeRoot16; - if (std::error_code EC = - windows::getVolumeRootFromPath(FullPath16, VolumeRoot16)) + if (windows::widenPath(path::root_path(Path8), VolumeRoot16)) return false; // Attempt to load AreShortNamesEnabled dynamically (it's avalible only on @@ -2565,7 +2522,7 @@ static std::string getShortPathUtf8(llvm::StringRef Path8) { static std::string stripPrefix(llvm::StringRef P) { if (P.starts_with(R"(\\?\UNC\)")) - return R"\\" + P.drop_front(8).str(); // or "\\" + P.drop_front(7).str(); + return "\\" + P.drop_front(7).str(); if (P.starts_with(R"(\\?\)")) return P.drop_front(4).str(); return P.str(); From 649e0fa1700ef5fda5a9866e79a27b58edeef33d Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Wed, 18 Jun 2025 10:17:17 +0100 Subject: [PATCH 12/20] Simplify python in lit config by calling PathLib.anchor rather than GetVolumePathNameW --- cross-project-tests/lit.site.cfg.py.in | 32 ++++++++++---------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/cross-project-tests/lit.site.cfg.py.in b/cross-project-tests/lit.site.cfg.py.in index a097d500e50bc..c15e3d1ed677b 100644 --- a/cross-project-tests/lit.site.cfg.py.in +++ b/cross-project-tests/lit.site.cfg.py.in @@ -27,35 +27,27 @@ lit.llvm.initialize(lit_config, config) # Let the main config do the real work. lit_config.load_config(config, "@CROSS_PROJECT_TESTS_SOURCE_DIR@/lit.cfg.py") -def is_short_names_enabled(path_utf8: str) -> bool: - import os, ctypes; from ctypes import wintypes - - k32 = ctypes.WinDLL("kernel32", use_last_error=True) +def are_short_names_enabled(p): + import os, ctypes; from ctypes import wintypes; from pathlib import Path try: - fn = k32.AreShortNamesEnabled + fn = ctypes.WinDLL("kernel32", use_last_error=True).AreShortNamesEnabled fn.restype = wintypes.BOOL fn.argtypes = [wintypes.HANDLE, ctypes.POINTER(wintypes.BOOL)] except AttributeError: return False - - path_w = ctypes.create_unicode_buffer(os.path.abspath(path_utf8), 260) - vol_w = ctypes.create_unicode_buffer(260) - if not k32.GetVolumePathNameW(path_w, vol_w, 260): - return False - # 0x80000000 = GENERIC_READ # 0x7 = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE # 3 = OPEN_EXISTING # 0x02000000 = FILE_FLAG_BACKUP_SEMANTICS - handle = k32.CreateFileW( - vol_w, 0x80000000, 0x7, None, 3, 0x02000000, None) - if handle == ctypes.c_void_p(-1).value: + h = ctypes.windll.kernel32.CreateFileW( + str(Path(p).resolve().anchor), + 0x80000000, 0x7, None, 3, 0x02000000, None) + if h == ctypes.c_void_p(-1).value: return False + r = wintypes.BOOL(); + ok = fn(h, ctypes.byref(r)) + ctypes.windll.kernel32.CloseHandle(h) + return bool(ok and r.value) - res = wintypes.BOOL() - ok = fn(handle, ctypes.byref(res)) - k32.CloseHandle(handle) - return bool(ok and res.value) - -if os.name == "nt" and is_short_names_enabled(config.environment.get('TEMP')): +if os.name == "nt" and are_short_names_enabled(config.environment.get('TEMP')): config.available_features.add('shortpaths') From d102386fc2b7c5fe632b76755d2e94bd0d003e5e Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Wed, 18 Jun 2025 10:40:54 +0100 Subject: [PATCH 13/20] Remove archive test-case from LIT test. --- cross-project-tests/dtlto/path.test | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/cross-project-tests/dtlto/path.test b/cross-project-tests/dtlto/path.test index da1da4261a358..e134bddf6c40d 100644 --- a/cross-project-tests/dtlto/path.test +++ b/cross-project-tests/dtlto/path.test @@ -6,12 +6,10 @@ RUN: rm -rf %t && split-file %s %t && cd %t -# Create an archive to confirm unpacked member paths are expanded. RUN: prospero-clang --target=x86_64-linux-gnu -c start.c f.c -flto=thin -O2 -RUN: prospero-llvm-ar rcs libf.a f.o --thin RUN: %python in-tilde-dir.py \ -RUN: ld.lld start.o libf.a -o start.elf \ +RUN: ld.lld start.o f.o -o start.elf \ RUN: --thinlto-distributor=%python \ RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \ RUN: --thinlto-remote-compiler=%clang \ @@ -39,14 +37,13 @@ int f(); int _start() { return f(); } #--- in-tilde-dir.py -import os, shutil, sys, uuid, subprocess -from pathlib import Path +import os, shutil, sys, uuid, subprocess; from pathlib import Path temp = Path(os.environ['TEMP']) assert temp.is_dir() # Copy the CWD to a unique directory inside TEMP and determine its 8.3 form. -# TEMP is likely to be on a drive that supports long+short paths. +# TEMP is likely to be on a drive that supports 8.3 form paths. d = (temp / str(uuid.uuid4()) / 'veryverylong').resolve() d83 = d.parent / (d.name[:6] + '~1') d.parent.mkdir(parents=True) From 826a25061bf9e3327bf3483c62dd4a262e56c1de Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Wed, 18 Jun 2025 12:57:33 +0100 Subject: [PATCH 14/20] Improve variable naming in unit test --- llvm/unittests/Support/Path.cpp | 70 ++++++++++++++++----------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index a8a4614c118f3..735ba68aa234f 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -2489,7 +2489,7 @@ static bool isShortNameEnabledForPath(llvm::StringRef Path8) { /// Returns the 8.3 path for the given UTF-8 path, or an empty string /// on failure. Uses Win32 GetShortPathNameW. -static std::string getShortPathUtf8(llvm::StringRef Path8) { +static std::string getShortPathName(llvm::StringRef Path8) { using namespace llvm; // Convert UTF-8 to UTF-16 @@ -2533,8 +2533,8 @@ TEST_F(FileSystemTest, makeLong) { GTEST_SKIP() << "Short names not enabled on volume."; // Get a short-path version of the test directory - std::string Short8 = getShortPathUtf8(TestDirectory); - ASSERT_FALSE(Short8.empty()) + std::string Short = getShortPathName(TestDirectory); + ASSERT_FALSE(Short.empty()) << "Expected short path form for test directory."; // Setup: Create a path where even if all components were reduced to short @@ -2542,53 +2542,53 @@ TEST_F(FileSystemTest, makeLong) { // exceed MAX_PATH. constexpr const char *OneDir = "\\123456789"; // >8 chars const size_t NLevels = (MAX_PATH / 8) + 1; - SmallString Long(TestDirectory); + SmallString Max(TestDirectory); for (size_t I = 0; I < NLevels; ++I) - Long.append(OneDir); + Max.append(OneDir); - ASSERT_NO_ERROR(fs::create_directories(Long)); - std::string LongShort8WithPrefix = getShortPathUtf8(Long); - ASSERT_TRUE(StringRef(LongShort8WithPrefix).starts_with(R"(\\?\)")) - << "Expected prefixed short path, got: " << LongShort8WithPrefix; + ASSERT_NO_ERROR(fs::create_directories(Max)); + std::string MaxShortWithPrefix = getShortPathName(Max); + ASSERT_TRUE(StringRef(MaxShortWithPrefix).starts_with(R"(\\?\)")) + << "Expected prefixed short path, got: " << MaxShortWithPrefix; - std::string LongShort8 = stripPrefix(LongShort8WithPrefix); + std::string MaxShort = stripPrefix(MaxShortWithPrefix); - // === Case 1: Non-existent short path should fail === - SmallString<128> NoExist("No Existy~1 Path"); + // === Case 1: Non-existent short path === + SmallString<128> NoExist("NotEre~1"); SmallString<128> NoExistResult; EXPECT_TRUE(windows::makeLong(NoExist, NoExistResult)); EXPECT_TRUE(NoExistResult.empty()); - // === Case 2: Short path that exists, no prefix === + // === Case 2: Short path that exists === SmallString<128> ShortResult; - ASSERT_FALSE(windows::makeLong(Short8, ShortResult)); + ASSERT_FALSE(windows::makeLong(Short, ShortResult)); fs::UniqueID ShortID1, ShortID2; - ASSERT_NO_ERROR(fs::getUniqueID(Short8, ShortID1)); + ASSERT_NO_ERROR(fs::getUniqueID(Short, ShortID1)); ASSERT_NO_ERROR(fs::getUniqueID(ShortResult, ShortID2)); EXPECT_EQ(ShortID1, ShortID2); - // === Case 3: Long short path (no prefix) === - SmallString<128> LongResult; - ASSERT_FALSE(windows::makeLong(LongShort8, LongResult)); - fs::UniqueID LongID1, LongID2; - ASSERT_NO_ERROR(fs::getUniqueID(LongShort8, LongID1)); - ASSERT_NO_ERROR(fs::getUniqueID(LongResult, LongID2)); - EXPECT_EQ(LongID1, LongID2); - EXPECT_FALSE(StringRef(LongResult).starts_with(R"(\\?\)")) - << "Expected unprefixed result, got: " << LongResult; - - // === Case 4: Long short path with prefix === - SmallString<128> LongPrefixedResult; - ASSERT_FALSE(windows::makeLong(LongShort8WithPrefix, LongPrefixedResult)); - fs::UniqueID LongPrefixedID1, LongPrefixedID2; - ASSERT_NO_ERROR(fs::getUniqueID(LongShort8WithPrefix, LongPrefixedID1)); - ASSERT_NO_ERROR(fs::getUniqueID(LongPrefixedResult, LongPrefixedID2)); - EXPECT_EQ(LongPrefixedID1, LongPrefixedID2); - EXPECT_TRUE(StringRef(LongPrefixedResult).starts_with(R"(\\?\)")) - << "Expected prefixed result, got: " << LongPrefixedResult; + // === Case 3: Short path greater than MAX_PATH, no prefix === + SmallString<128> MaxResult; + ASSERT_FALSE(windows::makeLong(MaxShort, MaxResult)); + fs::UniqueID MaxID1, MaxID2; + ASSERT_NO_ERROR(fs::getUniqueID(MaxShort, MaxID1)); + ASSERT_NO_ERROR(fs::getUniqueID(MaxResult, MaxID2)); + EXPECT_EQ(MaxID1, MaxID2); + EXPECT_FALSE(StringRef(MaxResult).starts_with(R"(\\?\)")) + << "Expected unprefixed result, got: " << MaxResult; + + // === Case 4: Short path greater than MAX_PATH, with prefix === + SmallString<128> MaxPrefixedResult; + ASSERT_FALSE(windows::makeLong(MaxShortWithPrefix, MaxPrefixedResult)); + fs::UniqueID MaxPrefixedID1, MaxPrefixedID2; + ASSERT_NO_ERROR(fs::getUniqueID(MaxShortWithPrefix, MaxPrefixedID1)); + ASSERT_NO_ERROR(fs::getUniqueID(MaxPrefixedResult, MaxPrefixedID2)); + EXPECT_EQ(MaxPrefixedID1, MaxPrefixedID2); + EXPECT_TRUE(StringRef(MaxPrefixedResult).starts_with(R"(\\?\)")) + << "Expected prefixed result, got: " << MaxPrefixedResult; // Cleanup - ASSERT_NO_ERROR(fs::remove(Long)); + ASSERT_NO_ERROR(fs::remove(Max)); } // Windows refuses lock request if file region is already locked by the same From fa8c95faaa1c8856ad3a61b22d75af6b4a83374f Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Wed, 18 Jun 2025 13:38:30 +0100 Subject: [PATCH 15/20] Fix removal of test directories. --- llvm/unittests/Support/Path.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index 735ba68aa234f..7ee6134096579 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -2555,6 +2555,7 @@ TEST_F(FileSystemTest, makeLong) { // === Case 1: Non-existent short path === SmallString<128> NoExist("NotEre~1"); + ASSERT_FALSE(fs::exists(NoExist)); SmallString<128> NoExistResult; EXPECT_TRUE(windows::makeLong(NoExist, NoExistResult)); EXPECT_TRUE(NoExistResult.empty()); @@ -2588,7 +2589,7 @@ TEST_F(FileSystemTest, makeLong) { << "Expected prefixed result, got: " << MaxPrefixedResult; // Cleanup - ASSERT_NO_ERROR(fs::remove(Max)); + ASSERT_NO_ERROR(fs::remove_directories(TestDirectory.str())); } // Windows refuses lock request if file region is already locked by the same From 0026ef66b87e52c79d88457f020545f56513b119 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Wed, 18 Jun 2025 14:49:18 +0100 Subject: [PATCH 16/20] Improve LIT test. Use more meaningful names and call GetShortPathNameW rather than guessing the 8.3 short-form name --- cross-project-tests/dtlto/path.test | 26 +++++++++++++++++++------- cross-project-tests/lit.site.cfg.py.in | 2 +- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/cross-project-tests/dtlto/path.test b/cross-project-tests/dtlto/path.test index e134bddf6c40d..901b11abdadbc 100644 --- a/cross-project-tests/dtlto/path.test +++ b/cross-project-tests/dtlto/path.test @@ -1,4 +1,4 @@ -# REQUIRES: x86-registered-target,ld.lld,llvm-ar,system-windows,shortpaths +# REQUIRES: x86-registered-target,ld.lld,llvm-ar,system-windows,tempshortpaths # Check that any shortened "8.3" paths containing '~' characters are expanded, # and their path seperators changed to Windows style ones, prior to being passed @@ -8,7 +8,7 @@ RUN: rm -rf %t && split-file %s %t && cd %t RUN: prospero-clang --target=x86_64-linux-gnu -c start.c f.c -flto=thin -O2 -RUN: %python in-tilde-dir.py \ +RUN: %python in-83-dir.py \ RUN: ld.lld start.o f.o -o start.elf \ RUN: --thinlto-distributor=%python \ RUN: --thinlto-distributor-arg=%llvm_src_root/utils/dtlto/local.py \ @@ -36,24 +36,36 @@ int f() { return _start(); } int f(); int _start() { return f(); } -#--- in-tilde-dir.py +#--- in-83-dir.py import os, shutil, sys, uuid, subprocess; from pathlib import Path +import ctypes; from ctypes import wintypes + +def get_short_path(p): + g = ctypes.windll.kernel32.GetShortPathNameW + g.argtypes = [wintypes.LPCWSTR, wintypes.LPWSTR, wintypes.DWORD] + g.restype = wintypes.DWORD + n = g(os.path.abspath(p), None, 0) # First call gets required buffer size. + b = ctypes.create_unicode_buffer(n) + if g(os.path.abspath(p), b, n): # Second call fills buffer. + return b.value + raise ctypes.WinError() temp = Path(os.environ['TEMP']) assert temp.is_dir() -# Copy the CWD to a unique directory inside TEMP and determine its 8.3 form. +# Copy the CWD to a unique directory inside TEMP and ensure that one of the path +# components is long enough to have a distinct 8.3 form. # TEMP is likely to be on a drive that supports 8.3 form paths. d = (temp / str(uuid.uuid4()) / 'veryverylong').resolve() -d83 = d.parent / (d.name[:6] + '~1') d.parent.mkdir(parents=True) try: shutil.copytree(Path.cwd(), d) # Replace the arguments of the command that name files with equivalents in # our temp directory, prefixed with the 8.3 form. - cmd = [a if not Path(a).is_file() else str(d83/a) \ - for a in sys.argv[1:]] + cmd = [a if Path(a).is_absolute() or not (d/a).is_file() + else get_short_path(d/a) for a in sys.argv[1:]] + print(cmd) sys.exit(subprocess.run(cmd).returncode) diff --git a/cross-project-tests/lit.site.cfg.py.in b/cross-project-tests/lit.site.cfg.py.in index c15e3d1ed677b..d6a2956ef9682 100644 --- a/cross-project-tests/lit.site.cfg.py.in +++ b/cross-project-tests/lit.site.cfg.py.in @@ -50,4 +50,4 @@ def are_short_names_enabled(p): return bool(ok and r.value) if os.name == "nt" and are_short_names_enabled(config.environment.get('TEMP')): - config.available_features.add('shortpaths') + config.available_features.add('tempshortpaths') From 7227c200852c605f7c98d3c9f455be35f93711d1 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Wed, 18 Jun 2025 16:01:35 +0100 Subject: [PATCH 17/20] Do some manual testing to ensure that UNC paths are handled as expected. I wasn't able to figure out how to test this without admin permissions for the tests. Therefore, I'm not going to try to add automated tests for this. --- cross-project-tests/dtlto/path.test | 12 +++++++++-- llvm/unittests/Support/Path.cpp | 33 +++++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/cross-project-tests/dtlto/path.test b/cross-project-tests/dtlto/path.test index 901b11abdadbc..76392a8a9efc5 100644 --- a/cross-project-tests/dtlto/path.test +++ b/cross-project-tests/dtlto/path.test @@ -50,13 +50,20 @@ def get_short_path(p): return b.value raise ctypes.WinError() +def replace_drive_with_admin_share(path: str) -> str: + p = Path(path).resolve() + if p.drive and len(p.drive) == 2 and p.drive[1] == ':': + share = f"\\\\localhost\\{p.drive[0]}$" + return os.path.join(share, *p.parts[1:]) + return str(p) + temp = Path(os.environ['TEMP']) assert temp.is_dir() # Copy the CWD to a unique directory inside TEMP and ensure that one of the path # components is long enough to have a distinct 8.3 form. # TEMP is likely to be on a drive that supports 8.3 form paths. -d = (temp / str(uuid.uuid4()) / 'veryverylong').resolve() +d = (temp / str(uuid.uuid4()) / 'veryverylong123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789').resolve() d.parent.mkdir(parents=True) try: shutil.copytree(Path.cwd(), d) @@ -64,9 +71,10 @@ try: # Replace the arguments of the command that name files with equivalents in # our temp directory, prefixed with the 8.3 form. cmd = [a if Path(a).is_absolute() or not (d/a).is_file() - else get_short_path(d/a) for a in sys.argv[1:]] + else replace_drive_with_admin_share(get_short_path(d/a)) for a in sys.argv[1:]] print(cmd) + #assert(cmd == "bob!") sys.exit(subprocess.run(cmd).returncode) diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index 7ee6134096579..042f96106315e 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -2528,6 +2528,21 @@ static std::string stripPrefix(llvm::StringRef P) { return P.str(); } +void swapDriveLetterWithUNC(llvm::SmallString &Max) { + if (Max.size() >= 2 && llvm::isAlpha(Max[0]) && Max[1] == ':') { + llvm::SmallString<16> UNC; + UNC.append("\\\\localhost\\"); + UNC += Max[0]; // drive letter + UNC += '$'; + + // Remove the first two characters (e.g., "C:") + Max.erase(Max.begin(), Max.begin() + 2); + + // Prepend UNC + Max.insert(Max.begin(), UNC.begin(), UNC.end()); + } +} + TEST_F(FileSystemTest, makeLong) { if (!isShortNameEnabledForPath(TestDirectory.str())) GTEST_SKIP() << "Short names not enabled on volume."; @@ -2546,6 +2561,10 @@ TEST_F(FileSystemTest, makeLong) { for (size_t I = 0; I < NLevels; ++I) Max.append(OneDir); + swapDriveLetterWithUNC(Max); + + std::cout << "Max: " << Max.str().str() << "\n"; + ASSERT_NO_ERROR(fs::create_directories(Max)); std::string MaxShortWithPrefix = getShortPathName(Max); ASSERT_TRUE(StringRef(MaxShortWithPrefix).starts_with(R"(\\?\)")) @@ -2553,14 +2572,14 @@ TEST_F(FileSystemTest, makeLong) { std::string MaxShort = stripPrefix(MaxShortWithPrefix); - // === Case 1: Non-existent short path === + // Case 1: Non-existent short path. SmallString<128> NoExist("NotEre~1"); ASSERT_FALSE(fs::exists(NoExist)); SmallString<128> NoExistResult; EXPECT_TRUE(windows::makeLong(NoExist, NoExistResult)); EXPECT_TRUE(NoExistResult.empty()); - // === Case 2: Short path that exists === + // Case 2: Short path that exists. SmallString<128> ShortResult; ASSERT_FALSE(windows::makeLong(Short, ShortResult)); fs::UniqueID ShortID1, ShortID2; @@ -2568,7 +2587,7 @@ TEST_F(FileSystemTest, makeLong) { ASSERT_NO_ERROR(fs::getUniqueID(ShortResult, ShortID2)); EXPECT_EQ(ShortID1, ShortID2); - // === Case 3: Short path greater than MAX_PATH, no prefix === + // Case 3: Short path greater than MAX_PATH, no prefix. SmallString<128> MaxResult; ASSERT_FALSE(windows::makeLong(MaxShort, MaxResult)); fs::UniqueID MaxID1, MaxID2; @@ -2578,7 +2597,10 @@ TEST_F(FileSystemTest, makeLong) { EXPECT_FALSE(StringRef(MaxResult).starts_with(R"(\\?\)")) << "Expected unprefixed result, got: " << MaxResult; - // === Case 4: Short path greater than MAX_PATH, with prefix === + + std::cout << "MaxResult: " << MaxResult.str().str() << "\n"; + + // Case 4: Short path greater than MAX_PATH, with prefix. SmallString<128> MaxPrefixedResult; ASSERT_FALSE(windows::makeLong(MaxShortWithPrefix, MaxPrefixedResult)); fs::UniqueID MaxPrefixedID1, MaxPrefixedID2; @@ -2588,6 +2610,9 @@ TEST_F(FileSystemTest, makeLong) { EXPECT_TRUE(StringRef(MaxPrefixedResult).starts_with(R"(\\?\)")) << "Expected prefixed result, got: " << MaxPrefixedResult; + + std::cout << "MaxPrefixedResult: " << MaxPrefixedResult.str().str() << "\n"; + // Cleanup ASSERT_NO_ERROR(fs::remove_directories(TestDirectory.str())); } From 2bf19f16c908c0643b9ed7422b6da16a7f06c911 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Wed, 18 Jun 2025 16:02:43 +0100 Subject: [PATCH 18/20] Revert "Do some manual testing to ensure that UNC paths are handled as expected. I wasn't able to figure out how to test this without admin permissions for the tests. Therefore, I'm not going to try to add automated tests for this." This reverts commit 7227c200852c605f7c98d3c9f455be35f93711d1. --- cross-project-tests/dtlto/path.test | 12 ++--------- llvm/unittests/Support/Path.cpp | 33 ++++------------------------- 2 files changed, 6 insertions(+), 39 deletions(-) diff --git a/cross-project-tests/dtlto/path.test b/cross-project-tests/dtlto/path.test index 76392a8a9efc5..901b11abdadbc 100644 --- a/cross-project-tests/dtlto/path.test +++ b/cross-project-tests/dtlto/path.test @@ -50,20 +50,13 @@ def get_short_path(p): return b.value raise ctypes.WinError() -def replace_drive_with_admin_share(path: str) -> str: - p = Path(path).resolve() - if p.drive and len(p.drive) == 2 and p.drive[1] == ':': - share = f"\\\\localhost\\{p.drive[0]}$" - return os.path.join(share, *p.parts[1:]) - return str(p) - temp = Path(os.environ['TEMP']) assert temp.is_dir() # Copy the CWD to a unique directory inside TEMP and ensure that one of the path # components is long enough to have a distinct 8.3 form. # TEMP is likely to be on a drive that supports 8.3 form paths. -d = (temp / str(uuid.uuid4()) / 'veryverylong123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789').resolve() +d = (temp / str(uuid.uuid4()) / 'veryverylong').resolve() d.parent.mkdir(parents=True) try: shutil.copytree(Path.cwd(), d) @@ -71,10 +64,9 @@ try: # Replace the arguments of the command that name files with equivalents in # our temp directory, prefixed with the 8.3 form. cmd = [a if Path(a).is_absolute() or not (d/a).is_file() - else replace_drive_with_admin_share(get_short_path(d/a)) for a in sys.argv[1:]] + else get_short_path(d/a) for a in sys.argv[1:]] print(cmd) - #assert(cmd == "bob!") sys.exit(subprocess.run(cmd).returncode) diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index 042f96106315e..7ee6134096579 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -2528,21 +2528,6 @@ static std::string stripPrefix(llvm::StringRef P) { return P.str(); } -void swapDriveLetterWithUNC(llvm::SmallString &Max) { - if (Max.size() >= 2 && llvm::isAlpha(Max[0]) && Max[1] == ':') { - llvm::SmallString<16> UNC; - UNC.append("\\\\localhost\\"); - UNC += Max[0]; // drive letter - UNC += '$'; - - // Remove the first two characters (e.g., "C:") - Max.erase(Max.begin(), Max.begin() + 2); - - // Prepend UNC - Max.insert(Max.begin(), UNC.begin(), UNC.end()); - } -} - TEST_F(FileSystemTest, makeLong) { if (!isShortNameEnabledForPath(TestDirectory.str())) GTEST_SKIP() << "Short names not enabled on volume."; @@ -2561,10 +2546,6 @@ TEST_F(FileSystemTest, makeLong) { for (size_t I = 0; I < NLevels; ++I) Max.append(OneDir); - swapDriveLetterWithUNC(Max); - - std::cout << "Max: " << Max.str().str() << "\n"; - ASSERT_NO_ERROR(fs::create_directories(Max)); std::string MaxShortWithPrefix = getShortPathName(Max); ASSERT_TRUE(StringRef(MaxShortWithPrefix).starts_with(R"(\\?\)")) @@ -2572,14 +2553,14 @@ TEST_F(FileSystemTest, makeLong) { std::string MaxShort = stripPrefix(MaxShortWithPrefix); - // Case 1: Non-existent short path. + // === Case 1: Non-existent short path === SmallString<128> NoExist("NotEre~1"); ASSERT_FALSE(fs::exists(NoExist)); SmallString<128> NoExistResult; EXPECT_TRUE(windows::makeLong(NoExist, NoExistResult)); EXPECT_TRUE(NoExistResult.empty()); - // Case 2: Short path that exists. + // === Case 2: Short path that exists === SmallString<128> ShortResult; ASSERT_FALSE(windows::makeLong(Short, ShortResult)); fs::UniqueID ShortID1, ShortID2; @@ -2587,7 +2568,7 @@ TEST_F(FileSystemTest, makeLong) { ASSERT_NO_ERROR(fs::getUniqueID(ShortResult, ShortID2)); EXPECT_EQ(ShortID1, ShortID2); - // Case 3: Short path greater than MAX_PATH, no prefix. + // === Case 3: Short path greater than MAX_PATH, no prefix === SmallString<128> MaxResult; ASSERT_FALSE(windows::makeLong(MaxShort, MaxResult)); fs::UniqueID MaxID1, MaxID2; @@ -2597,10 +2578,7 @@ TEST_F(FileSystemTest, makeLong) { EXPECT_FALSE(StringRef(MaxResult).starts_with(R"(\\?\)")) << "Expected unprefixed result, got: " << MaxResult; - - std::cout << "MaxResult: " << MaxResult.str().str() << "\n"; - - // Case 4: Short path greater than MAX_PATH, with prefix. + // === Case 4: Short path greater than MAX_PATH, with prefix === SmallString<128> MaxPrefixedResult; ASSERT_FALSE(windows::makeLong(MaxShortWithPrefix, MaxPrefixedResult)); fs::UniqueID MaxPrefixedID1, MaxPrefixedID2; @@ -2610,9 +2588,6 @@ TEST_F(FileSystemTest, makeLong) { EXPECT_TRUE(StringRef(MaxPrefixedResult).starts_with(R"(\\?\)")) << "Expected prefixed result, got: " << MaxPrefixedResult; - - std::cout << "MaxPrefixedResult: " << MaxPrefixedResult.str().str() << "\n"; - // Cleanup ASSERT_NO_ERROR(fs::remove_directories(TestDirectory.str())); } From f3394d0a750c0e767ccdf63652433b6b3aaf6485 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Wed, 18 Jun 2025 16:16:22 +0100 Subject: [PATCH 19/20] Reduce the size of the unit test by using a helper function to check path equality. --- llvm/unittests/Support/Path.cpp | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index 7ee6134096579..23574888d8030 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -2528,6 +2528,13 @@ static std::string stripPrefix(llvm::StringRef P) { return P.str(); } +static void verifyPathEquality(std::string& S1, SmallString<128>& S2) { + fs::UniqueID ID1, ID2; + ASSERT_NO_ERROR(fs::getUniqueID(S1, ID1)); + ASSERT_NO_ERROR(fs::getUniqueID(S2, ID2)); + EXPECT_EQ(ID1, ID2); +} + TEST_F(FileSystemTest, makeLong) { if (!isShortNameEnabledForPath(TestDirectory.str())) GTEST_SKIP() << "Short names not enabled on volume."; @@ -2553,38 +2560,29 @@ TEST_F(FileSystemTest, makeLong) { std::string MaxShort = stripPrefix(MaxShortWithPrefix); - // === Case 1: Non-existent short path === + // Case 1: Non-existent short path. SmallString<128> NoExist("NotEre~1"); ASSERT_FALSE(fs::exists(NoExist)); SmallString<128> NoExistResult; EXPECT_TRUE(windows::makeLong(NoExist, NoExistResult)); EXPECT_TRUE(NoExistResult.empty()); - // === Case 2: Short path that exists === + // Case 2: Short path that exists. SmallString<128> ShortResult; ASSERT_FALSE(windows::makeLong(Short, ShortResult)); - fs::UniqueID ShortID1, ShortID2; - ASSERT_NO_ERROR(fs::getUniqueID(Short, ShortID1)); - ASSERT_NO_ERROR(fs::getUniqueID(ShortResult, ShortID2)); - EXPECT_EQ(ShortID1, ShortID2); + verifyPathEquality(Short, ShortResult); - // === Case 3: Short path greater than MAX_PATH, no prefix === + // Case 3: Short path greater than MAX_PATH, no prefix. SmallString<128> MaxResult; ASSERT_FALSE(windows::makeLong(MaxShort, MaxResult)); - fs::UniqueID MaxID1, MaxID2; - ASSERT_NO_ERROR(fs::getUniqueID(MaxShort, MaxID1)); - ASSERT_NO_ERROR(fs::getUniqueID(MaxResult, MaxID2)); - EXPECT_EQ(MaxID1, MaxID2); + verifyPathEquality(MaxShort, MaxResult); EXPECT_FALSE(StringRef(MaxResult).starts_with(R"(\\?\)")) << "Expected unprefixed result, got: " << MaxResult; - // === Case 4: Short path greater than MAX_PATH, with prefix === + // Case 4: Short path greater than MAX_PATH, with prefix. SmallString<128> MaxPrefixedResult; ASSERT_FALSE(windows::makeLong(MaxShortWithPrefix, MaxPrefixedResult)); - fs::UniqueID MaxPrefixedID1, MaxPrefixedID2; - ASSERT_NO_ERROR(fs::getUniqueID(MaxShortWithPrefix, MaxPrefixedID1)); - ASSERT_NO_ERROR(fs::getUniqueID(MaxPrefixedResult, MaxPrefixedID2)); - EXPECT_EQ(MaxPrefixedID1, MaxPrefixedID2); + verifyPathEquality(MaxShortWithPrefix, MaxPrefixedResult); EXPECT_TRUE(StringRef(MaxPrefixedResult).starts_with(R"(\\?\)")) << "Expected prefixed result, got: " << MaxPrefixedResult; From 057b7c3710f262ef8483282ec33daf999bfdeba7 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Wed, 18 Jun 2025 16:16:22 +0100 Subject: [PATCH 20/20] Use a simple filesystem test rather than AreShortNamesEnabled Also: - Formatted all python with "black" - Minor test comment, naming and structure improvements. --- cross-project-tests/dtlto/path.test | 24 +++-- cross-project-tests/lit.site.cfg.py.in | 38 +++---- llvm/unittests/Support/Path.cpp | 133 ++++++++++++------------- 3 files changed, 91 insertions(+), 104 deletions(-) diff --git a/cross-project-tests/dtlto/path.test b/cross-project-tests/dtlto/path.test index 901b11abdadbc..2cea9dbe6763a 100644 --- a/cross-project-tests/dtlto/path.test +++ b/cross-project-tests/dtlto/path.test @@ -37,38 +37,44 @@ int f(); int _start() { return f(); } #--- in-83-dir.py -import os, shutil, sys, uuid, subprocess; from pathlib import Path -import ctypes; from ctypes import wintypes +import os, shutil, sys, uuid, subprocess +from pathlib import Path +import ctypes +from ctypes import wintypes + def get_short_path(p): g = ctypes.windll.kernel32.GetShortPathNameW g.argtypes = [wintypes.LPCWSTR, wintypes.LPWSTR, wintypes.DWORD] g.restype = wintypes.DWORD - n = g(os.path.abspath(p), None, 0) # First call gets required buffer size. + n = g(os.path.abspath(p), None, 0) # First call gets required buffer size. b = ctypes.create_unicode_buffer(n) - if g(os.path.abspath(p), b, n): # Second call fills buffer. + if g(os.path.abspath(p), b, n): # Second call fills buffer. return b.value raise ctypes.WinError() -temp = Path(os.environ['TEMP']) + +temp = Path(os.environ["TEMP"]) assert temp.is_dir() # Copy the CWD to a unique directory inside TEMP and ensure that one of the path # components is long enough to have a distinct 8.3 form. # TEMP is likely to be on a drive that supports 8.3 form paths. -d = (temp / str(uuid.uuid4()) / 'veryverylong').resolve() +d = (temp / str(uuid.uuid4()) / "veryverylong").resolve() d.parent.mkdir(parents=True) try: shutil.copytree(Path.cwd(), d) # Replace the arguments of the command that name files with equivalents in # our temp directory, prefixed with the 8.3 form. - cmd = [a if Path(a).is_absolute() or not (d/a).is_file() - else get_short_path(d/a) for a in sys.argv[1:]] + cmd = [ + a if Path(a).is_absolute() or not (d / a).is_file() else get_short_path(d / a) + for a in sys.argv[1:] + ] print(cmd) sys.exit(subprocess.run(cmd).returncode) finally: - shutil.rmtree(d.parent) + shutil.rmtree(d.parent) \ No newline at end of file diff --git a/cross-project-tests/lit.site.cfg.py.in b/cross-project-tests/lit.site.cfg.py.in index d6a2956ef9682..0b8651119951b 100644 --- a/cross-project-tests/lit.site.cfg.py.in +++ b/cross-project-tests/lit.site.cfg.py.in @@ -1,7 +1,8 @@ @LIT_SITE_CFG_IN_HEADER@ -import sys +import shutil, sys, os, uuid import lit.util +from pathlib import Path config.targets_to_build = "@TARGETS_TO_BUILD@".split() config.llvm_src_root = "@LLVM_SOURCE_DIR@" @@ -22,32 +23,21 @@ config.mlir_src_root = "@MLIR_SOURCE_DIR@" config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@" import lit.llvm + lit.llvm.initialize(lit_config, config) # Let the main config do the real work. lit_config.load_config(config, "@CROSS_PROJECT_TESTS_SOURCE_DIR@/lit.cfg.py") -def are_short_names_enabled(p): - import os, ctypes; from ctypes import wintypes; from pathlib import Path + +def are_short_names_enabled(path): + d = Path(path) / str(uuid.uuid4()) / "verylongdir" + d.mkdir(parents=True) try: - fn = ctypes.WinDLL("kernel32", use_last_error=True).AreShortNamesEnabled - fn.restype = wintypes.BOOL - fn.argtypes = [wintypes.HANDLE, ctypes.POINTER(wintypes.BOOL)] - except AttributeError: - return False - # 0x80000000 = GENERIC_READ - # 0x7 = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE - # 3 = OPEN_EXISTING - # 0x02000000 = FILE_FLAG_BACKUP_SEMANTICS - h = ctypes.windll.kernel32.CreateFileW( - str(Path(p).resolve().anchor), - 0x80000000, 0x7, None, 3, 0x02000000, None) - if h == ctypes.c_void_p(-1).value: - return False - r = wintypes.BOOL(); - ok = fn(h, ctypes.byref(r)) - ctypes.windll.kernel32.CloseHandle(h) - return bool(ok and r.value) - -if os.name == "nt" and are_short_names_enabled(config.environment.get('TEMP')): - config.available_features.add('tempshortpaths') + return (d.parent / "verylo~1").exists() + finally: + shutil.rmtree(d.parent) + + +if os.name == "nt" and are_short_names_enabled(config.environment.get("TEMP")): + config.available_features.add("tempshortpaths") diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp index 23574888d8030..0aafc78210bce 100644 --- a/llvm/unittests/Support/Path.cpp +++ b/llvm/unittests/Support/Path.cpp @@ -2450,54 +2450,35 @@ TEST_F(FileSystemTest, widenPath) { #ifdef _WIN32 -/// Checks whether short (8.3) names are enabled for the volume containing the -/// given UTF-8 path. -static bool isShortNameEnabledForPath(llvm::StringRef Path8) { - - llvm::SmallVector VolumeRoot16; - if (windows::widenPath(path::root_path(Path8), VolumeRoot16)) - return false; - - // Attempt to load AreShortNamesEnabled dynamically (it's avalible only on - // some versions of Windows). - using AreShortNamesEnabledFn = BOOL(WINAPI *)(HANDLE, BOOL *); - auto Kernel32 = ::GetModuleHandleW(L"kernel32.dll"); - if (!Kernel32) - return false; - - auto *FnPtr = - reinterpret_cast(reinterpret_cast( - ::GetProcAddress(Kernel32, "AreShortNamesEnabled"))); - if (!FnPtr) - return false; - - // Open the volume root for querying. - HANDLE VolumeHandle = ::CreateFileW( - VolumeRoot16.data(), GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, nullptr, - OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, nullptr); - - if (VolumeHandle == INVALID_HANDLE_VALUE) +/// Checks whether short (8.3) names are enabled in the given UTF-8 path. This +/// is the best way I have found of checking this. Note that 8.3 names can be +/// enabled and disabled on a per-folder, per-volume, and per-system basis. +static bool areShortNamesEnabled(llvm::StringRef Path8) { + // Create a directory under Path8 with a name long enough that Windows will + // provide a shortened 8.3 form name, if 8.3 names are enabled. + SmallString<256> Dir(Path8); + path::append(Dir, "verylongdir"); + if (std::error_code EC = fs::create_directories(Dir)) return false; - BOOL Enabled = FALSE; - bool Success = FnPtr(static_cast(VolumeHandle), &Enabled); - ::CloseHandle(VolumeHandle); + // Check if the expected 8.3 form name was created. + SmallString<256> Short(Path8); + path::append(Short, "verylo~1"); + bool Result = fs::is_directory(Short); - return Success && Enabled; + fs::remove_directories(Dir); + return Result; } -/// Returns the 8.3 path for the given UTF-8 path, or an empty string -/// on failure. Uses Win32 GetShortPathNameW. +/// Returns the 8.3 path for the given UTF-8 path, or an empty string on +/// failure. Uses Win32 GetShortPathNameW. static std::string getShortPathName(llvm::StringRef Path8) { - using namespace llvm; - - // Convert UTF-8 to UTF-16 + // Convert UTF-8 to UTF-16. SmallVector Path16; if (std::error_code EC = sys::windows::widenPath(Path8, Path16)) return {}; - // Get required buffer size for short path (includes null terminator) + // Get required buffer size for short path (includes null terminator). DWORD Required = ::GetShortPathNameW(Path16.data(), nullptr, 0); if (Required == 0) return {}; @@ -2520,6 +2501,8 @@ static std::string getShortPathName(llvm::StringRef Path8) { return std::string(Utf8Result); } +/// Removes the Windows extended path prefix (\\?\ or \\?\UNC\) from the given +/// UTF-8 path, if present. static std::string stripPrefix(llvm::StringRef P) { if (P.starts_with(R"(\\?\UNC\)")) return "\\" + P.drop_front(7).str(); @@ -2528,7 +2511,9 @@ static std::string stripPrefix(llvm::StringRef P) { return P.str(); } -static void verifyPathEquality(std::string& S1, SmallString<128>& S2) { +/// Verifies that the two paths refer to the same file or directory by comparing +/// their UniqueIDs. +static void verifyPathEquality(std::string &S1, SmallString<128> &S2) { fs::UniqueID ID1, ID2; ASSERT_NO_ERROR(fs::getUniqueID(S1, ID1)); ASSERT_NO_ERROR(fs::getUniqueID(S2, ID2)); @@ -2536,29 +2521,35 @@ static void verifyPathEquality(std::string& S1, SmallString<128>& S2) { } TEST_F(FileSystemTest, makeLong) { - if (!isShortNameEnabledForPath(TestDirectory.str())) - GTEST_SKIP() << "Short names not enabled on volume."; + if (!areShortNamesEnabled(TestDirectory.str())) + GTEST_SKIP() << "Short names not enabled in: " << TestDirectory; - // Get a short-path version of the test directory - std::string Short = getShortPathName(TestDirectory); - ASSERT_FALSE(Short.empty()) - << "Expected short path form for test directory."; + // Setup: A test directory longer than 8 characters for which a distinct + // short form will be created on Windows. Typically, 123456~1. + constexpr const char *OneDir = "\\123456789"; // >8 chars // Setup: Create a path where even if all components were reduced to short - // form (typically 8 characters e.g. 123456~1) the total length would - // exceed MAX_PATH. - constexpr const char *OneDir = "\\123456789"; // >8 chars + // form, the total length would exceed MAX_PATH. + SmallString Deep(TestDirectory); const size_t NLevels = (MAX_PATH / 8) + 1; - SmallString Max(TestDirectory); for (size_t I = 0; I < NLevels; ++I) - Max.append(OneDir); + Deep.append(OneDir); + + ASSERT_NO_ERROR(fs::create_directories(Deep)); - ASSERT_NO_ERROR(fs::create_directories(Max)); - std::string MaxShortWithPrefix = getShortPathName(Max); - ASSERT_TRUE(StringRef(MaxShortWithPrefix).starts_with(R"(\\?\)")) - << "Expected prefixed short path, got: " << MaxShortWithPrefix; + // Setup: Create prefixed and non-prefixed 8.3 forms of the deep test path we + // just created. + std::string DeepShortWithPrefix = getShortPathName(Deep); + ASSERT_TRUE(StringRef(DeepShortWithPrefix).starts_with(R"(\\?\)")) + << "Expected prefixed short path, got: " << DeepShortWithPrefix; + std::string DeepShort = stripPrefix(DeepShortWithPrefix); - std::string MaxShort = stripPrefix(MaxShortWithPrefix); + // Setup: Create an 8.3-form short path for the first-level directory. + SmallString<256> FirstLevel(TestDirectory); + FirstLevel.append(OneDir); + std::string Short = getShortPathName(FirstLevel); + ASSERT_FALSE(Short.empty()) + << "Expected 8.3 short path form for test directory."; // Case 1: Non-existent short path. SmallString<128> NoExist("NotEre~1"); @@ -2567,26 +2558,26 @@ TEST_F(FileSystemTest, makeLong) { EXPECT_TRUE(windows::makeLong(NoExist, NoExistResult)); EXPECT_TRUE(NoExistResult.empty()); - // Case 2: Short path that exists. + // Case 2: Valid short path. SmallString<128> ShortResult; ASSERT_FALSE(windows::makeLong(Short, ShortResult)); verifyPathEquality(Short, ShortResult); - // Case 3: Short path greater than MAX_PATH, no prefix. - SmallString<128> MaxResult; - ASSERT_FALSE(windows::makeLong(MaxShort, MaxResult)); - verifyPathEquality(MaxShort, MaxResult); - EXPECT_FALSE(StringRef(MaxResult).starts_with(R"(\\?\)")) - << "Expected unprefixed result, got: " << MaxResult; - - // Case 4: Short path greater than MAX_PATH, with prefix. - SmallString<128> MaxPrefixedResult; - ASSERT_FALSE(windows::makeLong(MaxShortWithPrefix, MaxPrefixedResult)); - verifyPathEquality(MaxShortWithPrefix, MaxPrefixedResult); - EXPECT_TRUE(StringRef(MaxPrefixedResult).starts_with(R"(\\?\)")) - << "Expected prefixed result, got: " << MaxPrefixedResult; - - // Cleanup + // Case 3: Deep short path without \\?\ prefix. + SmallString<128> DeepResult; + ASSERT_FALSE(windows::makeLong(DeepShort, DeepResult)); + verifyPathEquality(DeepShort, DeepResult); + EXPECT_FALSE(StringRef(DeepResult).starts_with(R"(\\?\)")) + << "Expected unprefixed result, got: " << DeepResult; + + // Case 4: Deep short path with \\?\ prefix. + SmallString<128> DeepPrefixedResult; + ASSERT_FALSE(windows::makeLong(DeepShortWithPrefix, DeepPrefixedResult)); + verifyPathEquality(DeepShortWithPrefix, DeepPrefixedResult); + EXPECT_TRUE(StringRef(DeepPrefixedResult).starts_with(R"(\\?\)")) + << "Expected prefixed result, got: " << DeepPrefixedResult; + + // Cleanup. ASSERT_NO_ERROR(fs::remove_directories(TestDirectory.str())); }