https://build.chromium.org/p/chromium.fyi/builders/CrWinClang64%28dbg%29%20tester/builds/1842/steps/webkit_unit_tests%20on%20Windows-7-SP1/logs/PaymentRequestTest.ResolvePromiseOnComplete has better stacks:

[ RUN      ] PaymentRequestTest.ResolvePromiseOnComplete
Backtrace:
	blink::GarbageCollectedFinalized<blink::ShippingAddress>::~GarbageCollectedFinalized<blink::ShippingAddress> [0x000000000A454979+28889]
	blink::PaymentRequest::OnPaymentResponse [0x000000000A44874E+2062]
	blink::PaymentRequest::OnPaymentResponse [0x000000000A448578+1592]
	blink::WebIDBKeyRange::reset [0x0000000140C75353+566643]
	InitializeBase [0x0000000142219862+11812482]
	InitializeBase [0x00000001422197CC+11812332]
	InitializeBase [0x000000014221A326+11815238]
	InitializeBase [0x000000014221B033+11818579]
	InitializeBase [0x0000000142221E65+11846789]
	InitializeBase [0x0000000142221A88+11845800]
	InitializeBase [0x000000014222190E+11845422]
	blink::WebIDBKeyRange::reset [0x0000000140DB9AC1+1895649]
	blink::WebIDBKeyRange::reset [0x0000000140DB90D4+1893108]
	(No symbol) [0x00000001400011CA]
	(No symbol) [0x000000014000293D]
	(No symbol) [0x00000001400028D3]
	(No symbol) [0x000000014000289B]
	blink::WebIDBKeyRange::reset [0x0000000140E2DB56+2370934]
	blink::WebIDBKeyRange::reset [0x0000000140E2ACBC+2359004]
	blink::WebIDBKeyRange::reset [0x0000000140E2AA05+2358309]
	(No symbol) [0x000000014000109B]
	IsSandboxedProcess [0x0000000142A31BA4+551252]
	IsSandboxedProcess [0x0000000142A31ACE+551038]
	IsSandboxedProcess [0x0000000142A3198E+550718]
	IsSandboxedProcess [0x0000000142A31BB9+551273]
	BaseThreadInitThunk [0x00000000772959ED+13]
	RtlUserThreadStart [0x00000000774CC541+33]

[ RUN      ] PaymentRequestTest.DontRejectShowPromiseForValidShippingAddress
Backtrace:
	blink::PaymentRequest::OnShippingAddressChange [0x000000000A437A31+1313]
	blink::WebIDBKeyRange::reset [0x0000000140C725FC+555036]
	InitializeBase [0x0000000142219862+11812482]
	InitializeBase [0x00000001422197CC+11812332]
	InitializeBase [0x000000014221A326+11815238]
	InitializeBase [0x000000014221B033+11818579]
	InitializeBase [0x0000000142221E65+11846789]
	InitializeBase [0x0000000142221A88+11845800]
	InitializeBase [0x000000014222190E+11845422]
	blink::WebIDBKeyRange::reset [0x0000000140DB9AC1+1895649]
	blink::WebIDBKeyRange::reset [0x0000000140DB90D4+1893108]
	(No symbol) [0x00000001400011CA]
	(No symbol) [0x000000014000293D]
	(No symbol) [0x00000001400028D3]
	(No symbol) [0x000000014000289B]
	blink::WebIDBKeyRange::reset [0x0000000140E2DB56+2370934]
	blink::WebIDBKeyRange::reset [0x0000000140E2ACBC+2359004]
	blink::WebIDBKeyRange::reset [0x0000000140E2AA05+2358309]
	(No symbol) [0x000000014000109B]
	IsSandboxedProcess [0x0000000142A31BA4+551252]
	IsSandboxedProcess [0x0000000142A31ACE+551038]
	IsSandboxedProcess [0x0000000142A3198E+550718]
	IsSandboxedProcess [0x0000000142A31BB9+551273]
	BaseThreadInitThunk [0x00000000772959ED+13]
	RtlUserThreadStart [0x00000000774CC541+33]

