Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Fix issue 19128 - argument to alloca may be too large #2258

Closed
belka-ew wants to merge 1 commit into from
Closed

Fix issue 19128 - argument to alloca may be too large #2258

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions src/rt/arrayassign.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,25 @@ extern (C) void[] _d_arrayassign(TypeInfo ti, void[] from, void[] to)

// Need a temporary buffer tmp[] big enough to hold one element
void[16] buf = void;
void* ptmp = (elementSize > buf.sizeof) ? alloca(elementSize) : buf.ptr;
return _d_arrayassign_l(ti, from, to, ptmp);
void* ptmp;
if (elementSize <= buf.sizeof)
{
ptmp = buf.ptr;
Copy link

@iain-buclaw-sociomantic iain-buclaw-sociomantic Jul 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue here that if we are going to fall back to malloc. Then having a pre-allocated buffer is pointless.

This should be:

if (elementSize <= maxAllocaSize)
  ptmp = alloca(elementSize);
else
  ptmp = malloc(elementSize);

Copy link

@iain-buclaw-sociomantic iain-buclaw-sociomantic Jul 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or at the very least, this should just be a simple s/alloca/malloc/ swap.

void* ptmp = (elementSize > buf.sizeof) ? malloc(elementSize) : buf.ptr;

// Later...
if (ptmp != buf.ptr)
  free(ptmp);

}
else if (elementSize < 1024)
Copy link
Contributor

@JinShil JinShil Aug 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How was 1024 chosen?

Copy link
Contributor Author

@belka-ew belka-ew Aug 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty random. Should be enough for a lot of data types, and not too much.

{
ptmp = alloca(elementSize);
}
else
{
ptmp = malloc(elementSize);
}
auto dst = _d_arrayassign_l(ti, from, to, ptmp);
if (elementSize >= 1024)
{
free(ptmp);
}
return dst;
}

/**
Expand Down Expand Up @@ -209,12 +226,18 @@ extern (C) void* _d_arraysetassign(void* p, void* value, int count, TypeInfo ti)
//Need a temporary buffer tmp[] big enough to hold one element
void[16] buf = void;
void[] tmp;
if (element_size > buf.sizeof)
if (element_size <= buf.sizeof)
{
tmp = buf[];
}
else if (element_size < 1024)
{
tmp = alloca(element_size)[0 .. element_size];
}
else
tmp = buf[];
{
tmp = malloc(element_size)[0 .. element_size];
}

foreach (i; 0 .. count)
{
Expand All @@ -224,6 +247,10 @@ extern (C) void* _d_arraysetassign(void* p, void* value, int count, TypeInfo ti)
ti.destroy(tmp.ptr);
p += element_size;
}
if (element_size >= 1024)
{
free(tmp.ptr);
}
return pstart;
}

Expand Down