Skip to content

Commit e3fe665

Browse files
Fix crash by using alignedAlloc rather than alloc (#10554) (#10585)
close #10553 need to use alignedAlloc for Complex Type Co-authored-by: ChangRui-Ryan <changrui82@gmail.com>
1 parent b50ff41 commit e3fe665

6 files changed

Lines changed: 15 additions & 108 deletions

File tree

dbms/src/AggregateFunctions/AggregateFunctionGroupArray.h

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

185-
Node * node = reinterpret_cast<Node *>(arena->alloc(sizeof(Node) + size));
185+
Node * node = reinterpret_cast<Node *>(arena->alignedAlloc(sizeof(Node) + size, alignof(Node)));
186186
node->size = size;
187187
buf.read(node->data(), size);
188188
return node;
@@ -198,7 +198,7 @@ struct GroupArrayListNodeString : public GroupArrayListNodeBase<GroupArrayListNo
198198
{
199199
StringRef string = static_cast<const ColumnString &>(column).getDataAt(row_num);
200200

201-
Node * node = reinterpret_cast<Node *>(arena->alloc(sizeof(Node) + string.size));
201+
Node * node = reinterpret_cast<Node *>(arena->alignedAlloc(sizeof(Node) + string.size, alignof(Node)));
202202
node->next = nullptr;
203203
node->size = string.size;
204204
memcpy(node->data(), string.data, string.size);
@@ -215,7 +215,7 @@ struct GroupArrayListNodeGeneral : public GroupArrayListNodeBase<GroupArrayListN
215215

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

221221
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
@@ -324,7 +324,7 @@ void ColumnAggregateFunction::insert(const Field & x)
324324

325325
Arena & arena = createOrGetArena();
326326

327-
getData().push_back(arena.alloc(function->sizeOfData()));
327+
getData().push_back(arena.alignedAlloc(function->sizeOfData(), function->alignOfData()));
328328
function->create(getData().back());
329329
ReadBufferFromString read_buffer(x.get<const String &>());
330330
function->deserialize(getData().back(), read_buffer, &arena);
@@ -336,7 +336,7 @@ void ColumnAggregateFunction::insertDefault()
336336

337337
Arena & arena = createOrGetArena();
338338

339-
getData().push_back(arena.alloc(function->sizeOfData()));
339+
getData().push_back(arena.alignedAlloc(function->sizeOfData(), function->alignOfData()));
340340
function->create(getData().back());
341341
}
342342

@@ -364,7 +364,7 @@ const char * ColumnAggregateFunction::deserializeAndInsertFromArena(
364364
*/
365365
Arena & dst_arena = createOrGetArena();
366366

367-
getData().push_back(dst_arena.alloc(function->sizeOfData()));
367+
getData().push_back(dst_arena.alignedAlloc(function->sizeOfData(), function->alignOfData()));
368368
function->create(getData().back());
369369

370370
/** 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
@@ -90,7 +90,7 @@ void DataTypeAggregateFunction::deserializeBinary(IColumn & column, ReadBuffer &
9090

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

9595
function->create(place);
9696
try
@@ -144,8 +144,7 @@ void DataTypeAggregateFunction::deserializeBinaryBulk(
144144
{
145145
if (istr.eof())
146146
break;
147-
148-
AggregateDataPtr place = arena.alloc(size_of_state);
147+
AggregateDataPtr place = arena.alignedAlloc(size_of_state, function->alignOfData());
149148

150149
function->create(place);
151150

@@ -176,7 +175,7 @@ static void deserializeFromString(const AggregateFunctionPtr & function, IColumn
176175

177176
Arena & arena = column_concrete.createOrGetArena();
178177
size_t size_of_state = function->sizeOfData();
179-
AggregateDataPtr place = arena.alloc(size_of_state);
178+
AggregateDataPtr place = arena.alignedAlloc(size_of_state, function->alignOfData());
180179

181180
function->create(place);
182181

dbms/src/Interpreters/AggregationCommon.h

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

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

225225
method.onNewKey(*it, params.keys_size, keys, *aggregates_pool);
226226

227-
AggregateDataPtr place = aggregates_pool->alloc(total_size_of_aggregate_states);
227+
AggregateDataPtr place
228+
= aggregates_pool->alignedAlloc(total_size_of_aggregate_states, align_aggregate_states);
228229

229230
AggregateFunctionsList::forEach(
230231
AggregateFunctionsCreator(aggregate_functions, offsets_of_aggregate_states, place));
@@ -304,4 +305,4 @@ void NO_INLINE Aggregator::executeSpecializedWithoutKey(
304305
extern "C" void __attribute__((__visibility__("default"), __noreturn__)) __cxa_pure_virtual()
305306
{
306307
abort();
307-
};
308+
}

0 commit comments

Comments
 (0)