Re: [PATCH] C/C++: Add -Waddress-of-packed-member

[Martin Sebor]

On 01/19/2018 10:14 AM, Martin Sebor wrote:

On 01/14/2018 07:29 AM, H.J. Lu wrote:

When address of packed member of struct or union is taken, it may result
in an unaligned pointer value.  This patch adds
-Waddress-of-packed-member
to warn it:

$ cat x.i
struct pair_t
{
  char c;
  int i;
} __attribute__ ((packed));

extern struct pair_t p;
int *addr = &p.i;
$ gcc -O2 -S x.i
x.i:8:13: warning: initialization of 'int *' from address of packed
member of 'struct pair_t' may result in an unaligned pointer value
[-Waddress-of-packed-member]
 int *addr = &p.i;
             ^
$

This warning is enabled by default.

I like this enhancement.  It would be useful for data types,
packed or not, such as casting int* to long*.

I noticed some differences from Clang for the test case below.
It seems that GCC should warn on all the cases Clang does.

Also, since converting the address of a struct to that of its
first member is common (especially in C and when the member
itself is a struct) I wonder if the warning should trigger
for those conversions as well.

struct A {
  int i;
} __attribute__ ((packed));

long* f8 (struct A *p) { return &p->i; }   // Clang only
int* f4 (struct A *p) { return &p->i; }    // Clang, GCC
short* f2 (struct A *p) { return &p->i; }  // Clang only
char* f1 (struct A *p) { return &p->i; }
void* f0 (struct A *p) { return &p->i; }

struct B { int i; };
struct C { struct B b; } __attribute__ ((packed));

long* g8 (struct C *p) { return p; }        // should warn?
int* g4 (struct C *p) { return &p->b; }     // Clang only

int* h4 (struct C *p) { return &p->b.i; }   // Clang only

After reading the Clang code review for the warning
(https://reviews.llvm.org/D20561) and experimenting with a few
more test cases I noticed some additional false negatives that
I think would be worthwhile diagnosing:

  struct A {
    int i;
  } __attribute__ ((packed));

  int f (struct A *p)
  {
    return *&p->i;
  }

An example similar to one of those discussed in the review is
one involving a conditional expression (Clang diagnoses this):

  int* f (struct A *p, int *q)
  {
    return q ? q : &p->i;   // missing warning
  }

Clang doesn't diagnose the conversion of a packed member array
to a pointer to its element.  It seems to me that it should be
diagnosed:

  struct B {
    int a[1];
  } __attribute__ ((packed));

  int* f (struct B *p)
  {
    return p->a;   // missing warning
  }

Similarly, in C++, binding to more strictly aligned references
should probably be diagnosed (Clang misses it):

int* g (struct A &r)
{
  int &ir = r.i;   // missing warning here
  return &ir;      // regardless of how ir is used
}

Martin