From: Guy Harris <guy@alum.mit.edu>
Date: Sat, 1 Feb 2014 22:02:17 +0000 (-0800)
Subject: Don't use the __attribute__((packed)) on most platforms.
X-Git-Tag: tcpdump-4.6.0~242
X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/78f0dde7c972f65de116912d48fb09d0a296a53e

Don't use the __attribute__((packed)) on most platforms.

It won't necessarily work with non-GCC-compatible compilers, so use it
only on GCC-compatible compilers.

Even with some GCC-compatible compilers (such as, err, umm, GCC), it
doesn't do the right thing on some platforms, e.g. 64-bit SPARC, where
the compiler generates code that assumes alignment even when using it.
The only platforms I know of where an unaligned big-endian load can be
done better than by loading bytes and shifting-and-ORing them together
are MIPS and possibly Alpha, so only do it there.
---

diff --git a/extract.h b/extract.h
index 2af90d07..4dfbaae3 100644
--- a/extract.h
+++ b/extract.h
@@ -26,20 +26,59 @@
 /*
  * The processor doesn't natively handle unaligned loads.
  */
-#ifdef HAVE___ATTRIBUTE__
+#if defined(__GNUC__) && defined(HAVE___ATTRIBUTE__) && \
+    (defined(__alpha) || defined(__alpha__) || \
+     defined(__mips) || defined(__mips__))
+
 /*
- * We have __attribute__; we assume that means we have __attribute__((packed)).
+ * This is a GCC-compatible compiler and we have __attribute__, which
+ * we assume that mean we have __attribute__((packed)), and this is
+ * MIPS or Alpha, which has instructions that can help when doing
+ * unaligned loads.
+ *
  * Declare packed structures containing a u_int16_t and a u_int32_t,
  * cast the pointer to point to one of those, and fetch through it;
  * the GCC manual doesn't appear to explicitly say that
  * __attribute__((packed)) causes the compiler to generate unaligned-safe
  * code, but it apppears to do so.
  *
- * We do this in case the compiler can generate, for this instruction set,
- * better code to do an unaligned load and pass stuff to "ntohs()" or
- * "ntohl()" than the code to fetch the bytes one at a time and
- * assemble them.  (That might not be the case on a little-endian platform,
- * where "ntohs()" and "ntohl()" might not be done inline.)
+ * We do this in case the compiler can generate code using those
+ * instructions to do an unaligned load and pass stuff to "ntohs()" or
+ * "ntohl()", which might be better than than the code to fetch the
+ * bytes one at a time and assemble them.  (That might not be the
+ * case on a little-endian platform, such as DEC's MIPS machines and
+ * Alpha machines, where "ntohs()" and "ntohl()" might not be done
+ * inline.)
+ *
+ * We do this only for specific architectures because, for example,
+ * at least some versions of GCC, when compiling for 64-bit SPARC,
+ * generate code that assumes alignment if we do this.
+ *
+ * XXX - add other architectures and compilers as possible and
+ * appropriate.
+ *
+ * HP's C compiler, indicated by __HP_cc being defined, supports
+ * "#pragma unaligned N" in version A.05.50 and later, where "N"
+ * specifies a number of bytes at which the typedef on the next
+ * line is aligned, e.g.
+ *
+ *	#pragma unalign 1
+ *	typedef u_int16_t unaligned_u_int16_t;
+ *
+ * to define unaligned_u_int16_t as a 16-bit unaligned data type.
+ * This could be presumably used, in sufficiently recent versions of
+ * the compiler, with macros similar to those below.  This would be
+ * useful only if that compiler could generate better code for PA-RISC
+ * or Itanium than would be generated by a bunch of shifts-and-ORs.
+ *
+ * DEC C, indicated by __DECC being defined, has, at least on Alpha,
+ * an __unaligned qualifier that can be applied to pointers to get the
+ * compiler to generate code that does unaligned loads and stores when
+ * dereferencing the pointer in question.
+ *
+ * XXX - what if the native C compiler doesn't support
+ * __attribute__((packed))?  How can we get it to generate unaligned
+ * accesses for *specific* items?
  */
 typedef struct {
 	u_int16_t	val;
@@ -66,12 +105,13 @@ EXTRACT_64BITS(const void *p)
 {
 	return ((u_int64_t)(((u_int64_t)ntohl(((const unaligned_u_int32_t *)(p) + 0)->val)) << 32 | \
 		((u_int64_t)ntohl(((const unaligned_u_int32_t *)(p) + 1)->val)) << 0));
-
 }
 
-#else /* HAVE___ATTRIBUTE__ */
+#else /* have to do it a byte at a time */
 /*
- * We don't have __attribute__, so do unaligned loads of big-endian
+ * This isn't a GCC-compatible compiler, we don't have __attribute__,
+ * or we do but we don't know of any better way with this instruction
+ * set to do unaligned loads, so do unaligned loads of big-endian
  * quantities the hard way - fetch the bytes one at a time and
  * assemble them.
  */
@@ -92,7 +132,7 @@ EXTRACT_64BITS(const void *p)
 		     (u_int64_t)*((const u_int8_t *)(p) + 5) << 16 | \
 		     (u_int64_t)*((const u_int8_t *)(p) + 6) << 8 | \
 		     (u_int64_t)*((const u_int8_t *)(p) + 7)))
-#endif /* HAVE___ATTRIBUTE__ */
+#endif /* must special-case unaligned accesses */
 #else /* LBL_ALIGN */
 /*
  * The processor natively handles unaligned loads, so we can just