Permalink
Browse files

fix some more ubsan failures

Summary:
Ubsan complains about a few undefined behavior, such as signed integer overflow, left shifting negative integer.
Some of them seem to be related to real bugs, e.g., taking log() on 0.0, uninitialized rusage in light process resource tracking.

Reviewed By: markw65

Differential Revision: D7284109

fbshipit-source-id: aa0aca1b60102c27ad49a2eee10ba68e9de2ebdf
  • Loading branch information...
binliu19 authored and hhvm-bot committed Mar 24, 2018
1 parent 0aaf0ca commit b69f64767206750e28077b9882d9a76592e33afb
@@ -737,7 +737,7 @@ void MemoryManager::checkGC() {
*/
void MemoryManager::updateNextGc() {
t_trigger_allocated = -1;
if (!isGCEnabled()) {
if (!isGCEnabled() || m_usageLimit == std::numeric_limits<int64_t>::max()) {
m_nextGC = kNoNextGC;
updateMMDebt();
return;
@@ -190,7 +190,11 @@ struct Mul {
Cell operator()(double a, int64_t b) const { return make_dbl(a * b); }
Cell operator()(double a, double b) const { return make_dbl(a * b); }
Cell operator()(int64_t a, double b) const { return make_dbl(a * b); }
Cell operator()(int64_t a, int64_t b) const { return make_int(a * b); }
Cell operator()(int64_t a, int64_t b) const
// TODO: T26068998 fix signed-integer-overflow undefined behavior
FOLLY_DISABLE_UNDEFINED_BEHAVIOR_SANITIZER("signed-integer-overflow") {
return make_int(a * b);
}
ArrayData* operator()(ArrayData* a1, ArrayData* /*a2*/) const {
throw_bad_array_operand(a1);
@@ -645,7 +649,7 @@ Cell cellShl(Cell c1, Cell c2) {
}
}
return make_int(lhs << (shift & 63));
return make_int(static_cast<uint64_t>(lhs) << (shift & 63));
}
Cell cellShr(Cell c1, Cell c2) {
@@ -108,8 +108,9 @@ double php_math_round(double value, int places,
int mode /* = PHP_ROUND_HALF_UP */) {
double tmp_value;
if (std::isinf(value)) {
return value;
// We need to avoid calculating log(0.0) later, but log(epsilon) is fine.
if (std::isinf(value) || value == 0.0) {
return value;
}
int precision_places = 14 - php_intlog10abs(value);
@@ -529,7 +529,7 @@ struct CompactWriter {
}
uint64_t i64ToZigzag(int64_t n) {
return (n << 1) ^ (n >> 63);
return (static_cast<uint64_t>(n) << 1) ^ (n >> 63);
}
};
@@ -262,7 +262,7 @@ void HttpRequestHandler::handleRequest(Transport *transport) {
Timer::GetMonotonicTime(now);
const timespec& queueTime = transport->getQueueTime();
if (gettime_diff_us(queueTime, now) > requestTimeoutSeconds * 1000000) {
if (gettime_diff_us(queueTime, now) > requestTimeoutSeconds * 1000000LL) {
transport->sendString("Service Unavailable", 503);
transport->onSendEnd();
m_requestTimedOutOnQueue->addValue(1);
@@ -153,7 +153,7 @@ void BootStats::done() {
StructuredLog::log("hhvm_boot_timer", cols);
cols.clear();
for (auto sample : s_instance->m_marks) {
cols.setInt(sample.first, sample.second.rssMb() << 20); // To bytes.
cols.setInt(sample.first, sample.second.rssMb() * (1 << 20)); // To bytes.
}
StructuredLog::log("hhvm_boot_memory", cols);
}
@@ -366,12 +366,13 @@ void do_waitpid(int afdt_fd) {
}
rusage ru;
int64_t time_us = 0;
const auto ret = ::wait4(pid, &stat, options, &ru);
alarm(0); // cancel the previous alarm if not triggered yet
waited = 0;
const auto time_us = ru2microseconds(ru);
int64_t events[] = { 0, 0, 0 };
if (ret > 0 && s_trackProcessTimes) {
time_us = ru2microseconds(ru);
auto it = s_pidToHCWMap.find(ret);
if (it == s_pidToHCWMap.end()) {
throw Exception("pid not in map: %s",
@@ -836,7 +837,7 @@ pid_t LightProcess::waitpid(pid_t pid, int *stat_loc, int options,
// light process is not really there
rusage ru;
const auto ret = wait4(pid, stat_loc, options, &ru);
if (s_trackProcessTimes) {
if (ret > 0 && s_trackProcessTimes) {
s_extra_request_nanoseconds += ru2microseconds(ru) * 1000;
}
return ret;
@@ -869,7 +870,7 @@ pid_t LightProcess::waitpid(pid_t pid, int *stat_loc, int options,
pid_t LightProcess::pcntl_waitpid(pid_t pid, int *stat_loc, int options) {
rusage ru;
const auto ret = wait4(pid, stat_loc, options, &ru);
if (s_trackProcessTimes) {
if (ret > 0 && s_trackProcessTimes) {
s_extra_request_nanoseconds += ru2microseconds(ru) * 1000;
}
return ret;

0 comments on commit b69f647

Please sign in to comment.