Fix initialization of LRU cache keys
In particular this fixes Blitter::Options state having uninitialized
bits after the bitfield, and Blitter::State having uninitialized padding
bytes after Options so 'sourceFormat' is 32-bit aligned.
Defines a Memset<T> class to be used as the first base class of cache
key types, which makes it explicit that their underlying memory will be
fully initialized before any member constructors are run.
Also adds is_memcmparable<T> for checking if memcmp() can be used to
implement operator==() for cache keys. It's equivalent to
std::is_trivially_copyable except it provides a fallback for STL
implementations that don't support it.
Bug: b/134932616
Change-Id: I6569e78b380e67aee02d5bfd39d80d210bd225fd
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/32929
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Chris Forbes <chrisforbes@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/Device/Blitter.hpp b/src/Device/Blitter.hpp
index 99feb22..b95f14c 100644
--- a/src/Device/Blitter.hpp
+++ b/src/Device/Blitter.hpp
@@ -20,7 +20,7 @@
#include "Vulkan/VkFormat.h"
#include <mutex>
-#include <string.h>
+#include <cstring>
namespace vk
{
@@ -59,15 +59,16 @@
bool clampToEdge : 1;
};
- struct State : Options
+ struct State : Memset<State>, Options
{
- State() = default;
- State(const Options &options) : Options(options) {}
+ State() : Memset(this, 0) {}
+ State(const Options &options) : Memset(this, 0), Options(options) {}
State(vk::Format sourceFormat, vk::Format destFormat, int srcSamples, int destSamples, const Options &options) :
- Options(options), sourceFormat(sourceFormat), destFormat(destFormat), srcSamples(srcSamples), destSamples(destSamples) {}
+ Memset(this, 0), Options(options), sourceFormat(sourceFormat), destFormat(destFormat), srcSamples(srcSamples), destSamples(destSamples) {}
bool operator==(const State &state) const
{
+ static_assert(is_memcmparable<State>::value, "Cannot memcmp State");
return memcmp(this, &state, sizeof(State)) == 0;
}
diff --git a/src/Device/LRUCache.hpp b/src/Device/LRUCache.hpp
index 68c7332..4fc3561 100644
--- a/src/Device/LRUCache.hpp
+++ b/src/Device/LRUCache.hpp
@@ -17,6 +17,9 @@
#include "System/Math.hpp"
+#include <cstring>
+#include <type_traits>
+
namespace sw
{
template<class Key, class Data>
@@ -29,7 +32,7 @@
Data *query(const Key &key) const;
Data *add(const Key &key, Data *data);
-
+
int getSize() {return size;}
Key &getKey(int i) {return key[i];}
@@ -43,6 +46,32 @@
Key **ref;
Data **data;
};
+
+ // Helper class for clearing the memory of objects at construction.
+ // Useful as the first base class or cache keys which may contain padding bytes or bits otherwise left uninitialized.
+ template<class T>
+ struct Memset
+ {
+ Memset(T *object, int val)
+ {
+ static_assert(std::is_base_of<Memset<T>, T>::value, "Memset<T> must only clear the memory of a type of which it is a base class");
+ memset(object, 0, sizeof(T));
+ }
+ };
+
+ // Traits-like helper class for checking if objects can be compared using memcmp().
+ // Useful for statically asserting if a cache key can implement operator==() with memcmp().
+ template<typename T>
+ struct is_memcmparable
+ {
+ // std::is_trivially_copyable is not available in older GCC versions.
+ #if !defined(__GNUC__) || __GNUC__ > 5
+ static const bool value = std::is_trivially_copyable<T>::value;
+ #else
+ // At least check it doesn't have virtual methods.
+ static const bool value = !std::is_polymorphic<T>::value;
+ #endif
+ };
}
namespace sw
