Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix several undefined behavior issues (strict aliasing and ODR violations) #4594

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Commits on Mar 20, 2024

  1. Fix strict aliasing issue in FloatFlip()

    Dereferencing type-punned pointers breaks C++'s strict aliasing
    rules and invokes undefined behavior.
    Use memcpy() to copy the float to an unsigned int variable to
    access the bit pattern.
    miller-alex committed Mar 20, 2024
    Configuration menu
    Copy the full SHA
    4a4f37a View commit details
    Browse the repository at this point in the history
  2. Fix strict aliasing issues in gim_math.h

    The macros GIM_IR(), GIM_SIR(), GIM_AIR(), and GIM_FR() dereference
    type-punned pointers, which is UB. The functions gim_inv_sqrt() and
    gim_sqrt() use these broken macros.
    
    Introduce two small inline functions copying the bits between the
    icompatible float and integer types using memcpy() and rewrite the
    macros so they use these functions.
    
    Note that three of the macros returned a reference before the change
    and thus could theoretically be used as lvalue; this is no longer
    possible. I couldn't find such usage in the code base and it would be
    rather odd for 3rd party code to abuse the API like that. However, if
    it is required that these macros return lvalues, then a more complicated
    approach would be needed with a custom class wrapping the reference
    and performing the copy on each access.
    miller-alex committed Mar 20, 2024
    Configuration menu
    Copy the full SHA
    fd2ec79 View commit details
    Browse the repository at this point in the history

Commits on Mar 21, 2024

  1. Fix a strict aliasing issue in b3RadixSort32CL.cpp

    Use b3MakeInt2() to contruct an object of the required type instead
    of type-punning b3SortData fillValue in a call to m_fill->execute()
    in b3RadixSort32CL::execute(b3OpenCLArray<b3SortData>&, int).
    miller-alex committed Mar 21, 2024
    Configuration menu
    Copy the full SHA
    ce53fd0 View commit details
    Browse the repository at this point in the history
  2. Fix strict aliasing issue in b3GpuPgsContactSolver.cpp

    csCfg of type b3ConstraintCfg is type-punned to b3SolverBase::ConstraintCfg&
    in solveContacts(). The two types are incompatible and this is UB.
    But they have almost the same definition, only m_staticIdx is initialized
    to a different value in the constructor. Declare b3ConstraintCfg to be
    a subclass of b3SolverBase::ConstraintCfg so we can convert between them.
    miller-alex committed Mar 21, 2024
    Configuration menu
    Copy the full SHA
    685a6c4 View commit details
    Browse the repository at this point in the history
  3. Fix invalid int to pointer conversion in btSoftBody::pointersToIndices()

    The code tries to reinterpret an int as a void* with type-punning.
    This is not only a strict aliasing violation, but completely
    invalid on architectures where ints and pointers have different size.
    
    The leaf btDbvtNode's payload is a union which contains an int dataAsInt
    member, so we set that instead of void *data.
    miller-alex committed Mar 21, 2024
    Configuration menu
    Copy the full SHA
    3ff0b81 View commit details
    Browse the repository at this point in the history
  4. Fix ODR violation by class GIM_CONTACT

    GIM_CONTACT is defined in btContactProcessingStructs.h and gim_contact.h,
    but with subtle differences, so there are two different types with the
    same name. This violates the C++ One Definition Rule and causes undefined
    behavior.
    
    Fix this by always using the same definition. Since gim_contact.h only
    provides the definition if btContactProcessingStructs.h isn't included
    before, we delete the diverging duplicate definition from the former
    header and #include the latter to provide the definition. Also drop
    unnecessary headers from btContactProcessingStructs.h and move over
    a comment from the deleted variant.
    miller-alex committed Mar 21, 2024
    Configuration menu
    Copy the full SHA
    e88b71b View commit details
    Browse the repository at this point in the history
  5. Fix ODR violation by btSoftSingleRayCallback

    Both btSoftMultiBodyDynamicsWorld.cpp and btSoftRigidDynamicsWorld.cpp
    define a type btSoftSingleRayCallback. Though the types are very similar
    they are not identical, causing an ODR violation.
    
    Move the type definitions into anonymous namespaces so their names no
    longer collide.
    miller-alex committed Mar 21, 2024
    Configuration menu
    Copy the full SHA
    b45d35d View commit details
    Browse the repository at this point in the history