[PATCH] D98734: [dfsan] Add -dfsan-fast-8-labels flag

[George Balatsouras via Phabricator via llvm-commits]

gbalats marked 2 inline comments as done.
gbalats added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:40-41
+///
+/// With fast-8 mode, each byte of application memory is backed by a single
+/// shadow memory byte. On Linux/x86_64, the memory layout becomes:
+///
----------------
stephan.yichao.zhao wrote:
> If users read top to down, it seems helpful if we move this sentence above the first layout. 
> I suggest we first explain there are two kinds of shadow layouts.  One uses 2 bytes bit for fast-16-labels mode with 16 labels and legacy mode with 2^16 labels, and the other uses 1 byte for fast-8-labels mode with 8 labels. Then we show the two layouts.
You're right the bottom part relates to the 16-shadow-bits layout. Changed the order and added more text to explain better.


================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:2050
+
+  // The shadow size in bytes is a multiple of 8, except for the case of load32
+  // in fast-8 mode when it's 4. For the latter case, we can fit the wide shadow
----------------
stephan.yichao.zhao wrote:
> A comment with more context could be
> "LLVM bitcode supports loading integers with byte size 1, 12, 20....
> But loadFast16ShadowFast only optimizes normal cases loading i32 i64 i128 etc.. 
> so it is fine to support only size=4 or size % 8 == 0. "
Expanded comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98734/new/

https://reviews.llvm.org/D98734