================ Comment at: CMakeLists.txt:210 @@ -209,2 +209,3 @@ append_list_if(COMPILER_RT_HAS_FNO_STACK_PROTECTOR_FLAG -fno-stack-protector SANITIZER_COMMON_CFLAGS) +append_list_if(COMPILER_RT_HAS_FNO_SAFE_STACK_FLAG -fno-safe-stack SANITIZER_COMMON_CFLAGS) append_list_if(COMPILER_RT_HAS_FVISIBILITY_HIDDEN_FLAG -fvisibility=hidden SANITIZER_COMMON_CFLAGS) ---------------- is this still valid? Don't we use -fsanitize= syntax?
================ Comment at: lib/safestack/safestack.cc:38 @@ +37,3 @@ +/// FIXME: is this in some header? +#define STACK_ALIGN 16 + ---------------- please no macros where constants work ================ Comment at: lib/safestack/safestack.cc:45 @@ +44,3 @@ + +// define MAP_STACK if undefined, this flag is used as a hint +#ifndef MAP_STACK ---------------- do you really need this? ================ Comment at: lib/safestack/safestack.cc:57 @@ +56,3 @@ +// all symbols from pthread that we use dynamically +#define __DECLARE_WRAPPER(fn) static __typeof__(fn)* __d_ ## fn = NULL; + ---------------- Mmm. I don't like that. Why can't we just ensure that pthread is linked? ================ Comment at: lib/safestack/safestack.cc:80 @@ +79,3 @@ + +#define __GET_UNSAFE_STACK_PTR() __safestack_unsafe_stack_ptr +#define __SET_UNSAFE_STACK_PTR(value) __safestack_unsafe_stack_ptr = (value) ---------------- no macros, please, unless you have too, in which case describe in comments why. ================ Comment at: lib/safestack/safestack.cc:101 @@ +100,3 @@ + // (such overrides might crash is they use the unsafe stack themselves) + void *addr = (void *)internal_mmap( + NULL, size + guard, PROT_WRITE | PROT_READ, ---------------- I would use MmapOrDie (or create a similar one in sanitizer_common/sanitizer_posix.cc) and get rid of mmap includes in this file. ================ Comment at: lib/safestack/safestack.cc:114 @@ +113,3 @@ + void* stack_ptr = (char*) start + size; + assert((((size_t)stack_ptr) & (STACK_ALIGN-1)) == 0); + ---------------- use CHECK_EQ ================ Comment at: lib/safestack/safestack.cc:126 @@ +125,3 @@ + // (such overrides might crash is they use the unsafe stack themselves) + (void)internal_munmap( + (char*) __GET_UNSAFE_STACK_START() - __GET_UNSAFE_STACK_GUARD(), ---------------- UnmapOrDie ================ Comment at: lib/safestack/safestack.cc:204 @@ +203,3 @@ + + assert(size != 0); + assert((size & (STACK_ALIGN-1)) == 0); ---------------- use CHECK ================ Comment at: lib/safestack/safestack.cc:276 @@ +275,3 @@ + +#ifdef __ELF__ +// Run safestack initialization before any other constructors. ---------------- please make this similar to other sanitizers (SANITIZER_CAN_USE_PREINIT_ARRAY). http://reviews.llvm.org/D6096 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
