Fixed a critical bug on 32-bit builds, and added much more Kokoro testing.

There was a bug in our arena code where we assumed that
sizeof(upb_array) would be a multiple of 8. On i386 it was
not, and this was causing memory corruption on 32-bit builds.
pull/13171/head
Joshua Haberman 4 years ago
parent 7543f851f1
commit 0497f8deed
  1. 11
      .bazelrc
  2. 26
      kokoro/ubuntu/build.sh
  3. 4
      tests/test_generated_code.c
  4. 3
      upb/decode.c
  5. 2
      upb/msg.c
  6. 9
      upb/msg.h
  7. 2
      upb/upb.h

@ -0,0 +1,11 @@
# Use our custom-configured c++ toolchain.
build:m32 --copt=-m32 --linkopt=-m32
build:asan --copt=-fsanitize=address --linkopt=-fsanitize=address
build:valgrind --run_under='valgrind --leak-check=full --error-exitcode=1'
build:ubsan --copt=-fsanitize=undefined --linkopt=-fsanitize=undefined --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1
# Workaround for the fact that Bazel links with $CC, not $CXX
# https://github.com/bazelbuild/bazel/issues/11122#issuecomment-613746748
build:ubsan --copt=-fno-sanitize=function --copt=-fno-sanitize=vptr

@ -11,29 +11,31 @@ fi
echo PATH=$PATH
ls -l `which cmake`
cmake --version
echo CC=${CC:-cc}
${CC:-cc} --version
# Log the bazel path and version.
which bazel
bazel version
cd $(dirname $0)/../..
bazel test --test_output=errors ...
if [[ $(uname) = "Linux" ]]; then
# Verify the ASAN build. Have to exclude test_conformance_upb as protobuf
# currently leaks memory in the conformance test runner.
bazel test --copt=-fsanitize=address --linkopt=-fsanitize=address --test_output=errors ...
if which gcc; then
gcc --version
CC=gcc bazel test --test_output=errors ...
fi
# Verify the UBSan build. Have to exclude Lua as the version we are using
# fails some UBSan tests.
if which clang; then
CC=clang bazel test --test_output=errors ...
CC=clang bazel test --test_output=errors --config=m32 ...
CC=clang bazel test --test_output=errors -c opt ...
# For some reason kokoro doesn't have Clang available right now.
#CC=clang CXX=clang++ bazel test -c dbg --copt=-fsanitize=undefined --copt=-fno-sanitize=function,vptr --linkopt=-fsanitize=undefined --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 -- :all -:test_lua
if [[ $(uname) = "Linux" ]]; then
CC=clang bazel test --test_output=errors --config=asan ...
# TODO: update to a newer Lua that hopefully does not trigger UBSAN.
CC=clang bazel test --test_output=errors --config=ubsan ... -- -tests/bindings/lua:test_lua
fi
fi
if which valgrind; then
bazel test --run_under='valgrind --leak-check=full --error-exitcode=1' ... -- -tests:test_conformance_upb -cmake:cmake_build
bazel test --config=valgrind ... -- -tests:test_conformance_upb -cmake:cmake_build
fi

@ -56,9 +56,9 @@ static void test_scalars(void) {
ASSERT(protobuf_test_messages_proto3_TestAllTypesProto3_optional_uint64(
msg2) == 40);
ASSERT(protobuf_test_messages_proto3_TestAllTypesProto3_optional_float(
msg2) == 50.5);
msg2) - 50.5 < 0.01);
ASSERT(protobuf_test_messages_proto3_TestAllTypesProto3_optional_double(
msg2) == 60.6);
msg2) - 60.6 < 0.01);
ASSERT(protobuf_test_messages_proto3_TestAllTypesProto3_optional_bool(
msg2) == 1);
ASSERT(upb_strview_eql(

@ -615,7 +615,8 @@ static const char *decode_msg(upb_decstate *d, const char *ptr, upb_msg *msg,
int ndx = field->descriptortype;
if (_upb_isrepeated(field)) ndx += 18;
ptr = decode_varint32(d, ptr, &val.size);
if (val.size >= INT32_MAX || ptr - d->end + val.size > d->limit) {
if (val.size >= INT32_MAX ||
ptr - d->end + (int32_t)val.size > d->limit) {
decode_err(d); /* Length overflow. */
}
op = delim_ops[ndx];

@ -11,7 +11,7 @@ static const size_t overhead = sizeof(upb_msg_internal);
static const upb_msg_internal *upb_msg_getinternal_const(const upb_msg *msg) {
ptrdiff_t size = sizeof(upb_msg_internal);
return UPB_PTR_AT(msg, -size, upb_msg_internal);
return (upb_msg_internal*)((char*)msg - size);
}
upb_msg *_upb_msg_new(const upb_msglayout *l, upb_arena *a) {

@ -94,7 +94,8 @@ UPB_INLINE upb_msg *_upb_msg_new_inl(const upb_msglayout *l, upb_arena *a) {
upb_msg *_upb_msg_new(const upb_msglayout *l, upb_arena *a);
UPB_INLINE upb_msg_internal *upb_msg_getinternal(upb_msg *msg) {
return UPB_PTR_AT(msg, -sizeof(upb_msg_internal), upb_msg_internal);
ptrdiff_t size = sizeof(upb_msg_internal);
return (upb_msg_internal*)((char*)msg - size);
}
/* Clears the given message. */
@ -189,9 +190,11 @@ typedef struct {
uintptr_t data; /* Tagged ptr: low 3 bits of ptr are lg2(elem size). */
size_t len; /* Measured in elements. */
size_t size; /* Measured in elements. */
uint64_t junk;
} upb_array;
UPB_INLINE const void *_upb_array_constptr(const upb_array *arr) {
UPB_ASSERT((arr->data & 7) <= 4);
return (void*)(arr->data & ~(uintptr_t)7);
}
@ -201,15 +204,17 @@ UPB_INLINE void *_upb_array_ptr(upb_array *arr) {
UPB_INLINE uintptr_t _upb_tag_arrptr(void* ptr, int elem_size_lg2) {
UPB_ASSERT(elem_size_lg2 <= 4);
UPB_ASSERT(((uintptr_t)ptr & 7) == 0);
return (uintptr_t)ptr | (unsigned)elem_size_lg2;
}
UPB_INLINE upb_array *_upb_array_new(upb_arena *a, size_t init_size,
int elem_size_lg2) {
const size_t arr_size = UPB_ALIGN_UP(sizeof(upb_array), 8);
const size_t bytes = sizeof(upb_array) + (init_size << elem_size_lg2);
upb_array *arr = (upb_array*)upb_arena_malloc(a, bytes);
if (!arr) return NULL;
arr->data = _upb_tag_arrptr(arr + 1, elem_size_lg2);
arr->data = _upb_tag_arrptr(UPB_PTR_AT(arr, arr_size, void), elem_size_lg2);
arr->len = 0;
arr->size = init_size;
return arr;

@ -314,7 +314,7 @@ UPB_INLINE uint64_t _upb_be_swap64(uint64_t val) {
}
UPB_INLINE int _upb_lg2ceil(int x) {
if (x == 0) return 0;
if (x <= 1) return 0;
#ifdef __GNUC__
return 32 - __builtin_clz(x - 1);
#else

Loading…
Cancel
Save