[PATCH] D20352: Add XRay flags to Clang

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 5 07:30:59 PDT 2016


aaron.ballman added inline comments.

================
Comment at: include/clang/Basic/Attr.td:436
@@ +435,3 @@
+                   CXX11<"clang", "xray_never_instrument">];
+  let Subjects = SubjectList<[CXXMethod, ObjCMethod, Function], WarnDiag,
+                              "ExpectedFunctionOrMethod">;
----------------
Then ClangAttrEmitter.cpp needs to be updated as well (this is a reasonable case to automatically support, I think).

================
Comment at: include/clang/Basic/AttrDocs.td:2457
@@ +2456,3 @@
+  let Content = [{
+``__attribute__((xray_always_instrument))`` or ``[[clang:xray_always_instrument]]`` is used to mark member functions (in C++), methods (in Objective C), and free functions (in C, C++, and Objective C) to be instrumented with XRay. This will cause the function to always have space at the beginning and exit points to allow for runtime patching.
+
----------------
I get the impression they do conflict because they have overlapping functionality (both provide prologue space for runtime patching, for instance). It would be best to have only one attribute if we can do it.

================
Comment at: lib/CodeGen/CodeGenFunction.cpp:696
@@ +695,3 @@
+    if (const auto *XRayAttr = D->getAttr<XRayInstrumentAttr>()) {
+      if (XRayAttr->alwaysXRayInstrument()) {
+        Fn->addFnAttr("function-instrument", "xray-always");
----------------
Yes, this is the right way to solve it. Thanks!

However, you should elide the braces when there's only a single statement (per our coding standards).


http://reviews.llvm.org/D20352





More information about the cfe-commits mailing list