[lld][test] Fix AArch64 build attribute test cleanup

[DavidSpickett]

This test has been reported to fail on a release bot, but nothing in the content of the test on the release branch should cause this problem:
ld.lld: error: cannot open output file /home/buildbot/as-worker-92/clang-with-thin-lto-ubuntu-rel/build/stage1/tools/lld/test/ELF/Output/aarch64-build-attributes.s.tmp: Is a directory

The reason this is happening is that if the same builder happened to run the extended version of this test, from a build of main, it would create a directory with that name. The test on main seemingly intends to stash files in that temporary directory (though not for all files, see #164396), but regardless, it creates the directory.

$ tree tools/lld/test/
tools/lld/test/
├── CMakeFiles
├── ELF
│   └── Output
│   ├── aarch64-build-attributes.s.tmp <<< This directory
│   │   ├── pauth-bti-gcs.s
│   │   └── pauth-bti-pac.s
│   ├── aarch64-build-attributes.s.tmp.merged.o
│   ├── aarch64-build-attributes.s.tmp1.o
│   ├── aarch64-build-attributes.s.tmp2.o
│   └── aarch64-build-attributes.s.tmp3.o
├── Unit
│   └── lit.site.cfg.py
├── cmake_install.cmake
└── lit.site.cfg.py

If you then had a 21.x build run, it would find that pre-existing directory and try to write to it as if it were a file.

ld.lld: error: cannot open output file /home/david.spickett/build-llvm-aarch64/tools/lld/test/ELF/Output/aarch64-build-attributes.s.tmp: Is a directory

To fix this, remove the file or directory names we're going to use, before the test starts.

(if this were main I'd put all the files in one direcory, but in the interest of keeping release changes small I'm just adding the rm line)

[llvmbot]

@llvm/pr-subscribers-lld-elf

Author: David Spickett (DavidSpickett)

Changes

This test has been reported to fail on a release bot, but nothing in the content of the test on the release branch should cause this problem: ld.lld: error: cannot open output file /home/buildbot/as-worker-92/clang-with-thin-lto-ubuntu-rel/build/stage1/tools/lld/test/ELF/Output/aarch64-build-attributes.s.tmp: Is a directory

The reason this is happening is that if the same builder happened to run the extended version of this test, from a build of main, it would create a directory with that name. The test on main stashes the temporary files in that directory.

$ tree tools/lld/test/
tools/lld/test/
├── CMakeFiles
├── ELF
│   └── Output
│   ├── aarch64-build-attributes.s.tmp
│   │   ├── pauth-bti-gcs.s
│   │   └── pauth-bti-pac.s
│   ├── aarch64-build-attributes.s.tmp.merged.o
│   ├── aarch64-build-attributes.s.tmp1.o
│   ├── aarch64-build-attributes.s.tmp2.o
│   └── aarch64-build-attributes.s.tmp3.o
├── Unit
│   └── lit.site.cfg.py
├── cmake_install.cmake
└── lit.site.cfg.py

If you then had a 21.x build run, it would find that pre-existing directory and try to write to it as if it were a file.

ld.lld: error: cannot open output file /home/david.spickett/build-llvm-aarch64/tools/lld/test/ELF/Output/aarch64-build-attributes.s.tmp: Is a directory

To fix this, remove the file or directory names we're going to use, before the test starts.

(if this were main I'd put all the files in one direcory, but in the interest of keeping release changes small I'm just adding the rm line)

---

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

1 Files Affected:

• (modified) lld/test/ELF/aarch64-build-attributes.s (+1)

diff --git a/lld/test/ELF/aarch64-build-attributes.s b/lld/test/ELF/aarch64-build-attributes.s
index 24e15f94e3d4a..815aed32f2aaa 100644
--- a/lld/test/ELF/aarch64-build-attributes.s
+++ b/lld/test/ELF/aarch64-build-attributes.s
@@ -1,4 +1,5 @@
 // REQUIRES: aarch64
+// RUN: rm -rf %t %t.o %t.so %t2.o
 // RUN: llvm-mc -triple=aarch64 %s -filetype=obj -o %t.o
 // RUN: ld.lld %t.o --shared -o %t.so
 // RUN: llvm-readelf --sections %t.so | FileCheck %s

[llvmbot]

@llvm/pr-subscribers-lld

Author: David Spickett (DavidSpickett)

Changes

This test has been reported to fail on a release bot, but nothing in the content of the test on the release branch should cause this problem: ld.lld: error: cannot open output file /home/buildbot/as-worker-92/clang-with-thin-lto-ubuntu-rel/build/stage1/tools/lld/test/ELF/Output/aarch64-build-attributes.s.tmp: Is a directory

The reason this is happening is that if the same builder happened to run the extended version of this test, from a build of main, it would create a directory with that name. The test on main stashes the temporary files in that directory.

$ tree tools/lld/test/
tools/lld/test/
├── CMakeFiles
├── ELF
│   └── Output
│   ├── aarch64-build-attributes.s.tmp
│   │   ├── pauth-bti-gcs.s
│   │   └── pauth-bti-pac.s
│   ├── aarch64-build-attributes.s.tmp.merged.o
│   ├── aarch64-build-attributes.s.tmp1.o
│   ├── aarch64-build-attributes.s.tmp2.o
│   └── aarch64-build-attributes.s.tmp3.o
├── Unit
│   └── lit.site.cfg.py
├── cmake_install.cmake
└── lit.site.cfg.py

If you then had a 21.x build run, it would find that pre-existing directory and try to write to it as if it were a file.

ld.lld: error: cannot open output file /home/david.spickett/build-llvm-aarch64/tools/lld/test/ELF/Output/aarch64-build-attributes.s.tmp: Is a directory

To fix this, remove the file or directory names we're going to use, before the test starts.

(if this were main I'd put all the files in one direcory, but in the interest of keeping release changes small I'm just adding the rm line)

---

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

1 Files Affected:

• (modified) lld/test/ELF/aarch64-build-attributes.s (+1)

diff --git a/lld/test/ELF/aarch64-build-attributes.s b/lld/test/ELF/aarch64-build-attributes.s
index 24e15f94e3d4a..815aed32f2aaa 100644
--- a/lld/test/ELF/aarch64-build-attributes.s
+++ b/lld/test/ELF/aarch64-build-attributes.s
@@ -1,4 +1,5 @@
 // REQUIRES: aarch64
+// RUN: rm -rf %t %t.o %t.so %t2.o
 // RUN: llvm-mc -triple=aarch64 %s -filetype=obj -o %t.o
 // RUN: ld.lld %t.o --shared -o %t.so
 // RUN: llvm-readelf --sections %t.so | FileCheck %s

[DavidSpickett]

I'm still not sure how that particular bot managed this, because the build path does have "rel" in it: clang-with-thin-lto-ubuntu-rel

And the non-release version builds in clang-with-thin-lto-ubuntu. But who knows maybe some symlink or configuration mistake that got fixed.

I confirmed locally that this problem can happen if you re-use the build directory.

[c-rhodes]

thanks for the quick fix! LGTM, but I wonder if it's worth fixing on main so this isn't an issue in the future? Grepping thru the tests it seems pretty common to do this

[DavidSpickett]

Main version: https://github.com/llvm/llvm-project/blob/main/lld/test/ELF/aarch64-build-attributes.s

Now I look closer, it's not putting all the files in that directory, though it claims to cd into it. I'll check it again locally.

[DavidSpickett]

#164396 (this wouldn't have fixed the issue on release, but I do think the authors did not intend the test to work that way)

[mstorsjo]

The test on main intends to stash all files in that temporary directory (it fails to, see #164396 for that fix), but regardless, it creates the directory.

As mentioned on the other review, I'm not sure if I'd read that much into it - and I think this kinda distracts from the core issue here; the old test produces a file named %t, while the new form on main creates % as a temp directory.

The change looks good to me though.

[mstorsjo]

LGTM, but I'd skip a couple sentences from the commit message.

[c-rhodes]

As mentioned on the other review, I'm not sure if I'd read that much into it - and I think this kinda distracts from the core issue here; the old test produces a file named %t, while the new form on main creates % as a temp directory.

ah ok, I wasn't aware of this when I mentioned also fixing on main, so it seems like this should only be a temporary issue for the current release. In that case I'll land this on the release branch and leave that other PR for you two to decide on

[DavidSpickett]

As mentioned on the other review, I'm not sure if I'd read that much into it - and I think this kinda distracts from the core issue here

I put that in for context to explain why the tree shown does not match what you (or at least I) would assume that test was doing. You're right that the point is it makes a directory, but, one might see the weird layout and say well why not fix it on main then. Which we are doing as well.

Idk, borderline but it's the kind of detail I will appreciate if this comes back up again in future. That said, I deal with random test failures all day, so I may have weird tastes there.