From: Jakub Jelinek <jakub@redhat.com>
To: Maxim Ostapenko <m.ostapenko@samsung.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
Ramana Radhakrishnan <ramana.radhakrishnan@foss.arm.com>
Subject: Re: [Ping][PATCH v3] Fix Incorrect ASan global variables alignment on arm (PR sanitizer/81697)
Date: Thu, 30 Nov 2017 11:55:00 -0000 [thread overview]
Message-ID: <20171130115452.GF2353@tucnak> (raw)
In-Reply-To: <5A1FEDB1.9040408@samsung.com>
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
next prev parent reply other threads:[~2017-11-30 11:55 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20171121084326eucas1p1b6791865d9d8c4d2b17939650b66da45@eucas1p1.samsung.com>
2017-11-21 8:57 ` Maxim Ostapenko
[not found] ` <CGME20171128070454eucas1p2ecf098de3fc9ced1e4e283b5e24f4c6f@eucas1p2.samsung.com>
2017-11-28 7:42 ` Maxim Ostapenko
2017-11-29 11:05 ` Jakub Jelinek
2017-11-30 11:55 ` Maxim Ostapenko
2017-11-30 11:55 ` Jakub Jelinek [this message]
2017-11-30 12:47 ` Maxim Ostapenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171130115452.GF2353@tucnak \
--to=jakub@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=m.ostapenko@samsung.com \
--cc=ramana.radhakrishnan@foss.arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).