[PATCH] Fix malloc attribute mishandling

[Richard Guenther]

On Wed, Feb 10, 2010 at 12:13 AM, Xinliang David Li <davidxl@google.com> wrote:
> On Tue, Feb 9, 2010 at 2:44 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Tue, Feb 9, 2010 at 8:18 PM, Xinliang David Li <davidxl@google.com> wrote:
>>> Why is this a compiler problem, but not a user error? malloc attribute
>>> should have clear semantics to allow/disallow such use.
>>
>> It does have clear semantics.  The semantics is that a function with
>> the malloc attribute returns a pointer to memory that is not aliased
>> by any other pointer.  No further restrictions are documented.
>
> Is it carved in stone or can further be refined?

It's carved in stone - we can't probably break existing uses.  Thus for
a more restrictive interpretation we have to invent a new attribute
(or amend the function annotation patch).

Richard.

> David
>
>>
>>> Besides, can
>>> the malloc wrapper recognition detect things like 1) initializing heap
>>> memory to point to global variables; 2) making heap memory exposed
>>> before returning?  A warning can be emitted if needed when a violation
>>> is detected ...
>>
>> IPA-PTA can properly compute malloc property of malloc wrappers.
>> I don't understand your question fully though.
>
>
>
>>
>> Richard.
>>
>>> David
>>>
>>> On Tue, Feb 9, 2010 at 6:36 AM, Richard Guenther <rguenther@suse.de> wrote:
>>>>
>>>> As we now handle malloc results more precise we forget to consider
>>>> constructors with malloc attribute.  This causes some perl testcases
>>>> to fail.
>>>>
>>>> Fixed as follows -- note this pessimizes simple malloc wrappers again,
>>>> we probably need another attribute for more strict semantics (thus,
>>>> the returned memory doesn't point to anything, but is either
>>>> uninitialized or zero-initialized).  Not that this pessimization
>>>> would be a regression to 4.4 (but it is to previous 4.5 snapshots).
>>>>
>>>> Bootstrap and regtest running on x86_64-unknown-linux-gnu, will apply
>>>> if that succeeds.
>>>>
>>>> Richard.
>>>>
>>>>
>>>> 2010-02-09  Richard Guenther  <rguenther@suse.de>
>>>>
>>>>        PR tree-optimization/43008
>>>>        * tree-ssa-structalias.c (handle_lhs_call): Pass in the fndecl,
>>>>        make HEAP variables initialized from global memory if they
>>>>        are not known builtin functions.
>>>>        (find_func_aliases): Adjust.
>>>>
>>>>        * gcc.c-torture/execute/pr43008.c: New testcase.
>>>>
>>>> Index: gcc/tree-ssa-structalias.c
>>>> ===================================================================
>>>> *** gcc/tree-ssa-structalias.c  (revision 156620)
>>>> --- gcc/tree-ssa-structalias.c  (working copy)
>>>> *************** handle_rhs_call (gimple stmt, VEC(ce_s,
>>>> *** 3482,3488 ****
>>>>     the LHS point to global and escaped variables.  */
>>>>
>>>>  static void
>>>> ! handle_lhs_call (tree lhs, int flags, VEC(ce_s, heap) *rhsc)
>>>>  {
>>>>    VEC(ce_s, heap) *lhsc = NULL;
>>>>
>>>> --- 3482,3488 ----
>>>>     the LHS point to global and escaped variables.  */
>>>>
>>>>  static void
>>>> ! handle_lhs_call (tree lhs, int flags, VEC(ce_s, heap) *rhsc, tree fndecl)
>>>>  {
>>>>    VEC(ce_s, heap) *lhsc = NULL;
>>>>
>>>> *************** handle_lhs_call (tree lhs, int flags, VE
>>>> *** 3496,3501 ****
>>>> --- 3496,3507 ----
>>>>           it escapes.  */
>>>>        DECL_EXTERNAL (vi->decl) = 0;
>>>>        vi->is_global_var = 0;
>>>> +       /* If this is not a real malloc call assume the memory was
>>>> +          initialized and thus may point to global memory.  All
>>>> +        builtin functions with the malloc attribute behave in a sane way.  */
>>>> +       if (!fndecl
>>>> +         || DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
>>>> +       make_constraint_from (vi, nonlocal_id);
>>>>      }
>>>>    else if (VEC_length (ce_s, rhsc) > 0)
>>>>      {
>>>> *************** find_func_aliases (gimple origt)
>>>> *** 3798,3804 ****
>>>>            handle_rhs_call (t, &rhsc);
>>>>          if (gimple_call_lhs (t)
>>>>              && could_have_pointers (gimple_call_lhs (t)))
>>>> !           handle_lhs_call (gimple_call_lhs (t), flags, rhsc);
>>>>          VEC_free (ce_s, heap, rhsc);
>>>>        }
>>>>        else
>>>> --- 3804,3810 ----
>>>>            handle_rhs_call (t, &rhsc);
>>>>          if (gimple_call_lhs (t)
>>>>              && could_have_pointers (gimple_call_lhs (t)))
>>>> !           handle_lhs_call (gimple_call_lhs (t), flags, rhsc, fndecl);
>>>>          VEC_free (ce_s, heap, rhsc);
>>>>        }
>>>>        else
>>>> Index: gcc/testsuite/gcc.c-torture/execute/pr43008.c
>>>> ===================================================================
>>>> *** gcc/testsuite/gcc.c-torture/execute/pr43008.c       (revision 0)
>>>> --- gcc/testsuite/gcc.c-torture/execute/pr43008.c       (revision 0)
>>>> ***************
>>>> *** 0 ****
>>>> --- 1,23 ----
>>>> + int i;
>>>> + struct X {
>>>> +   int *p;
>>>> + };
>>>> + struct X * __attribute__((malloc))
>>>> + my_alloc (void)
>>>> + {
>>>> +   struct X *p = __builtin_malloc (sizeof (struct X));
>>>> +   p->p = &i;
>>>> +   return p;
>>>> + }
>>>> + extern void abort (void);
>>>> + int main()
>>>> + {
>>>> +   struct X *p, *q;
>>>> +   p = my_alloc ();
>>>> +   q = my_alloc ();
>>>> +   *(p->p) = 1;
>>>> +   *(q->p) = 0;
>>>> +   if (*(p->p) != 0)
>>>> +     abort ();
>>>> +   return 0;
>>>> + }
>>>>
>>>
>>
>