List of failing tests:

PaymentRequestTest.CannotCallCompleteTwice
PaymentRequestTest.DontRejectShowPromiseForValidShippingAddress
PaymentRequestTest.OnShippingOptionChange
PaymentRequestTest.RejectCompletePromiseOnError
PaymentRequestTest.RejectShowPromiseOnError
PaymentRequestTest.RejectShowPromiseOnInvalidShippingAddress
PaymentRequestTest.ResolvePromiseOnComplete
PaymentRequestTest.ResolveShowPromiseWithoutShippingAddressInResponse


Those are exactly the tests that do `static_cast<mojom::blink::PaymentRequestClient*>(request)` in PaymentRequestTest.cpp.

If I edit PaymentRequest.h to make the overridden PaymentRequestClient methods public and then in RejectShowPromiseOnInvalidShippingAddress change

    static_cast<mojom::blink::PaymentRequestClient*>(request)->OnShippingAddressChange(mojom::blink::ShippingAddress::New());

to

    (request)->OnShippingAddressChange(mojom::blink::ShippingAddress::New());

(i.e. just remove the static_cast), then that test starts passing.

I wonder if we are accidentally treating that static_cast as a reinterpret_cast and failing to adjust the pointer.

Generated assembly for PaymentRequestTest with and without that cast.

Works:

	calll	*"__imp_?then@ScriptPromise@blink@@QAE?AV12@V?$Local@VFunction@v8@@@v8@@0@Z"
	movl	"__imp_??1ScriptPromise@blink@@QAE@XZ", %ebx
	movl	%esi, %ecx
	calll	*%ebx
	movl	%edi, %ecx
	calll	*%ebx
	movl	%esp, %esi
	movl	-48(%ebp), %edi         # 4-byte Reload
	addl	$40, %edi
	movl	$4, %eax
	calll	__chkstk
	movl	%esp, %eax
	pushl	%eax
	calll	"?New@ShippingAddress@blink@mojom@2@SA?AV?$StructPtr@VShippingAddress@blink@mojom@2@@mojo@@XZ"
	addl	$4, %esp
	movl	%edi, %ecx
	calll	*"__imp_?OnShippingAddressChange@PaymentRequest@blink@@UAEXV?$StructPtr@VShippingAddress@blink@mojom@2@@mojo@@@Z"
	movl	%esi, %esp
	movl	-72(%ebp), %ecx
	calll	*"__imp_?Exit@Context@v8@@QAEXXZ"

broken:

	calll	*"__imp_?then@ScriptPromise@blink@@QAE?AV12@V?$Local@VFunction@v8@@@v8@@0@Z"
	movl	"__imp_??1ScriptPromise@blink@@QAE@XZ", %ebx
	movl	%esi, %ecx
	calll	*%ebx
	movl	%edi, %ecx
	calll	*%ebx
	movl	%esp, %esi
	movl	$4, %eax
	calll	__chkstk
	movl	%esp, %eax
	pushl	%eax
	calll	"?New@ShippingAddress@blink@mojom@2@SA?AV?$StructPtr@VShippingAddress@blink@mojom@2@@mojo@@XZ"
	addl	$4, %esp
	movl	-48(%ebp), %ecx         # 4-byte Reload
	calll	*"__imp_?OnShippingAddressChange@PaymentRequest@blink@@UAEXV?$StructPtr@VShippingAddress@blink@mojom@2@@mojo@@@Z"
	movl	%esi, %esp
	movl	-72(%ebp), %ecx
	calll	*"__imp_?Exit@Context@v8@@QAEXXZ"


The reload is in a difference place (seems irrelevant), and the working version has a `	addl	$40, %edi`.

Deleted: works_full.asm 868 KB

works_full.asm 868 KB View Download

Deleted: broken_full.asm 868 KB

broken_full.asm 868 KB View Download

I isolated the static_cast + virtual call into a 'doit' function and I noticed that clang is devirtualizing the call without doing a 'this' adjustment:

