[PATCH] D19168: [esan] EfficiencySanitizer base runtime library

Filipe Cabecinhas via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 20 08:38:09 PDT 2016


filcab added a subscriber: filcab.
filcab added a comment.

Thanks again, for working on this.
Please add tests so that we're always sure the current code is building+running correctly.


================
Comment at: lib/esan/esan.cc:66
@@ +65,3 @@
+  processRangeAccess(PC, Addr, Size, IsWrite);
+}
+
----------------
If you end up calling the "range access" function on both the above cases, then I see no reason to have it split from the other two.
`processRangeAccess` should be able to do everything.

It's trivial to think of occasions when this wouldn't happen. And I understand this is a skeleton, but I'd prefer (for now) to either:
Remove the `processRangeAccess` be called only when the others aren't. Or only have `processRangeAccess`.
(In the future, the "other path" will be added, I'm sure. But I don't think it's necessary for the current skeleton proposal, and it's very easy for some of this to fall into dead-code land)

================
Comment at: lib/esan/esan.h:41
@@ +40,3 @@
+void processMemAccess(uptr PC, uptr Addr, int Size, bool IsWrite);
+void processUnalignedAccess(uptr PC, uptr Addr, int Size, bool IsWrite);
+void processRangeAccess(uptr PC, uptr Addr, int Size, bool IsWrite);
----------------
`Mem` vs `Unaligned` seems weird. Maybe `Aligned` vs `Unaligned` explains it better.

================
Comment at: lib/esan/esan_interface.cc:23
@@ +22,3 @@
+  initializeLibrary();
+}
+
----------------
`__esan_init()` will be called more than once.
We should make sure we're always called with the same tool as argument.


http://reviews.llvm.org/D19168





More information about the llvm-commits mailing list