apple / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies. This fork is used to manage Apple’s stable releases of Clang as well as support the Swift project.
https://llvm.org
Other
1.1k stars 320 forks source link

[🍒 stable/20230725] [Support] Handle delete_pending case for Windows fs::status (#90655) #8850

Closed z2oh closed 4 weeks ago

z2oh commented 4 weeks ago

Explanation: Adds a new delete_pending error code which is returned from fs::status on Windows when the destination path is marked STATUS_DELETE_PENDING. Previously, this would return a misleading permission_denied error code. Scope: This change is needed for https://github.com/apple/llvm-project/pull/8848, which fixes a common compiler crash on Windows when indexing-while-building. Issue: https://github.com/llvm/llvm-project/issues/89137 Risk: Very low risk, this change introduces a new error code that can only be generated under specific conditions on Windows. Testing: Tested through LLVM CI and locally. Reviewer: @compnerd @bnbarham

If a delete is pending on the file queried for status, a misleading permission_denied error code will be returned (this is the correct mapping of the error set by GetFileAttributesW). By querying the underlying NTSTATUS code via ntdll's RtlGetLastNtStatus, this case can be disambiguated. If this underlying error code indicates a pending delete, fs::status will return a new pending_delete error code to be handled by callers

Fixes #89137

(cherry picked from commit cb7690af09b95bb944baf1b5a9ffb18f86c12130)

compnerd commented 4 weeks ago

@swift-ci please test

etcwilde commented 4 weeks ago
/usr/bin/ld: /home/build-user/build/Ninja-ReleaseAssert/libdispatch-linux-x86_64/src/swift/CMakeFiles/swiftDispatch.dir/Block.swift.o: relocation R_X86_64_PC32 against protected symbol `$s8Dispatch0A13WorkItemFlagsVSYAAMc' can not be used when making a shared object
/usr/bin/ld: final link failed: bad value

Checkout:

/tmp/jenkins-ceb2551a/workspace/pr-apple-llvm-project-linux/branch-stable/20230725/swift/utils/update-checkout --clone --reset-to-remote --scheme stable/20230725 --skip-repository llvm-project --github-comment '@swift-ci please test'

Okay, so you're picking up the new top-of-main swift-driver. This suggests that the build-script invocation isn't using one of the new presets, so it's either not checking out a new Swift checkout, or using a raw build-script invocation instead of going through a preset. The clang-linker would then default to use bfd....

+ docker run --name swift-pr --security-opt=no-new-privileges --cap-add=SYS_PTRACE --security-opt seccomp=unconfined -e SWIFT_SOURCE_ROOT -e SWIFT_BUILD_ROOT -w /home/build-user/ -v /tmp/jenkins-ceb2551a/workspace/pr-apple-llvm-project-linux/branch-stable/20230725:/source -v swift-pr:/home/build-user 608094965728.dkr.ecr.us-west-2.amazonaws.com/main-ci-ecr-8ce5c7b:swift-ci-ubuntu2004 /bin/bash -lc 'env; cp -r /source/* /home/build-user/; ls -al  /home/build-user/; cd /home/build-user/; ./swift/utils/build-script --foundation --libicu --libdispatch --test --release --lldb -- --skip-test-cmark --skip-test-swift --lldb-test-swift-only --skip-test-foundation'

And it looks like the second one is our issue... now to figure out how to change that build-script invocation to set the clang-linker-driver's default linker to not-bfd. 🤔

z2oh commented 4 weeks ago

Thanks for looking at that failure Evan!

now to figure out how to change that build-script invocation to set the clang-linker-driver's default linker to not-bfd.

Is this anything I could help out with? I'm afraid I don't have much context into how this CI is pieced together. (Is this stuff public-facing or internal to Apple?)

etcwilde commented 4 weeks ago

Is this anything I could help out with? I'm afraid I don't have much context into how this CI is pieced together. (Is this stuff public-facing or internal to Apple?)

I think this one is internal, unfortunately. There isn't a preset for this job so it will take modifying the CI job directly to fix it. CC @shahmishal

z2oh commented 4 weeks ago

Thanks for for fixing that Evan! Much appreciated. It looks like the CI jobs here pull from a branch in apple/swift, so re-triggering CI will pick up your changes?

etcwilde commented 4 weeks ago

@swift-ci please test Linux platform

etcwilde commented 4 weeks ago

Thanks for for fixing that Evan! Much appreciated. It looks like the CI jobs here pull from a branch in apple/swift, so re-triggering CI will pick up your changes?

Yeah, just landed the changes on the CI itself so it should pick up that preset now.