[Driver] Correct -f(no-)xray-function-index behavior

[ilammy]

Summary

This option has undergone several refactorings and got inverted along
the way. The XRayOmitFunctionIndex flag governs codegen behavior,
*omitting* "xray_fn_idx" section if it is set. But the command-line
flag behavior was not adjusted at the time. Right now it's like this:

               (default): no function index
   -fxray-function-index: no function index
-fno-xray-function-index: "xray_fn_idx" is present

While the default behavior should be keep "xray_fn_idx", unless
-fno-xray-function-index is given:

               (default): "xray_fn_idx" is present
   -fxray-function-index: same, present, but explicitly
-fno-xray-function-index: no function index

Flip the flags to make it so.

[ianlevesque]

Good catch, thanks for the fix.

[MaskRay]

clang/test/CodeGen/xray-function-index.cpp

2

Add // REQUIRES: x86-registered-target

9

.section xray_fn_idx

[ilammy]

Addressed feedback by @MaskRay. Thanks!

[MaskRay]

Thanks!

clang/test/CodeGen/xray-function-index.cpp

2

Move // REQUIRES: to the top

[ilammy]

Addressing feedback by @MaskRay:

• Moved // REQUIRES: directive to the top of the test file

[MaskRay]

clang/test/CodeGen/xray-function-index.cpp

1

We generally want to avoid assembly output tests in clang/test.

However, CodeGenOpts.XRayOmitFunctionIndex only affects llvm::TargetOptions, not LLVM IR, so I think this seems fine.

(Note: a lot of xray tests do not follow the best practice, and we should be careful for new tests, not necessarily copying the old xray testing practice.)

2

We don't usually do alignment like this. You may place -S earlier to make the compile action (emit assembly) clearer.

[MaskRay]

This option has undergone several refactorings and got inverted along the way. The XRayOmitFunctionIndex flag governs codegen behavior, *omitting* "xray_fn_idx" section if it is set. But the command-line flag behavior was not adjusted at the time. Right now it's like this:

d8a8e5d6240a1db809cd95106910358e69bbf299 accidentally omitted xray_fn_idx by default.
I pushed 849f1dd15e92fda2b83dbb6144e6b28b2cb946e0 to rename the confusing XRayOmitFunctionIndex and fix -fno-xray-function-index/-fxray-function-index.

[MaskRay]

clang/include/clang/Driver/Options.td

2219

The change should be accompanies by a change to clang/lib/Driver/XRayArgs.cpp.
I'll fix this issue myself.
Thanks for reporting the bug, though.

[ilammy]

@MaskRay, thanks!

clang/include/clang/Driver/Options.td

 
 defm xray_ignore_loops : BoolFOption<"xray-ignore-loops",
   CodeGenOpts<"XRayIgnoreLoops">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Don't instrument functions with loops"
           " unless they also meet the minimum function size">,
   NegFlag<SetFalse>>;
 
 defm xray_function_index : BoolFOption<"xray-function-index",
-  CodeGenOpts<"XRayOmitFunctionIndex">, DefaultTrue,
-  NegFlag<SetFalse, [CC1Option], "Omit function index section at the"
+  CodeGenOpts<"XRayOmitFunctionIndex">, DefaultFalse,
+  NegFlag<SetTrue, [CC1Option], "Omit function index section at the"
           " expense of single-function patching performance">,
-  PosFlag<SetTrue>>;
+  PosFlag<SetFalse>>;
 
 def fxray_link_deps : Flag<["-"], "fxray-link-deps">, Group<f_Group>,
   Flags<[CC1Option]>,
   HelpText<"Tells clang to add the link dependencies for XRay.">;
 def fnoxray_link_deps : Flag<["-"], "fnoxray-link-deps">, Group<f_Group>,
   Flags<[CC1Option]>;
 
 def fxray_instrumentation_bundle :

clang/test/CodeGen/xray-function-index.cpp

+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 -fxray-instrument                          -x c++ -std=c++11 -triple x86_64-unknown-linux-gnu -S -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-ENABLED
+// RUN: %clang_cc1 -fxray-instrument -fno-xray-function-index -x c++ -std=c++11 -triple x86_64-unknown-linux-gnu -S -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-DISABLED
+
+[[clang::xray_always_instrument]] void foo() {}
+
+// CHECK-LABEL: .section xray_instr_map,"ao",@progbits,_Z3foov
+// CHECK-ENABLED: .section xray_fn_idx,"awo",@progbits,_Z3foov
+// CHECK-DISABLED-NOT: .section xray_fn_idx