[PATCH] D20561: Warn when taking address of packed member

[Roger Ferrer Ibanez via cfe-commits]

rogfer01 added a comment.

Firefox build shows a couple of warnings in the sctp library that already gave warnings with the earlier patches.

That code has the following structure.

  #define SCTP_PACKED __attribute__((packed))
  #define SCTP_IDENTIFICATION_SIZE 16
  // ...
  /* state cookie header */
  struct sctp_state_cookie {	/* this is our definition... */
  	uint8_t identification[SCTP_IDENTIFICATION_SIZE];/* id of who we are */
  	struct timeval time_entered;	/* the time I built cookie */
          // other fields
  } SCTP_PACKED;

The warning is triggered by the following code (the other occurence is almost exact).

  net->RTO = sctp_calculate_rto(stcb, asoc, net,
  				      &cookie->time_entered,   // ← warning here!
  				      sctp_align_unsafe_makecopy,
  				      SCTP_RTT_FROM_NON_DATA);

the called function being declared as

  uint32_t
  sctp_calculate_rto(struct sctp_tcb *stcb,
  		   struct sctp_association *asoc,
  		   struct sctp_nets *net,
  		   struct timeval *told,
  		   int safe, int rtt_from_sack)
  {
    ...

So I think that is a legitimate warning.

But the code is working (assuming this function is ever called that I didn't check). Checking the code, though, reveals a curious usage of `told`.

  	/* Copy it out for sparc64 */
  	if (safe == sctp_align_unsafe_makecopy) {
  		old = &then;
  		memcpy(&then, told, sizeof(struct timeval));
  	} else if (safe == sctp_align_safe_nocopy) {
  		old = told;
  	} else {
  		/* error */
  		SCTP_PRINTF("Huh, bad rto calc call\n");
  		return (0);
  	}

which suggests that the code somehow knows that the pointer is unaligned and cannot be copied straightforwardly.

We can cast to `void*` and back to the original pointer type.

  net->RTO = sctp_calculate_rto(stcb, asoc, net,
  				      (struct timeval*)(void*)&cookie->time_entered, // note: the cast is because this pointer is unaligned
  				      sctp_align_unsafe_makecopy,
  				      SCTP_RTT_FROM_NON_DATA);

Which looks a bit ugly to me but clearly pinpoints a problem and can be wrapped in a macro.

  #define UNALIGNED_ADDRESS(x) ((__typeof__(x))(void*)(x))
  
  ...
  
  net->RTO = sctp_calculate_rto(stcb, asoc, net,
  				      UNALIGNED_ADDRESS(&cookie->time_entered),
  				      sctp_align_unsafe_makecopy,
  				      SCTP_RTT_FROM_NON_DATA);

What do you think?


http://reviews.llvm.org/D20561