Inplace new-ing an array overwrites the square of the memory

[AlisdairM]

Bugzilla Link	42096
Version	8.0
OS	Linux
CC	@dwblaikie,@DougGregor,@jfbastien,@riccibruno,@zygoloid

Extended Description

When in-place new-ing a local variable of an array of trivial type, the generated code calls memset with the square of the size of the array, corrupting the stack.

Quick example:

#include <new>

template <typename TYPE>
void f()
{
    typedef TYPE TArray[7];

    TArray x;
    new(&x) TArray();
}

int main()
{
    f<char>();
    f<int>();
}

Sample code generation can be seen for Clang 7 and 8 via godbolt:
https://godbolt.org/z/WjhFrc

[dwblaikie]

Neat! I'll take a look.

[dwblaikie]

Seems to only happen in the template case, not if you inline it into a non-template situation. Huh.

[dwblaikie]

Talked to Richard Smith & summarizing that discussion:

In general Clang currently strips the first array bound of an array new to handle it differently (because it can be a runtime bound, etc) - but in a non-dependent case then adjusts the type of the new to the nested type (removing that outer bound). But in a dependent case it does not perform that type adjustment - so later on uses both the size of the outer bound that was split off, plus the size of the un-adjusted array, that includes that outer bound.

ActOnCXXNew is where all new expressions are handled, and it does the top-bound stripping off for non-dependent types.
CreateCXXNew is where it gets weird & the dependent type case gets the bound extracted but the type is not adjusted.

General thought: Representationally it'd probably be better not to "adjust" the type - extract the top bound for runtime bound/other reasons, but keep the original type (with possible runtime bound). Then when computing the size, always strip off the top bound and multiply it by the extracted bound.

[AlisdairM]

Updated godbolt link showing this issue is still present on trunk, 2020/12/23:
https://godbolt.org/z/zre3j4

[mahtohappy]

Hi, I added a fix for this, but couldn't link it somehow, now sure why. #83124

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: None (e84ee981-20b7-441e-8de0-adb2190144d6)

| | | | --- | --- | | Bugzilla Link | [42096](https://llvm.org/bz42096) | | Version | 8.0 | | OS | Linux | | CC | @dwblaikie,@DougGregor,@jfbastien,@riccibruno,@zygoloid |

Extended Description

When in-place new-ing a local variable of an array of trivial type, the generated code calls 'memset' with the square of the size of the array, corrupting the stack.

Quick example:

#include <new>

template <typename TYPE>
void f()
{
typedef TYPE TArray[7];

TArray x;
new(&x) TArray();

}

int main()
{
f<char>();
f<int>();
}

Sample code generation can be seen for Clang 7 and 8 via godbolt:
https://godbolt.org/z/WjhFrc

[awson]

The "fix" has been reverted, the problem is still with us.

CreateCXXNew is where it gets weird & the dependent type case gets the bound extracted but the type is not adjusted

@dwblaikie, I can't find CreateCXXNew anywhere in the code (even after preprocessing), do you mean BuildCXXNew?

[dwblaikie]

The "fix" has been reverted, the problem is still with us.

CreateCXXNew is where it gets weird & the dependent type case gets the bound extracted but the type is not adjusted

@dwblaikie, I can't find CreateCXXNew anywhere in the code (even after preprocessing), do you mean BuildCXXNew?

Yeah, been a while - but at a cursory glance that correction sounds right.

[dwblaikie]

(patch was reverted in e1321fa)

[awson]

@djtodoro, how is this "completed"?

I see neither my PR (#96464) is merged, nor any newer one is referenced in this issue.

[djtodoro]

@djtodoro, how is this "completed"?

I see neither my PR (#96464) is merged, nor any newer one is referenced in this issue.

I have no idea what happened here. Never touched or visited this issue.