This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Remove dead code in asan.c
- From: Martin Liška <mliska at suse dot cz>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 30 Jun 2017 15:34:09 +0200
- Subject: Re: [PATCH] Remove dead code in asan.c
- Authentication-results: sourceware.org; auth=none
- References: <e620fd3b-3f39-4d32-1197-330c6d00eb52@suse.cz> <20170630101531.GH2123@tucnak>
On 06/30/2017 12:15 PM, Jakub Jelinek wrote:
> On Fri, Jun 30, 2017 at 12:00:36PM +0200, Martin Liška wrote:
>> Hi.
>>
>> Following crap code was added by me when I added use-after-scope.
>> Actually decl always points to LASANPC, so asan_handled_variables->contains (decl)
>> is always false.
>>
>> Well, originally the idea was to not clear content (place in shadow memory in between
>> red zoner) of auto variables, but as we emit 0xf5 in order to have working use-after-return,
>> it probably does not worth for doing an optimization?
>
> use-after-return is only runtime conditional, defaults to off.
> And your patch doesn't bring the code to anything close to what we had
> before the -fsanitize-use-after-scope changes, just look what it did before
> - only cleared the shadow spots that weren't known to be 0, clearing the
> whole shadow might be too expensive. Consider many KB large local
> variables.
>
> You can find the right decl in decls[l / 2] or decls[l / 2 - 1] or so.
>
> Jakub
>
Huh, sorry, I overlooked that shadow stack used by use-after-return has it's own
code that does shadow memory modification.
Please take a look at v2 of the patch where I reverted the hunk to pre use-after-scope
and then I added condition to unpoison variables used in use-after-scope.
I'm going to test it.
Martin
>From 2e249424e28a0d66ffb1c9cf8f54c1160043a2cc Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Fri, 30 Jun 2017 15:08:55 +0200
Subject: [PATCH] Make stack epilogue more efficient
gcc/ChangeLog:
2017-06-30 Martin Liska <mliska@suse.cz>
* asan.c (asan_emit_stack_protection): Unpoison just red zones
and shadow memory of auto variables which are subject of
use-after-scope sanitization.
(asan_expand_mark_ifn): Add do set only when is_poison.
---
gcc/asan.c | 79 +++++++++++++++++++++++++++++++-------------------------------
1 file changed, 40 insertions(+), 39 deletions(-)
diff --git a/gcc/asan.c b/gcc/asan.c
index 2de16402c51..5cce2be9962 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1062,7 +1062,7 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
HOST_WIDE_INT base_offset = offsets[length - 1];
HOST_WIDE_INT base_align_bias = 0, offset, prev_offset;
HOST_WIDE_INT asan_frame_size = offsets[0] - base_offset;
- HOST_WIDE_INT last_offset;
+ HOST_WIDE_INT last_offset, last_size;
int l;
unsigned char cur_shadow_byte = ASAN_STACK_MAGIC_LEFT;
tree str_cst, decl, id;
@@ -1297,58 +1297,55 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
if (STRICT_ALIGNMENT)
set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
- /* Unpoison shadow memory of a stack at the very end of a function.
- As we're poisoning stack variables at the end of their scope,
- shadow memory must be properly unpoisoned here. The easiest approach
- would be to collect all variables that should not be unpoisoned and
- we unpoison shadow memory of the whole stack except ranges
- occupied by these variables. */
+ prev_offset = base_offset;
last_offset = base_offset;
- HOST_WIDE_INT current_offset = last_offset;
- if (length)
+ last_size = 0;
+ for (l = length; l; l -= 2)
{
- HOST_WIDE_INT var_end_offset = 0;
- HOST_WIDE_INT stack_start = offsets[length - 1];
- gcc_assert (last_offset == stack_start);
-
- for (int l = length - 2; l > 0; l -= 2)
+ offset = base_offset + ((offsets[l - 1] - base_offset)
+ & ~(ASAN_RED_ZONE_SIZE - HOST_WIDE_INT_1));
+ if (last_offset + last_size != offset)
{
- HOST_WIDE_INT var_offset = offsets[l];
- current_offset = var_offset;
- var_end_offset = offsets[l - 1];
- HOST_WIDE_INT rounded_size = ROUND_UP (var_end_offset - var_offset,
- BITS_PER_UNIT);
+ shadow_mem = adjust_address (shadow_mem, VOIDmode,
+ (last_offset - prev_offset)
+ >> ASAN_SHADOW_SHIFT);
+ prev_offset = last_offset;
+ asan_clear_shadow (shadow_mem, last_size >> ASAN_SHADOW_SHIFT);
+ last_offset = offset;
+ last_size = 0;
+ }
+ last_size += base_offset + ((offsets[l - 2] - base_offset)
+ & ~(ASAN_RED_ZONE_SIZE - HOST_WIDE_INT_1))
+ - offset;
- /* Should we unpoison the variable? */
+ /* Unpoison shadow memory that corresponds to a variable that was
+ is subject of use-after-return sanitization. */
+ if (l > 2)
+ {
+ decl = decls[l / 2 - 2];
if (asan_handled_variables != NULL
&& asan_handled_variables->contains (decl))
{
+ HOST_WIDE_INT size = offsets[l - 3] - offsets[l - 2];
if (dump_file && (dump_flags & TDF_DETAILS))
{
const char *n = (DECL_NAME (decl)
? IDENTIFIER_POINTER (DECL_NAME (decl))
: "<unknown>");
fprintf (dump_file, "Unpoisoning shadow stack for variable: "
- "%s (%" PRId64 "B)\n", n,
- var_end_offset - var_offset);
+ "%s (%" PRId64 " B)\n", n, size);
}
- unsigned HOST_WIDE_INT s
- = shadow_mem_size (current_offset - last_offset);
- asan_clear_shadow (shadow_mem, s);
- HOST_WIDE_INT shift
- = shadow_mem_size (current_offset - last_offset + rounded_size);
- shadow_mem = adjust_address (shadow_mem, VOIDmode, shift);
- last_offset = var_offset + rounded_size;
- current_offset = last_offset;
+ last_size += size & ~(ASAN_RED_ZONE_SIZE - HOST_WIDE_INT_1);
}
-
}
-
- /* Handle last redzone. */
- current_offset = offsets[0];
- asan_clear_shadow (shadow_mem,
- shadow_mem_size (current_offset - last_offset));
+ }
+ if (last_size)
+ {
+ shadow_mem = adjust_address (shadow_mem, VOIDmode,
+ (last_offset - prev_offset)
+ >> ASAN_SHADOW_SHIFT);
+ asan_clear_shadow (shadow_mem, last_size >> ASAN_SHADOW_SHIFT);
}
/* Clean-up set with instrumented stack variables. */
@@ -2802,9 +2799,13 @@ asan_expand_mark_ifn (gimple_stmt_iterator *iter)
decl = TREE_OPERAND (decl, 0);
gcc_checking_assert (TREE_CODE (decl) == VAR_DECL);
- if (asan_handled_variables == NULL)
- asan_handled_variables = new hash_set<tree> (16);
- asan_handled_variables->add (decl);
+
+ if (is_poison)
+ {
+ if (asan_handled_variables == NULL)
+ asan_handled_variables = new hash_set<tree> (16);
+ asan_handled_variables->add (decl);
+ }
tree len = gimple_call_arg (g, 2);
gcc_assert (tree_fits_shwi_p (len));
--
2.13.1