Skip to content

Commit e2fae1d

Browse files
Fix crash by using alignedAlloc rather than alloc (#10554)
close #10553 need to use alignedAlloc for Complex Type
1 parent d0b28bd commit e2fae1d

File tree

6 files changed

+14
-107
lines changed

6 files changed

+14
-107
lines changed

dbms/src/AggregateFunctions/AggregateFunctionGroupArray.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ struct GroupArrayListNodeBase
184184
UInt64 size;
185185
readVarUInt(size, buf);
186186

187-
Node * node = reinterpret_cast<Node *>(arena->alloc(sizeof(Node) + size));
187+
Node * node = reinterpret_cast<Node *>(arena->alignedAlloc(sizeof(Node) + size, alignof(Node)));
188188
node->size = size;
189189
buf.read(node->data(), size);
190190
return node;
@@ -200,7 +200,7 @@ struct GroupArrayListNodeString : public GroupArrayListNodeBase<GroupArrayListNo
200200
{
201201
StringRef string = static_cast<const ColumnString &>(column).getDataAt(row_num);
202202

203-
Node * node = reinterpret_cast<Node *>(arena->alloc(sizeof(Node) + string.size));
203+
Node * node = reinterpret_cast<Node *>(arena->alignedAlloc(sizeof(Node) + string.size, alignof(Node)));
204204
node->next = nullptr;
205205
node->size = string.size;
206206
memcpy(node->data(), string.data, string.size);
@@ -217,7 +217,7 @@ struct GroupArrayListNodeGeneral : public GroupArrayListNodeBase<GroupArrayListN
217217

218218
static Node * allocate(const IColumn & column, size_t row_num, Arena * arena)
219219
{
220-
const char * begin = arena->alloc(sizeof(Node));
220+
const char * begin = arena->alignedAlloc(sizeof(Node), alignof(Node));
221221
StringRef value = column.serializeValueIntoArena(row_num, *arena, begin);
222222

223223
Node * node = reinterpret_cast<Node *>(const_cast<char *>(begin));

dbms/src/Columns/ColumnAggregateFunction.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ void ColumnAggregateFunction::insert(const Field & x)
327327

328328
Arena & arena = createOrGetArena();
329329

330-
getData().push_back(arena.alloc(function->sizeOfData()));
330+
getData().push_back(arena.alignedAlloc(function->sizeOfData(), function->alignOfData()));
331331
function->create(getData().back());
332332
ReadBufferFromString read_buffer(x.get<const String &>());
333333
function->deserialize(getData().back(), read_buffer, &arena);
@@ -339,7 +339,7 @@ void ColumnAggregateFunction::insertDefault()
339339

340340
Arena & arena = createOrGetArena();
341341

342-
getData().push_back(arena.alloc(function->sizeOfData()));
342+
getData().push_back(arena.alignedAlloc(function->sizeOfData(), function->alignOfData()));
343343
function->create(getData().back());
344344
}
345345

@@ -367,7 +367,7 @@ const char * ColumnAggregateFunction::deserializeAndInsertFromArena(
367367
*/
368368
Arena & dst_arena = createOrGetArena();
369369

370-
getData().push_back(dst_arena.alloc(function->sizeOfData()));
370+
getData().push_back(dst_arena.alignedAlloc(function->sizeOfData(), function->alignOfData()));
371371
function->create(getData().back());
372372

373373
/** We will read from src_arena.

dbms/src/DataTypes/DataTypeAggregateFunction.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ void DataTypeAggregateFunction::deserializeBinary(IColumn & column, ReadBuffer &
9292

9393
Arena & arena = column_concrete.createOrGetArena();
9494
size_t size_of_state = function->sizeOfData();
95-
AggregateDataPtr place = arena.alloc(size_of_state);
95+
AggregateDataPtr place = arena.alignedAlloc(size_of_state, function->alignOfData());
9696

9797
function->create(place);
9898
try
@@ -146,8 +146,7 @@ void DataTypeAggregateFunction::deserializeBinaryBulk(
146146
{
147147
if (istr.eof())
148148
break;
149-
150-
AggregateDataPtr place = arena.alloc(size_of_state);
149+
AggregateDataPtr place = arena.alignedAlloc(size_of_state, function->alignOfData());
151150

152151
function->create(place);
153152

@@ -178,7 +177,7 @@ static void deserializeFromString(const AggregateFunctionPtr & function, IColumn
178177

179178
Arena & arena = column_concrete.createOrGetArena();
180179
size_t size_of_state = function->sizeOfData();
181-
AggregateDataPtr place = arena.alloc(size_of_state);
180+
AggregateDataPtr place = arena.alignedAlloc(size_of_state, function->alignOfData());
182181

183182
function->create(place);
184183

dbms/src/Interpreters/AggregationCommon.h

Lines changed: 0 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -237,100 +237,6 @@ static inline UInt128 ALWAYS_INLINE hash128(
237237
return key;
238238
}
239239

240-
241-
/// Copy keys to the pool. Then put into pool StringRefs to them and return the pointer to the first.
242-
static inline StringRef * ALWAYS_INLINE placeKeysInPool(size_t keys_size, StringRefs & keys, Arena & pool)
243-
{
244-
for (size_t j = 0; j < keys_size; ++j)
245-
{
246-
char * place = pool.alloc(keys[j].size);
247-
memcpy(place, keys[j].data, keys[j].size); /// TODO padding in Arena and memcpySmall
248-
keys[j].data = place;
249-
}
250-
251-
/// Place the StringRefs on the newly copied keys in the pool.
252-
char * res = pool.alloc(keys_size * sizeof(StringRef));
253-
memcpy(res, keys.data(), keys_size * sizeof(StringRef));
254-
255-
return reinterpret_cast<StringRef *>(res);
256-
}
257-
258-
/*
259-
/// Copy keys to the pool. Then put into pool StringRefs to them and return the pointer to the first.
260-
static inline StringRef * ALWAYS_INLINE extractKeysAndPlaceInPool(
261-
size_t i,
262-
size_t keys_size,
263-
const ColumnRawPtrs & key_columns,
264-
StringRefs & keys,
265-
Arena & pool)
266-
{
267-
for (size_t j = 0; j < keys_size; ++j)
268-
{
269-
keys[j] = key_columns[j]->getDataAtWithTerminatingZero(i);
270-
char * place = pool.alloc(keys[j].size);
271-
memcpy(place, keys[j].data, keys[j].size);
272-
keys[j].data = place;
273-
}
274-
275-
/// Place the StringRefs on the newly copied keys in the pool.
276-
char * res = pool.alloc(keys_size * sizeof(StringRef));
277-
memcpy(res, keys.data(), keys_size * sizeof(StringRef));
278-
279-
return reinterpret_cast<StringRef *>(res);
280-
}
281-
282-
283-
/// Copy the specified keys to a continuous memory chunk of a pool.
284-
/// Subsequently append StringRef objects referring to each key.
285-
///
286-
/// [key1][key2]...[keyN][ref1][ref2]...[refN]
287-
/// ^ ^ : | |
288-
/// +-----|--------:-----+ |
289-
/// : +--------:-----------+
290-
/// : :
291-
/// <-------------->
292-
/// (1)
293-
///
294-
/// Return a StringRef object, referring to the area (1) of the memory
295-
/// chunk that contains the keys. In other words, we ignore their StringRefs.
296-
inline StringRef ALWAYS_INLINE extractKeysAndPlaceInPoolContiguous(
297-
size_t i,
298-
size_t keys_size,
299-
const ColumnRawPtrs & key_columns,
300-
StringRefs & keys,
301-
const TiDB::TiDBCollators & collators,
302-
std::vector<std::string> & sort_key_containers,
303-
Arena & pool)
304-
{
305-
size_t sum_keys_size = 0;
306-
for (size_t j = 0; j < keys_size; ++j)
307-
{
308-
keys[j] = key_columns[j]->getDataAtWithTerminatingZero(i);
309-
if (!collators.empty() && collators[j] != nullptr)
310-
{
311-
// todo check if need to handle the terminating zero
312-
keys[j] = collators[j]->sortKey(keys[j].data, keys[j].size - 1, sort_key_containers[j]);
313-
}
314-
sum_keys_size += keys[j].size;
315-
}
316-
317-
char * res = pool.alloc(sum_keys_size + keys_size * sizeof(StringRef));
318-
char * place = res;
319-
320-
for (size_t j = 0; j < keys_size; ++j)
321-
{
322-
memcpy(place, keys[j].data, keys[j].size);
323-
keys[j].data = place;
324-
place += keys[j].size;
325-
}
326-
327-
/// Place the StringRefs on the newly copied keys in the pool.
328-
memcpy(place, keys.data(), keys_size * sizeof(StringRef));
329-
330-
return {res, sum_keys_size};
331-
}
332-
*/
333-
334240
/** Serialize keys into a continuous chunk of memory.
335241
*/
336242
static inline StringRef ALWAYS_INLINE serializeKeysToPoolContiguous(

dbms/src/Interpreters/JoinPartition.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ void RowsNotInsertToMap::insertRow(Block * stored_block, size_t index, bool need
8383
}
8484
else
8585
{
86-
auto * elem = reinterpret_cast<RowRefList *>(pool.arena.alloc(sizeof(RowRefList)));
86+
auto * elem = reinterpret_cast<RowRefList *>(pool.arena.alignedAlloc(sizeof(RowRefList), alignof(RowRefList)));
8787
new (elem) RowRefList(stored_block, index);
8888
/// don't need cache column since it will explicitly materialize of need_materialize is true
8989
insertRowToList(pool, &head, elem, 0);
@@ -514,7 +514,8 @@ struct Inserter<ASTTableJoin::Strictness::All, Map, KeyGetter>
514514
* We will insert each time the element into the second place.
515515
* That is, the former second element, if it was, will be the third, and so on.
516516
*/
517-
auto elem = reinterpret_cast<MappedType *>(pool.arena.alloc(sizeof(MappedType)));
517+
auto elem
518+
= reinterpret_cast<MappedType *>(pool.arena.alignedAlloc(sizeof(MappedType), alignof(MappedType)));
518519
new (elem) typename Map::mapped_type(stored_block, i);
519520
insertRowToList(pool, &emplace_result.getMapped(), elem, cache_column_threshold);
520521
}

dbms/src/Interpreters/SpecializedAggregator.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,8 @@ void NO_INLINE Aggregator::executeSpecializedCase(
226226

227227
method.onNewKey(*it, params.keys_size, keys, *aggregates_pool);
228228

229-
AggregateDataPtr place = aggregates_pool->alloc(total_size_of_aggregate_states);
229+
AggregateDataPtr place
230+
= aggregates_pool->alignedAlloc(total_size_of_aggregate_states, align_aggregate_states);
230231

231232
AggregateFunctionsList::forEach(
232233
AggregateFunctionsCreator(aggregate_functions, offsets_of_aggregate_states, place));

0 commit comments

Comments
 (0)