[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