Repository: kudu Updated Branches: refs/heads/master af86a1fbd -> 1fc87e993
KUDU-2378: fix another aligned load crash The TestKuduBackup.testRandomBackupAndRestore test from the kudu-backup project would crash a TSAN master from time to time, with a stack trace ending in Variant::Reset. After some debugging, this turned out to be a holdover from KUDU-2378, where clang emitted a movaps (Aligned Load) instruction on a value that wasn't aligned. The holdover was missed because it was a static_cast (the original search covered reinterpret_casts); I did a search for other bad casts but found none. Why TSAN? Probably because it's the only build to use libc++, whose std::string uses the Small String Optimization to embed the data into the string itself, thus raising the possibility of the string's data being unaligned. By comparison, libstdc++'s COW std::string uses a separate allocation for string data, which is likely always 16-byte aligned. The patch includes a unit test that would repro the crash 100% of the time when built for TSAN. Change-Id: I13e23a9d88db878071faeff7b9de06d1bdd98357 Reviewed-on: http://gerrit.cloudera.org:8080/11911 Tested-by: Adar Dembo <[email protected]> Reviewed-by: Todd Lipcon <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/1fc87e99 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/1fc87e99 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/1fc87e99 Branch: refs/heads/master Commit: 1fc87e9930258384b981ab166b3c2fca93d5e21c Parents: af86a1f Author: Adar Dembo <[email protected]> Authored: Thu Nov 8 12:18:36 2018 -0800 Committer: Adar Dembo <[email protected]> Committed: Fri Nov 9 04:20:13 2018 +0000 ---------------------------------------------------------------------- src/kudu/common/types.h | 2 +- src/kudu/common/wire_protocol-test.cc | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/1fc87e99/src/kudu/common/types.h ---------------------------------------------------------------------- diff --git a/src/kudu/common/types.h b/src/kudu/common/types.h index db527d1..b3603da 100644 --- a/src/kudu/common/types.h +++ b/src/kudu/common/types.h @@ -703,7 +703,7 @@ class Variant { break; case DECIMAL128: case INT128: - numeric_.i128 = *static_cast<const int128_t *>(value); + numeric_.i128 = UnalignedLoad<int128_t>(value); break; case FLOAT: numeric_.float_val = *static_cast<const float *>(value); http://git-wip-us.apache.org/repos/asf/kudu/blob/1fc87e99/src/kudu/common/wire_protocol-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/common/wire_protocol-test.cc b/src/kudu/common/wire_protocol-test.cc index 9c11de3..35aa7ea 100644 --- a/src/kudu/common/wire_protocol-test.cc +++ b/src/kudu/common/wire_protocol-test.cc @@ -37,6 +37,7 @@ #include "kudu/util/bitmap.h" #include "kudu/util/bloom_filter.h" #include "kudu/util/faststring.h" +#include "kudu/util/hash.pb.h" #include "kudu/util/hexdump.h" #include "kudu/util/memory/arena.h" #include "kudu/util/pb_util.h" @@ -447,6 +448,15 @@ TEST_F(WireProtocolTest, TestColumnDefaultValue) { ASSERT_EQ(write_default_u32, *static_cast<const uint32_t *>(col5fpb.write_default_value())); } +// Regression test for KUDU-2378; the call to ColumnSchemaFromPB yielded a crash. +TEST_F(WireProtocolTest, TestCrashOnAlignedLoadOf128BitReadDefault) { + ColumnSchemaPB pb; + pb.set_name("col"); + pb.set_type(DECIMAL128); + pb.set_read_default_value(string(16, 'a')); + ColumnSchemaFromPB(pb); +} + TEST_F(WireProtocolTest, TestColumnPredicateInList) { ColumnSchema col1("col1", INT32); vector<ColumnSchema> cols = { col1 };
