From 00076b529cf19b9cdb67b52465716678e03ad37c Mon Sep 17 00:00:00 2001 From: dANW34V3R Date: Tue, 3 Sep 2024 17:27:01 +0100 Subject: [PATCH 1/4] Initial fix for Wmaybe-uninitialized --- src/include/simeng/RegisterValue.hh | 10 +++++----- src/include/simeng/arch/aarch64/helpers/neon.hh | 11 +++++++++-- src/lib/RegisterValue.cc | 4 ++-- src/lib/arch/aarch64/ExceptionHandler.cc | 2 +- src/lib/arch/riscv/ExceptionHandler.cc | 2 +- 5 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/include/simeng/RegisterValue.hh b/src/include/simeng/RegisterValue.hh index 4faadb301d..6dcb3d7cfe 100644 --- a/src/include/simeng/RegisterValue.hh +++ b/src/include/simeng/RegisterValue.hh @@ -28,12 +28,12 @@ class RegisterValue { typename std::enable_if_t, T>* = nullptr> RegisterValue(T value, uint16_t bytes = sizeof(T)) : bytes(bytes) { if (isLocal()) { - T* view = reinterpret_cast(this->value); + T* view = reinterpret_cast(this->localValue); view[0] = value; if (bytes > sizeof(T)) { // Zero the remaining bytes not set by the provided value - std::fill(this->value + sizeof(T), this->value + bytes, + std::fill(this->localValue + sizeof(T), this->localValue + bytes, 0); } } else { @@ -57,7 +57,7 @@ class RegisterValue { assert(capacity >= bytes && "Capacity is less than requested bytes"); char* dest; if (isLocal()) { - dest = this->value; + dest = this->localValue; } else { dest = static_cast(pool.allocate(capacity)); std::memset(dest, 0, capacity); @@ -96,7 +96,7 @@ class RegisterValue { "Attempted to access a RegisterValue as a datatype larger than the " "data held"); if (isLocal()) { - return reinterpret_cast(value); + return reinterpret_cast(localValue); } else { return reinterpret_cast(ptr.get()); } @@ -128,7 +128,7 @@ class RegisterValue { /** The underlying local member value. Aligned to 8 bytes to prevent * potential alignment issue when casting. */ - alignas(8) char value[MAX_LOCAL_BYTES]; + alignas(8) char localValue[MAX_LOCAL_BYTES] = {}; }; inline bool operator==(const RegisterValue& lhs, const RegisterValue& rhs) { diff --git a/src/include/simeng/arch/aarch64/helpers/neon.hh b/src/include/simeng/arch/aarch64/helpers/neon.hh index 0fcf04f03f..96deb78e12 100644 --- a/src/include/simeng/arch/aarch64/helpers/neon.hh +++ b/src/include/simeng/arch/aarch64/helpers/neon.hh @@ -785,14 +785,21 @@ RegisterValue vecSumElems_2ops(srcValContainer& sourceValues) { template RegisterValue vecXtn(srcValContainer& sourceValues, bool isXtn2) { const D* d; - if (isXtn2) d = sourceValues[0].getAsVector(); - const N* n = sourceValues[isXtn2 ? 1 : 0].getAsVector(); + const N* n; + if (isXtn2) { + d = sourceValues[0].getAsVector(); + n = sourceValues[1].getAsVector(); + } else { + d = {}; + n = sourceValues[0].getAsVector(); + } D out[16 / sizeof(D)] = {0}; int index = 0; for (int i = 0; i < I; i++) { if (isXtn2 & (i < (I / 2))) { + assert(isXtn2 && "isXtn2 is false so d is not initialised"); out[i] = d[i]; } else { out[i] = static_cast(n[index]); diff --git a/src/lib/RegisterValue.cc b/src/lib/RegisterValue.cc index ea27a2a5f3..adddeb375e 100644 --- a/src/lib/RegisterValue.cc +++ b/src/lib/RegisterValue.cc @@ -19,8 +19,8 @@ RegisterValue RegisterValue::zeroExtend(uint16_t fromBytes, auto extended = RegisterValue(0, toBytes); // Get the appropriate source/destination pointers and copy the data - const char* src = (isLocal() ? value : ptr.get()); - char* dest = (extended.isLocal() ? extended.value : extended.ptr.get()); + const char* src = (isLocal() ? localValue : ptr.get()); + char* dest = (extended.isLocal() ? extended.localValue : extended.ptr.get()); std::memcpy(dest, src, fromBytes); diff --git a/src/lib/arch/aarch64/ExceptionHandler.cc b/src/lib/arch/aarch64/ExceptionHandler.cc index 36aff03781..0f52030d19 100644 --- a/src/lib/arch/aarch64/ExceptionHandler.cc +++ b/src/lib/arch/aarch64/ExceptionHandler.cc @@ -329,7 +329,7 @@ bool ExceptionHandler::init() { return readStringThen(filename, filenamePtr, kernel::Linux::LINUX_PATH_MAX, [=](auto length) { // Invoke the kernel - kernel::stat statOut; + kernel::stat statOut = {}; uint64_t retval = linux_.newfstatat( dfd, filename, statOut, flag); ProcessStateChange stateChange = { diff --git a/src/lib/arch/riscv/ExceptionHandler.cc b/src/lib/arch/riscv/ExceptionHandler.cc index 15a5518c64..423c7d1435 100644 --- a/src/lib/arch/riscv/ExceptionHandler.cc +++ b/src/lib/arch/riscv/ExceptionHandler.cc @@ -327,7 +327,7 @@ bool ExceptionHandler::init() { return readStringThen(filename, filenamePtr, kernel::Linux::LINUX_PATH_MAX, [=](auto length) { // Invoke the kernel - kernel::stat statOut; + kernel::stat statOut = {}; uint64_t retval = linux_.newfstatat( dfd, filename, statOut, flag); ProcessStateChange stateChange = { From 042307a0f10d3822519dd13bca7fae32b091f71f Mon Sep 17 00:00:00 2001 From: dANW34V3R Date: Mon, 23 Sep 2024 18:04:36 +0100 Subject: [PATCH 2/4] Force clangformat --- src/include/simeng/RegisterValue.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/include/simeng/RegisterValue.hh b/src/include/simeng/RegisterValue.hh index 6dcb3d7cfe..5590427af7 100644 --- a/src/include/simeng/RegisterValue.hh +++ b/src/include/simeng/RegisterValue.hh @@ -33,8 +33,8 @@ class RegisterValue { if (bytes > sizeof(T)) { // Zero the remaining bytes not set by the provided value - std::fill(this->localValue + sizeof(T), this->localValue + bytes, - 0); + std::fill(this->localValue + sizeof(T), + this->localValue + bytes, 0); } } else { void* data = pool.allocate(bytes); From a3f6f53d68c03eaafebed5168475d3c04cf83317 Mon Sep 17 00:00:00 2001 From: dANW34V3R Date: Mon, 23 Sep 2024 19:12:31 +0100 Subject: [PATCH 3/4] Hacky fix for failing test --- test/unit/RegisterValueTest.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/unit/RegisterValueTest.cc b/test/unit/RegisterValueTest.cc index c365ff8f7e..b80ccbe593 100644 --- a/test/unit/RegisterValueTest.cc +++ b/test/unit/RegisterValueTest.cc @@ -40,7 +40,10 @@ TEST(RegisterValueTest, Reinterpret) { TEST(RegisterValueTest, Vector) { uint64_t value = 0x0000000200000001; auto registerValue = simeng::RegisterValue(value, 8); - auto vector = registerValue.getAsVector(); + const uint32_t* vector = registerValue.getAsVector(); + // Print vector's address to prevent it from being optimised away causing the + // test to fail + std::cout << vector << std::endl; EXPECT_EQ(vector[0], 1); EXPECT_EQ(vector[1], 2); } From d3d97e02253983258e64d1fe1e23957c3449388a Mon Sep 17 00:00:00 2001 From: dANW34V3R Date: Thu, 26 Sep 2024 11:55:47 +0100 Subject: [PATCH 4/4] Add no strict ailiasing flag --- CMakeLists.txt | 2 +- test/unit/RegisterValueTest.cc | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 542049f2f8..2381ccc954 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -72,7 +72,7 @@ set(CMAKE_MACOSX_RPATH 1) set(CMAKE_POSITION_INDEPENDENT_CODE ON) # Create variable to enable additional compiler warnings for SimEng targets only -set(SIMENG_COMPILE_OPTIONS -Wall -pedantic -Werror) #-Wextra +set(SIMENG_COMPILE_OPTIONS -Wall -pedantic -Werror -fno-strict-aliasing) #-Wextra # Disable RTTI for all targets add_compile_options($<$:-fno-rtti>) diff --git a/test/unit/RegisterValueTest.cc b/test/unit/RegisterValueTest.cc index b80ccbe593..034d6d8766 100644 --- a/test/unit/RegisterValueTest.cc +++ b/test/unit/RegisterValueTest.cc @@ -41,9 +41,6 @@ TEST(RegisterValueTest, Vector) { uint64_t value = 0x0000000200000001; auto registerValue = simeng::RegisterValue(value, 8); const uint32_t* vector = registerValue.getAsVector(); - // Print vector's address to prevent it from being optimised away causing the - // test to fail - std::cout << vector << std::endl; EXPECT_EQ(vector[0], 1); EXPECT_EQ(vector[1], 2); }