[PATCH] D31452: [XRay][compiler-rt] Add an end-to-end test for FDR Logging

[Keith via Phabricator via llvm-commits]

kpw added a comment.

Thanks for the quick fix and for figuring out the lit configuration.



================
Comment at: lib/xray/xray_fdr_logging.cc:65
   FDROptions.reset(new FDRLoggingOptions());
-  *FDROptions = *reinterpret_cast<FDRLoggingOptions *>(Options);
-  if (FDROptions->ReportErrors)
-    SetPrintfAndReportCallback(printToStdErr);
-
+  memcpy(FDROptions.get(), Options, OptionsSize);
   bool Success = false;
----------------
Cool. I didn't grok the proper use of the void * args in xray_log_interface.h or how to switch on fdr via flags and just use the top level interface.


================
Comment at: lib/xray/xray_utils.cc:96
 int getLogFD() XRAY_NEVER_INSTRUMENT {
   // FIXME: Figure out how to make this less stderr-dependent.
   // Open a temporary file once for the log.
----------------
This comment dangles now. There may still be a fix, necessary somewhere for how to configure Report, but the FIXME shouldn't be here.


================
Comment at: test/xray/TestCases/Linux/fdr-mode.cc:3
+// RUN: XRAY_OPTIONS="patch_premain=false xray_naive_log=false xray_logfile_base=fdr-logging-test- xray_fdr_log=true verbosity=1" %run %t 2>&1 | FileCheck %s
+// FIXME: %llvm_xray convert -instr_map=%t "`ls fdr-logging-test-* | head -1`" | FileCheck %s --check-prefix TRACE
+// RUN: rm fdr-logging-test-*
----------------
I suspect we'll lose the exit code due to unix pipes returning the final commands exit code.

If the command returns a TRACE prefix as expected, then crashes, we want a failure.
If the shell that runs these is guaranteed to be bash, then the pipefail builtin option could help.


================
Comment at: test/xray/TestCases/Linux/fdr-mode.cc:25-27
+[[clang::xray_always_instrument]] void __attribute__((noinline)) fD() {
+  std::this_thread::sleep_for(std::chrono::milliseconds(100));
+}
----------------
This is never called AFAICT.


================
Comment at: test/xray/TestCases/Linux/fdr-mode.cc:51
+    fA();
+    std::cout << "Thread execution var = " << var << std::endl;
+  });
----------------
Might as well lose this. It's not CHECKED.


https://reviews.llvm.org/D31452