arch-arm: Fix aapcs32/aapcs64 compilation issues

Some compilers won't build ARM due to how guest ABI
has been implemented.

The error is: "left shift count >= width of type"
[-Werror=shift-count-overflow]

The error is triggered when there is a left shift > the variable size
(in bits); this leads to undefined behaviour.

This is a compile time vs run time problem; the code is technically
fine, but the compiler is not able to understand this.

For example in aapcs64:

struct Argument<Aapcs64, Integer, typename std::enable_if<
 std::is_integral<Integer>::value>::type> : public Aapcs64ArgumentBase
{
    [...]
    if (sizeof(Integer) == 16 && state.ngrn + 1 <= state.MAX_GRN) {
        Integer low = tc->readIntReg(state.ngrn++);
        Integer high = tc->readIntReg(state.ngrn++);
        high = high << 64;
        return high | low;
    }
}

Even if the sizeof operator will be evaluated at compile time,
the block will be executed at runtime: the block will still be part of
the code if Integer = uint32_t.
The compiler will then throw an error because we are left shifting an
uint32_t by 64 bits.

Error arising on:
Compiler: gcc/5.4.0
Distro: Ubuntu 16.04 LTS

Change-Id: Iaafe030b7262c5fb162afe7118ae592a1a759a58
Signed-off-by: Giacomo Travaglini <giacomo.travaglini@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/26990
Reviewed-by: Gabe Black <gabeblack@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>

diff --git a/src/arch/arm/aapcs32.hh b/src/arch/arm/aapcs32.hh
index bfeb55374b540d0bfa06ba43c4760aeb3b4e9ff0..fd63483ef426299484fd6a1efc3f98648c24a6ec 100644 (file)
--- a/src/arch/arm/aapcs32.hh
+++ b/src/arch/arm/aapcs32.hh
@@ -131,39 +131,78 @@ struct Aapcs32ArgumentBase
 
 template <typename Integer>
 struct Result<Aapcs32, Integer, typename std::enable_if<
-    std::is_integral<Integer>::value>::type>
+    std::is_integral<Integer>::value && (sizeof(Integer) < sizeof(uint32_t))
+    >::type>
 {
     static void
     store(ThreadContext *tc, const Integer &i)
     {
-        if (sizeof(Integer) < sizeof(uint32_t)) {
-            uint32_t val = std::is_signed<Integer>::value ?
-                    sext<sizeof(Integer) * 8>(i) : i;
-            tc->setIntReg(ArmISA::INTREG_R0, val);
-        } else if (sizeof(Integer) == sizeof(uint32_t) ||
-                   std::is_same<Integer, Addr>::value) {
+        uint32_t val = std::is_signed<Integer>::value ?
+                sext<sizeof(Integer) * 8>(i) : i;
+        tc->setIntReg(ArmISA::INTREG_R0, val);
+    }
+};
+
+template <typename Integer>
+struct Result<Aapcs32, Integer, typename std::enable_if<
+    std::is_integral<Integer>::value && (sizeof(Integer) == sizeof(uint32_t))
+    >::type>
+{
+    static void
+    store(ThreadContext *tc, const Integer &i)
+    {
+        tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)i);
+    }
+};
+
+template <typename Integer>
+struct Result<Aapcs32, Integer, typename std::enable_if<
+    std::is_integral<Integer>::value && (sizeof(Integer) == sizeof(uint64_t))
+    >::type>
+{
+    static void
+    store(ThreadContext *tc, const Integer &i)
+    {
+        if (std::is_same<Integer, Addr>::value) {
             tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)i);
-        } else if (sizeof(Integer) == sizeof(uint64_t)) {
-            if (ArmISA::byteOrder(tc) == LittleEndianByteOrder) {
-                tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 0));
-                tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 32));
-            } else {
-                tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 32));
-                tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 0));
-            }
+        } else if (ArmISA::byteOrder(tc) == LittleEndianByteOrder) {
+            tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 0));
+            tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 32));
+        } else {
+            tc->setIntReg(ArmISA::INTREG_R0, (uint32_t)(i >> 32));
+            tc->setIntReg(ArmISA::INTREG_R1, (uint32_t)(i >> 0));
         }
     }
 };
 
 template <typename Integer>
 struct Argument<Aapcs32, Integer, typename std::enable_if<
