Commit 8d2cf752 by Nicolas Capens Committed by Nicolas Capens

Fix unaligned access undefined behavior.

Unaligned accesses are undefined behavior, but they're common in our ELF binary patching code for variable length instruction sets (namely x86). We can use memcpy() and rely on target-specific compiler optimizations to make it efficient. Utility functions and classes were added to aid readability. Bug b/119823623 Change-Id: I8a82672a0d18d1e1783f580eb629f8cc09a009cd Reviewed-on: https://swiftshader-review.googlesource.com/c/22828Tested-by: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarAlexis Hétu <sugoi@google.com>
parent 0e6a044d
...@@ -15,8 +15,9 @@ ...@@ -15,8 +15,9 @@
#ifndef rr_ExecutableMemory_hpp #ifndef rr_ExecutableMemory_hpp
#define rr_ExecutableMemory_hpp #define rr_ExecutableMemory_hpp
#include <stddef.h> #include <cstddef>
#include <stdint.h> #include <cstdint>
#include <cstring>
namespace rr namespace rr
{ {
...@@ -25,6 +26,67 @@ size_t memoryPageSize(); ...@@ -25,6 +26,67 @@ size_t memoryPageSize();
void *allocateExecutable(size_t bytes); // Allocates memory that can be made executable using markExecutable() void *allocateExecutable(size_t bytes); // Allocates memory that can be made executable using markExecutable()
void markExecutable(void *memory, size_t bytes); void markExecutable(void *memory, size_t bytes);
void deallocateExecutable(void *memory, size_t bytes); void deallocateExecutable(void *memory, size_t bytes);
template<typename P>
P unaligned_read(P *address)
{
P value;
memcpy(&value, address, sizeof(P));
return value;
}
template<typename P, typename V>
void unaligned_write(P *address, V value)
{
static_assert(sizeof(V) == sizeof(P), "value size must match pointee size");
memcpy(address, &value, sizeof(P));
}
template<typename P>
class unaligned_ref
{
public:
explicit unaligned_ref(void *ptr) : ptr((P*)ptr) {}
template<typename V>
P operator=(V value)
{
unaligned_write(ptr, value);
return value;
}
operator P()
{
return unaligned_read((P*)ptr);
}
private:
P *ptr;
};
template<typename P>
class unaligned_ptr
{
template<typename S>
friend class unaligned_ptr;
public:
unaligned_ptr(P *ptr) : ptr(ptr) {}
unaligned_ref<P> operator*()
{
return unaligned_ref<P>(ptr);
}
template<typename S>
operator S()
{
return S(ptr);
}
private:
void *ptr;
};
} }
#endif // rr_ExecutableMemory_hpp #endif // rr_ExecutableMemory_hpp
...@@ -217,8 +217,6 @@ namespace rr ...@@ -217,8 +217,6 @@ namespace rr
{ {
const SectionHeader *target = elfSection(elfHeader, relocationTable.sh_info); const SectionHeader *target = elfSection(elfHeader, relocationTable.sh_info);
intptr_t address = (intptr_t)elfHeader + target->sh_offset;
int32_t *patchSite = (int*)(address + relocation.r_offset);
uint32_t index = relocation.getSymbol(); uint32_t index = relocation.getSymbol();
int table = relocationTable.sh_link; int table = relocationTable.sh_link;
void *symbolValue = nullptr; void *symbolValue = nullptr;
...@@ -250,6 +248,9 @@ namespace rr ...@@ -250,6 +248,9 @@ namespace rr
} }
} }
intptr_t address = (intptr_t)elfHeader + target->sh_offset;
unaligned_ptr<int32_t> patchSite = (int32_t*)(address + relocation.r_offset);
if(CPUID::ARM) if(CPUID::ARM)
{ {
switch(relocation.getType()) switch(relocation.getType())
...@@ -301,8 +302,6 @@ namespace rr ...@@ -301,8 +302,6 @@ namespace rr
{ {
const SectionHeader *target = elfSection(elfHeader, relocationTable.sh_info); const SectionHeader *target = elfSection(elfHeader, relocationTable.sh_info);
intptr_t address = (intptr_t)elfHeader + target->sh_offset;
int32_t *patchSite = (int*)(address + relocation.r_offset);
uint32_t index = relocation.getSymbol(); uint32_t index = relocation.getSymbol();
int table = relocationTable.sh_link; int table = relocationTable.sh_link;
void *symbolValue = nullptr; void *symbolValue = nullptr;
...@@ -334,19 +333,23 @@ namespace rr ...@@ -334,19 +333,23 @@ namespace rr
} }
} }
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);
switch(relocation.getType()) switch(relocation.getType())
{ {
case R_X86_64_NONE: case R_X86_64_NONE:
// No relocation // No relocation
break; break;
case R_X86_64_64: case R_X86_64_64:
*(int64_t*)patchSite = (int64_t)((intptr_t)symbolValue + *(int64_t*)patchSite) + relocation.r_addend; *patchSite64 = (int64_t)((intptr_t)symbolValue + *patchSite64 + relocation.r_addend);
break; break;
case R_X86_64_PC32: case R_X86_64_PC32:
*patchSite = (int32_t)((intptr_t)symbolValue + *patchSite - (intptr_t)patchSite) + relocation.r_addend; *patchSite32 = (int32_t)((intptr_t)symbolValue + *patchSite32 - (intptr_t)patchSite32 + relocation.r_addend);
break; break;
case R_X86_64_32S: case R_X86_64_32S:
*patchSite = (int32_t)((intptr_t)symbolValue + *patchSite) + relocation.r_addend; *patchSite32 = (int32_t)((intptr_t)symbolValue + *patchSite32 + relocation.r_addend);
break; break;
default: default:
assert(false && "Unsupported relocation type"); assert(false && "Unsupported relocation type");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment