Cuttlefish dEQP-VK.spirv_assembly.instruction.graphics.spirv_ids_abuse#lots_ids_[frag|vert] failures [173785481]

Verified

Bug

Status Update

No update yet.

Description

na...@google.com

created issue #1

Nov 20, 2020 03:37PM

cd <android-repo-directory>
source build/envsetup.sh
lunch aosp_cf_x86_64_phone-userdebug
m && m cts

# In one terminal
launch_cvd --gpu_mode=guest_swiftshader

# In another terminal
cts-tradefed run commandAndExit cts-dev -s 0.0.0.0:6520 -m CtsDeqpTestCases --module-arg CtsDeqpTestCases:include-filter:dEQP-VK.spirv_assembly.instruction.graphics.spirv_ids_abuse#lots_ids_frag

results in

*** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
Build fingerprint: 'generic/aosp_cf_x86_64_phone/vsoc_x86_64:S/AOSP.MASTER/eng.natsu.20201119.103802:userdebug/test-keys'
Revision: '0'
ABI: 'x86_64'
Timestamp: 2020-11-19 13:31:24.438365939+0000
pid: 3927, tid: 3958, name: deqp:testercore  >>> com.drawelements.deqp:testercore <<<
uid: 10115
signal 11 (SIGSEGV), code 2 (SEGV_ACCERR), fault addr 0x750a53654fe8
Cause: stack pointer is not in a rw map; likely due to stack overflow.
    rax 0000000000000000  rbx 0000000000000004  rcx 0000000000000000  rdx 000000000000002e
    r8  0000750a53655100  r9  0000750c4ea2fb70  r10 0000750a4f8f9040  r11 0000000000000000
    r12 0000000000000000  r13 0000750c1f005358  r14 0000750a53655100  r15 0000750a5374a2f0
    rdi 0000750a53655010  rsi 0000750c1f005358
    rbp 0000750bcf0f13d0  rsp 0000750a53654ff0  rip 0000750a519f1770
 
backtrace:
      #00 pc 0000000000fec770  /vendor/lib64/hw/vulkan.pastel.so (llvm::SelectionDAG::isSplatValue(llvm::SDValue, llvm::APInt const&, llvm::APInt&)+128) (BuildId: 8c704cabac9617d731b25a3b9630718b)
      #01 pc 0000000000fec90d  /vendor/lib64/hw/vulkan.pastel.so (llvm::SelectionDAG::isSplatValue(llvm::SDValue, llvm::APInt const&, llvm::APInt&)+541) (BuildId: 8c704cabac9617d731b25a3b9630718b)
      #02 pc 0000000000fec90d  /vendor/lib64/hw/vulkan.pastel.so (llvm::SelectionDAG::isSplatValue(llvm::SDValue, llvm::APInt const&, llvm::APInt&)+541) (BuildId: 8c704cabac9617d731b25a3b9630718b)
#03 pc 0000000000fec90d  /vendor/lib64/hw/vulkan.pastel.so (llvm::SelectionDAG::isSplatValue(llvm::SDValue, llvm::APInt const&, llvm::APInt&)+541) (BuildId: 8c704cabac9617d731b25a3b9630718b)
      #04 pc 0000000000fec90d  /vendor/lib64/hw/vulkan.pastel.so (llvm::SelectionDAG::isSplatValue(llvm::SDValue, llvm::APInt const&, llvm::APInt&)+541) (BuildId: 8c704cabac9617d731b25a3b9630718b)
      #05 pc 0000000000fec90d  /vendor/lib64/hw/vulkan.pastel.so (llvm::SelectionDAG::isSplatValue(llvm::SDValue, llvm::APInt const&, llvm::APInt&)+541) (BuildId: 8c704cabac9617d731b25a3b9630718b)
      #06 pc 0000000000fec90d  /vendor/lib64/hw/vulkan.pastel.so (llvm::SelectionDAG::isSplatValue(llvm::SDValue, llvm::APInt const&, llvm::APInt&)+541) (BuildId: 8c704cabac9617d731b25a3b9630718b)
      #07 pc 0000000000fec90d  /vendor/lib64/hw/vulkan.pastel.so (llvm::SelectionDAG::isSplatValue(llvm::SDValue, llvm::APInt const&, llvm::APInt&)+541) (BuildId: 8c704cabac9617d731b25a3b9630718b)
      #08 pc 0000000000fec90d  /vendor/lib64/hw/vulkan.pastel.so (llvm::SelectionDAG::isSplatValue(llvm::SDValue, llvm::APInt const&, llvm::APInt&)+541) (BuildId: 8c704cabac9617d731b25a3b9630718b)
      #09 pc 0000000000fec90d  /vendor/lib64/hw/vulkan.pastel.so (llvm::SelectionDAG::isSplatValue(llvm::SDValue, llvm::APInt const&, llvm::APInt&)+541) (BuildId: 8c704cabac9617d731b25a3b9630718b)
      #10 pc 0000000000fec90d  /vendor/lib64/hw/vulkan.pastel.so (llvm::SelectionDAG::isSplatValue(llvm::SDValue, llvm::APInt const&, llvm::APInt&)+541) (BuildId: 8c704cabac9617d731b25a3b9630718b)
