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]
+ 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.
+ /* Check alignment of the object. */
+ if (TREE_CODE (object) == COMPONENT_REF)
+ {
+ field = TREE_OPERAND (object, 1);
+ if (TREE_CODE (field) == FIELD_DECL && DECL_PACKED (field))
+ {
+ type_align = TYPE_ALIGN (type);
+ context = DECL_CONTEXT (field);
+ record_align = TYPE_ALIGN (context);
+ if ((record_align % type_align) != 0)
+ return context;
+ }
+ }
Why doesn't this recurse? What if you have a packed field three
COMPONENT_REFs down?
+ 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);
+ if (context)
+ rhs = op1;
+ else
+ {
+ /* Check the ELSE path. */
+ rhs = TREE_OPERAND (rhs, 2);
+ context = check_address_of_packed_member (type, rhs);
+ }
+ }
Likewise, what if you have more levels of COND_EXPR? Or COMPOUND_EXPR
within COND_EXPR?
@@ -7470,6 +7470,9 @@ convert_for_arg_passing (tree type, tree val,
tsubst_flags_t complain)
+ warn_for_address_or_pointer_of_packed_member (true, type, val);
@@ -8914,6 +8914,8 @@ convert_for_assignment (tree type, tree rhs,
+ warn_for_address_or_pointer_of_packed_member (true, type, rhs);
Why would address_p be true in these calls? It seems that you are
warning at the point of assignment but looking for the warning about
taking the address rather than the one about assignment.
If you want to warn about taking the address, shouldn't that happen
under cp_build_addr_expr? Alternately, drop the address_p parameter and
choose your path inside warn_for_*_packed_member based on whether rhs is
an ADDR_EXPR there rather than in the caller.
Jason
