Hi,
I wrote a small writeup to summarize the two approaches to generate
.ACCESS_WITH_SIZE for pointers with counted_by,
In this writeup, I described the major issue for the approach we agreed on last
week to fix PR120929, due to the problem,
the previous implementation in the committed (and reverted) patches should be
kept.
I plan to:
1. Keep the patches that were implemented in version #7;
2. Add the fix for PR120929 in gcc/tree-object-size.cc, and testing case;
3. Rebase the version #7 patch on top of the latest trunk (in which the #of
argument of .ACCESS_WITH_SIZE has been reduced from 6 to 4).
Let me know if you have any comment or suggestion.
thanks.
Qing
==========================================
The 7th version of the patch for extending counted_by to pointer field
has been committed into trunk, but triggered PR120929.
I reverted the patches from trunk due to PR120929.
In order to fix PR120929, we agreed on the following solution:
for a pointer field with counted_by attribute:
struct S {
int n;
int *p __attribute__((counted_by(n)));
} *f;
f->p = malloc (size);
when generating the call to .ACCESS_WITH_SIZE for f->p, there are two
approaches:
A. Pass the ADDRESS of the original pointer &(f->p) as the first argument,
and also return the ADDRESS of the original pointer:
*.ACCESS_WITH_SIZE (&f->p, &f->n,...)
B. Pass the VALUE of the original pointer f->p as the first argument,
and also return the original pointer:
.ACCESS_WITH_SIZE (f->p, &f->n,...)
I used the above A in the previous committed (and reverted) 7th version of the
patch;
the above B was suggested and agreed to fix PR120929 based on our previous
discussion on PR120929.
I re-implemented the patch based on B to fix PR120929, however, the approach
B brings undefined behavior into the application.
(Actually, I met this issue in the previous implementation but forgot to
documented it.
This issue is the exact reason I chose A in my committed patch).
f->p = malloc (size);
***** With the approach B: the IL for the above is:
tmp1 = f->p;
tmp2 = &f->n;
tmp3 = .ACCESS_WITH_SIZE (tmp1, tmp2, ...);
tmp4 = malloc (size);
tmp3 = tmp4;
In the above, in order to generate a call to .ACCESS_WITH_SIZE for the pointer
reference f->p, the new GIMPLE tmp1 = f->p is necessary to pass the value of
the pointer f->p to the call to .ACCESS_WITH_SIZE. However, this new GIMPLE is
the one that brings UB into the application since the value of f->p is not
initialized yet
when it is assigned to "tmp1".
the above IL will be expanded to the following when .ACCESS_WITH_SIZE is
expanded
to its first argument:
tmp1 = f->p;
tmp2 = &f->n;
tmp3 = tmp1;
tmp4 = malloc (size);
tmp3 = tmp4;
the final optimized IL will be:
tmp3 = f->p;
tmp3 = malloc (size);;
As a result, the f->p will NOT be set correctly to the pointer
returned by malloc (size).
***** With the Approach A, the IL is:
tmp1 = &f->p;
tmp2 = &f->n;
tmp3 = .ACCESS_WITH_SIZE (tmp1, tmp2, ...);
tmp4 = malloc (size);
*tmp3 = tmp4;
After .ACCESS_WITH_SIZE is expanded to its first argument:
tmp1 = &f->p;
tmp2 = &f->n;
tmp3 = tmp1;
tmp4 = malloc (size);
*tmp3 = tmp4;
The final optimized IL will be:
*(&f->p) = malloc (size);
Which is the correct IL for the original source code.
Based on the above, I think that we should take the Approach A for the
pointers with counted_by attribute. i.e:
A. Pass the ADDRESS of the original pointer &(f->p) as the first argument,
and also return the ADDRESS of the original pointer:
*.ACCESS_WITH_SIZE (&f->p, &f->n,…)
As A result,
1. I will keep the patches that were implemented in version #7;
2. Add the fix for PR120929 in gcc/tree-object-size.cc, and testing case;
3. Rebase the version #7 patch on top of the latest trunk (in which the #of
argument of .ACCESS_WITH_SIZE has been reduced from 6 to 4).
