[AMDGPU] Fix leak and self-assignment in copy assignment operator (#107847)

A static analyzer identified that this operator was unsafe in the case
of self-assignment.

In the placement new statement, StringValue's copy constructor was being
implicitly called, which received a reference to "itself". In fact, it
was being passed an old StringValue at the same address - one whose
lifetime had already ended. The copy constructor was thus copying fields
from a dead object.

We need to be careful when switching active union members, and calling
the destructor on the old StringValue will avoid memory leaks which I
believe the old code exhibited.
This commit is contained in:
Fraser Cormack 2024-09-11 10:23:41 +01:00 committed by GitHub
parent a4b0153c4f
commit f4dd1bc8fc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -100,19 +100,25 @@ struct SIArgument {
SIArgument() : IsRegister(false), StackOffset(0) {}
SIArgument(const SIArgument &Other) {
IsRegister = Other.IsRegister;
if (IsRegister) {
::new ((void *)std::addressof(RegisterName))
StringValue(Other.RegisterName);
} else
if (IsRegister)
new (&RegisterName) StringValue(Other.RegisterName);
else
StackOffset = Other.StackOffset;
Mask = Other.Mask;
}
SIArgument &operator=(const SIArgument &Other) {
// Default-construct or destruct the old RegisterName in case of switching
// union members
if (IsRegister != Other.IsRegister) {
if (Other.IsRegister)
new (&RegisterName) StringValue();
else
RegisterName.~StringValue();
}
IsRegister = Other.IsRegister;
if (IsRegister) {
::new ((void *)std::addressof(RegisterName))
StringValue(Other.RegisterName);
} else
if (IsRegister)
RegisterName = Other.RegisterName;
else
StackOffset = Other.StackOffset;
Mask = Other.Mask;
return *this;