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?