You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2018/11/09 04:20:28 UTC
kudu git commit: KUDU-2378: fix another aligned load crash
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 <ad...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
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 <ad...@cloudera.com>
Authored: Thu Nov 8 12:18:36 2018 -0800
Committer: Adar Dembo <ad...@cloudera.com>
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 };