[PATCH] D34917: ELF: Only unlink regular files

[Rafael Avila de Espindola via llvm-commits]

LGTM

FileOutputBuffer has something similar, so this is all that we have to
do.

It is a pity that we have to unlink at all. I will do a quick benchmark
to see if rename without unlink is still slow.

Cheers,
Rafael

Tom Stellard via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> tstellar created this revision.
> Herald added a subscriber: emaste.
>
> If the output file is a character file (e.g. /dev/null) unlinking it could
> potentially destabilize the user's system.  For example,
> unlinking causes `lld %input -o /dev/null` to replace /dev/null with a
> regular file, which will lead to unexpected behavior in other programs
> that read from /dev/null, and worse than expected peformance for
> programs that write to /dev/null.
>
> This makes it possible to run the test-release.sh script as root.
> Prior to this patch, the ELF/basic.s test would replace
> /dev/null with a regular file, which would cause crashes in llvm
> test-suite programs that piped /dev/null to stdin.
>
> For example, if you run the test-relase.sh script as root,
>
>
> https://reviews.llvm.org/D34917
>
> Files:
>   ELF/Filesystem.cpp
>
>
> Index: ELF/Filesystem.cpp
> ===================================================================
> --- ELF/Filesystem.cpp
> +++ ELF/Filesystem.cpp
> @@ -38,7 +38,8 @@
>  // This function spawns a background thread to call unlink.
>  // The calling thread returns almost immediately.
>  void elf::unlinkAsync(StringRef Path) {
> -  if (!Config->Threads || !sys::fs::exists(Config->OutputFile))
> +  if (!Config->Threads || !sys::fs::exists(Config->OutputFile) ||
> +      !sys::fs::is_regular_file(Config->OutputFile))
>      return;
>  
>    // First, rename Path to avoid race condition. We cannot remove
>
>
> Index: ELF/Filesystem.cpp
> ===================================================================
> --- ELF/Filesystem.cpp
> +++ ELF/Filesystem.cpp
> @@ -38,7 +38,8 @@
>  // This function spawns a background thread to call unlink.
>  // The calling thread returns almost immediately.
>  void elf::unlinkAsync(StringRef Path) {
> -  if (!Config->Threads || !sys::fs::exists(Config->OutputFile))
> +  if (!Config->Threads || !sys::fs::exists(Config->OutputFile) ||
> +      !sys::fs::is_regular_file(Config->OutputFile))
>      return;
>  
>    // First, rename Path to avoid race condition. We cannot remove
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits