search

facebookincubator / velox

A composable and fully extensible C++ execution engine library for data management systems.
https://velox-lib.io/
Apache License 2.0
3.54k stars 1.17k forks source link

AsyncDataCache::canTryAllocate signature incorrectly used type int32_t for MachinePageCount (uint64_t) parameter being passed #11738

Open aditi-pandit opened 11 hours ago

aditi-pandit commented 11 hours ago

Bug description

Caught by IBM internal security scan.

AsynDataCache::canTryAllocate used int32_t as parameter type for numPages, https://github.com/facebookincubator/velox/blob/main/velox/common/caching/AsyncDataCache.cpp#L867, when parameter passed in was MachinePageCount (uint64_t) from AsyncDataCache::makeSpace https://github.com/facebookincubator/velox/blob/main/velox/common/caching/AsyncDataCache.cpp#L779

This could've caused crashes in production.

Have fixed the signature in https://github.com/facebookincubator/velox/pull/11684, but creating an issue to get broader consensus on the issue and fix.

System information

Velox System Info v0.0.2 Commit: c14348ebf64d0dd7a6a4532ea2e5fe32c411a11f CMake Version: 3.31.0 System: Darwin-23.6.0 Arch: arm64 C++ Compiler: /Library/Developer/CommandLineTools/usr/bin/c++ C++ Compiler Version: 15.0.0.15000309 C Compiler: /Library/Developer/CommandLineTools/usr/bin/cc C Compiler Version: 15.0.0.15000309 CMake Prefix Path: /Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk/usr;/opt/homebrew;/usr/local;/usr;/;/opt/homebrew;/usr/local;/usr/X11R6;/usr/pkg;/opt;/sw;/opt/local

Relevant logs

No response

aditi-pandit commented 11 hours ago

@majetideepak @Yuhta @xiaoxmeng