1 From 5177b28400e03d8666dfb3e65c3016b5062ce0f7 Mon Sep 17 00:00:00 2001 2 From: Chih-Hung Hsieh <chh (a] google.com> 3 Date: Thu, 15 Nov 2018 16:07:02 -0800 4 Subject: [PATCH] Update asserts in mojo about int64_t's alignment 5 6 The purpose of these asserts is to check that the options structs are aligned 7 sufficiently that an int64_t (or similar) could be added to them. The asserts 8 were added in https://codereview.chromium.org/295383012 9 10 However, the alignment of int64_t on 32-bit x86 is actually 4 bytes, not 8. So 11 how did this ever work? Before Clang r345419, the alignof() operator (which is what 12 MOJO_ALIGNOF maps to) would return the *preferred alignment* of a type, not 13 the *ABI alignment*. That's not what the C and C++ standards specify, so the 14 bug was fixed and the asserts started firing. (See also 15 https://llvm.org/pr26547) 16 17 To fix the build, change the asserts to check that int64_t requires 8 bytes 18 alignment *or less*. 19 20 Bug: 900406 21 Bug: 119634736 22 Change-Id: Icf80cea80ac31082423faab8c192420d0b98d515 23 Reviewed-on: https://chromium-review.googlesource.com/c/1318971 24 Commit-Queue: Ken Rockot <rockot (a] google.com> 25 Reviewed-by: Ken Rockot <rockot (a] google.com> 26 Cr-Commit-Position: refs/heads/master@{#605699} 27 --- 28 mojo/core/options_validation_unittest.cc | 2 +- 29 mojo/public/c/system/buffer.h | 2 +- 30 mojo/public/c/system/data_pipe.h | 2 +- 31 mojo/public/c/system/message_pipe.h | 2 +- 32 4 files changed, 4 insertions(+), 4 deletions(-) 33 34 diff --git a/mojo/core/options_validation_unittest.cc b/mojo/core/options_validation_unittest.cc 35 index 52e0032..b4b02dc 100644 36 --- a/mojo/core/options_validation_unittest.cc 37 +++ b/mojo/core/options_validation_unittest.cc 38 @@ -18,7 +18,7 @@ namespace { 39 40 using TestOptionsFlags = uint32_t; 41 42 -static_assert(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment"); 43 +static_assert(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment"); 44 struct MOJO_ALIGNAS(8) TestOptions { 45 uint32_t struct_size; 46 TestOptionsFlags flags; 47 diff --git a/mojo/public/c/system/buffer.h b/mojo/public/c/system/buffer.h 48 index 2cc5427..83b198b 100644 49 --- a/mojo/public/c/system/buffer.h 50 +++ b/mojo/public/c/system/buffer.h 51 @@ -30,7 +30,7 @@ struct MOJO_ALIGNAS(8) MojoCreateSharedBufferOptions { 52 // See |MojoCreateSharedBufferFlags|. 53 MojoCreateSharedBufferFlags flags; 54 }; 55 -MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment"); 56 +MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment"); 57 MOJO_STATIC_ASSERT(sizeof(MojoCreateSharedBufferOptions) == 8, 58 "MojoCreateSharedBufferOptions has wrong size"); 59 60 diff --git a/mojo/public/c/system/data_pipe.h b/mojo/public/c/system/data_pipe.h 61 index 3702cdb..c72f553 100644 62 --- a/mojo/public/c/system/data_pipe.h 63 +++ b/mojo/public/c/system/data_pipe.h 64 @@ -40,7 +40,7 @@ struct MOJO_ALIGNAS(8) MojoCreateDataPipeOptions { 65 // system-dependent capacity of at least one element in size. 66 uint32_t capacity_num_bytes; 67 }; 68 -MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment"); 69 +MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment"); 70 MOJO_STATIC_ASSERT(sizeof(MojoCreateDataPipeOptions) == 16, 71 "MojoCreateDataPipeOptions has wrong size"); 72 73 diff --git a/mojo/public/c/system/message_pipe.h b/mojo/public/c/system/message_pipe.h 74 index 9f222f9..0f642dd 100644 75 --- a/mojo/public/c/system/message_pipe.h 76 +++ b/mojo/public/c/system/message_pipe.h 77 @@ -35,7 +35,7 @@ struct MOJO_ALIGNAS(8) MojoCreateMessagePipeOptions { 78 // See |MojoCreateMessagePipeFlags|. 79 MojoCreateMessagePipeFlags flags; 80 }; 81 -MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment"); 82 +MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment"); 83 MOJO_STATIC_ASSERT(sizeof(MojoCreateMessagePipeOptions) == 8, 84 "MojoCreateMessagePipeOptions has wrong size"); 85 86 -- 87 2.20.0.rc0.387.gc7a69e6b6c-goog 88 89
