This is a part of https://reviews.llvm.org/D95835.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
compiler-rt/include/sanitizer/dfsan_interface.h | ||
---|---|---|
121 | Is this printed to stdout? Should we introduce an output stream argument instead to make it more versatile? | |
compiler-rt/lib/dfsan/dfsan.cpp | ||
794 | This is not very clear. It might be mistaken to mean that origin tracking is not supported. Also, it doesn't indicate to the user what they are doing wrong. | |
794 | Should this be an error perhaps? | |
801 | ||
826–828 | Is there a test for this code path? | |
845 | If the loop is not entered at all, should the return value be 0 (now) or origin? | |
compiler-rt/test/dfsan/origin_ldst.c | ||
32 | Why not typedefs instead? | |
compiler-rt/test/dfsan/origin_limit.c | ||
31 | You can use CHECK-COUNT here to remove duplication. |
compiler-rt/include/sanitizer/dfsan_interface.h | ||
---|---|---|
121 | Agree that would be better. dfsan_print_origin_trace eventually uses stack.Print() by sanitizer's Printf to print to stderr. If we assume this API is always called from customer code, I feel another stream can work if we can change stack.Print() works with a configurable output. I changed the comments to mention 'stderr'. | |
compiler-rt/lib/dfsan/dfsan.cpp | ||
794 | Since we print all the messages to stderr, changed to use colors (d.Warning()) for this kind of messages. | |
801 | Also set the warning color. | |
826–828 | Added a test case by using asm to bypass DFSan instrumentation. | |
845 | That would be 0. But the print out stack trace can tell which we get a valid chain, because a real very origin should be an stack about dfsan_set_label. | |
compiler-rt/test/dfsan/origin_ldst.c | ||
32 | Thank you. |
compiler-rt/test/dfsan/origin_limit.c | ||
---|---|---|
31 | Thank you. |
compiler-rt/lib/dfsan/dfsan.cpp | ||
---|---|---|
851–858 | Iiuc, raw_id is initialized to 0 so we could place origin_id var outside the loop so that we don't have to check the condition twice. |
compiler-rt/lib/dfsan/dfsan.cpp | ||
---|---|---|
851–858 | Thank you. This simplified the code. |
compiler-rt/include/sanitizer/dfsan_interface.h | ||
---|---|---|
124 | At some point we may want an API to traverse the origin chain. But this is fine for now. | |
compiler-rt/lib/dfsan/dfsan.cpp | ||
711 | Do we also need a dfsw wrapper? | |
802–803 | ||
810–811 | ||
813 | For addr, do we need %p? | |
831–833 | ||
842–843 | ||
847–848 | ||
compiler-rt/test/dfsan/origin_add_label.c | ||
20 | Why do we need this cast? |
addressed comments.
LLVM testing does not have debug info by default.
Added a dfsan config with debug info on to test stack traces with file/line information.
compiler-rt/lib/dfsan/dfsan.cpp | ||
---|---|---|
711 | When origin_tracking is off, we do not generate wrappers with origin arguments. | |
813 | yes. %p works perfectly than using 0x%x. Thank you. | |
compiler-rt/test/dfsan/origin_add_label.c | ||
20 | Thank you. we only need cast at the next line to shift 4 bytes. This one is not useful. |
Is this printed to stdout? Should we introduce an output stream argument instead to make it more versatile?