Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/sanitizer_common/sanitizer_common.h | ||
---|---|---|
662–665 ↗ | (On Diff #28831) | This could probably be _ReadWriteBarrier(); |
Use _ReadWriteBarrier
lib/sanitizer_common/sanitizer_common.h | ||
---|---|---|
662–665 ↗ | (On Diff #28831) | I think it can; done. |
lib/sanitizer_common/sanitizer_atomic_msvc.h | ||
---|---|---|
157 ↗ | (On Diff #28833) | 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 ↗ | (On Diff #28833) | SANITIZER_WINDOWS |
lib/ubsan/ubsan_flags.cc | ||
75 ↗ | (On Diff #28833) | What does this function mean? |
lib/ubsan/ubsan_platform.h | ||
20 ↗ | (On Diff #28833) | Group OS together. Also, consider #including sanitizer_platform.h and using SANITIZER_LINUX, SANITIZER_WINDOWS etc. |
lib/sanitizer_common/sanitizer_atomic_msvc.h | ||
---|---|---|
157 ↗ | (On Diff #28833) | VS 2013 is the minimum toolchain requirement for LLVM, so this should be fine. |
lib/ubsan/ubsan_flags.cc | ||
75 ↗ | (On Diff #28833) | 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 ↗ | (On Diff #28833) | Done |
lib/ubsan/ubsan_flags.cc | ||
75 ↗ | (On Diff #28833) | 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 ↗ | (On Diff #28833) |
Won't work, at least with cl.exe; that compiler does not define the architecture macros. |
LGTM
lib/ubsan/ubsan_platform.h | ||
---|---|---|
20 ↗ | (On Diff #28844) | 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 ↗ | (On Diff #28844) | Done |