Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/12643 )
Change subject: [thirdparty] Bump RapidJSON version to 1.1.0 ...................................................................... Patch Set 6: (6 comments) > (6 comments) > > > looks like lots of test failures from this new version in ASAN > builds. I think Mike and Adar have been looking at rapidjson UBSAN > problems as well > > I pulled out one of the failures below. Like Todd said, let us know > if you need help debugging this. > > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/installed/common/include/rapidjson/internal/dtoa.h:88:16: > runtime error: unsigned integer overflow: 4294967292 + 4 cannot be > represented in type 'unsigned int' > #0 0x7f3b9b8dabd1 in rapidjson::internal::DigitGen(rapidjson::internal::DiyFp > const&, rapidjson::internal::DiyFp const&, unsigned long, char*, > int*, int*) > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/installed/common/include/rapidjson/internal/dtoa.h > #1 0x7f3b9b8d8638 in rapidjson::internal::Grisu2(double, char*, > int*, int*) > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/installed/common/include/rapidjson/internal/dtoa.h:123:5 > #2 0x7f3b9b8d8039 in rapidjson::internal::dtoa(double, char*, int) > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/installed/common/include/rapidjson/internal/dtoa.h:233:9 > #3 0x7f3b9b982f0c in rapidjson::Writer<kudu::UTF8StringStreamBuffer, > rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator, > 0u>::WriteDouble(double) > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/installed/common/include/rapidjson/writer.h:338:21 > #4 0x7f3b9b97da93 in > kudu::JsonWriter::ProtobufField(google::protobuf::Message > const&, google::protobuf::Reflection const*, > google::protobuf::FieldDescriptor > const*) > /home/jenkins-slave/workspace/kudu-master/1/src/kudu/util/jsonwriter.cc > #5 0x7f3b9b97c472 in kudu::JsonWriter::Protobuf(google::protobuf::Message > const&) > /home/jenkins-slave/workspace/kudu-master/1/src/kudu/util/jsonwriter.cc:214:7 > #6 0x7f3b9b9e54a3 in kudu::Histogram::WriteAsJson(kudu::JsonWriter*, > kudu::MetricJsonOptions const&) const > /home/jenkins-slave/workspace/kudu-master/1/src/kudu/util/metrics.cc:658:11 > #7 0x7f3b9b9df851 in kudu::MetricEntity::WriteAsJson(kudu::JsonWriter*, > std::vector<std::string, std::allocator<std::string> > const&, > kudu::MetricJsonOptions const&) const > /home/jenkins-slave/workspace/kudu-master/1/src/kudu/util/metrics.cc:264:7 > #8 0x7f3b9b9e18fc in kudu::MetricRegistry::WriteAsJson(kudu::JsonWriter*, > std::vector<std::string, std::allocator<std::string> > const&, > kudu::MetricJsonOptions const&) const > /home/jenkins-slave/workspace/kudu-master/1/src/kudu/util/metrics.cc:356:5 > #9 0x7f3ba6efd4c6 in kudu::WriteMetricsAsJson(kudu::MetricRegistry > const*, kudu::WebCallbackRegistry::WebRequest const&, > kudu::WebCallbackRegistry::PrerenderedWebResponse*) > /home/jenkins-slave/workspace/kudu-master/1/src/kudu/server/default_path_handlers.cc:359:3 > #10 0x7f3ba6f07f41 in void boost::_bi::bind_t<void, void > (*)(kudu::MetricRegistry const*, kudu::WebCallbackRegistry::WebRequest > const&, kudu::WebCallbackRegistry::PrerenderedWebResponse*), > boost::_bi::list3<boost::_bi::value<kudu::MetricRegistry const*>, > boost::arg<1>, boost::arg<2> > > >::operator()<kudu::WebCallbackRegistry::WebRequest > const&, > kudu::WebCallbackRegistry::PrerenderedWebResponse*>(kudu::WebCallbackRegistry::WebRequest > const&, kudu::WebCallbackRegistry::PrerenderedWebResponse*&&) > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/installed/uninstrumented/include/boost/bind/bind.hpp:1246:16 > #11 0x7f3ba6f07aef in > boost::detail::function::void_function_obj_invoker2<boost::_bi::bind_t<void, > void (*)(kudu::MetricRegistry const*, kudu::WebCallbackRegistry::WebRequest > const&, kudu::WebCallbackRegistry::PrerenderedWebResponse*), > boost::_bi::list3<boost::_bi::value<kudu::MetricRegistry const*>, > boost::arg<1>, boost::arg<2> > >, void, kudu::WebCallbackRegistry::WebRequest > const&, > kudu::WebCallbackRegistry::PrerenderedWebResponse*>::invoke(boost::detail::function::function_buffer&, > kudu::WebCallbackRegistry::WebRequest const&, > kudu::WebCallbackRegistry::PrerenderedWebResponse*) > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/installed/uninstrumented/include/boost/function/function_template.hpp:159:11 > #12 0x7f3ba6f86a19 in boost::function2<void, > kudu::WebCallbackRegistry::WebRequest > const&, > kudu::WebCallbackRegistry::PrerenderedWebResponse*>::operator()(kudu::WebCallbackRegistry::WebRequest > const&, kudu::WebCallbackRegistry::PrerenderedWebResponse*) const > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/installed/uninstrumented/include/boost/function/function_template.hpp:770:14 > #13 0x7f3ba6f7e082 in > kudu::Webserver::RunPathHandler(kudu::Webserver::PathHandler > const&, sq_connection*, sq_request_info*) > /home/jenkins-slave/workspace/kudu-master/1/src/kudu/server/webserver.cc:469:5 > #14 0x7f3ba6f7cd86 in kudu::Webserver::BeginRequestCallback(sq_connection*, > sq_request_info*) > /home/jenkins-slave/workspace/kudu-master/1/src/kudu/server/webserver.cc:403:10 > #15 0x7f3ba6fad436 in handle_request > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/src/squeasel-9335b81317a6451d5a37c5dc7ec088eecbf68c82/squeasel.c:3865 > #16 0x7f3ba6fafbb2 in process_new_connection > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/src/squeasel-9335b81317a6451d5a37c5dc7ec088eecbf68c82/squeasel.c:4496 > #17 0x7f3ba6fb03d7 in worker_thread > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/src/squeasel-9335b81317a6451d5a37c5dc7ec088eecbf68c82/squeasel.c:4628 > #18 0x7f3ba1bee183 in start_thread > /build/eglibc-SvCtMH/eglibc-2.19/nptl/pthread_create.c:312 > #19 0x7f3b96f45ffc in clone sysdeps/unix/sysv/linux/x86_64/clone.S:111 > > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior > /home/jenkins-slave/workspace/kudu-master/1/thirdparty/installed/common/include/rapidjson/internal/dtoa.h:88:16 > in Interesting, all unit tests in ASAN build type passed in my own environment. For this stack, I find something unexpected: #5 0x7f3b9b97c472 in kudu::JsonWriter::Protobuf(google::protobuf::Message const&) /home/jenkins-slave/workspace/kudu-master/1/src/kudu/util/jsonwriter.cc:214:7 It seemed that some field in HistogramSnapshotPB was interpreted as FieldDescriptor::CPPTYPE_INT64 type, but there is no field type is int64 in HistogramSnapshotPB. http://gerrit.cloudera.org:8080/#/c/12643/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12643/5//COMMIT_MSG@16 PS5, Line 16: > If the only usage is test-only, It'd be simpler to just create a new JsonWr No other use case now, I'll remove it. http://gerrit.cloudera.org:8080/#/c/12643/5/src/kudu/util/jsonreader-test.cc File src/kudu/util/jsonreader-test.cc: http://gerrit.cloudera.org:8080/#/c/12643/5/src/kudu/util/jsonreader-test.cc@147 PS5, Line 147: constexpr auto kMaxInt32 = std::numeric_limits<int32_t>::max(); : constexpr auto kMaxInt64 = std::numeric_limits<int64_t>::max(); : constexpr auto kMaxUint32 = std::numeric_limits<uint32_t>::max(); : constexpr auto kMaxUint64 = std::numeric_limits<uint64_t>::max(); : constexpr auto kMinInt32 = std::numeric_limits<int32_t>::min(); : conste > We should be able to remove this now. Done http://gerrit.cloudera.org:8080/#/c/12643/5/src/kudu/util/jsonwriter-test.cc File src/kudu/util/jsonwriter-test.cc: http://gerrit.cloudera.org:8080/#/c/12643/5/src/kudu/util/jsonwriter-test.cc@186 PS5, Line 186: for (int i = 0; i < 10000; i++) { > Could you run these benchmarks before and after the upgrade in RELEASE mode old version: TestJsonWriter.BenchmarkAllTypes Throughput: 119.812MB/sec TestJsonWriter.BenchmarkNestedMessage Throughput: 88.6562MB/sec TestJsonWriter.BenchmarkRepeatedInt64 Throughput: 115.427MB/sec ----------------- new version: TestJsonWriter.BenchmarkAllTypes Throughput: 136.701MB/sec TestJsonWriter.BenchmarkNestedMessage Throughput: 87.075MB/sec TestJsonWriter.BenchmarkRepeatedInt64 Throughput: 122.741MB/sec http://gerrit.cloudera.org:8080/#/c/12643/5/src/kudu/util/jsonwriter.h File src/kudu/util/jsonwriter.h: http://gerrit.cloudera.org:8080/#/c/12643/5/src/kudu/util/jsonwriter.h@83 PS5, Line 83: // Convert the given protobuf to JSON format. > Could you document what this does in a comment? removed http://gerrit.cloudera.org:8080/#/c/12643/5/src/kudu/util/jsonwriter.cc File src/kudu/util/jsonwriter.cc: http://gerrit.cloudera.org:8080/#/c/12643/5/src/kudu/util/jsonwriter.cc@30 PS5, Line 30: #include <rapidjson/prettywriter.h> > What was wrong with the existing inclusion of rapidjson/rapidjson.h? IWYU report an error: @@ -27,8 +27,8 @@ #include <google/protobuf/descriptor.pb.h> #include <google/protobuf/message.h> #include <rapidjson/encodings.h> +#include <rapidjson/internal/../rapidjson.h> #include <rapidjson/prettywriter.h> -#include <rapidjson/rapidjson.h> #include <rapidjson/writer.h> I'm not clear how to fix it.. http://gerrit.cloudera.org:8080/#/c/12643/5/src/kudu/util/jsonwriter.cc@378 PS5, Line 378: > Nit: add an empty line after this one. Done -- To view, visit http://gerrit.cloudera.org:8080/12643 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c5e6bf6b8c6802bde28716487abb2b242a7a578 Gerrit-Change-Number: 12643 Gerrit-PatchSet: 6 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Tue, 05 Mar 2019 07:28:17 +0000 Gerrit-HasComments: Yes
