microsoft / microsoft/DirectXShaderCompiler

UBSAN "downcast of address" runtime error on `createSentinal` for `ilist`s

Open
#6,446 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
C++
Stars
3.7k
Forks
900
Avg merge
2d 11h
Merged PRs (30d)
44

Description

ubsan-compiled dxc will fail with errors like:

/home/amaiorano/src/external/DirectXShaderCompiler/include/llvm/ADT/SparseBitVector.h:256:44: runtime error: downcast of address 0x7ffc07f29348 with insufficient space for an object of type 'llvm::ilist_traits<llvm::SparseBitVectorElement<128>>::Element' (aka 'SparseBitVectorElement<128U>')
0x7ffc07f29348: note: pointer points here
 00 00 00 00  00 00 00 00 00 00 00 00  18 ab 02 00 10 61 00 00  38 ab 02 00 10 61 00 00  68 ab 02 00
              ^ 
    #0 0x5572402d0e1d in createSentinel /home/amaiorano/src/external/DirectXShaderCompiler/include/llvm/ADT/SparseBitVector.h:256:44
    #1 0x5572402d0e1d in provideInitialHead /home/amaiorano/src/external/DirectXShaderCompiler/include/llvm/ADT/SparseBitVector.h:260:48
    #2 0x5572402d0e1d in iplist /home/amaiorano/src/external/DirectXShaderCompiler/include/llvm/ADT/ilist.h:350:25
    #3 0x5572402d0e1d in ilist /home/amaiorano/src/external/DirectXShaderCompiler/include/llvm/ADT/ilist.h:603:3
    #4 0x5572402d0e1d in SparseBitVector /home/amaiorano/src/external/DirectXShaderCompiler/include/llvm/ADT/SparseBitVector.h:437:3
    #5 0x5572402d0e1d in (anonymous namespace)::SparseBitVectorTest_TrivialOperation_Test::TestBody() /home/amaiorano/src/external/DirectXShaderCompiler/unittests/ADT/SparseBitVectorTest.cpp:18:21
<snip>

The reason this fails is that most implementations of ilist_traits<T>::createSentinal return a down-casted instance of a ilist_half_node<T> or ilist_node<T>, which although can be cast-down to T (since T derives from ilist_node), means we're returning a pointer to a sliced object of type T. And although we never dereference the pointer, this is still UB, which is why UBSAN flags it.

This issue was already flagged and addressed in LLVM by @dexonsmith, as can be seen by most of his changes here, where Duncan ended up rewriting the ilist system to be clearer, potentially more efficient, and non-UB. See Duncan's original llvm-dev post where he describes the issues in even more detail.

We should back-port those fixes into DXC.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with include/llvm/ADT/SparseBitVector.h, include/llvm/ADT/ilist.h, and include/llvm/ADT/ilist_node.h, then compare the referenced upstream LLVM ilist changes with the current DXC copies. Use unittests/ADT/SparseBitVectorTest.cpp under an UBSAN build to verify the downcast report is gone and existing SparseBitVector behavior remains covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.