This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix malloc attribute mishandling


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;
>>>> + }
>>>>
>>>
>>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]