morehouse added a subscriber: Dor1s.
morehouse added a comment.
In D84371#2168367 <https://reviews.llvm.org/D84371#2168367>, @kcc wrote:
> In what cases do we still call __dfsan_union?
We still call `__dfsan_union_load` when we load sizes greater than 2 bytes but
not divisible by 4. I actually am not sure what instruction sequence would hit
this case, but it was already in DFSan so I left it there.
`__dfsan_union_load` then calls `__dfsan_union`.
================
Comment at: compiler-rt/lib/dfsan/dfsan.cpp:173
+// Configures the DFSan runtime to use fast16labels mode.
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE void dfsan_use_fast16labels() {
+ use_fast16labels = true;
----------------
kcc wrote:
> I wonder if we can get away w/o these flags.
> Just assume that fast16mode works only when the fast16mode instrumentation is
> enabled.
> (remember, fast16mode is currently experimental, we are free to change its
> behavior)
SGTM. I only know of @Dor1s using it at the moment.
================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1079
+ cast<Constant>(DFSanUseFast16LabelsFn.getCallee()),
+ "__dfsan_fast16labels_preinit");
+ UseFast16LabelsInit->setSection(".preinit_array");
----------------
vitalybuka wrote:
> what we are going to do in __dfsan_fast16labels_preinit?
This global is simply a pointer to `dfsan_use_fast16labels()` so that it runs
during preinit. `dfsan_use_fast16labels` just enables fast16labels mode in the
runtime.
================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1339
+ IRB.CreateAlignedLoad(IRB.getInt64Ty(), WideAddr, ShadowAlign);
+ for (uint64_t Ofs = 64 / DFS.ShadowWidthBits; Ofs != Size;
+ Ofs += 64 / DFS.ShadowWidthBits) {
----------------
vitalybuka wrote:
> have you considered a loop?
>
I think we should avoid loop overhead at least for the common case here
(loading 8-16 bytes of shadow).
I think it's also possible to hit this case with vector instructions, but
vanilla DFSan currently doesn't use a loop in that case either. If we want to
use a loop for vector loads, I think we should do it in a different patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84371/new/
https://reviews.llvm.org/D84371
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
