I'm also seeing the jsonreader-test crash in RELEASE mode on Ubuntu bionic.
The symptom is a nullptr dereference but really it's caused by an assertion
failure.
I modified the test to get a proper failure and it fails here:
// Min signed 32-bit integer.
const char* const signed_small32 = "signed_small32";
Status s = r.ExtractUint32(r.root(), signed_small32, &dummy_u32);
ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
That status object is OK, which means jsonreader incorrectly detected the
negative integer as a uint in the following piece of code:
Status JsonReader::ExtractUint32(const Value* object,
const char* field,
uint32_t* result) const {
const Value* val;
RETURN_NOT_OK(ExtractField(object, field, &val));
if (PREDICT_FALSE(!val->IsUint())) {
return Status::InvalidArgument(Substitute(
"wrong type during field extraction: expected uint32 but got $0",
TypeToString(val->GetType())));
}
*result = val->GetUint();
return Status::OK();
}
Note that this only happens in RELEASE mode for me. In this test, there is
a comment preventing UB testing that looks like this:
// The rapidjson code has some improper handling of the min int32 and min
// int64 that exposes UB.
#if defined(ADDRESS_SANITIZER)
LOG(WARNING) << "this test is skipped in ASAN builds";
return;
#endif
It's surprising we have never noticed this before. Maybe only the newer
compilers are causing problems due to the undefined behavior.
When I comment out that #ifdef and run this under ASAN with UBSAN enabled,
I get the following warning:
[ RUN ] JsonReaderTest.SignedAndUnsignedInts
thirdparty/installed/common/include/rapidjson/reader.h:644:18: runtime
error: negation of -2147483648 cannot be represented in type 'int'; cast to
an unsigned type to negate this value to itself
#0 0x8b15e4 in void rapidjson::GenericReader<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::ParseNumber<0u,
rapidjson::GenericStringStream<rapidjson::UTF8<char> >,
rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >
>(rapidjson::GenericStringStream<rapidjson::UTF8<char> >&,
rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&)
../thirdparty/installed/common/include/rapidjson/reader.h:644:18
#1 0x8aaf00 in void rapidjson::GenericReader<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::ParseValue<0u,
rapidjson::GenericStringStream<rapidjson::UTF8<char> >,
rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >
>(rapidjson::GenericStringStream<rapidjson::UTF8<char> >&,
rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&)
../thirdparty/installed/common/include/rapidjson/reader.h:663:14
#2 0x8a8b96 in void rapidjson::GenericReader<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::ParseObject<0u,
rapidjson::GenericStringStream<rapidjson::UTF8<char> >,
rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >
>(rapidjson::GenericStringStream<rapidjson::UTF8<char> >&,
rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&)
../thirdparty/installed/common/include/rapidjson/reader.h:290:4
#3 0x8a7829 in bool rapidjson::GenericReader<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::Parse<0u,
rapidjson::GenericStringStream<rapidjson::UTF8<char> >,
rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >
>(rapidjson::GenericStringStream<rapidjson::UTF8<char> >&,
rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&)
../thirdparty/installed/common/include/rapidjson/reader.h:243:15
#4 0x8a6f10 in rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&
rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::ParseStream<0u,
rapidjson::GenericStringStream<rapidjson::UTF8<char> >
>(rapidjson::GenericStringStream<rapidjson::UTF8<char> >&)
../thirdparty/installed/common/include/rapidjson/document.h:712:23
#5 0x8a34f2 in rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&
rapidjson::GenericDocument<rapidjson::UTF8<char>,
rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >::Parse<0u>(char
const*) ../thirdparty/installed/common/include/rapidjson/document.h:745:10
#6 0x89d804 in kudu::JsonReader::Init()
../src/kudu/util/jsonreader.cc:65:13
#7 0x7754f7 in
kudu::JsonReaderTest_SignedAndUnsignedInts_Test::TestBody()
../src/kudu/util/jsonreader-test.cc:172:3
#8 0xe69c6c in void
testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test,
void>(testing::Test*, void (testing::Test::*)(), char const*)
/home/mpercy/src/kudu/thirdparty/src/googletest-release-1.8.0/googletest/src/gtest.cc:2402
#9 0xe69c6c in void
testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,
void>(testing::Test*, void (testing::Test::*)(), char const*)
/home/mpercy/src/kudu/thirdparty/src/googletest-release-1.8.0/googletest/src/gtest.cc:2438
#10 0xe60391 in testing::Test::Run()
/home/mpercy/src/kudu/thirdparty/src/googletest-release-1.8.0/googletest/src/gtest.cc:2474
#11 0xe60473 in testing::TestInfo::Run()
/home/mpercy/src/kudu/thirdparty/src/googletest-release-1.8.0/googletest/src/gtest.cc:2656
#12 0xe605b6 in testing::TestCase::Run()
/home/mpercy/src/kudu/thirdparty/src/googletest-release-1.8.0/googletest/src/gtest.cc:2774
#13 0xe60a87 in testing::internal::UnitTestImpl::RunAllTests()
/home/mpercy/src/kudu/thirdparty/src/googletest-release-1.8.0/googletest/src/gtest.cc:4649
#14 0xe6a14c in bool
testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
bool>(testing::internal::UnitTestImpl*, bool
(testing::internal::UnitTestImpl::*)(), char const*)
/home/mpercy/src/kudu/thirdparty/src/googletest-release-1.8.0/googletest/src/gtest.cc:2402
#15 0xe6a14c in bool
testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
bool>(testing::internal::UnitTestImpl*, bool
(testing::internal::UnitTestImpl::*)(), char const*)
/home/mpercy/src/kudu/thirdparty/src/googletest-release-1.8.0/googletest/src/gtest.cc:2438
#16 0xe60be0 in testing::UnitTest::Run()
/home/mpercy/src/kudu/thirdparty/src/googletest-release-1.8.0/googletest/src/gtest.cc:4257
#17 0x7a61bb in RUN_ALL_TESTS()
../thirdparty/installed/uninstrumented/include/gtest/gtest.h:2233:46
#18 0x7a2b24 in main ../src/kudu/util/test_main.cc:106:13
#19 0x7f36f9ffeb96 in __libc_start_main csu/libc-start.c:310
#20 0x663409 in _start
(/home/mpercy/src/kudu/build/asandebug/bin/jsonreader-test+0x663409)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
thirdparty/installed/common/include/rapidjson/reader.h:644:18 in
When I rebased on top of this patch to upgrade RapidJSON to 1.1.0, the test
passed and the crash did not occur: https://gerrit.cloudera.org/c/12643/
Looking at the RapidJSON change log for the 1.1.0 release, they mention
fixing the undefined behavior <
http://rapidjson.org/md__c_h_a_n_g_e_l_o_g.html>:
1.1.0 - 2016-08-25
Added
* Fix undefined behaviour (#646) which references
https://github.com/Tencent/rapidjson/pull/646/commits ... I am not sure if
the actual relevant bugfix is in that PR or not but it seems that PR
finally got RapidJSON passing the UBSAN tests.
I think we should run another RC since the UB is actually causing
observable undefined / incorrect behavior with new compiler optimizations,
at least in a test.
Mike
On Fri, Mar 1, 2019 at 3:40 PM Greg Solovyev <[email protected]>
wrote:
> +1
> I built on Ubuntu 16, Ubuntu 18 and OSX 10.14.3. I also tested Docker
> build.
>
> The following test failed consistently on release and debug builds on
> Ubuntu 16:
>
> - client_examples-test
>
> The following test was flaky on Ubuntu 16:
>
> - authn_token_expire-itest
>
> The following test failed consistently on release build on Ubuntu18:
>
> - jsonreader-test
>
> Docker build took about 2 hours after increasing Docker RAM to 6GB. The
> first 3 attempts failed when I had Docker RAM set to 4.2GB.
>
> On OSX 10.14.3 the following tests failed on release build:
> 23 - hybrid_clock-test (Failed)
> 175 - sentry_authz_provider-test (Failed)
> 197 - sentry_client-test (Failed)
> 261 - kudu-tool-test.2 (Failed)
> 313 - jsonreader-test (Failed)
>
> Greg
>
> On Wed, Feb 27, 2019 at 6:39 PM Adar Lieber-Dembo
> <[email protected]>
> wrote:
>
> > +1
> >
> > I ran the C++ tests (DEBUG build) in slow mode on Ubuntu 18, CentOS
> > 6.6, and CentOS 7.3. I saw a couple failures but I'm pretty sure
> > they're just flakes; filed KUDU-2718 and KUDU-2719 for them.
> >
> >
> > On Wed, Feb 27, 2019 at 1:51 AM Andrew Wong <[email protected]> wrote:
> > >
> > > Hello Kudu devs!
> > >
> > > As suggested on the RC1 voting thread, I've included the fix for
> > KUDU-2710
> > > and some other bug fixes that landed on master since RC1, and created a
> > new
> > > release candidate for Apache Kudu 1.9.0.
> > >
> > > Apache Kudu 1.9.0 is a minor release that offers many improvements and
> > > fixes since the prior release.
> > >
> > > This is a source-only release. The artifacts have been staged here:
> > > https://dist.apache.org/repos/dist/dev/kudu/1.9.0-RC2/
> > >
> > > Java convenience binaries in the form of a Maven repository are staged
> > here:
> > > https://repository.apache.org/content/repositories/orgapachekudu-1029/
> > >
> > > It is tagged in Git as 1.9.0-RC2 and the corresponding hash is the
> > > following:
> > >
> >
> https://gitbox.apache.org/repos/asf?p=kudu.git;a=commit;h=deb0f04b010c5399771879829285c284b8f20008
> > >
> > > A draft of the release notes can be found here:
> > > https://gerrit.cloudera.org/c/12389/
> > >
> > > The KEYS file to verify the artifact signatures can be found here:
> > > https://dist.apache.org/repos/dist/release/kudu/KEYS
> > >
> > > I'd suggest going through the README and the release notes, building
> > Kudu,
> > > and
> > > running the unit tests. Testing out the Maven repo would also be
> > > appreciated.
> > >
> > > The vote will run until this coming Friday, March 1st at 11:59AM PST.
> > >
> > >
> > > Thank you,
> > > Andrew
> >
>
