Skip to content

Commit

Permalink
Fix small ubsan issue in EventBaseAtomicNotificationQueue
Browse files Browse the repository at this point in the history
Summary:
`sizeof()` produces an unsigned value, and `result` can be `-1`, so this comparison was technically UB. For extra fun, triggering a ubsan issue in this location changes `errno`. This probably shouldn't actually be detected as a ubsan issue, since the `sizeof()` is trivially known at compile time, but C++ semantics say otherwise :(

Fixes: #1953

Reviewed By: vitalii-topoliuk

Differential Revision: D44110957

fbshipit-source-id: f41f7d3ef9fb71c0e5368f5c4ebbabd814b91609
  • Loading branch information
Orvid authored and facebook-github-bot committed Mar 16, 2023
1 parent b83a95d commit e288b43
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion folly/io/async/EventBaseAtomicNotificationQueue-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,9 @@ void EventBaseAtomicNotificationQueue<Task, Consumer>::drainFd() {
uint64_t message = 0;
if (eventfd_ >= 0) {
auto result = readNoInt(eventfd_, &message, sizeof(message));
CHECK(result == sizeof(message) || errno == EAGAIN || errno == EWOULDBLOCK)
CHECK(
result == (int)sizeof(message) || errno == EAGAIN ||
errno == EWOULDBLOCK)
<< "result = " << result << "; errno = " << errno;
writesObserved_ += message;
} else {
Expand Down

0 comments on commit e288b43

Please sign in to comment.