Hi,
this is the second attempt to fix PR sanitizer/81697.
For architectures that use section anchors there is a problem with
asan_protect_global because it's involved in a circular dependency:
1) categorize_decl_for_section calls asan_protect_global that returns
false because DECL_RTL is not set at this point and assigns decl to
SECCAT_RODATA_MERGE_CONST section.
2) make_decl_rtl sets up SET_DECL_RTL (decl, x).
3) Following asan_protect_global calls (e.g. from asan_finish_file) will
return true and we end up with mismatch between section and alignment
requirements.
Previous patch
(https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00569.html) just disabled
section anchors if ASan is enabled, but it was rejected.
This patch just introduces an additional ignore_decl_rtl_set_p parameter
for asan_protect_global to break dependence mentioned above.
Tested on arm-linux-gnueabi, ppc64-unknown-linux-gnu and x86{,
_64}-linux-gnu, bootstrapped on x86_64-unknown-linux-gnu.
-Maxim
gcc/ChangeLog:
2017-10-31 Maxim Ostapenko <m.ostape...@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/ChangeLog:
2017-10-31 Maxim Ostapenko <m.ostape...@samsung.com>
PR sanitizer/81697
* g++.dg/asan/global-alignment.C: New test.
diff --git a/gcc/asan.c b/gcc/asan.c
index d5128aa..78c3b60 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,6 +1657,9 @@ asan_protect_global (tree decl)
|| is_odr_indicator (decl))
return false;
+ if (ignore_decl_rtl_set_p)
+ return true;
+
rtl = DECL_RTL (decl);
if (!MEM_P (rtl) || GET_CODE (XEXP (rtl, 0)) != SYMBOL_REF)
return false;
diff --git a/gcc/asan.h b/gcc/asan.h
index c82d4d9..885b47e 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/g++.dg/asan/global-alignment.C b/gcc/testsuite/g++.dg/asan/global-alignment.C
new file mode 100644
index 0000000..84dac37
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asan/global-alignment.C
@@ -0,0 +1,18 @@
+/* { dg-options "-fmerge-all-constants" } */
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
+
+#include <string>
+#include <map>
+
+const char kRecoveryInstallString[] = "NEW";
+const char kRecoveryUpdateString[] = "UPDATE";
+const char kRecoveryUninstallationString[] = "UNINSTALL";
+
+const std::map<std::string, int> kStringToRequestMap = {
+ {kRecoveryInstallString, 0},
+ {kRecoveryUpdateString, 0},
+ {kRecoveryUninstallationString, 0},
+};
+
+/* { dg-final { scan-assembler-times {\.section\s+\.rodata\n(?:(?!\.section).)*\.\w+\s+"NEW} 1 } } */
diff --git a/gcc/varasm.c b/gcc/varasm.c
index a139151..849eae0 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -6508,7 +6508,7 @@ categorize_decl_for_section (const_tree decl, int reloc)
else if (TREE_CODE (decl) == STRING_CST)
{
if ((flag_sanitize & SANITIZE_ADDRESS)
- && asan_protect_global (CONST_CAST_TREE (decl)))
+ && asan_protect_global (CONST_CAST_TREE (decl), true))
/* or !flag_merge_constants */
return SECCAT_RODATA;
else
@@ -6536,7 +6536,7 @@ 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))))
+ && asan_protect_global (CONST_CAST_TREE (decl), true)))
/* 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). */
