Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 99e9db75 authored by Andy Hung's avatar Andy Hung
Browse files

AudioTrackShared: Ensure proper subtraction 

Optimizing compilers may skip masking if the result of
subtraction is assigned to larger integral type.

Test: long term test with 64 bit audioserver
Bug: 113763960
Change-Id: Ida405a5a26e819891e7da7f29ebd0b78d2de1fc5
parent 05fa8856

media/libaudioclient/AudioTrackShared.cpp

+25 −7
Original line number Diff line number Diff line
@@ -26,6 +26,24 @@ 


namespace android {



// TODO: consider pulling this into a shared header.

// safe_sub_overflow is used ensure that subtraction occurs in the same native type

// with proper 2's complement overflow.  Without calling this function, it is possible,

// for example, that optimizing compilers may elect to treat 32 bit subtraction

// as 64 bit subtraction when storing into a 64 bit destination as integer overflow is

// technically undefined.

template<typename T,

         typename U,

         typename = std::enable_if_t<std::is_same<std::decay_t<T>,

                                       std::decay_t<U>>{}>>

         // ensure arguments are same type (ignoring volatile, which is used in cblk variables).

auto safe_sub_overflow(const T& a, const U& b) {

    std::decay_t<T> result;

    (void)__builtin_sub_overflow(a, b, &result);

    // note if __builtin_sub_overflow returns true, an overflow occurred.

    return result;

}



// used to clamp a value to size_t.  TODO: move to another file.

template <typename T>

size_t clampToSize(T x) {
@@ -186,7 +204,7 @@ status_t ClientProxy::obtainBuffer(Buffer* buffer, const struct timespec *reques 
            front = cblk->u.mStreaming.mFront;

        }

        // write to rear, read from front

        ssize_t filled = rear - front;

        ssize_t filled = safe_sub_overflow(rear, front);

        // pipe should not be overfull

        if (!(0 <= filled && (size_t) filled <= mFrameCount)) {

            if (mIsOut) {
@@ -684,7 +702,7 @@ void ServerProxy::flushBufferIfNeeded() 
        const size_t overflowBit = mFrameCountP2 << 1;

        const size_t mask = overflowBit - 1;

        int32_t newFront = (front & ~mask) | (flush & mask);

        ssize_t filled = rear - newFront;

        ssize_t filled = safe_sub_overflow(rear, newFront);

        if (filled >= (ssize_t)overflowBit) {

            // front and rear offsets span the overflow bit of the p2 mask

            // so rebasing newFront on the front offset is off by the overflow bit.
@@ -726,7 +744,7 @@ int32_t AudioTrackServerProxy::getRear() const 
        const size_t overflowBit = mFrameCountP2 << 1;

        const size_t mask = overflowBit - 1;

        int32_t newRear = (rear & ~mask) | (stop & mask);

        ssize_t filled = newRear - front;

        ssize_t filled = safe_sub_overflow(newRear, front);

        // overflowBit is unsigned, so cast to signed for comparison.

        if (filled >= (ssize_t)overflowBit) {

            // front and rear offsets span the overflow bit of the p2 mask
@@ -778,7 +796,7 @@ status_t ServerProxy::obtainBuffer(Buffer* buffer, bool ackFlush) 
        front = android_atomic_acquire_load(&cblk->u.mStreaming.mFront);

        rear = cblk->u.mStreaming.mRear;

    }

    ssize_t filled = rear - front;

    ssize_t filled = safe_sub_overflow(rear, front);

    // pipe should not already be overfull

    if (!(0 <= filled && (size_t) filled <= mFrameCount)) {

        ALOGE("Shared memory control block is corrupt (filled=%zd, mFrameCount=%zu); shutting down",
@@ -905,7 +923,7 @@ size_t AudioTrackServerProxy::framesReady() 
        return mFrameCount;

    }

    const int32_t rear = getRear();

    ssize_t filled = rear - cblk->u.mStreaming.mFront;

    ssize_t filled = safe_sub_overflow(rear, cblk->u.mStreaming.mFront);

    // pipe should not already be overfull

    if (!(0 <= filled && (size_t) filled <= mFrameCount)) {

        ALOGE("Shared memory control block is corrupt (filled=%zd, mFrameCount=%zu); shutting down",
@@ -931,7 +949,7 @@ size_t AudioTrackServerProxy::framesReadySafe() const 
        return mFrameCount;

    }

    const int32_t rear = getRear();

    const ssize_t filled = rear - cblk->u.mStreaming.mFront;

    const ssize_t filled = safe_sub_overflow(rear, cblk->u.mStreaming.mFront);

    if (!(0 <= filled && (size_t) filled <= mFrameCount)) {

        return 0; // error condition, silently return 0.

    }
@@ -1241,7 +1259,7 @@ size_t AudioRecordServerProxy::framesReadySafe() const 
    }

    const int32_t front = android_atomic_acquire_load(&mCblk->u.mStreaming.mFront);

    const int32_t rear = mCblk->u.mStreaming.mRear;

    const ssize_t filled = rear - front;

    const ssize_t filled = safe_sub_overflow(rear, front);

    if (!(0 <= filled && (size_t) filled <= mFrameCount)) {

        return 0; // error condition, silently return 0.

    }