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

Sanitizer on RenderTargetPool.cpp reports undefined behaviour #439

Closed
Zefz opened this issue Oct 30, 2018 · 2 comments
Closed

Sanitizer on RenderTargetPool.cpp reports undefined behaviour #439

Zefz opened this issue Oct 30, 2018 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@Zefz
Copy link

Zefz commented Oct 30, 2018

Describe the bug
Using an undefined behaviour sanitizer with RenderTargetPool.cpp:167 reports an issue of unsigned integer undeflow:

if (UTILS_UNLIKELY(mDeepPurgeCountDown-- == 0)) {
        mDeepPurgeCountDown = POOL_ENTRY_MAX_AGE;
        uint32_t age = mCacheAge - POOL_ENTRY_MAX_AGE;
        // remove all entries that are older than CACHE_ENTRY_MAX_AGE
        auto last = std::remove_if(cache.begin(), cache.end(),
                [this, &driver, age](const Entry* entry) {
                    bool remove = entry->age <= age;
                    if (remove) {
                        destroyEntry(driver, entry);
                    }
                    return remove;
                });
        cache.erase(last, cache.end());
    }

In this case mDeepPurgeCountDown (an uint32_t) is zero when this block is reached when it becomes decremented and underflows.

Sanitizer report:

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /filament/filament/src/Camera.cpp:205:23 in
filament/filament/src/RenderTargetPool.cpp:167:9: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'uint32_t' (aka 'unsigned int')
#0 0xf7d904 in filament::RenderTargetPool::gc()
filament/filament/src/RenderTargetPool.cpp:167:9
#1 0xf203fa in filament::details::FRenderer::endFrame()
filament/filament/src/Renderer.cpp:371:9
#2 0xf21aa9 in filament::Renderer::endFrame()
filament/filament/src/Renderer.cpp:460:19

Expected behavior
I am quite sure this is not the desired behaviour. E.g something like this would solve the issue:
UTILS_UNLIKELY(mDeepPurgeCountDown == 0 || mDeepPurgeCountDown-- == 0)

Desktop (please complete the following information):

  • OS: Linux Mint 18
  • GPU: [NVIDIA GTX 1050]
  • Backend: [OpenGL]

Additional context
Seems that this happens on shutdown, so it might not actually cause any runtime issues.

@romainguy
Copy link
Collaborator

Thanks for the report!

@romainguy romainguy added the bug Something isn't working label Oct 30, 2018
@pixelflinger
Copy link
Collaborator

I don't think there is problem here. mDeepPurgeCountDown is tested before being decremented. If it is 0, then it will indeed underflow, but it is immediately reset to POOL_ENTRY_MAX_AGE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants