thakis@ is OS=Mac intended here?

No, sorry. crbug defaults to the OS I'm running chrome on and I forgot to change it.

I'm guessing it's this one:

------------------------------------------------------------------------
r345419 | rsmith | 2018-10-26 21:26:45 +0200 (Fri, 26 Oct 2018) | 9 lines

PR26547: alignof should return ABI alignment, not preferred alignment

Summary:
- Add `UETT_PreferredAlignOf` to account for the difference between `__alignof` and `alignof`
- `AlignOfType` now returns ABI alignment instead of preferred alignment iff clang-abi-compat > 7, and one uses _Alignof or alignof

Patch by Nicole Mazzuca!

Differential Revision: https://reviews.llvm.org/D53207
------------------------------------------------------------------------

Seems like MOJO_ALIGNOF(int64_t) returns 4.

Short repro:

$ /work/llvm.combined/build.release/bin/clang++ --target=i686-linux-android -S -o - /tmp/a.cc | grep -A1 'x:'
x:
        .long   4                       # 0x4


Hang on, why is the triple i686? Isn't this supposed to be a 64-bit build? Oh, I suppose maybe we build both and do a fat binary or something?

Anyhow, i686 doesn't require more than 4-byte alignment for 64-bit ints, so this makes sense.

Wait, how does this work on regular 32-bit x86 builds?

> Wait, how does this work on regular 32-bit x86 builds?

I guess we don't do 32-bit Linux anymore? And on 32-bit Windows, the alignment is 8..

Patch: https://chromium-review.googlesource.com/c/chromium/src/+/1318971

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/16035729f1ca7fcc5815caf702c457f7f5257689

commit 16035729f1ca7fcc5815caf702c457f7f5257689
Author: Hans Wennborg <hans@chromium.org>
Date: Tue Nov 06 16:13:27 2018

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 
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}
[modify] https://crrev.com/16035729f1ca7fcc5815caf702c457f7f5257689/mojo/core/options_validation_unittest.cc
[modify] https://crrev.com/16035729f1ca7fcc5815caf702c457f7f5257689/mojo/public/c/system/buffer.h
[modify] https://crrev.com/16035729f1ca7fcc5815caf702c457f7f5257689/mojo/public/c/system/data_pipe.h
[modify] https://crrev.com/16035729f1ca7fcc5815caf702c457f7f5257689/mojo/public/c/system/message_pipe.h

New failures are found in MacrosTest.Alignof test

https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8930420859439810624/+/steps/mojo_unittests/0/logs/MacrosTest.Alignof/0

Thanks! It's unfortunate that this didn't get caught in the CQ.

Anyway, the test needs to be updated, it currently looks like this:

TEST(MacrosTest, Alignof) {
  // Strictly speaking, this isn't a portable test, but I think it'll pass on
  // all the platforms we currently support.
  EXPECT_EQ(1u, MOJO_ALIGNOF(char));
  EXPECT_EQ(4u, MOJO_ALIGNOF(int32_t));
  EXPECT_EQ(8u, MOJO_ALIGNOF(int64_t));
  EXPECT_EQ(8u, MOJO_ALIGNOF(double));
}


The "I think it'll pass on all the platforms we currently support." part doesn't hold.

dvadym: I'd suggest disabling the test to green the build until it's fixed.

 Issue 903223  has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86e6511b6b1ee086f50639c9a64346790dea350e

commit 86e6511b6b1ee086f50639c9a64346790dea350e
Author: Reid Kleckner <rnk@google.com>
Date: Thu Nov 08 22:26:12 2018

Fix mojo double and int64_t alignment expectations

Previously, alignof(int64_t) was 8 on i686, but now it is 4. This is
true in GCC 8+ and the latest version of clang. These types have always
only been aligned to 4 bytes in struct layout, so this should not have
wide ranging implications for ABI stability.

Bug:  900406 
Change-Id: Ia2f0d87300679148d7ca653b7cc52fe9cf910bbc
Reviewed-on: https://chromium-review.googlesource.com/c/1327606
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Reid Kleckner <rnk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606622}
[modify] https://crrev.com/86e6511b6b1ee086f50639c9a64346790dea350e/mojo/public/c/system/tests/macros_unittest.cc