Details
Diff Detail
Event Timeline
| lib/sanitizer_common/sanitizer_common.h | ||
|---|---|---|
| 662–665 | This could probably be _ReadWriteBarrier(); | |
Use _ReadWriteBarrier
| lib/sanitizer_common/sanitizer_common.h | ||
|---|---|---|
| 662–665 | I think it can; done. | |
| lib/sanitizer_common/sanitizer_atomic_msvc.h | ||
|---|---|---|
| 157 | IIRC this is only available since Visual Studio 2013. I'm not aware of the minimum version we support, adding Reid. | |
| lib/ubsan/ubsan_diag.cc | ||
| 189 | SANITIZER_WINDOWS | |
| lib/ubsan/ubsan_flags.cc | ||
| 75 | What does this function mean? | |
| lib/ubsan/ubsan_platform.h | ||
| 20 | Group OS together. Also, consider #including sanitizer_platform.h and using SANITIZER_LINUX, SANITIZER_WINDOWS etc. | |
| lib/sanitizer_common/sanitizer_atomic_msvc.h | ||
|---|---|---|
| 157 | VS 2013 is the minimum toolchain requirement for LLVM, so this should be fine. | |
| lib/ubsan/ubsan_flags.cc | ||
| 75 | Peter, at some point we might implement attribute weak in clang the way mingw does, where we automatically create a new symbol and a weak alias to it with the original name, so you might want to make this #elif SANITIZER_WINDOWS. | |
- Address review comments
| lib/ubsan/ubsan_diag.cc | ||
|---|---|---|
| 189 | Done | |
| lib/ubsan/ubsan_flags.cc | ||
| 75 | Did you misread line 69 of the new code as #if SANITIZER_SUPPORTS_WEAK_HOOKS? I think SANITIZER_SUPPORTS_WEAK_HOOKS means "do the sanitizers support hooks (either through attribute weak or /alternatename)" rather than "does the compiler support attribute weak". This does seem a little silly, but I was trying to be consistent with the stuff in lib/asan/asan_win.cc. | |
| lib/ubsan/ubsan_platform.h | ||
| 20 |
Won't work, at least with cl.exe; that compiler does not define the architecture macros. | |
LGTM
| lib/ubsan/ubsan_platform.h | ||
|---|---|---|
| 20 | Alright... but the boolean expression is really hard to parse. Just move _WIN32 to another branch? #if defined(_WIN32) # define CAN_SANITIZE_UB 1 #elif (defined(__linux__) || .... # define CAN_SANITIZE_UB 1 #else # define CAN_SANITIZE_UB 0 #endif | |
| lib/ubsan/ubsan_platform.h | ||
|---|---|---|
| 20 | Done | |
IIRC this is only available since Visual Studio 2013. I'm not aware of the minimum version we support, adding Reid.