Skip to content

Commit

Permalink
FileDataLoader fails to read the file when size > INT32_MAX (#4435)
Browse files Browse the repository at this point in the history
Summary:
On macOS, the `read` function will fail with an `EINVAL` error if the size parameter exceeds `INT32_MAX`. This update addresses the issue by adding a check to ensure that the read size does not surpass `INT32_MAX`. On Linux, the maximum permissible read size is 2,147,479,552 bytes ( < `INT32_MAX`), so attempting to read beyond this limit is inconsequential.

Pull Request resolved: #4435

Test Plan:
Exporting llama3 with `python -m examples.models.llama2.export_llama --checkpoint examples/models/llama-2-7B/consolidated.00.pth --params examples/models/llama-2-7B/params.json --coreml --disable_dynamic_shape -kv `

Without fix
Fails with `invalid argument` error.

With fix
Succeeds.

Reviewed By: kirklandsign

Differential Revision: D60321719

Pulled By: shoumikhin

fbshipit-source-id: fca265c6c1edc628b38a5044693ec7bbe0c0b43a
  • Loading branch information
cymbalrush authored and facebook-github-bot committed Jul 29, 2024
1 parent 5a20a49 commit 1e4603d
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion extension/data_loader/file_data_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@

#include <executorch/extension/data_loader/file_data_loader.h>

#include <algorithm>
#include <cerrno>
#include <cstddef>
#include <cstring>
#include <limits>

#include <fcntl.h>
#include <sys/stat.h>
Expand Down Expand Up @@ -189,7 +191,12 @@ Result<FreeableBuffer> FileDataLoader::load(
size_t needed = size;
uint8_t* buf = reinterpret_cast<uint8_t*>(aligned_buffer);
while (needed > 0) {
ssize_t nread = ::read(fd_, buf, needed);
// Reads on macos will fail with EINVAL if size > INT32_MAX.
ssize_t nread = ::read(
fd_,
buf,
std::min<size_t>(
needed, static_cast<size_t>(std::numeric_limits<int32_t>::max())));
if (nread < 0 && errno == EINTR) {
// Interrupted by a signal; zero bytes read.
continue;
Expand Down

0 comments on commit 1e4603d

Please sign in to comment.