Re: [PATCH, RFC] Introduce -fsanitize=use-after-scope (v2)

[Martin Liška <mliska at suse dot cz>]

On 11/02/2016 10:40 AM, Richard Biener wrote:
> On Tue, Nov 1, 2016 at 4:12 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Tue, Nov 01, 2016 at 03:53:46PM +0100, Martin Liška wrote:
>>> @@ -1504,7 +1505,7 @@ non_rewritable_lvalue_p (tree lhs)
>>>
>>>  static void
>>>  maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs,
>>> -                 bitmap suitable_for_renaming)
>>> +                 bitmap suitable_for_renaming, bitmap marked_nonaddressable)
>>>  {
>>>    /* Global Variables, result decls cannot be changed.  */
>>>    if (is_global_var (var)
>>> @@ -1522,6 +1523,7 @@ maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs,
>>>         || !bitmap_bit_p (not_reg_needs, DECL_UID (var))))
>>>      {
>>>        TREE_ADDRESSABLE (var) = 0;
>>> +      bitmap_set_bit (marked_nonaddressable, DECL_UID (var));
>>
>> Why do you need the marked_nonaddressable bitmap?
>>
>>>        if (is_gimple_reg (var))
>>>       bitmap_set_bit (suitable_for_renaming, DECL_UID (var));
>>>        if (dump_file)
>>> @@ -1550,20 +1552,43 @@ maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs,
>>>      }
>>>  }
>>>
>>> -/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.  */
>>> +/* Return true when STMT is ASAN mark where second argument is an address
>>> +   of a local variable.  */
>>>
>>> -void
>>> -execute_update_addresses_taken (void)
>>> +static bool
>>> +is_asan_mark_p (gimple *stmt)
>>> +{
>>> +  if (!gimple_call_internal_p (stmt, IFN_ASAN_MARK))
>>> +    return false;
>>> +
>>> +  tree addr = get_base_address (gimple_call_arg (stmt, 1));
>>> +  if (TREE_CODE (addr) == ADDR_EXPR
>>> +      && TREE_CODE (TREE_OPERAND (addr, 0)) == VAR_DECL)
>>
>> Just check here if dropping TREE_ADDRESSABLE from the VAR (use VAR_P btw)
>> would turn it into is_gimple_reg), and don't return true if not.
>>
>>> +    return true;
>>> +
>>> +  return false;
>>> +}
>>> +
>>> +/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.
>>> +   If SANITIZE_ASAN_MARK is set to true, sanitize also ASAN_MARK built-ins.  */
>>> +
>>> +
>>> +static void
>>> +execute_update_addresses_taken (bool sanitize_asan_mark = false)
>>
>> I wonder if the sanitize_asan_mark wouldn't better be some PROP_* property
>> set during the asan pass and kept on until end of compilation of that
>> function.  That means even if a var only addressable because of ASAN_MARK is
>> discovered after this pass we'd still be able to rewrite it into SSA.
> 
> Note that being TREE_ADDRESSABLE also has effects on alias analysis
> (didn't follow the patches to see whether you handle ASAN_MARK specially
> in points-to analysis and/or alias analysis).

Currently all manipulation with shadow memory is done via a pointer type
which has created a separate aliasing set:

static void
asan_init_shadow_ptr_types (void)
{
  asan_shadow_set = new_alias_set ();
  tree types[3] = { signed_char_type_node, short_integer_type_node,
		    integer_type_node };

  for (unsigned i = 0; i < 3; i++)
    {
      shadow_ptr_types[i] = build_distinct_type_copy (types[i]);
      TYPE_ALIAS_SET (shadow_ptr_types[i]) = asan_shadow_set;
      shadow_ptr_types[i] = build_pointer_type (shadow_ptr_types[i]);
    }
...

Martin

> 
> Generally in update-address-taken you can handle ASAN_MARK similar to
> MEM_REF (and drop it in the rewrite phase?).
> 
> As said, I didnt look at the patches and just came by here seeing
> tree-ssa.c pieces...
> 
> Richard.
> 
>>         Jakub