[PATCH] [tsan] Move some suppressions-related code to common. Factor out code to be reused in LSan. Also switch from linked list to vector.

Sergey Matveev earthdok at google.com
Wed Jun 26 07:47:57 PDT 2013



================
Comment at: lib/sanitizer_common/sanitizer_suppressions.h:45
@@ +44,3 @@
+  uptr SuppressionCount();
+  void GetMatched(InternalMmapVector<Suppression *> *matched);
+
----------------
Dmitry Vyukov wrote:
> I would add PrintMatched() method, then this is unnecesasry
> 
PrintMatched() *is* tool-specific.

================
Comment at: lib/sanitizer_common/sanitizer_suppressions.h:36
@@ +35,3 @@
+
+class SuppressionContextTest;
+
----------------
Dmitry Vyukov wrote:
> this is unnecessary, remove
ok

================
Comment at: lib/sanitizer_common/sanitizer_suppressions.h:21
@@ +20,3 @@
+
+enum SuppressionType {
+  SuppressionNone,
----------------
Dmitry Vyukov wrote:
> why it is here now?
> it's tool-specific.
I think this is better and cleaner than having to pass strings around. It's awkward to not have the string-enum correspondence encapsulated in this module. I also don't see any good reasons why tools can't share this enum. (We're already sharing the Suppression struct, after all - and that is also tool-specific to some extent.) At some point we might even want to read suppressions for several tools out of one file.

================
Comment at: lib/sanitizer_common/sanitizer_suppressions.h:42
@@ +41,3 @@
+  void Parse(const char *str);
+  void FinishInitialization() { initialized_ = true; }
+  bool Match(const char* str, SuppressionType type, Suppression **s);
----------------
Dmitry Vyukov wrote:
> set initialized_=true in Match()?
I think explicit is better than implicit, but ok.


http://llvm-reviews.chandlerc.com/D1037



More information about the llvm-commits mailing list