-    std::is_integral<Integer>::value>::type> : public Aapcs32ArgumentBase
+    std::is_integral<Integer>::value && (sizeof(Integer) <= sizeof(uint32_t))
+    >::type> : public Aapcs32ArgumentBase
+{
+    static Integer
+    get(ThreadContext *tc, Aapcs32::State &state)
+    {
+        if (state.ncrn <= state.MAX_CRN) {
+            return tc->readIntReg(state.ncrn++);
+        }
+
+        // Max out the ncrn since we effectively exhausted it.
+        state.ncrn = state.MAX_CRN + 1;
+
+        return loadFromStack<Integer>(tc, state);
+    }
+};
+
+template <typename Integer>
+struct Argument<Aapcs32, Integer, typename std::enable_if<
+    std::is_integral<Integer>::value && (sizeof(Integer) > sizeof(uint32_t))
+    >::type> : public Aapcs32ArgumentBase
 {
     static Integer
     get(ThreadContext *tc, Aapcs32::State &state)
     {
-        if ((sizeof(Integer) <= sizeof(uint32_t) ||
-                std::is_same<Integer, Addr>::value) &&
+        if (std::is_same<Integer, Addr>::value &&
                 state.ncrn <= state.MAX_CRN) {
             return tc->readIntReg(state.ncrn++);
         }

diff --git a/src/arch/arm/aapcs64.hh b/src/arch/arm/aapcs64.hh
index 9d7623a058f89bd98f7fe74ac78e7dbce40c7a09..203846d43cbfadbd4313f3ee8668afdf80c3499c 100644 (file)
--- a/src/arch/arm/aapcs64.hh
+++ b/src/arch/arm/aapcs64.hh
@@ -229,14 +229,30 @@ struct Result<Aapcs64, Float, typename std::enable_if<
 // This will pick up Addr as well, which should be used for guest pointers.
 template <typename Integer>
 struct Argument<Aapcs64, Integer, typename std::enable_if<
-    std::is_integral<Integer>::value>::type> : public Aapcs64ArgumentBase
+    std::is_integral<Integer>::value && (sizeof(Integer) <= 8)
+    >::type> : public Aapcs64ArgumentBase
 {
     static Integer
     get(ThreadContext *tc, Aapcs64::State &state)
     {
-        if (sizeof(Integer) <= 8 && state.ngrn <= state.MAX_GRN)
+        if (state.ngrn <= state.MAX_GRN)
             return tc->readIntReg(state.ngrn++);
 
+        // Max out ngrn since we've effectively saturated it.
+        state.ngrn = state.MAX_GRN + 1;
+
+        return loadFromStack<Integer>(tc, state);
+    }
+};
+
+template <typename Integer>
+struct Argument<Aapcs64, Integer, typename std::enable_if<
+    std::is_integral<Integer>::value && (sizeof(Integer) > 8)
+    >::type> : public Aapcs64ArgumentBase
+{
+    static Integer
+    get(ThreadContext *tc, Aapcs64::State &state)
+    {
         if (alignof(Integer) == 16 && (state.ngrn % 2))
             state.ngrn++;
 
@@ -256,17 +272,26 @@ struct Argument<Aapcs64, Integer, typename std::enable_if<
 
 template <typename Integer>
 struct Result<Aapcs64, Integer, typename std::enable_if<
-    std::is_integral<Integer>::value>::type>
+    std::is_integral<Integer>::value && (sizeof(Integer) <= 8)
+    >::type>
 {
     static void
     store(ThreadContext *tc, const Integer &i)
     {
-        if (sizeof(Integer) <= 8) {
-            tc->setIntReg(0, i);
-        } else {
-            tc->setIntReg(0, (uint64_t)i);
-            tc->setIntReg(1, (uint64_t)(i >> 64));
-        }
+        tc->setIntReg(0, i);
+    }
+};
+
+template <typename Integer>
+struct Result<Aapcs64, Integer, typename std::enable_if<
+    std::is_integral<Integer>::value && (sizeof(Integer) > 8)
+    >::type>
+{
+    static void
+    store(ThreadContext *tc, const Integer &i)
+    {
+        tc->setIntReg(0, (uint64_t)i);
+        tc->setIntReg(1, (uint64_t)(i >> 64));
     }
 };