On 12/17/18 7:42 AM, H.J. Lu wrote:
On Mon, Dec 17, 2018 at 1:39 AM Richard Biener
<richard.guent...@gmail.com> wrote:
On Fri, Dec 14, 2018 at 11:48 PM H.J. Lu <hjl.to...@gmail.com> wrote:
On Fri, Dec 14, 2018 at 2:10 PM Jason Merrill <ja...@redhat.com> wrote:
On 12/13/18 6:56 PM, H.J. Lu wrote:
On Thu, Dec 13, 2018 at 12:50 PM Jason Merrill <ja...@redhat.com> wrote:
On 9/25/18 11:46 AM, H.J. Lu wrote:
On Fri, Aug 31, 2018 at 2:04 PM, Jason Merrill <ja...@redhat.com> wrote:
On 07/23/2018 05:24 PM, H.J. Lu wrote:
On Mon, Jun 18, 2018 at 12:26 PM, Joseph Myers <jos...@codesourcery.com>
wrote:
On Mon, 18 Jun 2018, Jason Merrill wrote:
On Mon, Jun 18, 2018 at 11:59 AM, Joseph Myers <jos...@codesourcery.com>
wrote:
On Mon, 18 Jun 2018, Jason Merrill wrote:
+ if (TREE_CODE (rhs) == COND_EXPR)
+ {
+ /* Check the THEN path first. */
+ tree op1 = TREE_OPERAND (rhs, 1);
+ context = check_address_of_packed_member (type, op1);
This should handle the GNU extension of re-using operand 0 if operand
1 is omitted.
Doesn't that just use a SAVE_EXPR?
Hmm, I suppose it does, but many places in the compiler seem to expect
that it produces a COND_EXPR with TREE_OPERAND 1 as NULL_TREE.
Maybe that's used somewhere inside the C++ front end. For C a SAVE_EXPR
is produced directly.
Here is the updated patch. Changes from the last one:
1. Handle COMPOUND_EXPR.
2. Fixed typos in comments.
3. Combined warn_for_pointer_of_packed_member and
warn_for_address_of_packed_member into
warn_for_address_or_pointer_of_packed_member.
c.i:4:33: warning: converting a packed ‘struct C *’ pointer increases the
alignment of ‘long int *’ pointer from 1 to 8 [-Waddress-of-packed-member]
I think this would read better as
c.i:4:33: warning: converting a packed ‘struct C *’ pointer (alignment 1) to
‘long int *’ (alignment 8) may result in an unaligned pointer value
[-Waddress-of-packed-member]
Fixed.
+ while (TREE_CODE (base) == ARRAY_REF)
+ base = TREE_OPERAND (base, 0);
+ if (TREE_CODE (base) != COMPONENT_REF)
+ return NULL_TREE;
Are you deliberately not handling the other handled_component_p cases? If
so, there should be a comment.
I changed it to
while (handled_component_p (base))
{
enum tree_code code = TREE_CODE (base);
if (code == COMPONENT_REF)
break;
switch (code)
{
case ARRAY_REF:
base = TREE_OPERAND (base, 0);
break;
default:
/* FIXME: Can it ever happen? */
gcc_unreachable ();
break;
}
}
Is there a testcase to trigger this ICE? I couldn't find one.
You can take the address of an element of complex:
__complex int i;
int *p = &__real(i);
You may get VIEW_CONVERT_EXPR with location wrappers.
Fixed. I replaced gcc_unreachable with return NULL_TREE;
Then we're back to my earlier question: are you deliberately not
handling the other cases? Why not look through them as well? What if
e.g. the operand of __real is a packed field?
Here is the updated patch with
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index 615134cfdac..f105742598e 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -2669,6 +2669,9 @@ check_address_of_packed_member (tree type, tree rhs)
switch (code)
{
case ARRAY_REF:
+ case REALPART_EXPR:
+ case IMAGPART_EXPR:
+ case VIEW_CONVERT_EXPR:
base = TREE_OPERAND (base, 0);
break;
default:
don't we have handled_component_p () for this? (you're still
missing BIT_FIELD_REF which might be used for vector
element accesses)
Do you have a testcase?
Is there a reason you only want to handle some component references and
not others? If not, checking handled_component_p is simpler and more
future proof than enumerating specific codes.
Jason
