re PR sanitizer/81697 (Incorrect ASan global variables alignment on arm)

gcc/

2017-11-30  Maxim Ostapenko  <m.ostapenko@samsung.com>

	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  <m.ostapenko@samsung.com>

	PR sanitizer/81697
	* c-c++-common/asan/pr81697.c: New test.

From-SVN: r255283

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 400ce9776b554d6223baf9a2915715671c605946..227396abe64ff252865110a26d3777d65d393ca6 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@
+2017-12-01  Maxim Ostapenko  <m.ostapenko@samsung.com>
+
+       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  <jimw@sifive.com>
 
        * doc/invoke.texi (RISC-V Options): Delete nonexistent -mmemcpy and

diff --git a/gcc/asan.c b/gcc/asan.c
index ca5fceed9fcafcb4d11fb7196f999750f03da7af..873687f04877670c763cff264857dc154e3b5a49 100644 (file)
--- 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 c82d4d901e50a9242b5070206a4fe60027160aae..885b47e8cc430d287414908fe251a31ffd785269 100644 (file)
--- 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 972d1cbe6ef3b0e5f1b2cefaae36423728fb1400..045be1a4b549dc0c913d898e5c75b6660dc85846 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-12-01  Maxim Ostapenko  <m.ostapenko@samsung.com>
+
+       PR sanitizer/81697
+       * c-c++-common/asan/pr81697.c: New test.
+
 2017-11-30  Claudiu Zissulescu  <claziss@synopsys.com>
 
        * 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 (file)
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 0c7b26ebab747e871e180539bbe20c9b4dd4f1b4..392ac443f14fadfd4107c8ff665a711fe73baea4 100644 (file)
--- 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).  */