Add a pointer alignment helper function.

Also use a slightly more conservative pattern. Instead of aligning the
pointer as a uintptr_t and casting back, compute the offset and advance
in pointer space. C guarantees that casting from pointer to uintptr_t
and back gives the same pointer, but general integer-to-pointer
conversions are generally implementation-defined. GCC does define it in
the useful way, but this makes fewer dependencies.

Change-Id: I70c7af735e892fe7a8333b78b39d7b1f3f1cdbef
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48405
Reviewed-by: Adam Langley <[email protected]>

Author: davidben

crypto/hrss/hrss.c

@@ -1871,9 +1871,7 @@ static struct public_key *public_key_from_external(
       sizeof(struct HRSS_public_key) >= sizeof(struct public_key) + 15,
       "HRSS public key too small");
 
-  uintptr_t p = (uintptr_t)ext;
-  p = (p + 15) & ~15;
-  return (struct public_key *)p;
+  return align_pointer(ext->opaque, 16);
 }
 
 // private_key_from_external does the same thing as |public_key_from_external|,
@@ -1885,9 +1883,7 @@ static struct private_key *private_key_from_external(
       sizeof(struct HRSS_private_key) >= sizeof(struct private_key) + 15,
       "HRSS private key too small");
 
-  uintptr_t p = (uintptr_t)ext;
-  p = (p + 15) & ~15;
-  return (struct private_key *)p;
+  return align_pointer(ext->opaque, 16);
 }
 
 void HRSS_generate_key(

crypto/internal.h

@@ -209,6 +209,9 @@ typedef __uint128_t uint128_t;
 #define OPENSSL_SSE2
 #endif
 
+
+// Pointer utility functions.
+
 // buffers_alias returns one if |a| and |b| alias and zero otherwise.
 static inline int buffers_alias(const uint8_t *a, size_t a_len,
                                 const uint8_t *b, size_t b_len) {
@@ -221,6 +224,23 @@ static inline int buffers_alias(const uint8_t *a, size_t a_len,
   return a_u + a_len > b_u && b_u + b_len > a_u;
 }
 
+// align_pointer returns |ptr|, advanced to |alignment|. |alignment| must be a
+// power of two, and |ptr| must have at least |alignment - 1| bytes of scratch
+// space.
+static inline void *align_pointer(void *ptr, size_t alignment) {
+  // |alignment| must be a power of two.
+  assert(alignment != 0 && (alignment & (alignment - 1)) == 0);
+  // Instead of aligning |ptr| as a |uintptr_t| and casting back, compute the
+  // offset and advance in pointer space. C guarantees that casting from pointer
+  // to |uintptr_t| and back gives the same pointer, but general
+  // integer-to-pointer conversions are implementation-defined. GCC does define
+  // it in the useful way, but this makes fewer assumptions.
+  uintptr_t offset = (0u - (uintptr_t)ptr) & (alignment - 1);
+  ptr = (char *)ptr + offset;
+  assert(((uintptr_t)ptr & (alignment - 1)) == 0);
+  return ptr;
+}
+
 
 // Constant-time utility functions.
 //

crypto/poly1305/poly1305.c

@@ -56,7 +56,7 @@ OPENSSL_STATIC_ASSERT(
 
 static inline struct poly1305_state_st *poly1305_aligned_state(
     poly1305_state *state) {
-  return (struct poly1305_state_st *)(((uintptr_t)state + 63) & ~63);
+  return align_pointer(state, 64);
 }
 
 // poly1305_blocks updates |state| given some amount of input data. This

tool/speed.cc

@@ -342,12 +342,6 @@ static bool SpeedRSAKeyGen(const std::string &selected) {
   return true;
 }
 
-static uint8_t *align(uint8_t *in, unsigned alignment) {
-  return reinterpret_cast<uint8_t *>(
-      (reinterpret_cast<uintptr_t>(in) + alignment) &
-      ~static_cast<size_t>(alignment - 1));
-}
-
 static std::string ChunkLenSuffix(size_t chunk_len) {
   char buf[32];
   snprintf(buf, sizeof(buf), " (%zu byte%s)", chunk_len,
@@ -384,13 +378,17 @@ static bool SpeedAEADChunk(const EVP_AEAD *aead, std::string name,
       new uint8_t[overhead_len + kAlignment]);
 
 
-  uint8_t *const in = align(in_storage.get(), kAlignment);
+  uint8_t *const in =
+      static_cast<uint8_t *>(align_pointer(in_storage.get(), kAlignment));
   OPENSSL_memset(in, 0, chunk_len);
-  uint8_t *const out = align(out_storage.get(), kAlignment);
+  uint8_t *const out =
+      static_cast<uint8_t *>(align_pointer(out_storage.get(), kAlignment));
   OPENSSL_memset(out, 0, chunk_len + overhead_len);
-  uint8_t *const tag = align(tag_storage.get(), kAlignment);
+  uint8_t *const tag =
+      static_cast<uint8_t *>(align_pointer(tag_storage.get(), kAlignment));
   OPENSSL_memset(tag, 0, overhead_len);
-  uint8_t *const in2 = align(in2_storage.get(), kAlignment);
+  uint8_t *const in2 =
+      static_cast<uint8_t *>(align_pointer(in2_storage.get(), kAlignment));
 
   if (!EVP_AEAD_CTX_init_with_direction(ctx.get(), aead, key.get(), key_len,
                                         EVP_AEAD_DEFAULT_TAG_LENGTH,