From bee7ed760e03cc96c626d6c6be624be320e53df6 Mon Sep 17 00:00:00 2001 From: Denis Khalikov Date: Mon, 19 Mar 2018 09:43:09 +0300 Subject: [PATCH] [ASan] Fix for TTC-5 (PR sanitizer/81697). 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. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@255283 138bc75d-0d04-0410-961f-82ee72b054a4 Change-Id: I4568c3d228286d2a372c1e5c3a1dde62561ace74 --- gcc/asan.c | 28 +++++++++++++++++++--------- gcc/asan.h | 2 +- gcc/testsuite/c-c++-common/asan/pr81697.c | 20 ++++++++++++++++++++ gcc/varasm.c | 13 ++++++++++++- 4 files changed, 52 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/asan/pr81697.c diff --git a/gcc/asan.c b/gcc/asan.c index 2483eec..4806c3f 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -1333,7 +1333,7 @@ asan_needs_local_alias (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; @@ -1355,7 +1355,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. */ @@ -1378,14 +1384,18 @@ asan_protect_global (tree decl) || TREE_TYPE (decl) == ubsan_get_source_location_type ()) 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 ef58922..9182c60 100644 --- a/gcc/asan.h +++ b/gcc/asan.h @@ -25,7 +25,7 @@ extern void asan_function_start (void); extern void asan_finish_file (void); extern rtx_insn *asan_emit_stack_protection (rtx, rtx, unsigned int, HOST_WIDE_INT *, tree *, int); -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/c-c++-common/asan/pr81697.c b/gcc/testsuite/c-c++-common/asan/pr81697.c new file mode 100644 index 0000000..3a85813 --- /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 da88f5b..9317273 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -6387,6 +6387,7 @@ categorize_decl_for_section (const_tree decl, int reloc) } else if (TREE_CODE (decl) == VAR_DECL) { + tree d = CONST_CAST_TREE (decl); if (bss_initializer_p (decl)) ret = SECCAT_BSS; else if (! TREE_READONLY (decl) @@ -6406,7 +6407,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). */ -- 2.7.4