[PATCH] D20352: Add XRay flags to Clang

[Dean Michael Berris via cfe-commits]

dberris marked 2 inline comments as done.

================
Comment at: include/clang/Basic/Attr.td:436
@@ +435,3 @@
+                   CXX11<"clang", "xray_never_instrument">];
+  let Subjects = SubjectList<[CXXMethod, ObjCMethod, Function], WarnDiag,
+                              "ExpectedFunctionOrMethod">;
----------------
aaron.ballman wrote:
> Then ClangAttrEmitter.cpp needs to be updated as well (this is a reasonable case to automatically support, I think).
That makes sense, I can do it in another change that's more focused on just that if you don't mind?

================
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.
+
----------------
aaron.ballman wrote:
> 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.
I've explored that option (using `patchable-function` instead) and while I haven't excluded that outright, there needs to be a discussion about how we're going to implement complimentary instrumentation functionality that do nop-sled inserting. I suspect that has to happen in D19904 where we can still make those changes to instead use `patchable-function`.

The response to my comment in D19908 seems to be that there should be no conflicts in the platforms XRay currently cares about -- but that if we do ever want to get XRay implemented/supported in x86 32-bit, then we're going to have issues. This suggests that a harder look at `patchable-function` and exclusive modes (or special-casing the supported configurations) might need to happen sooner than later.


http://reviews.llvm.org/D20352