...

Comments

ni...@google.com <ni...@google.com> #2Nov 20, 2020 03:54PM

Cause: stack pointer is not in a rw map; likely due to stack overflow.

What is Android's stack size? We had a similar issue for b/149829034. Could you try defining JIT_IN_SEPARATE_THREAD?

na...@google.com <na...@google.com> #3Nov 20, 2020 10:05PM

What is Android's stack size?

adb shell ulimit -a

is reporting 8MB.

Adding "-DJIT_IN_SEPARATE_THREAD=1", to the cflags section of libswiftshader_llvm_defaults in external/swiftshader/Android.bp did not help.

Without any LLVM background knowledge, replacing isSplatValue() with a isSplatValueRecursive(..., int depth) and printing out the depth shows the function hitting a depth of ~5600 before crashing. Is it possible we're hitting an infinite recursion case?

ni...@google.com <ni...@google.com> #4Nov 21, 2020 05:29AM

Accepted by ni...@google.com.

Thanks, that matches typical desktop Linux stack size. Additional threads by default have a 2 MB stack on Linux, so it looks like Android is the same which explains why -DJIT_IN_SEPARATE_THREAD=1 doesn't help at all.

These tests currently pass on desktop Linux. Differences in compiler optimizations that affect the stack frame sizes of functions can make one build more prone to stack overflow than the other though.

While this call stack might seem like an infinite recursion, the test actually chains together 10,000 add instructions, and LLVM needs to walk up that entire chain of dependencies to know with certainty whether the final sum is a 'splat' result or not.

However, this is just for optimization purposes. If we return false early because the recursion goes deeper than expected, we just lose a (potential) optimization opportunity. Note that methods such as SelectionDAG::computeKnownBits take a Depth parameter to avoid going deeper than MaxRecursionDepth = 6. I'll patch isSplatValue to do the same.

ap...@google.com <ap...@google.com> #5Nov 23, 2020 08:10PM

Project: SwiftShader
Branch: master

commit 24350b8dcb6db79b73f0d9fa4a9403771477d5f2
Author: Nicolas Capens <capn@google.com>
Date: Sat Nov 21 01:00:55 2020

Limit the recursion depth of llvm::SelectionDAG::isSplatValue()

This method previously always recursively checked both the left-hand
side and right-hand side of binary operations for splatted (broadcast)
values to determine if the parent DAG node is a splat. For chained
operations, such as the 10,000 adds of the dEQP-VK.spirv_assembly.
instruction.graphics.spirv_ids_abuse.lots_ids_frag/vert tests,
very deep recursion could result in stack overflow.

Like several other SelectionDAG methods, limit the recursion depth to
MaxRecursionDepth (6).

Bug:

b/173785481

Change-Id: I22ee0453db7cf1311267291a331ad8fa3b57b1d9
Reviewed-on:

https://swiftshader-review.googlesource.com/c/SwiftShader/+/50672
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Jason Macnak <natsu@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Tested-by: Jason Macnak <natsu@google.com>

M third_party/llvm-10.0/llvm/include/llvm/CodeGen/SelectionDAG.h
M third_party/llvm-10.0/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

https://swiftshader-review.googlesource.com/50672

ni...@google.com <ni...@google.com> #6Nov 23, 2020 09:05PM

Should be upstreamed to LLVM before we call this fixed. Also needs cherry-picking for b/165000222

https://reviews.llvm.org/D92421

Message last modified on Dec 1, 2020 08:38PM

ap...@google.com <ap...@google.com> #7Dec 2, 2020 07:35PM

Project: platform/external/swiftshader
Branch: master

commit c2754b7bfd030ec146c6712f290425d57050acee
Author: Jason Macnak <natsu@google.com>
Date: Tue Nov 24 14:59:35 2020

Merge remote-tracking branch 'aosp/upstream-swiftshader-master' into 'aosp/master'

... to update SwiftShader for Cuttlefish to pull in

https://swiftshader-review.googlesource.com/c/SwiftShader/+/50672
to fix dEQP spirv_ids_abuse tests.

Bug:

b/173785481

Test: cts -m CtsDeqpTestCases w/ spirv_ids_abuse#*
Change-Id: I25cb2eb520edc6a5955f08f075fb9b0c0e080b60

https://android-review.googlesource.com/1510514

na...@google.com <na...@google.com> Dec 3, 2020 03:07PM

Marked as fixed.

ni...@google.com <ni...@google.com> #8Dec 9, 2020 08:01PM

Verified by ni...@google.com.

https://reviews.llvm.org/D92421 has landed.