From: Maxim Ostapenko Date: Thu, 30 Nov 2017 21:38:16 +0000 (+0000) Subject: re PR sanitizer/81697 (Incorrect ASan global variables alignment on arm) X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=1069dc251bd97f187ff42b543ca90e08203465ae;p=gcc.git re PR sanitizer/81697 (Incorrect ASan global variables alignment on arm) gcc/ 2017-11-30 Maxim Ostapenko PR sanitizer/81697 * asan.c (asan_protect_global): Add new ignore_decl_rtl_set_p parameter. Return true if ignore_decl_rtl_set_p is true and other conditions are satisfied. * asan.h (asan_protect_global): Add new parameter. * varasm.c (categorize_decl_for_section): Pass true as second parameter to asan_protect_global calls. gcc/testsuite/ 2017-11-30 Maxim Ostapenko PR sanitizer/81697 * c-c++-common/asan/pr81697.c: New test. From-SVN: r255283 --- diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 400ce9776b5..227396abe64 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,13 @@ +2017-12-01 Maxim Ostapenko + + PR sanitizer/81697 + * asan.c (asan_protect_global): Add new ignore_decl_rtl_set_p + parameter. Return true if ignore_decl_rtl_set_p is true and other + conditions are satisfied. + * asan.h (asan_protect_global): Add new parameter. + * varasm.c (categorize_decl_for_section): Pass true as second parameter + to asan_protect_global calls. + 2017-11-30 Jim Wilson * doc/invoke.texi (RISC-V Options): Delete nonexistent -mmemcpy and diff --git a/gcc/asan.c b/gcc/asan.c index ca5fceed9fc..873687f0487 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1605,7 +1605,7 @@ is_odr_indicator (tree decl) ASAN_RED_ZONE_SIZE bytes. */ bool -asan_protect_global (tree decl) +asan_protect_global (tree decl, bool ignore_decl_rtl_set_p) { if (!ASAN_GLOBALS) return false; @@ -1627,7 +1627,13 @@ asan_protect_global (tree decl) || DECL_THREAD_LOCAL_P (decl) /* Externs will be protected elsewhere. */ || DECL_EXTERNAL (decl) - || !DECL_RTL_SET_P (decl) + /* PR sanitizer/81697: For architectures that use section anchors first + call to asan_protect_global may occur before DECL_RTL (decl) is set. + We should ignore DECL_RTL_SET_P then, because otherwise the first call + to asan_protect_global will return FALSE and the following calls on the + same decl after setting DECL_RTL (decl) will return TRUE and we'll end + up with inconsistency at runtime. */ + || (!DECL_RTL_SET_P (decl) && !ignore_decl_rtl_set_p) /* Comdat vars pose an ABI problem, we can't know if the var that is selected by the linker will have padding or not. */ @@ -1651,14 +1657,18 @@ asan_protect_global (tree decl) || is_odr_indicator (decl)) return false; - rtl = DECL_RTL (decl); - if (!MEM_P (rtl) || GET_CODE (XEXP (rtl, 0)) != SYMBOL_REF) - return false; - symbol = XEXP (rtl, 0); + if (!ignore_decl_rtl_set_p || DECL_RTL_SET_P (decl)) + { - if (CONSTANT_POOL_ADDRESS_P (symbol) - || TREE_CONSTANT_POOL_ADDRESS_P (symbol)) - return false; + rtl = DECL_RTL (decl); + if (!MEM_P (rtl) || GET_CODE (XEXP (rtl, 0)) != SYMBOL_REF) + return false; + symbol = XEXP (rtl, 0); + + if (CONSTANT_POOL_ADDRESS_P (symbol) + || TREE_CONSTANT_POOL_ADDRESS_P (symbol)) + return false; + } if (lookup_attribute ("weakref", DECL_ATTRIBUTES (decl))) return false; diff --git a/gcc/asan.h b/gcc/asan.h index c82d4d901e5..885b47e8cc4 100644 --- a/gcc/asan.h +++ b/gcc/asan.h @@ -26,7 +26,7 @@ extern void asan_finish_file (void); extern rtx_insn *asan_emit_stack_protection (rtx, rtx, unsigned int, HOST_WIDE_INT *, tree *, int); extern rtx_insn *asan_emit_allocas_unpoison (rtx, rtx, rtx_insn *); -extern bool asan_protect_global (tree); +extern bool asan_protect_global (tree, bool ignore_decl_rtl_set_p = false); extern void initialize_sanitizer_builtins (void); extern tree asan_dynamic_init_call (bool); extern bool asan_expand_check_ifn (gimple_stmt_iterator *, bool); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 972d1cbe6ef..045be1a4b54 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2017-12-01 Maxim Ostapenko + + PR sanitizer/81697 + * c-c++-common/asan/pr81697.c: New test. + 2017-11-30 Claudiu Zissulescu * gcc.target/arc/loop-3.c: New test. diff --git a/gcc/testsuite/c-c++-common/asan/pr81697.c b/gcc/testsuite/c-c++-common/asan/pr81697.c new file mode 100644 index 00000000000..3a85813261d --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/pr81697.c @@ -0,0 +1,20 @@ +/* { dg-options "-fmerge-all-constants" } */ +/* { dg-do run } */ +/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */ + +const char kRecoveryInstallString[] = "NEW"; +const char kRecoveryUpdateString[] = "UPDATE"; +const char kRecoveryUninstallationString1[] = "INSTALL"; +const char kRecoveryUninstallationString2[] = "UNINSTALL"; + +volatile const int zero = 0; + +int +main() +{ + char x1 = kRecoveryInstallString[zero + 0]; + char x2 = kRecoveryUpdateString[zero + 0]; + char x3 = kRecoveryUninstallationString1[zero + 0]; + char x4 = kRecoveryUninstallationString2[zero + 0]; + return (x1 + x2 + x3 + x4) == 0; +} diff --git a/gcc/varasm.c b/gcc/varasm.c index 0c7b26ebab7..392ac443f14 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -6530,6 +6530,7 @@ categorize_decl_for_section (const_tree decl, int reloc) } else if (VAR_P (decl)) { + tree d = CONST_CAST_TREE (decl); if (bss_initializer_p (decl)) ret = SECCAT_BSS; else if (! TREE_READONLY (decl) @@ -6550,7 +6551,17 @@ categorize_decl_for_section (const_tree decl, int reloc) ret = reloc == 1 ? SECCAT_DATA_REL_RO_LOCAL : SECCAT_DATA_REL_RO; else if (reloc || flag_merge_constants < 2 || ((flag_sanitize & SANITIZE_ADDRESS) - && asan_protect_global (CONST_CAST_TREE (decl)))) + /* PR 81697: for architectures that use section anchors we + need to ignore DECL_RTL_SET_P (decl) for string constants + inside this asan_protect_global call because otherwise + we'll wrongly put them into SECCAT_RODATA_MERGE_CONST + section, set DECL_RTL (decl) later on and add DECL to + protected globals via successive asan_protect_global + calls. In this scenario we'll end up with wrong + alignment of these strings at runtime and possible ASan + false positives. */ + && asan_protect_global (d, use_object_blocks_p () + && use_blocks_for_decl_p (d)))) /* C and C++ don't allow different variables to share the same location. -fmerge-all-constants allows even that (at the expense of not conforming). */