RE-UE4SS icon indicating copy to clipboard operation
RE-UE4SS copied to clipboard

[Investigation - Release] FName Alignment

Open bitonality opened this issue 11 months ago • 5 comments

I think we should spend some time investigating what breaks when we align UE4SS FNames as 4 instead of 8.

// NameTypes.hpp
    // TODO:   Figure out what's going on here
    //         It shouldn't be required to use 'alignas' here to make sure it's aligned properly in containers (like TArray)
    //         I've never seen an FName not be 8-byte aligned in memory,
    //         but it is 4-byte aligned in the source so hopefully this doesn't cause any problems
    // UPDATE: This matters in the UE VM, when ElementSize is 0xC in memory for case-preserving games, it must be aligned by 0x4 in that case
#pragma warning(disable: 4324) // Suppressing warning about struct alignment
#ifdef WITH_CASE_PRESERVING_NAME
    struct alignas(4) RC_UE_API FName
#else
    struct alignas(8) RC_UE_API FName
#endif

The alignas(8) causes issues for structs like the following

    struct FPalItemRecipe : public FTableRowBase
    {
        FName Product_Id;                                                                 // 0x0008 (size: 0x8)
        int32 Product_Count;                                                              // 0x0010 (size: 0x4)
        float WorkAmount;                                                                 // 0x0014 (size: 0x4)
        int32 WorkableAttribute;                                                        // 0x0018 (size: 0x4)
        FName UnlockItemID;                                                             // 0x001C (size: 0x8)
        FName Material1_Id;                                                               // 0x0024 (size: 0x8)
     }

The struct is aligned in memory with alignas(4) for the FName members (UE FNames are alignas(4)'ed), but with the current UE4SS alignas(8), we lose the ability to do stuff like

auto val = reinterpret_cast<FPalItemRecipe*>(pair->Value);

And instead have to resort to manually incrementing and reading the bytes.

  auto valAddr = reinterpret_cast<std::uintptr_t>(pair->Value);
  auto val = FPalItemRecipe{};

  auto productId = reinterpret_cast<FName*>(valAddr);
  val.Product_Id = *productId;
  valAddr += 8;

  auto productCount = reinterpret_cast<int32_t*>(valAddr);
  val.Product_Count = *productCount;
  valAddr += 4;

  auto workAmount = reinterpret_cast<float*>(valAddr);
  val.WorkAmount = *workAmount;
  valAddr += 4;

I'm curious if anyone still has GitLab access that could look at a commit history/blame for NameTypes.hpp to see if they could deduce anything about what breaks when FNames are alignas(4)'ed. The current GitHub commit history only has the initial commit for the alignas lines, so I don't have much of a lead to go off of.

This investigation/resolution is currently blocking the UDataTable work. I think this specific FName alignment issue deserves its own commit history/discussion threads since the scope extends far beyond just the UDataTable additions.

bitonality avatar Feb 29 '24 02:02 bitonality

The gitlab commit wasn't very helpful. Message:

FName: Forcing FName to be 8-byte aligned to fix some problems.
Hopefully this doesn't cause other problems.

Diff:

    // TODO: Figure out what's going on here
    //       It shouldn't be required to use 'alignas' here to make sure it's aligned properly in containers (like TArray)
-   struct /*alignas(8)*/ RC_UE_API FName
+   //       I've never seen an FName not be 8-byte aligned in memory,
+   //       but it is 4-byte aligned in the source so hopefully this doesn't cause any problems
+   struct alignas(8) RC_UE_API FName
    {
    public:
        enum class EFindName

The commit that added the commented out alignas is just as useless. All we have to go on is the comment, particularly this: It shouldn't be required to use 'alignas' here to make sure it's aligned properly in containers (like TArray) So presumably, there were alignment problems with FNames in TArray. The UEnum names array might be a good place to test this, and you might need to test both the 4.14 version (TArray<FName, uint8>) and the 4.15+ version (TArray<FName, int64>), I think that might be where the problem was but it's difficult to recall.

UE4SS avatar Feb 29 '24 03:02 UE4SS

https://github.com/EpicGames/UnrealEngine/blob/6957dd274b9408cd6dc08e761eed4880e663e68a/Engine/Source/Runtime/Core/Public/Containers/Map.h#L45

So it looks like TPair is a legitimate class in UE 4.14. I replicated the functionality locally, but it still doesn't get what we need.

Doing some digging,

UE 4.20 inline TArray<TTuple<FName, int64>>
9e 5b 00 00 00 00 00 00  Enum FName
00 00 00 00 00 00 00 00  Enum Value
9f 5b 00 00 00 00 00 00  
01 00 00 00 00 00 00 00  
a0 5b 00 00 00 00 00 00  
02 00 00 00 00 00 00 00  
a1 5b 00 00 00 00 00 00  
03 00 00 00 00 00 00 00  
a3 5b 00 00 00 00 00 00  
04 00 00 00 00 00 00 00  
UE 4.14 inline TArray<TPair<FName, uint8>>
UE 4.14
84 31 00 00 00 00 00 00  Enum FName
00 00 b4 b3 12 02 00 00  First byte is Enum value, other 7 bytes are???
85 31 00 00 00 00 00 00  
01 00 b4 b3 12 02 00 00  
86 31 00 00 00 00 00 00  
02 00 b4 b3 12 02 00 00  
87 31 00 00 00 00 00 00  
03 00 b4 b3 12 02 00 00  
88 31 00 00 00 00 00 00  
04 00 b4 b3 12 02 00 00  

When FName is aligned to 8, we get a happy accident behavior in 4.14 where we just ignore those 7 unknown bytes... When aligning FName to 4, each TPair<FName, uint8> is seen as a 12 byte block and so the alignment of future inline elements gets offset and causes a failure.

bitonality avatar Mar 11 '24 05:03 bitonality

Pre 4.22 FName

/** 
 * Do we want to support case-variants for FName?
 * This will add an extra NAME_INunrealDEX variable to FName, but means that ToString() will return you the exact same 
 * string that FName::Init was called with (which is useful if your FNames are shown to the end user)
 * Currently this is enabled for the Editor and any Programs (such as UHT), but not the Runtime
 */
#ifndef WITH_CASE_PRESERVING_NAME
	#define WITH_CASE_PRESERVING_NAME WITH_EDITORONLY_DATA
#endif

class FName {
	union
	{
		struct
		{
			/** Index into the Names array (used to find String portion of the string/number pair used for comparison) */
			NAME_INDEX		ComparisonIndex;
		#if WITH_CASE_PRESERVING_NAME
			/** Index into the Names array (used to find String portion of the string/number pair used for display) */
			NAME_INDEX		DisplayIndex;
		#endif
			/** Number portion of the string/number pair (stored internally as 1 more than actual, so zero'd memory will be the default, no-instance case) */
			uint32			Number;
		};

		// Used to perform a single comparison in FName::operator==
		#if !WITH_CASE_PRESERVING_NAME
			uint64 CompositeComparisonValue;
		#endif
	};
}

4.22+ FName

class FName {
private:

    /** Index into the Names array (used to find String portion of the string/number pair used for comparison) */
    FNameEntryId    ComparisonIndex;
#if WITH_CASE_PRESERVING_NAME
    /** Index into the Names array (used to find String portion of the string/number pair used for display) */
    FNameEntryId    DisplayIndex;
#endif // WITH_CASE_PRESERVING_NAME
    /** Number portion of the string/number pair (stored internally as 1 more than actual, so zero'd memory will be the default, no-instance case) */
    uint32            Number;
}

To maintain compatibility, we need to align FName as follows: Any Case Preserving Version: alignas(4) UE < 4.22: alignas(8) UE >= 4.22: alignas(4)

bitonality avatar Mar 12 '24 06:03 bitonality

FName Alignment Writeup

The problem

You will sometimes have unexpected results when using the FName type in UE4SS. This is due to an alignment change that occurred in UE 4.22 that is currently not accounted for in UE4SS at the time of writing. Caution should be taken when using FName in the following ways/contexts:

  • TArray<FName>
  • TArray<SomeContainerWithFName>
  • struct with FName member
  • SomeUnrealContainers<FName>

Detailed explanation

In UE versions before 4.22, FName had an alignment of 8. This is due to FName's alignment being the largest alignment of the unioned members.

/** 
 * Do we want to support case-variants for FName?
 * This will add an extra NAME_INunrealDEX variable to FName, but means that ToString() will return you the exact same 
 * string that FName::Init was called with (which is useful if your FNames are shown to the end user)
 * Currently this is enabled for the Editor and any Programs (such as UHT), but not the Runtime
 */
#ifndef WITH_CASE_PRESERVING_NAME
	#define WITH_CASE_PRESERVING_NAME WITH_EDITORONLY_DATA
#endif

class FName {
	union
	{
		struct
		{
			/** Index into the Names array (used to find String portion of the string/number pair used for comparison) */
			NAME_INDEX		ComparisonIndex;
		#if WITH_CASE_PRESERVING_NAME
			/** Index into the Names array (used to find String portion of the string/number pair used for display) */
			NAME_INDEX		DisplayIndex;
		#endif
			/** Number portion of the string/number pair (stored internally as 1 more than actual, so zero'd memory will be the default, no-instance case) */
			uint32			Number;
		};

		// Used to perform a single comparison in FName::operator==
		#if !WITH_CASE_PRESERVING_NAME
			uint64 CompositeComparisonValue; // uint64 aligns as 8, so the union is 8 aligned.
		#endif
	};
}

In UE versions 4.22+, the FName alignment changed to 4 due to some refactoring of the FName members.

class FName {
    private:

        /** Index into the Names array (used to find String portion of the string/number pair used for comparison) */
        FNameEntryId    ComparisonIndex;
    #if WITH_CASE_PRESERVING_NAME
        /** Index into the Names array (used to find String portion of the string/number pair used for display) */
        FNameEntryId    DisplayIndex;
    #endif // WITH_CASE_PRESERVING_NAME
        /** Number portion of the string/number pair (stored internally as 1 more than actual, so zero'd memory will be the default, no-instance case) */
        uint32            Number;
    }

Case Preserving?

In versions before UE 4.22, a case-preserving FName is aligned as 4 and has size 12. In UE versions 4.22+, case-preserving FName is also aligned as 4 and has size 12.

Real Example

struct MyCoolStruct {
    FName Name;
    uint8 SomeNumber;
    FName AnotherName;
}

// When FName alignas(8). Total size = 24 bytes.
struct MyCoolStruct {
    FName Name; // 8 bytes
    uint8 SomeNumber; // 1 byte
    char gap_7[7]; // 7 bytes
    FName AnotherName; // 8 bytes
}

// XX XX XX XX XX XX XX XX
// XX cc cc cc cc cc cc cc
// XX XX XX XX XX XX XX XX

// When FName alignas(4). Total size = 20 bytes.
struct MyCoolStruct {
    FName Name; // 8 bytes
    uint8 SomeNumber; // 1 byte
    char gap_3[3]; // 3 bytes
    FName AnotherName; // 8 bytes
}

// XX XX XX XX XX XX XX XX
// XX cc cc cc XX XX XX XX
// XX XX XX XX

Observe the following behavior:

static auto Offset = MemberOffsets.find(STR("MyArrayOfCoolStructs"));
return *Helper::Casting::ptr_cast<TArray<MyCoolStruct>*>(this, Offset->second);

Let's say that the pointer to the TArray data pointed to something like this...

UE Memory with FName aligned as 4

FName Name:
ed cc 31 3f 
fe a1 09 7d
uint8 SomeNumber:
b1 cc cc cc
FName AnotherName:
e0 bf 9c 55
5a 10 9e 4f

When FName is alignas(8) in UE4SS, then our MyCoolStruct would look like the following...

FName Name:
ed cc 31 3f fe a1 09 7d
uint8 SomeNumber:
b1 cc cc cc e0 bf 9c 55
FName AnotherName:
5a 10 9e 4f ?? ?? ?? ??

Hopefully the issue is somewhat obvious...

bitonality avatar Mar 13 '24 07:03 bitonality

Afaik, need to implement either one of these libs in ue4ss to solve this issue: https://github.com/localcc/DynamicType https://github.com/Archengius/DynamicTypeLib

Buckminsterfullerene02 avatar Apr 06 '24 11:04 Buckminsterfullerene02