This is an archive of the discontinued LLVM Phabricator instance.

[XRay] Remove the deprecated __xray_log_init API
ClosedPublic

Authored by phosek on Sep 3 2018, 5:02 PM.

Details

Summary

This API has been deprecated three months ago and shouldn't be used
anymore, all clients should migrate to the new string based API.

Diff Detail

Repository
rL LLVM

Event Timeline

phosek created this revision.Sep 3 2018, 5:02 PM
Herald added subscribers: Restricted Project, llvm-commits, jfb, mgorny. · View Herald TranscriptSep 3 2018, 5:02 PM
dberris requested changes to this revision.Sep 4 2018, 4:33 PM
dberris added inline comments.
compiler-rt/include/xray/xray_log_interface.h
173 ↗(On Diff #163753)

This is an API+ABI change which we haven't communicated yet to users.

Please leave the interface as-is, update the comment that the first two arguments are no longer needed, and keep everything else as-is. We can tackle changing this at a later time, when we've had a chance to implement a replacement API to let users migrate to instead.

This revision now requires changes to proceed.Sep 4 2018, 4:33 PM
phosek added inline comments.Sep 4 2018, 5:49 PM
compiler-rt/include/xray/xray_log_interface.h
173 ↗(On Diff #163753)

Shall I keep FDRLoggingOptions and BasicLoggingOptions structs around as well or is it fine to remove those?

dberris added inline comments.Sep 4 2018, 5:52 PM
compiler-rt/include/xray/xray_log_interface.h
173 ↗(On Diff #163753)

Please remove those along with __xray_log_init(...). Thanks!

phosek updated this revision to Diff 163958.Sep 4 2018, 6:25 PM
phosek marked 3 inline comments as done.
dberris accepted this revision.Sep 4 2018, 9:02 PM

LGTM -- Thanks, @phosek!

This revision is now accepted and ready to land.Sep 4 2018, 9:02 PM
This revision was automatically updated to reflect the committed changes.

Who is responsible for updating the clients? Currently in the test suite MicroBenchmarks/XRay/FDRMode/fdrmode-bench.cc is failing to compile because [it is using __xray::FDRLoggingOptions](https://github.com/llvm-mirror/test-suite/blob/75acfc14c04d4d95c42b7f6dcd44d3e4ac6b5270/MicroBenchmarks/XRay/FDRMode/fdrmode-bench.cc#L29).

Can you please fix fdrmode-bench.cc to use correct API? Or if it is going to take a lot of time, revert this change?

Who is responsible for updating the clients? Currently in the test suite MicroBenchmarks/XRay/FDRMode/fdrmode-bench.cc is failing to compile because [it is using __xray::FDRLoggingOptions](https://github.com/llvm-mirror/test-suite/blob/75acfc14c04d4d95c42b7f6dcd44d3e4ac6b5270/MicroBenchmarks/XRay/FDRMode/fdrmode-bench.cc#L29).

Can you please fix fdrmode-bench.cc to use correct API? Or if it is going to take a lot of time, revert this change?

Good point -- yes, I'll work on changing the test-suite benchmarks, I can do that quickly.

test-suite should be fixed by r342426.

test-suite should be fixed by r342426.

Thanks for the fix, Dean. There are still some issues remaining. Jobs

are still failing for the test test-suite :: MicroBenchmarks/XRay/FDRMode/fdrmode-bench.test. According to my understanding the built binary fdrmode-bench is crashing:

******************** TEST 'test-suite :: MicroBenchmarks/XRay/FDRMode/fdrmode-bench.test' FAILED ********************

/Users/buildslave/jenkins/workspace/lnt-test-suite-x86_64-Os-flto/lnt-sandbox/build/MicroBenchmarks/XRay/FDRMode/fdrmode-bench --benchmark_format=csv > /Users/buildslave/jenkins/workspace/lnt-test-suite-x86_64-Os-flto/lnt-sandbox/build/MicroBenchmarks/XRay/FDRMode/Output/fdrmode-bench.test.bench.csv

Run on (8 X 2600 MHz CPU s)
2018-09-18 11:14:39
/Users/buildslave/jenkins/workspace/lnt-test-suite-x86_64-Os-flto/lnt-sandbox/build/MicroBenchmarks/XRay/FDRMode/Output/fdrmode-bench.test_run.script: line 1: 86215 Abort trap: 6           /Users/buildslave/jenkins/workspace/lnt-test-suite-x86_64-Os-flto/lnt-sandbox/build/MicroBenchmarks/XRay/FDRMode/fdrmode-bench --benchmark_format=csv > /Users/buildslave/jenkins/workspace/lnt-test-suite-x86_64-Os-flto/lnt-sandbox/build/MicroBenchmarks/XRay/FDRMode/Output/fdrmode-bench.test.bench.csv

********************

Can you or somebody else knowledgeable in XRay please take a look?

Is this on Darwin? If it is, then @devnexen has been the one working on making that work. I'm not sure whether there's a way to exclude the XRay test from macOS/Darwin, but the most prudent thing should be to disable it there until the Darwin support is properly fixed.

Do you have a way of reproducing these in isolation? I'm not familiar enough with Jenkins to figure this one out on my own. :(

I have published D52278 for disabling the test on Darwin. So far my investigation shows that __xray_log_select_mode("xray-fdr") returns 2 (aka XRAY_MODE_NOT_FOUND) and that's why we are calling std::abort.

if (__xray_log_select_mode("xray-fdr") !=
    XRayLogRegisterStatus::XRAY_REGISTRATION_OK)
  std::abort();

Any advices on how to troubleshoot this situation? I tried XRAY_OPTIONS="verbosity=1" and XRAY_FDR_OPTIONS="verbosity=1" but there is no extra logging. I am using clang version 8.0.0 (trunk 342513) and it has libclang_rt.xray-basic_osx.a, libclang_rt.xray-fdr_osx.a, libclang_rt.xray-profiling_osx.a, libclang_rt.xray_osx.a.

I have a smallish repro but I need to clean it up. Writing this I realized that in that repro we are using pieces of host compiler, not compiler-under-test (-isysroot, -B). Depending on the time FDR mode was added, host compiler might not have support for it.

I have published D52278 for disabling the test on Darwin. So far my investigation shows that __xray_log_select_mode("xray-fdr") returns 2 (aka XRAY_MODE_NOT_FOUND) and that's why we are calling std::abort.

if (__xray_log_select_mode("xray-fdr") !=
    XRayLogRegisterStatus::XRAY_REGISTRATION_OK)
  std::abort();

Any advices on how to troubleshoot this situation? I tried XRAY_OPTIONS="verbosity=1" and XRAY_FDR_OPTIONS="verbosity=1" but there is no extra logging. I am using clang version 8.0.0 (trunk 342513) and it has libclang_rt.xray-basic_osx.a, libclang_rt.xray-fdr_osx.a, libclang_rt.xray-profiling_osx.a, libclang_rt.xray_osx.a.

There won't be much to log if we don't find the mode available for selection, unfortunately.

I have a smallish repro but I need to clean it up. Writing this I realized that in that repro we are using pieces of host compiler, not compiler-under-test (-isysroot, -B). Depending on the time FDR mode was added, host compiler might not have support for it.

That would do it.

I don't know whether @devnexen has done any testing with the test-suite, but I'm confident that at least the compiler-rt tests for XRay don't get run on macOS yet. There was some effort to make that work and be properly supported but that work has stalled and I've not gotten back to it myself.

The repro is

// $COMPILER_DIR/bin/clang++ -O0 -arch x86_64 -Wl,-search_paths_first -Wl,-headerpad_max_install_names  main.cpp  -o a.out -Wl,-no_pie -fxray-instrument
//
// or
//
// $COMPILER_DIR/bin/clang++ main.cpp -o a.out -fxray-instrument

#include "xray/xray_log_interface.h"

int main(int argc, char *argv[]) {
    XRayLogRegisterStatus status = __xray_log_select_mode("xray-fdr");
    return status;
}

I checked that we link with correct libraries, so seems like XRay currently is just not working on macOS.

compiler-rt/trunk/include/xray/xray_log_interface.h