commit 5c95af798ba41e5c8bb09891ee7398e7bfe2b1fb
author David Benjamin <davidben@google.com> Thu Oct 26 12:33:44 2023 -0400
committer swiftshader-scoped@luci-project-accounts.iam.gserviceaccount.com <swiftshader-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Oct 30 18:25:39 2023 +0000
tree f55ff940dc0e8a7584c9f4c37ffbc8674a721709
parent d9ec9befba05a8dfca09c1e88f3f7be0e4b153c6

Fix unaligned_ptr in swiftshader

For something used in three places, this type is probably overkill, but
ah well. Casting an unaligned pointer to uint32_t* is UB, even if you
don't dereference it. So this unaligned_ptr type isn't actually avoiding
UB. Keeping things as void* under the hood, to avoid this.

While I'm here, trim the API surface a bit, so this is a bit easier to
reason about.

Bug: chromium:1394755
Change-Id: I3a198c78051508c6b2ec806ce0ea6b4335cf4ebe
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/72608
Reviewed-by: Shahbaz Youssefi <syoussefi@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Shahbaz Youssefi <syoussefi@google.com>
Commit-Queue: Shahbaz Youssefi <syoussefi@google.com>

src/Reactor/ExecutableMemory.hpp

diff --git a/src/Reactor/ExecutableMemory.hpp b/src/Reactor/ExecutableMemory.hpp
index dbbe32a..2cd5bec 100644
--- a/src/Reactor/ExecutableMemory.hpp
+++ b/src/Reactor/ExecutableMemory.hpp
@@ -41,17 +41,16 @@
 void deallocateMemoryPages(void *memory, size_t bytes);
 
 template<typename P>
-P unaligned_read(P *address)
+P unaligned_read(void *address)
 {
 	P value;
 	memcpy(&value, address, sizeof(P));
 	return value;
 }
 
-template<typename P, typename V>
-void unaligned_write(P *address, V value)
+template<typename P>
+void unaligned_write(void *address, P value)
 {
-	static_assert(sizeof(V) == sizeof(P), "value size must match pointee size");
 	memcpy(address, &value, sizeof(P));
 }
 
@@ -60,33 +59,29 @@
 {
 public:
 	explicit unaligned_ref(void *ptr)
-	    : ptr((P *)ptr)
+	    : ptr(ptr)
 	{}
 
-	template<typename V>
-	P operator=(V value)
+	unaligned_ref &operator=(P value)
 	{
 		unaligned_write(ptr, value);
-		return value;
+		return *this;
 	}
 
 	operator P()
 	{
-		return unaligned_read((P *)ptr);
+		return unaligned_read<P>(ptr);
 	}
 
 private:
-	P *ptr;
+	void *ptr;
 };
 
 template<typename P>
 class unaligned_ptr
 {
-	template<typename S>
-	friend class unaligned_ptr;
-
 public:
-	unaligned_ptr(P *ptr)
+	unaligned_ptr(void *ptr)
 	    : ptr(ptr)
 	{}
 
@@ -95,10 +90,9 @@
 		return unaligned_ref<P>(ptr);
 	}
 
-	template<typename S>
-	operator S()
+	explicit operator intptr_t()
 	{
-		return S(ptr);
+		return (intptr_t)ptr;
 	}
 
 private:

src/Reactor/SubzeroReactor.cpp

diff --git a/src/Reactor/SubzeroReactor.cpp b/src/Reactor/SubzeroReactor.cpp
index 0a4e9d2..c0c6b1d 100644
--- a/src/Reactor/SubzeroReactor.cpp
+++ b/src/Reactor/SubzeroReactor.cpp
@@ -518,7 +518,7 @@
 	}
 
 	intptr_t address = (intptr_t)elfHeader + target->sh_offset;
-	unaligned_ptr<int32_t> patchSite = (int32_t *)(address + relocation.r_offset);
+	unaligned_ptr<int32_t> patchSite = (void *)(address + relocation.r_offset);
 
 	if(CPUID::ARM)
 	{
@@ -603,8 +603,8 @@
 	}
 
 	intptr_t address = (intptr_t)elfHeader + target->sh_offset;
-	unaligned_ptr<int32_t> patchSite32 = (int32_t *)(address + relocation.r_offset);
-	unaligned_ptr<int64_t> patchSite64 = (int64_t *)(address + relocation.r_offset);
+	unaligned_ptr<int32_t> patchSite32 = (void *)(address + relocation.r_offset);
+	unaligned_ptr<int64_t> patchSite64 = (void *)(address + relocation.r_offset);
 
 	switch(relocation.getType())
 	{