Use of alloca to allocate storage for string manipulation is extremely unsafe

[Kaldaien]

I do not use the engine, but came across this function in a random GitHub search and was horrified:

        inline void set_object_name(ID3D12Object* object, const c8* name)
        {
            usize len = utf8_to_utf16_len(name);
            wchar_t* buf = (wchar_t*)alloca(sizeof(wchar_t) * (len + 1));
            utf8_to_utf16((c16*)buf, len + 1, name);
            object->SetName(buf);
        }

https://github.com/JX-Master/LunaGB/blob/93b16e99606b00b1cfbf638d0a3173ed32d315a2/Modules/Luna/RHI/Source/D3D12/D3D12Common.hpp#L241-L247

alloca has a serious flaw: it cannot detect stack overflow and failure behavior is undefined.

If a stack overflow does occur, and the chances of that are extremely high for a variable-length string (especially given the 2x storage size required for utf16), results are undefined. You can expect random crashes with a corrupt callstack to make debugging the problem even harder.

Microsoft has a non-standard implementation of alloca (https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/alloca?view=msvc-170) that can detect and even recover from stack overflow using Structured Exception Handling.

Frankly I would avoid even bothering with that. Just go ahead and allocate heap memory for strings, given they can have completely arbitrary length.

There are a number of other places using alloca in the codebase too following a quick search. They are less likely to overflow, but when they do overflow it will be just as difficult to debug. I would suggest using a safer approach even if that means writing a custom stack allocator wrapping platform-specific code to detect stack overflow. It is critical to use heap allocation if the stack cannot accommodate your allocation without overflow and you cannot grow the stack manually.

[JX-Master]

Hi Kaldaien, you are right. The reason I use alloca to allocate string buffer is that the debug string is relatively short in most cases (probably shorter than 128 bytes), so allocating such buffer on stack can avoid bothering the heap. But since the string length is (theoretically) uncertain, we definitively need to check the required buffer size and use different allocation policies for different size. I will review all alloca calls and see if they have potential overflow risk.

[JX-Master]

I implemented a new custom stack allocator to replace the original alloca functionality. The stack allocator works like this:

1. The user calls begin_stack_alloc_scope to open a new stack allocation scope, then calls stack_alloc to allocate stack memory. stack_alloc is compatible to alloca so that all original alloca calls can be replaced with no extra work.
2. The scope opened by begin_stack_alloc_scope should be explicitly closed by end_stack_alloc_scope, which frees all memory allocated after the nearest unclosed begin_stack_alloc_scope. The allocation scope can be nested by another allocation scope.
3. The user can use StackAllocator type to automatically open / close scopes using RAII.
4. The memory allocated from stack_alloc comes from a 4MB-sized thread local memory block that is allocated per thread when begin_stack_alloc_scope is called for the first time on that thread. Since the allocation is stack-based, allocating/deallocating memory from the block can be done by simply shifting the allocation pointer forward/backward. Currently the memory block never grows, stack_alloc returns nullptr when the stack overflows. It is the caller's responsibility to handle such overflow and use heap allocation instead when necessary.