Improved assert messages for numeric comparisons

This commit is contained in:
Russell Sears 2020-07-02 11:28:08 -07:00
parent 58df3ba29f
commit 92ab2c84b6
3 changed files with 128 additions and 20 deletions

View File

@ -253,8 +253,8 @@ namespace FDB {
for(size_t i = 0; i < value.size(); i++) {
size_t offset = value.offsets[i];
size_t next_offset = (i+1 < value.offsets.size() ? value.offsets[i+1] : value.data.size());
ASSERT(offset < value.data.size());
ASSERT(next_offset <= value.data.size());
ASSERT_LT(offset, value.data.size());
ASSERT_LE(next_offset, value.data.size());
uint8_t code = value.data[offset];
if(code == NULL_CODE) {
data.push_back( data.arena(), NULL_CODE );
@ -363,7 +363,7 @@ namespace FDB {
int64_t swap;
bool neg = false;
ASSERT(offsets[index] < data.size());
ASSERT_LT(offsets[index], data.size());
uint8_t code = data[offsets[index]];
if(code < NEG_INT_START || code > POS_INT_END) {
throw invalid_tuple_data_type();
@ -392,7 +392,7 @@ namespace FDB {
if(index >= offsets.size()) {
throw invalid_tuple_index();
}
ASSERT(offsets[index] < data.size());
ASSERT_LT(offsets[index], data.size());
uint8_t code = data[offsets[index]];
if(code == FALSE_CODE) {
return false;
@ -407,7 +407,7 @@ namespace FDB {
if(index >= offsets.size()) {
throw invalid_tuple_index();
}
ASSERT(offsets[index] < data.size());
ASSERT_LT(offsets[index], data.size());
uint8_t code = data[offsets[index]];
if(code != FLOAT_CODE) {
throw invalid_tuple_data_type();
@ -415,7 +415,7 @@ namespace FDB {
float swap;
uint8_t* bytes = (uint8_t*)&swap;
ASSERT(offsets[index] + 1 + sizeof(float) <= data.size());
ASSERT_LE(offsets[index] + 1 + sizeof(float), data.size());
swap = *(float*)(data.begin() + offsets[index] + 1);
adjust_floating_point( bytes, sizeof(float), false );
@ -426,7 +426,7 @@ namespace FDB {
if(index >= offsets.size()) {
throw invalid_tuple_index();
}
ASSERT(offsets[index] < data.size());
ASSERT_LT(offsets[index], data.size());
uint8_t code = data[offsets[index]];
if(code != DOUBLE_CODE) {
throw invalid_tuple_data_type();
@ -434,7 +434,7 @@ namespace FDB {
double swap;
uint8_t* bytes = (uint8_t*)&swap;
ASSERT(offsets[index] + 1 + sizeof(double) <= data.size());
ASSERT_LE(offsets[index] + 1 + sizeof(double), data.size());
swap = *(double*)(data.begin() + offsets[index] + 1);
adjust_floating_point( bytes, sizeof(double), false );
@ -446,12 +446,12 @@ namespace FDB {
throw invalid_tuple_index();
}
size_t offset = offsets[index];
ASSERT(offset < data.size());
ASSERT_LT(offset, data.size());
uint8_t code = data[offset];
if(code != UUID_CODE) {
throw invalid_tuple_data_type();
}
ASSERT(offset + Uuid::SIZE + 1 <= data.size());
ASSERT_LE(offset + Uuid::SIZE + 1, data.size());
StringRef uuidData(data.begin() + offset + 1, Uuid::SIZE);
return Uuid(uuidData);
}
@ -461,15 +461,15 @@ namespace FDB {
throw invalid_tuple_index();
}
size_t offset = offsets[index];
ASSERT(offset < data.size());
ASSERT_LT(offset, data.size());
uint8_t code = data[offset];
if(code != NESTED_CODE) {
throw invalid_tuple_data_type();
}
size_t next_offset = (index + 1 < offsets.size() ? offsets[index+1] : data.size());
ASSERT(next_offset <= data.size());
ASSERT(data[next_offset - 1] == (uint8_t)0x00);
ASSERT_LE(next_offset, data.size());
ASSERT_EQ(data[next_offset - 1], (uint8_t)0x00);
Standalone<VectorRef<uint8_t>> dest;
dest.reserve(dest.arena(), next_offset - offset);
std::vector<size_t> dest_offsets;
@ -493,21 +493,21 @@ namespace FDB {
}
} else {
// A null object within the nested tuple.
ASSERT(i + 1 < next_offset - 1);
ASSERT(data[i+1] == 0xff);
ASSERT_LT(i + 1, next_offset - 1);
ASSERT_EQ(data[i+1], 0xff);
i += 2;
}
}
else if(code == BYTES_CODE || code == STRING_CODE) {
size_t next_i = find_string_terminator(data, i+1) + 1;
ASSERT(next_i <= next_offset - 1);
ASSERT_LE(next_i, next_offset - 1);
size_t length = next_i - i - 1;
dest.append(dest.arena(), data.begin() + i + 1, length);
i = next_i;
}
else if(code >= NEG_INT_START && code <= POS_INT_END) {
size_t int_size = abs(code - INT_ZERO_CODE);
ASSERT(i + int_size <= next_offset - 1);
ASSERT_LE(i + int_size, next_offset - 1);
dest.append(dest.arena(), data.begin() + i + 1, int_size);
i += int_size + 1;
}
@ -515,17 +515,17 @@ namespace FDB {
i += 1;
}
else if(code == UUID_CODE) {
ASSERT(i + 1 + Uuid::SIZE <= next_offset - 1);
ASSERT_LE(i + 1 + Uuid::SIZE, next_offset - 1);
dest.append(dest.arena(), data.begin() + i + 1, Uuid::SIZE);
i += Uuid::SIZE + 1;
}
else if(code == FLOAT_CODE) {
ASSERT(i + 1 + sizeof(float) <= next_offset - 1);
ASSERT_LE(i + 1 + sizeof(float), next_offset - 1);
dest.append(dest.arena(), data.begin() + i + 1, sizeof(float));
i += sizeof(float) + 1;
}
else if(code == DOUBLE_CODE) {
ASSERT(i + 1 + sizeof(double) <= next_offset - 1);
ASSERT_LE(i + 1 + sizeof(double), next_offset - 1);
dest.append(dest.arena(), data.begin() + i + 1, sizeof(double));
i += sizeof(double) + 1;
}

View File

@ -21,6 +21,7 @@
#include "flow/Error.h"
#include "flow/Trace.h"
#include "flow/Knobs.h"
#include "flow/UnitTest.h"
#include <iostream>
using std::cout;
using std::endl;
@ -70,6 +71,21 @@ Error internal_error_impl(const char* msg, const char* file, int line) {
return Error(error_code_internal_error);
}
Error internal_error_impl(const char* a_nm, long long a, const char * op_nm, const char * b_nm, long long b, const char * file, int line) {
fprintf(stderr, "Assertion %s (%lld) %s %s (%lld) failed @ %s %d:\n", a_nm, a, op_nm, b_nm, b, file, line, platform::get_backtrace().c_str());
TraceEvent(SevError, "InternalError")
.error(Error::fromCode(error_code_internal_error))
.detailf("FailedAssertion", "%s %s %s", a_nm, op_nm, b_nm)
.detail("LeftValue", a)
.detail("RightValue", b)
.detail("File", file)
.detail("Line", line)
.backtrace();
flushTraceFileVoid();
return Error(error_code_internal_error);
}
Error::Error(int error_code)
: error_code(error_code), flags(0)
{
@ -132,3 +148,20 @@ bool isAssertDisabled(int line) {
void breakpoint_me() {
return;
}
TEST_CASE("/flow/AssertTest") {
// this is mostly checking bug for bug compatibility with the C integer / sign promotion rules.
ASSERT_LT(-1, 0);
ASSERT_EQ(0, 0u);
ASSERT_GT(1, -1);
ASSERT_EQ((int32_t)0xFFFFFFFF, (int32_t)-1);
// ASSERT_LT(-1, 0u); // fails: -1 is promoted to unsigned value in comparison.
// ASSERT(-1 < 0u); // also fails
int sz = 42;
size_t ln = 43;
ASSERT(sz < ln);
ASSERT_EQ(0xFFFFFFFF, (int32_t)-1);
return Void();
}

View File

@ -84,6 +84,8 @@ enum { error_code_actor_cancelled = error_code_operation_cancelled };
extern Error internal_error_impl( const char* file, int line );
extern Error internal_error_impl(const char* msg, const char* file, int line);
extern Error internal_error_impl(const char * a_nm, long long a, const char * op_nm, const char * b_nm, long long b, const char * file, int line);
#define inernal_error_msg(msg) internal_error_impl(msg, __FILE__, __LINE__)
extern bool isAssertDisabled( int line );
@ -110,6 +112,79 @@ extern bool isAssertDisabled( int line );
#define UNREACHABLE() \
{ throw internal_error_impl("unreachable", __FILE__, __LINE__); }
enum assert_op { EQ, NE, LT, GT, LE, GE };
// TODO: magic so this works even if const-ness doesn not match.
template<typename T, typename U>
void assert_num_impl(char const * a_nm,
T const & a,
assert_op op,
char const * b_nm,
U const & b,
char const * file,
int line) {
bool success;
char const * op_name;
switch (op) {
case EQ:
success = a == b;
op_name = "==";
break;
case NE:
success = a != b;
op_name = "!=";
break;
case LT:
success = a < b;
op_name = "<";
break;
case GT:
success = a > b;
op_name = ">";
break;
case LE:
success = a <= b;
op_name = "<=";
break;
case GE:
success = a >= b;
op_name = ">=";
break;
default:
success = false;
op_name = "UNKNOWN OP";
}
if (!success) {
throw internal_error_impl(a_nm, (long long)a, op_name, b_nm, (long long)b, file, line);
}
}
#define ASSERT_EQ(a, b) \
do { \
assert_num_impl((#a), (a), assert_op::EQ, (#b), (b), __FILE__, __LINE__); \
} while (0)
#define ASSERT_NE(a, b) \
do { \
assert_num_impl((#a), (a), assert_op::NE, (#b), (b), __FILE__, __LINE__); \
} while (0)
#define ASSERT_LT(a, b) \
do { \
assert_num_impl((#a), (a), assert_op::LT, (#b), (b), __FILE__, __LINE__); \
} while (0)
#define ASSERT_LE(a, b) \
do { \
assert_num_impl((#a), (a), assert_op::LE, (#b), (b), __FILE__, __LINE__); \
} while (0)
#define ASSERT_GT(a, b) \
do { \
assert_num_impl((#a), (a), assert_op::GT, (#b), (b), __FILE__, __LINE__); \
} while (0)
#define ASSERT_GE(a, b) \
do { \
assert_num_impl((#a), (a), assert_op::GE, (#b), (b), __FILE__, __LINE__); \
} while (0)
// ASSERT_WE_THINK() is to be used for assertions that we want to validate in testing, but which are judged too
// risky to evaluate at runtime, because the code should work even if they are false and throwing internal_error() would
// result in a bug. Don't use it for assertions that are *expensive*; look at EXPENSIVE_VALIDATION.