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>
This commit is contained in:
Giacomo Travaglini
2020-03-20 17:29:52 +00:00
parent 87cf5a99ee
commit 2fbd03599d
2 changed files with 91 additions and 27 deletions

View File

@@ -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 ((sizeof(Integer) <= sizeof(uint32_t) ||
std::is_same<Integer, Addr>::value) &&
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 (std::is_same<Integer, Addr>::value &&
state.ncrn <= state.MAX_CRN) {
return tc->readIntReg(state.ncrn++);
}

View File

@@ -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));
}
};