arm: Fixed undefined behaviours identified by gcc

This patch fixes the runtime errors highlighted by the undefined
behaviour sanitizer. In the end there were two issues. First, when
rotating an immediate, we ended up shifting an uint32_t by 32 in some
cases. This case is fixed by checking for a rotation by 0
positions. Second, the Mrc15 and Mcr15 are operating on an IntReg and
a MiscReg, but we used the type RegRegImmOp and passed a MiscRegIndex
as an IntRegIndex. This issue is resolved by introducing a
MiscRegRegImmOp and RegMiscRegImmOp with the appropriate types.

With these fixes there are no runtime errors identified for the full
ARM regressions.

diff --git a/src/arch/arm/insts/misc.cc b/src/arch/arm/insts/misc.cc
index efc334c4bdaaabfb4adca264a51d1719cf0e06fa..7432f436e3c2a6e58d5b3d9d3590489f07d92b0d 100644 (file)
--- a/src/arch/arm/insts/misc.cc
+++ b/src/arch/arm/insts/misc.cc
@@ -255,6 +255,30 @@ RegRegImmOp::generateDisassembly(Addr pc, const SymbolTable *symtab) const
     return ss.str();
 }
 
+std::string
+MiscRegRegImmOp::generateDisassembly(Addr pc, const SymbolTable *symtab) const
+{
+    std::stringstream ss;
+    printMnemonic(ss);
+    printReg(ss, dest);
+    ss << ", ";
+    printReg(ss, op1);
+    ccprintf(ss, ", #%d", imm);
+    return ss.str();
+}
+
+std::string
+RegMiscRegImmOp::generateDisassembly(Addr pc, const SymbolTable *symtab) const
+{
+    std::stringstream ss;
+    printMnemonic(ss);
+    printReg(ss, dest);
+    ss << ", ";
+    printReg(ss, op1);
+    ccprintf(ss, ", #%d", imm);
+    return ss.str();
+}
+
 std::string
 RegImmImmOp::generateDisassembly(Addr pc, const SymbolTable *symtab) const
 {

diff --git a/src/arch/arm/insts/misc.hh b/src/arch/arm/insts/misc.hh
index 3d947a27292399181b9bbd9d91ccef06742fb54d..4217dc6f1a2d7f75678f3f5275e1d08aa530a181 100644 (file)
--- a/src/arch/arm/insts/misc.hh
+++ b/src/arch/arm/insts/misc.hh
@@ -256,6 +256,40 @@ class RegRegImmOp : public PredOp
     std::string generateDisassembly(Addr pc, const SymbolTable *symtab) const;
 };
 
