[PATCH] D37925: Allow specifying sanitizers in blacklists

[Dean Michael Berris via Phabricator via cfe-commits]

dberris added inline comments.


================
Comment at: lib/Basic/XRayLists.cpp:29
   // whether it's treated as a "never" instrument function.
-  if (AlwaysInstrument->inSection("fun", FunctionName, "arg1"))
+  if (AlwaysInstrument->inSection("xray_always_instrument", "fun", FunctionName,
+                                  "arg1"))
----------------
vlad.tsyrklevich wrote:
> eugenis wrote:
> > It feels redundant to have AlwaysInstrument and NeverInstrument lists, and then the same distinction in section names. Maybe sections could be named "xray" in  both cases? Or, better, the lists could be merged into a single list with always and never sections? There is also an issue of backward compatibility. Anyway, that's for xray devs to decide. @dberris 
> I chose this approach for backwards compatibility, but I'd defer to what @dberris thinks is best.
Sorry for being late here.

I'm fine with keeping this as-is, then merging the lists into a single one. At the time this was designed/implemented, I hadn't thought about whether we could have used a single list. At the time it made sense to separate the always/never lists and applied in a set order (always wins over never).

If this is all-new functionality anyway, I'd think using "xray" as the section header and then using per-entry "always" and "never" identifiers/sections make sense.

If you leave a TODO here (and/or file a bug on XRay) I can do the migration to a single list later. I'm fine with how this is set as-is.


https://reviews.llvm.org/D37925