[XRay] Fix fdr-thread-order.cc when current directory contains fdr-thread-order.cc

[MaskRay]

Summary

Currently,

cd test/xray/TestCases/Posix
$build/bin/llvm-lit fdr-thread-order.cc

fails because rm fdr-thread-order.* deletes the .cc file.

This patch uses:

• %t as temporary directory name containing log files
• %t.exe as executable name

it does not delete %t after the test finishes for debugging convenience
if something goes wrong. This matches the behavior of many other LLVM
components.

[MaskRay]

Migrate from deprecaed options

[dberris]

Thanks for the patch -- some questions and suggestions below.

test/xray/TestCases/Posix/fdr-thread-order.cc

2 ↗

(On Diff #167418)

I'm not a fan of the .exe extension here -- consider -bin instead?

4–5 ↗

(On Diff #167418)

While we're here, can we change xray_fdr_log=true to xray_mode=xray-fdr in the XRAY_OPTIONS environment variable?

12 ↗

(On Diff #167418)

Unfortunately, doing this makes it really easy to clog up the filesystem with log files upon failure (which I learned the hard way while working on these). The suggestion has been to re-create the sequence by hand if we want to debug, rather than to keep them lying around when the tests fail.

Thoughts?

[MaskRay]

xray_fdr_log=true -> xray_mode=xray-fdr

[MaskRay]

test/xray/TestCases/Posix/fdr-thread-order.cc

2 ↗

(On Diff #167418)

I am not a fan of Windows but chose .exe because I've seen %t.exe used in llvm-readobj gold lld tests. They are also used in compiler-rt/test/asan/TestCases/{Posix,Linux,Darwin}. In ELF, such executables are called ET_EXEC before PIE era. The .exe suffix should not be too exotic.

12 ↗

(On Diff #167418)

If the tests use unique directory names to isolate, it shouldn't clog up the filesystem.

Are sizes of log files an issue? If not, I think not cleaning the temporary files should be favored, but right now tests put xray-log.* in the current directory, which easily lead to conflicts.

I've added back rm -rf %t here. We may discuss this later. If we reach a consensus, I can create a refactor revision for other tests as well, if needed.

[dberris]

LGTM

test/xray/TestCases/Posix/fdr-thread-order.cc

2 ↗

(On Diff #167418)

I see, that's a convincing reason to pick .exe too then. :)

12 ↗

(On Diff #167418)

It's both the number and the potential size(s) of the logs generated. Because each new invocation will attempt to create a unique filename, it will be a problem if files accumulate upon failures. I see that there's a cleanup of the directory at the beginning of the test which somewhat mitigates this, which I somehow thought was removed.

So I think leaving the files behind is fine given that we'll delete the directory anyway at the beginning.

compiler-rt/trunk/test/xray/TestCases/Posix/fdr-thread-order.cc

-// RUN: %clangxx_xray -g -std=c++11 %s -o %t
-// RUN: rm fdr-thread-order.* || true
-// RUN: XRAY_OPTIONS="patch_premain=false xray_naive_log=false \
-// RUN:    xray_logfile_base=fdr-thread-order. xray_fdr_log=true verbosity=1 \
-// RUN:    xray_fdr_log_func_duration_threshold_us=0" %run %t 2>&1 | \
+// RUN: rm -rf %t && mkdir %t
+// RUN: %clangxx_xray -g -std=c++11 %s -o %t.exe
+// RUN: XRAY_OPTIONS="patch_premain=false \
+// RUN:    xray_logfile_base=%t/ xray_mode=xray-fdr verbosity=1" \
+// RUN:    XRAY_FDR_OPTIONS=func_duration_threshold_us=0 %run %t.exe 2>&1 | \
 // RUN:    FileCheck %s
-// RUN: %llvm_xray convert --symbolize --output-format=yaml -instr_map=%t \
-// RUN:    "`ls fdr-thread-order.* | head -1`"
-// RUN: %llvm_xray convert --symbolize --output-format=yaml -instr_map=%t \
-// RUN:    "`ls fdr-thread-order.* | head -1`" | \
-// RUN:    FileCheck %s --check-prefix TRACE
-// RUN: rm fdr-thread-order.*
+// RUN: %llvm_xray convert --symbolize --output-format=yaml -instr_map=%t.exe %t/*
+// RUN: %llvm_xray convert --symbolize --output-format=yaml -instr_map=%t.exe %t/* | \
+// RUN:   FileCheck %s --check-prefix TRACE
 // FIXME: Make llvm-xray work on non-x86_64 as well.
 // REQUIRES: x86_64-target-arch
 // REQUIRES: built-in-llvm-tree
 
 #include "xray/xray_log_interface.h"
 #include <atomic>
 #include <cassert>
 #include <thread>