+class MiscRegRegImmOp : public PredOp
+{
+  protected:
+    MiscRegIndex dest;
+    IntRegIndex op1;
+    uint64_t imm;
+
+    MiscRegRegImmOp(const char *mnem, ExtMachInst _machInst, OpClass __opClass,
+                    MiscRegIndex _dest, IntRegIndex _op1,
+                    uint64_t _imm) :
+        PredOp(mnem, _machInst, __opClass),
+        dest(_dest), op1(_op1), imm(_imm)
+    {}
+
+    std::string generateDisassembly(Addr pc, const SymbolTable *symtab) const;
+};
+
+class RegMiscRegImmOp : public PredOp
+{
+  protected:
+    IntRegIndex dest;
+    MiscRegIndex op1;
+    uint64_t imm;
+
+    RegMiscRegImmOp(const char *mnem, ExtMachInst _machInst, OpClass __opClass,
+                    IntRegIndex _dest, MiscRegIndex _op1,
+                    uint64_t _imm) :
+        PredOp(mnem, _machInst, __opClass),
+        dest(_dest), op1(_op1), imm(_imm)
+    {}
+
+    std::string generateDisassembly(Addr pc, const SymbolTable *symtab) const;
+};
+
 class RegImmImmOp : public PredOp
 {
   protected:

diff --git a/src/arch/arm/insts/pred_inst.hh b/src/arch/arm/insts/pred_inst.hh
index c5e2ab386190d40cf096f0d2303e91993ce50f05..8a335879b73aa5be6af8da864137bc4e38ebcd6e 100644 (file)
--- a/src/arch/arm/insts/pred_inst.hh
+++ b/src/arch/arm/insts/pred_inst.hh
@@ -48,10 +48,11 @@
 namespace ArmISA
 {
 static inline uint32_t
-rotate_imm(uint32_t immValue, int rotateValue)
+rotate_imm(uint32_t immValue, uint32_t rotateValue)
 {
-    return ((immValue >> (rotateValue & 31)) |
-            (immValue << (32 - (rotateValue & 31))));
+    rotateValue &= 31;
+    return rotateValue == 0 ? immValue :
+        (immValue >> rotateValue) | (immValue << (32 - rotateValue));
 }
 
 static inline uint32_t

diff --git a/src/arch/arm/isa/formats/misc.isa b/src/arch/arm/isa/formats/misc.isa
index 0ba114e86da78c2a7e6f246c0fb00308688a82ce..7d3865104a02108b023a296297c18a1698c02431 100644 (file)
--- a/src/arch/arm/isa/formats/misc.isa
+++ b/src/arch/arm/isa/formats/misc.isa
@@ -214,8 +214,8 @@ let {{
 
             if (miscRegInfo[miscReg][MISCREG_IMPLEMENTED]) {
                 if (isRead)
-                    return new Mrc15(machInst, rt, (IntRegIndex)miscReg, iss);
-                return new Mcr15(machInst, (IntRegIndex)miscReg, rt, iss);
+                    return new Mrc15(machInst, rt, miscReg, iss);
+                return new Mcr15(machInst, miscReg, rt, iss);
             } else {
                 return new FailUnimplemented(isRead ? "mrc" : "mcr", machInst,
                     csprintf("%s %s", isRead ? "mrc" : "mcr",

diff --git a/src/arch/arm/isa/insts/misc.isa b/src/arch/arm/isa/insts/misc.isa
index 76fc1fbedfdea781c30853c047639f44c87e0ea9..00c907acd639a896974137ba54a3305155d5021a 100644 (file)
--- a/src/arch/arm/isa/insts/misc.isa
+++ b/src/arch/arm/isa/insts/misc.isa
@@ -860,11 +860,11 @@ let {{
     Dest = MiscNsBankedOp1;
     '''
 
-    mrc15Iop = InstObjParams("mrc", "Mrc15", "RegRegImmOp",
+    mrc15Iop = InstObjParams("mrc", "Mrc15", "RegMiscRegImmOp",
                              { "code": mrc15code,
                                "predicate_test": predicateTest }, [])
-    header_output += RegRegImmOpDeclare.subst(mrc15Iop)
-    decoder_output += RegRegImmOpConstructor.subst(mrc15Iop)
+    header_output += RegMiscRegImmOpDeclare.subst(mrc15Iop)
+    decoder_output += RegMiscRegImmOpConstructor.subst(mrc15Iop)
     exec_output += PredOpExecute.subst(mrc15Iop)
 
 
@@ -887,12 +887,12 @@ let {{
     }
     MiscNsBankedDest = Op1;
     '''
-    mcr15Iop = InstObjParams("mcr", "Mcr15", "RegRegImmOp",
+    mcr15Iop = InstObjParams("mcr", "Mcr15", "MiscRegRegImmOp",
                              { "code": mcr15code,
                                "predicate_test": predicateTest },
                                ["IsSerializeAfter","IsNonSpeculative"])
-    header_output += RegRegImmOpDeclare.subst(mcr15Iop)
-    decoder_output += RegRegImmOpConstructor.subst(mcr15Iop)
+    header_output += MiscRegRegImmOpDeclare.subst(mcr15Iop)
+    decoder_output += MiscRegRegImmOpConstructor.subst(mcr15Iop)
     exec_output += PredOpExecute.subst(mcr15Iop)
 
 

diff --git a/src/arch/arm/isa/templates/misc.isa b/src/arch/arm/isa/templates/misc.isa
index c3866a51fb73099aa5adbc16a20b7818001d8727..5cd4637a6b5f4534cda54a4dfa4cfa5f533b20e7 100644 (file)
--- a/src/arch/arm/isa/templates/misc.isa
+++ b/src/arch/arm/isa/templates/misc.isa
@@ -433,6 +433,66 @@ def template RegRegImmOpConstructor {{
     }
 }};
 
+def template MiscRegRegImmOpDeclare {{
+class %(class_name)s : public %(base_class)s
+{
+  protected:
+    public:
+        // Constructor
+        %(class_name)s(ExtMachInst machInst,
+                       MiscRegIndex _dest, IntRegIndex _op1,
+                       uint64_t _imm);
+        %(BasicExecDeclare)s
+};
+}};
+
+def template MiscRegRegImmOpConstructor {{
+    %(class_name)s::%(class_name)s(ExtMachInst machInst,
+                                          MiscRegIndex _dest,
+                                          IntRegIndex _op1,
+                                          uint64_t _imm)
+        : %(base_class)s("%(mnemonic)s", machInst, %(op_class)s,
+                         _dest, _op1, _imm)
+    {
+        %(constructor)s;
+        if (!(condCode == COND_AL || condCode == COND_UC)) {
+            for (int x = 0; x < _numDestRegs; x++) {
+                _srcRegIdx[_numSrcRegs++] = _destRegIdx[x];
+            }
+        }
+    }
+}};
+
+def template RegMiscRegImmOpDeclare {{
+class %(class_name)s : public %(base_class)s
+{
+  protected:
+    public:
+        // Constructor
+        %(class_name)s(ExtMachInst machInst,
+                       IntRegIndex _dest, MiscRegIndex _op1,
+                       uint64_t _imm);
+        %(BasicExecDeclare)s
+};
+}};
+
+def template RegMiscRegImmOpConstructor {{
+    %(class_name)s::%(class_name)s(ExtMachInst machInst,
+                                          IntRegIndex _dest,
+                                          MiscRegIndex _op1,
+                                          uint64_t _imm)
+        : %(base_class)s("%(mnemonic)s", machInst, %(op_class)s,
+                         _dest, _op1, _imm)
+    {
+        %(constructor)s;
+        if (!(condCode == COND_AL || condCode == COND_UC)) {
+            for (int x = 0; x < _numDestRegs; x++) {
+                _srcRegIdx[_numSrcRegs++] = _destRegIdx[x];
+            }
+        }
+    }
+}};
+
 def template RegImmImmOpDeclare {{
 class %(class_name)s : public %(base_class)s
 {

diff --git a/src/arch/arm/tlb.cc b/src/arch/arm/tlb.cc
index ece4e5a1c073f458a91a97d2edbbb3722a30a595..94343c1c21ad58f62a8f1cff59b9678ce19eda20 100644 (file)
--- a/src/arch/arm/tlb.cc
+++ b/src/arch/arm/tlb.cc
@@ -71,9 +71,10 @@ using namespace ArmISA;
 
 TLB::TLB(const ArmTLBParams *p)
     : BaseTLB(p), table(new TlbEntry[p->size]), size(p->size),
-    isStage2(p->is_stage2), tableWalker(p->walker), stage2Tlb(NULL),
-    stage2Mmu(NULL), rangeMRU(1), bootUncacheability(false),
-    miscRegValid(false), curTranType(NormalTran)
+      isStage2(p->is_stage2), stage2Req(false), _attr(0),
+      directToStage2(false), tableWalker(p->walker), stage2Tlb(NULL),
+      stage2Mmu(NULL), rangeMRU(1), bootUncacheability(false),
+      miscRegValid(false), curTranType(NormalTran)
 {
     tableWalker->setTlb(this);