summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChih-Hung Hsieh <chh@google.com>2018-11-15 16:07:02 -0800
committerChih-Hung Hsieh <chh@google.com>2018-11-26 13:47:22 -0800
commit9525530ea4ee4d70b55b5b376ad96ecdd587bd39 (patch)
treed2935e8a1391c495bc45667b01520d726183ffdc
parent263dedf8254f22ef57e5746b5bc51d659bf3cdfb (diff)
downloadlibchrome-9525530ea4ee4d70b55b5b376ad96ecdd587bd39.tar.gz
The purpose of these asserts is to check that the options structs are aligned sufficiently that an int64_t (or similar) could be added to them. The asserts were added in https://codereview.chromium.org/295383012 However, the alignment of int64_t on 32-bit x86 is actually 4 bytes, not 8. So how did this ever work? Before Clang r345419, the alignof() operator (which is what MOJO_ALIGNOF maps to) would return the *preferred alignment* of a type, not the *ABI alignment*. That's not what the C and C++ standards specify, so the bug was fixed and the asserts started firing. (See also https://llvm.org/pr26547) To fix the build, change the asserts to check that int64_t requires 8 bytes alignment *or less*. Bug: 900406 Bug: 119634736 Test: make checkbuild Change-Id: Icf80cea80ac31082423faab8c192420d0b98d515 Reviewed-on: https://chromium-review.googlesource.com/c/1318971 Commit-Queue: Ken Rockot <rockot@google.com> Reviewed-by: Ken Rockot <rockot@google.com> Cr-Commit-Position: refs/heads/master@{#605699}
-rw-r--r--libchrome_tools/patch/alignof_int64.patch88
-rw-r--r--mojo/core/options_validation_unittest.cc2
-rw-r--r--mojo/public/c/system/buffer.h2
-rw-r--r--mojo/public/c/system/data_pipe.h2
-rw-r--r--mojo/public/c/system/message_pipe.h2
5 files changed, 92 insertions, 4 deletions
diff --git a/libchrome_tools/patch/alignof_int64.patch b/libchrome_tools/patch/alignof_int64.patch
new file mode 100644
index 0000000000..121def4d39
--- /dev/null
+++ b/libchrome_tools/patch/alignof_int64.patch
@@ -0,0 +1,88 @@
+From 5177b28400e03d8666dfb3e65c3016b5062ce0f7 Mon Sep 17 00:00:00 2001
+From: Chih-Hung Hsieh <chh@google.com>
+Date: Thu, 15 Nov 2018 16:07:02 -0800
+Subject: [PATCH] Update asserts in mojo about int64_t's alignment
+
+The purpose of these asserts is to check that the options structs are aligned
+sufficiently that an int64_t (or similar) could be added to them. The asserts
+were added in https://codereview.chromium.org/295383012
+
+However, the alignment of int64_t on 32-bit x86 is actually 4 bytes, not 8. So
+how did this ever work? Before Clang r345419, the alignof() operator (which is what
+MOJO_ALIGNOF maps to) would return the *preferred alignment* of a type, not
+the *ABI alignment*. That's not what the C and C++ standards specify, so the
+bug was fixed and the asserts started firing. (See also
+https://llvm.org/pr26547)
+
+To fix the build, change the asserts to check that int64_t requires 8 bytes
+alignment *or less*.
+
+Bug: 900406
+Bug: 119634736
+Change-Id: Icf80cea80ac31082423faab8c192420d0b98d515
+Reviewed-on: https://chromium-review.googlesource.com/c/1318971
+Commit-Queue: Ken Rockot <rockot@google.com>
+Reviewed-by: Ken Rockot <rockot@google.com>
+Cr-Commit-Position: refs/heads/master@{#605699}
+---
+ mojo/core/options_validation_unittest.cc | 2 +-
+ mojo/public/c/system/buffer.h | 2 +-
+ mojo/public/c/system/data_pipe.h | 2 +-
+ mojo/public/c/system/message_pipe.h | 2 +-
+ 4 files changed, 4 insertions(+), 4 deletions(-)
+
+diff --git a/mojo/core/options_validation_unittest.cc b/mojo/core/options_validation_unittest.cc
+index 52e0032..b4b02dc 100644
+--- a/mojo/core/options_validation_unittest.cc
++++ b/mojo/core/options_validation_unittest.cc
+@@ -18,7 +18,7 @@ namespace {
+
+ using TestOptionsFlags = uint32_t;
+
+-static_assert(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment");
++static_assert(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment");
+ struct MOJO_ALIGNAS(8) TestOptions {
+ uint32_t struct_size;
+ TestOptionsFlags flags;
+diff --git a/mojo/public/c/system/buffer.h b/mojo/public/c/system/buffer.h
+index 2cc5427..83b198b 100644
+--- a/mojo/public/c/system/buffer.h
++++ b/mojo/public/c/system/buffer.h
+@@ -30,7 +30,7 @@ struct MOJO_ALIGNAS(8) MojoCreateSharedBufferOptions {
+ // See |MojoCreateSharedBufferFlags|.
+ MojoCreateSharedBufferFlags flags;
+ };
+-MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment");
++MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment");
+ MOJO_STATIC_ASSERT(sizeof(MojoCreateSharedBufferOptions) == 8,
+ "MojoCreateSharedBufferOptions has wrong size");
+
+diff --git a/mojo/public/c/system/data_pipe.h b/mojo/public/c/system/data_pipe.h
+index 3702cdb..c72f553 100644
+--- a/mojo/public/c/system/data_pipe.h
++++ b/mojo/public/c/system/data_pipe.h
+@@ -40,7 +40,7 @@ struct MOJO_ALIGNAS(8) MojoCreateDataPipeOptions {
+ // system-dependent capacity of at least one element in size.
+ uint32_t capacity_num_bytes;
+ };
+-MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment");
++MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment");
+ MOJO_STATIC_ASSERT(sizeof(MojoCreateDataPipeOptions) == 16,
+ "MojoCreateDataPipeOptions has wrong size");
+
+diff --git a/mojo/public/c/system/message_pipe.h b/mojo/public/c/system/message_pipe.h
+index 9f222f9..0f642dd 100644
+--- a/mojo/public/c/system/message_pipe.h
++++ b/mojo/public/c/system/message_pipe.h
+@@ -35,7 +35,7 @@ struct MOJO_ALIGNAS(8) MojoCreateMessagePipeOptions {
+ // See |MojoCreateMessagePipeFlags|.
+ MojoCreateMessagePipeFlags flags;
+ };
+-MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment");
++MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment");
+ MOJO_STATIC_ASSERT(sizeof(MojoCreateMessagePipeOptions) == 8,
+ "MojoCreateMessagePipeOptions has wrong size");
+
+--
+2.20.0.rc0.387.gc7a69e6b6c-goog
+
diff --git a/mojo/core/options_validation_unittest.cc b/mojo/core/options_validation_unittest.cc
index 52e0032a89..b4b02dc735 100644
--- a/mojo/core/options_validation_unittest.cc
+++ b/mojo/core/options_validation_unittest.cc
@@ -18,7 +18,7 @@ namespace {
using TestOptionsFlags = uint32_t;
-static_assert(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment");
+static_assert(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment");
struct MOJO_ALIGNAS(8) TestOptions {
uint32_t struct_size;
TestOptionsFlags flags;
diff --git a/mojo/public/c/system/buffer.h b/mojo/public/c/system/buffer.h
index 2cc54270ad..83b198b2a8 100644
--- a/mojo/public/c/system/buffer.h
+++ b/mojo/public/c/system/buffer.h
@@ -30,7 +30,7 @@ struct MOJO_ALIGNAS(8) MojoCreateSharedBufferOptions {
// See |MojoCreateSharedBufferFlags|.
MojoCreateSharedBufferFlags flags;
};
-MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment");
+MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment");
MOJO_STATIC_ASSERT(sizeof(MojoCreateSharedBufferOptions) == 8,
"MojoCreateSharedBufferOptions has wrong size");
diff --git a/mojo/public/c/system/data_pipe.h b/mojo/public/c/system/data_pipe.h
index 3702cdb624..c72f55387a 100644
--- a/mojo/public/c/system/data_pipe.h
+++ b/mojo/public/c/system/data_pipe.h
@@ -40,7 +40,7 @@ struct MOJO_ALIGNAS(8) MojoCreateDataPipeOptions {
// system-dependent capacity of at least one element in size.
uint32_t capacity_num_bytes;
};
-MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment");
+MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment");
MOJO_STATIC_ASSERT(sizeof(MojoCreateDataPipeOptions) == 16,
"MojoCreateDataPipeOptions has wrong size");
diff --git a/mojo/public/c/system/message_pipe.h b/mojo/public/c/system/message_pipe.h
index 9f222f9aa8..0f642dd965 100644
--- a/mojo/public/c/system/message_pipe.h
+++ b/mojo/public/c/system/message_pipe.h
@@ -35,7 +35,7 @@ struct MOJO_ALIGNAS(8) MojoCreateMessagePipeOptions {
// See |MojoCreateMessagePipeFlags|.
MojoCreateMessagePipeFlags flags;
};
-MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) == 8, "int64_t has weird alignment");
+MOJO_STATIC_ASSERT(MOJO_ALIGNOF(int64_t) <= 8, "int64_t has weird alignment");
MOJO_STATIC_ASSERT(sizeof(MojoCreateMessagePipeOptions) == 8,
"MojoCreateMessagePipeOptions has wrong size");