On Thu, Nov 30, 2017 at 02:38:25PM +0300, Maxim Ostapenko wrote:
> Hi Jakub, thanks for review.
>
> I've fixed the issues you've pointed in review.
> Regarding a testcase -- I've cooked a runtime test, but it shows FP on
> unpatched GCC version only when linking with Gold (because it strips
> redzones more aggressively).
I think we can live with that.
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -6550,7 +6550,19 @@ 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 (CONST_CAST_TREE (decl),
> + use_object_blocks_p ()
> + && use_blocks_for_decl_p (
> + CONST_CAST_TREE (decl)))))
Formatting is too bad here. && should go below use_object_block_p..
The opening ( should either go on the next line, like:
use_object_blocks_p ()
&& use_blocks_for_decl_p
(CONST_CAST_TREE (decl)))))
or perhaps better just introduce a temporary somewhere:
else if (VAR_P (decl))
{
+ tree d = CONST_CAST_TREE (decl);
if (bss_initializer_p (decl))
ret = SECCAT_BSS;
and use d instead of CONST_CAST_TREE (decl) later?
Ok with those changes.
Jakub
