[PATCH] D30388: [XRay] Add -fxray-{always, never}-instrument= flags to clang

Dean Michael Berris via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 23 00:21:40 PDT 2017


dberris added a comment.

In https://reviews.llvm.org/D30388#707719, @rnk wrote:

> I think you need some driver logic to make sure the xray lists show up in .d files so that modifications to them trigger rebuilds in most build systems. See the SanitizerArgs.cpp driver logic for this.


Good catch -- yes, added.

Also took the opportunity to refactor the flag logic out into its own class (similar to how we do it with the sanitizers).



================
Comment at: include/clang/Basic/XRayFunctionFilter.h:1
+//===--- XRayFunctionFilter.h - XRay automatic-attribution ------*- C++ -*-===//
+//
----------------
rnk wrote:
> Do you think XRayLists.h might be a better name for this header? It's analagous to SanitizerBlacklist.h, but it's not a "black" list.
Good idea -- although the names of the type defined doesn't have any resemblance to the lists. I suppose that's fine too.


================
Comment at: lib/Basic/XRayFunctionFilter.cpp:21
+    : AlwaysInstrument(
+          llvm::SpecialCaseList::createOrDie(AlwaysInstrumentPaths)),
+      NeverInstrument(llvm::SpecialCaseList::createOrDie(NeverInstrumentPaths)),
----------------
rnk wrote:
> Hm, phab ate my comment about this. I don't think we should use this API in clang. Instead, we should use the `::create(...)` version and issue a diagnostic similar to err_drv_malformed_sanitizer_blacklist from the caller of this function.
That's fair -- although I am just looking at what SanitizerLists already does. I've added the validation of the flags though at the driver level to ensure that we don't initialise this in a manner that's not valid.


https://reviews.llvm.org/D30388





More information about the cfe-commits mailing list