net_unittests QuicFramerTests failing on Windows Clang builds |
||||||||
Issue descriptionFor example: https://build.chromium.org/p/chromium.fyi/builders/CrWinClang%20tester/builds/5884 Also on the ToT bots: https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin%20tester/builds/4403 The test is new: https://codereview.chromium.org/1904213002
,
Apr 26 2016
Fix in flight.
,
Apr 26 2016
Opps, may have jumped the gun a bit. My change broke all net_unittests on android, and your examples only failed on a few windows builders.
,
Apr 26 2016
It reproduces locally in a BUILD_TYPE=Official build.. wnwen: Do you have some more context? Like a link to your original change, fix, or what it's fixing?
,
Apr 26 2016
Does it repro in buildtype=Official builds with msvc?
,
Apr 26 2016
This should have enough context on my bug: https://codereview.chromium.org/1919283002/
,
Apr 26 2016
> Does it repro in buildtype=Official builds with msvc? Trying that now. > This should have enough context on my bug: https://codereview.chromium.org/1919283002/ Thanks. The change you're referencing is more recent than this breakage, so probably not related.
,
Apr 26 2016
>> Does it repro in buildtype=Official builds with msvc? > Trying that now. Nope, they pass with MSVC in that mode :-/
,
Apr 26 2016
Also doesn't repro on Linux when built with: GYP_DEFINES="BUILD_TYPE=Official target_arch=ia32"
,
Apr 26 2016
Classic Heisenbug: this patch makes the test failures go away:
diff --git a/net/quic/quic_framer_test.cc b/net/quic/quic_framer_test.cc
index 2039bd3..a1e7cc6 100644
--- a/net/quic/quic_framer_test.cc
+++ b/net/quic/quic_framer_test.cc
@@ -2645,6 +2645,7 @@ TEST_P(QuicFramerTest, AckFrame500NacksVersion32) {
};
// clang-format on
+ printf("framer_.version() = %d\n", framer_.version());
if (framer_.version() <= QUIC_VERSION_31) {
return;
}
Seems like the if statement somehow gets optimized away, despite framer_.version() not being constant, but depending on the parameter of the test..
Value parameterized tests are documented here: https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuide.md#value-parameterized-tests
,
Apr 26 2016
,
Apr 26 2016
It's starting to look like a codegen bug.. the test body starts off like this:
TEST_P(QuicFramerTest, AckFrame500NacksVersion32) {
// clang-format off
unsigned char packet[] = {
// public flags (8 byte connection_id)
static_cast<unsigned char>(
framer_.version() > QUIC_VERSION_32 ? 0x38 : 0x3C),
// connection_id
0x10, 0x32, 0x54, 0x76, 0x98, 0xBA, 0xDC, 0xFE,
// packet number
0xA8, 0x9A, 0x78, 0x56, 0x34, 0x12,
[some more bytes]
};
// clang-format on
if (framer_.version() <= QUIC_VERSION_31) {
return;
}
... more test stuff below
The generated code looks like:
?TestBody@QuicFramerTest_AckFrame500NacksVersion32_Test@test@net@@UAEXXZ (public: virtual void __thiscall net::test::QuicFramerTest_AckFrame500NacksVersion32_Test::TestBody(void)):
00000000: 83 B9 AC 00 00 00 cmp dword ptr [ecx+0ACh],20h
20
00000007: B0 38 mov al,38h
00000009: 7F 02 jg 0000000D
0000000B: B0 3C mov al,3Ch
0000000D: 55 push ebp
0000000E: 89 E5 mov ebp,esp
00000010: 53 push ebx
00000011: 57 push edi
00000012: 56 push esi
00000013: 83 E4 F8 and esp,0FFFFFFF8h
00000016: 8D A4 24 88 FE FF lea esp,[esp-178h]
FF
0000001D: 88 44 24 30 mov byte ptr [esp+30h],al
00000021: C6 44 24 31 10 mov byte ptr [esp+31h],10h
00000026: C6 44 24 32 32 mov byte ptr [esp+32h],32h
0000002B: C6 44 24 33 54 mov byte ptr [esp+33h],54h
00000030: C6 44 24 34 76 mov byte ptr [esp+34h],76h
00000035: C6 44 24 35 98 mov byte ptr [esp+35h],98h
0000003A: C6 44 24 36 BA mov byte ptr [esp+36h],0BAh
0000003F: C6 44 24 37 DC mov byte ptr [esp+37h],0DCh
00000044: C6 44 24 38 FE mov byte ptr [esp+38h],0FEh
00000049: C6 44 24 39 A8 mov byte ptr [esp+39h],0A8h
0000004E: C6 44 24 3A 9A mov byte ptr [esp+3Ah],9Ah
00000053: C6 44 24 3B 78 mov byte ptr [esp+3Bh],78h
00000058: C6 44 24 3C 56 mov byte ptr [esp+3Ch],56h
0000005D: C6 44 24 3D 34 mov byte ptr [esp+3Dh],34h
00000062: C6 44 24 3E 12 mov byte ptr [esp+3Eh],12h
00000067: C6 44 24 3F 01 mov byte ptr [esp+3Fh],1
0000006C: 0F 28 05 00 00 00 movaps xmm0,xmmword ptr [__xmm@ff00f30102000000123456789abfba6c]
00
00000073: 0F 11 44 24 40 movups xmmword ptr [esp+40h],xmm0
00000078: 0F 8C 47 08 00 00 jl 000008C5 <--- Jumps to return.
.. more test stuff
It seems LLVM is cleverly re-using the "framer_.version() > QUIC_VERSION_32" comparison at 0x00 in the "jl" at 0x78, but the "and" at 0x13 has clobbered EFLAGS. This looks like shrink-wrapping gone wrong.
,
Apr 26 2016
Filed http://llvm.org/PR27531 upstream.
,
Apr 26 2016
Should be fixed by LLVM r267634. It'll take a while before the bots cycle, and then we'll need a roll.
,
Apr 27 2016
,
Apr 27 2016
,
May 4 2016
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by h...@chromium.org
, Apr 26 2016