Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

More fixes for signed integer overflow.

Browse Source 
Fence.cpp:

My previous fix to the fix was insufficient because a signed integer
overflow could still occur as follows (this was observed in an internal
test at Google).

If the conditional on line 87 is true, then a clamped value is passed
into `wait`, which causes the conditional on line 133 to fail, which
causes `wait_for` to be called inappropriately, which is where an
overflow would occur.

MetalHandles.mm:

This is more theoretical, but to prevent potential integer overflow
within STL, I think we should call `wait()` rather than `wait_for()`
when an infinite timeout is desired.

This is our fourth fix to fences in since November 2; see also dcca236,
e9c9e45, and 8969416.  :)
This commit is contained in:
Philip Rideout
2020-11-06 12:35:33 -08:00
parent 89694168a6
commit 96d3f2f16b
2 changed files with 6 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
filament/backend/src/metal/MetalHandles.mm
View File

@@ -849,12 +849,13 @@ void MetalFence::onSignal(MetalFenceSignalBlock block) {
FenceStatus MetalFence::wait(uint64_t timeoutNs) {
if (@available(macOS 10.14, iOS 12, *)) {
using ns = std::chrono::nanoseconds;
std::unique_lock<std::mutex> guard(state->mutex);
timeoutNs = std::min(timeoutNs, (uint64_t) std::chrono::nanoseconds::max().count());
while (state->status == FenceStatus::TIMEOUT_EXPIRED) {
if (timeoutNs == 0 ||
state->cv.wait_for(guard, std::chrono::nanoseconds(timeoutNs)) ==
std::cv_status::timeout) {
if (timeoutNs == FENCE_WAIT_FOR_EVER) {
state->cv.wait(guard);
} else if (timeoutNs == 0 ||
state->cv.wait_for(guard, ns(timeoutNs)) == std::cv_status::timeout) {
return FenceStatus::TIMEOUT_EXPIRED;
}
}

4
filament/src/Fence.cpp
View File

@@ -71,8 +71,6 @@ FenceStatus FFence::waitAndDestroy(FFence* fence, Mode mode) noexcept {
UTILS_NOINLINE
FenceStatus FFence::wait(Mode mode, uint64_t timeout) noexcept {
ASSERT_PRECONDITION(UTILS_HAS_THREADING || timeout == 0, "Non-zero timeout requires threads.");
const bool waitForever = timeout == FENCE_WAIT_FOR_EVER;
timeout = std::min(timeout, (uint64_t) ns::max().count());
FEngine& engine = mEngine;
@@ -98,7 +96,7 @@ FenceStatus FFence::wait(Mode mode, uint64_t timeout) noexcept {
}
engine.pumpPlatformEvents();
const auto elapsed = std::chrono::system_clock::now() - startTime;
if (!waitForever && elapsed >= ns(timeout)) {
if (timeout != FENCE_WAIT_FOR_EVER && elapsed >= ns(timeout)) {
break;
}
}