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.