Skip to content

Commit 59a42e6

Browse files
committed
Fix crash by using alignedAlloc rather than alloc
1 parent d187d42 commit 59a42e6

6 files changed

Lines changed: 19 additions & 113 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
@@ -297,7 +297,7 @@ void ColumnAggregateFunction::insert(const Field & x)
297297

298298
Arena & arena = createOrGetArena();
299299

300-
getData().push_back(arena.alloc(function->sizeOfData()));
300+
getData().push_back(arena.alignedAlloc(function->sizeOfData(), function->alignOfData()));
301301
function->create(getData().back());
302302
ReadBufferFromString read_buffer(x.get<const String &>());
303303
function->deserialize(getData().back(), read_buffer, &arena);
@@ -309,7 +309,7 @@ void ColumnAggregateFunction::insertDefault()
309309

310310
Arena & arena = createOrGetArena();
311311

312-
getData().push_back(arena.alloc(function->sizeOfData()));
312+
getData().push_back(arena.alignedAlloc(function->sizeOfData(), function->alignOfData()));
313313
function->create(getData().back());
314314
}
315315

@@ -337,7 +337,7 @@ const char * ColumnAggregateFunction::deserializeAndInsertFromArena(
337337
*/
338338
Arena & dst_arena = createOrGetArena();
339339

340-
getData().push_back(dst_arena.alloc(function->sizeOfData()));
340+
getData().push_back(dst_arena.alignedAlloc(function->sizeOfData(), function->alignOfData()));
341341
function->create(getData().back());
342342

343343
/** We will read from src_arena.

dbms/src/DataTypes/DataTypeAggregateFunction.cpp

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ std::string DataTypeAggregateFunction::getName() const
6363

6464
void DataTypeAggregateFunction::serializeBinary(const Field & field, WriteBuffer & ostr) const
6565
{
66-
const String & s = get<const String &>(field);
66+
const auto & s = get<const String &>(field);
6767
writeVarUInt(s.size(), ostr);
6868
writeString(s, ostr);
6969
}
@@ -73,7 +73,7 @@ void DataTypeAggregateFunction::deserializeBinary(Field & field, ReadBuffer & is
7373
UInt64 size;
7474
readVarUInt(size, istr);
7575
field = String();
76-
String & s = get<String &>(field);
76+
auto & s = get<String &>(field);
7777
s.resize(size);
7878
istr.readStrict(&s[0], size);
7979
}
@@ -85,11 +85,11 @@ void DataTypeAggregateFunction::serializeBinary(const IColumn & column, size_t r
8585

8686
void DataTypeAggregateFunction::deserializeBinary(IColumn & column, ReadBuffer & istr) const
8787
{
88-
ColumnAggregateFunction & column_concrete = static_cast<ColumnAggregateFunction &>(column);
88+
auto & column_concrete = static_cast<ColumnAggregateFunction &>(column);
8989

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

9494
function->create(place);
9595
try
@@ -143,8 +143,7 @@ void DataTypeAggregateFunction::deserializeBinaryBulk(
143143
{
144144
if (istr.eof())
145145
break;
146-
147-
AggregateDataPtr place = arena.alloc(size_of_state);
146+
AggregateDataPtr place = arena.alignedAlloc(size_of_state, function->alignOfData());
148147

149148
function->create(place);
150149

@@ -171,11 +170,11 @@ static String serializeToString(const AggregateFunctionPtr & function, const ICo
171170

172171
static void deserializeFromString(const AggregateFunctionPtr & function, IColumn & column, const String & s)
173172
{
174-
ColumnAggregateFunction & column_concrete = static_cast<ColumnAggregateFunction &>(column);
173+
auto & column_concrete = static_cast<ColumnAggregateFunction &>(column);
175174

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

180179
function->create(place);
181180

@@ -317,7 +316,7 @@ static DataTypePtr create(const ASTPtr & arguments)
317316
"name of aggregate function and list of data types for arguments",
318317
ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH);
319318

320-
if (const ASTFunction * parametric = typeid_cast<const ASTFunction *>(arguments->children[0].get()))
319+
if (const auto * parametric = typeid_cast<const ASTFunction *>(arguments->children[0].get()))
321320
{
322321
if (parametric->parameters)
323322
throw Exception("Unexpected level of parameters to aggregate function", ErrorCodes::SYNTAX_ERROR);
@@ -328,7 +327,7 @@ static DataTypePtr create(const ASTPtr & arguments)
328327

329328
for (size_t i = 0; i < parameters.size(); ++i)
330329
{
331-
const ASTLiteral * lit = typeid_cast<const ASTLiteral *>(parameters[i].get());
330+
const auto * lit = typeid_cast<const ASTLiteral *>(parameters[i].get());
332331
if (!lit)
333332
throw Exception(
334333
"Parameters to aggregate functions must be literals",

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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ void RowsNotInsertToMap::insertRow(Block * stored_block, size_t index, bool need
6666
}
6767
else
6868
{
69-
auto * elem = reinterpret_cast<RowRefList *>(pool.alloc(sizeof(RowRefList)));
69+
auto * elem = reinterpret_cast<RowRefList *>(pool.alignedAlloc(sizeof(RowRefList), alignof(RowRefList)));
7070
new (elem) RowRefList(stored_block, index);
7171
insertRowToList(&head, elem);
7272
}
@@ -496,7 +496,7 @@ struct Inserter<ASTTableJoin::Strictness::All, Map, KeyGetter>
496496
* We will insert each time the element into the second place.
497497
* That is, the former second element, if it was, will be the third, and so on.
498498
*/
499-
auto elem = reinterpret_cast<MappedType *>(pool.alloc(sizeof(MappedType)));
499+
auto elem = reinterpret_cast<MappedType *>(pool.alignedAlloc(sizeof(MappedType), alignof(MappedType)));
500500
new (elem) typename Map::mapped_type(stored_block, i);
501501
insertRowToList(&emplace_result.getMapped(), elem);
502502
}

dbms/src/Interpreters/SpecializedAggregator.h

Lines changed: 2 additions & 1 deletion
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));

0 commit comments

Comments
 (0)