0:000> uf webkit_unit_tests!doit
webkit_unit_tests!doit [D:\src\chromium\src\third_party\WebKit\Source\modules\payments\PaymentRequestTest.cpp @ 217]:
...
  218 01a5c76e ff155cb80f02    call    dword ptr [webkit_unit_tests!_imp_?OnShippingAddressChangePaymentRequestblinkEAEXV?

I think we're doing this because PlatformRequest is final. We probably just need to fix our IRGen to do the right adjustment.

Yeah, it's our bug:

$ cat t.cpp
#include <stdio.h>
struct A { virtual void f() {} };
struct B { virtual void g() {} };
struct C final : A, B {
  virtual void h() {}
  void g() override {
    printf("%p\n", this);
  }
};
int main() {
  C *p = new C();
  printf("%p\n", p);
  static_cast<B*>(p)->g();
}

$ clang-cl t.cpp && ./t.exe && cl -nologo t.cpp && ./t.exe
0000000000358730
0000000000358728
t.cpp
0000000000378730
0000000000378730

The two pointer values should be the same.

Ok, I'll remove "final" from PaymentRequest as a workaround until we've rolled in the fix.

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

commit 9306e66e5e680801132688b987fe850f1bb43b7a
Author: Nico Weber <thakis@chromium.org>
Date: Tue May 03 18:36:45 2016

Work around a clang-cl devirtualization bug.

clang-cl currently miscompiles calls to virtual methods of base classes
of a final class.  Since the type is final, it tries to devirtualize the call,
but then it forgets to apply the right this adjustment.

https://codereview.chromium.org/1938853002 added static_casts to a super class
for visibility reasons, exposing this bug.

BUG= 608705 
R=rouslan@chromium.org

Review URL: https://codereview.chromium.org/1949653002 .

Cr-Commit-Position: refs/heads/master@{#391307}

[modify] https://crrev.com/9306e66e5e680801132688b987fe850f1bb43b7a/third_party/WebKit/Source/modules/payments/PaymentRequest.h

Fixed in LLVM r268418, and we can close this in the next clang roll.

Please don't forget to put "final" back on PaymentRequest.

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

commit 7f225037d3373bc6b4c37cd122b3235902e1602c
Author: rnk <rnk@chromium.org>
Date: Thu May 05 20:10:11 2016

Roll clang 268373:268419

Picks up Windows fix for a miscompile of a virtual method call
on a final class in blink.

R=thakis@chromium.org,hans@chromium.org
BUG= 608705 

Review-Url: https://codereview.chromium.org/1949113002
Cr-Commit-Position: refs/heads/master@{#391887}

[modify] https://crrev.com/7f225037d3373bc6b4c37cd122b3235902e1602c/tools/clang/scripts/update.py

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

commit c19da3c46af754416c299b6dacf2223f402b1b34
Author: thakis <thakis@chromium.org>
Date: Mon May 09 18:25:38 2016

Revert of Work around a clang-cl devirtualization bug. (patchset #2 id:20001 of https://codereview.chromium.org/1949653002/ )

Reason for revert:
We fixed the compiler bug, this should no longer be needed.

Original issue's description:
> Work around a clang-cl devirtualization bug.
>
> clang-cl currently miscompiles calls to virtual methods of base classes
> of a final class.  Since the type is final, it tries to devirtualize the call,
> but then it forgets to apply the right this adjustment.
>
> https://codereview.chromium.org/1938853002 added static_casts to a super class
> for visibility reasons, exposing this bug.
>
> BUG= 608705 
> R=rouslan@chromium.org
>
> Committed: https://chromium.googlesource.com/chromium/src/+/9306e66e5e680801132688b987fe850f1bb43b7a

TBR=rouslan@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 608705 

Review-Url: https://codereview.chromium.org/1958183002
Cr-Commit-Position: refs/heads/master@{#392371}

[modify] https://crrev.com/c19da3c46af754416c299b6dacf2223f402b1b34/third_party/WebKit/Source/modules/payments/PaymentRequest.h