changeset 7a76e13f0101 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=7a76e13f0101
description:
        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.

diffstat:

 src/arch/arm/insts/misc.cc          |  24 ++++++++++++++
 src/arch/arm/insts/misc.hh          |  34 ++++++++++++++++++++
 src/arch/arm/insts/pred_inst.hh     |   7 ++-
 src/arch/arm/isa/formats/misc.isa   |   4 +-
 src/arch/arm/isa/insts/misc.isa     |  12 +++---
 src/arch/arm/isa/templates/misc.isa |  60 +++++++++++++++++++++++++++++++++++++
 src/arch/arm/tlb.cc                 |   7 ++-
 7 files changed, 134 insertions(+), 14 deletions(-)

diffs (231 lines):

diff -r 710ee116eb68 -r 7a76e13f0101 src/arch/arm/insts/misc.cc
--- a/src/arch/arm/insts/misc.cc        Sat Sep 27 09:08:36 2014 -0400
+++ b/src/arch/arm/insts/misc.cc        Sat Sep 27 09:08:37 2014 -0400
@@ -256,6 +256,30 @@
 }
 
 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
 {
     std::stringstream ss;
diff -r 710ee116eb68 -r 7a76e13f0101 src/arch/arm/insts/misc.hh
--- a/src/arch/arm/insts/misc.hh        Sat Sep 27 09:08:36 2014 -0400
+++ b/src/arch/arm/insts/misc.hh        Sat Sep 27 09:08:37 2014 -0400
@@ -256,6 +256,40 @@
     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 -r 710ee116eb68 -r 7a76e13f0101 src/arch/arm/insts/pred_inst.hh
--- a/src/arch/arm/insts/pred_inst.hh   Sat Sep 27 09:08:36 2014 -0400
+++ b/src/arch/arm/insts/pred_inst.hh   Sat Sep 27 09:08:37 2014 -0400
@@ -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 -r 710ee116eb68 -r 7a76e13f0101 src/arch/arm/isa/formats/misc.isa
--- a/src/arch/arm/isa/formats/misc.isa Sat Sep 27 09:08:36 2014 -0400
+++ b/src/arch/arm/isa/formats/misc.isa Sat Sep 27 09:08:37 2014 -0400
@@ -214,8 +214,8 @@
 
             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 -r 710ee116eb68 -r 7a76e13f0101 src/arch/arm/isa/insts/misc.isa
--- a/src/arch/arm/isa/insts/misc.isa   Sat Sep 27 09:08:36 2014 -0400
+++ b/src/arch/arm/isa/insts/misc.isa   Sat Sep 27 09:08:37 2014 -0400
@@ -860,11 +860,11 @@
     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 @@
     }
     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 -r 710ee116eb68 -r 7a76e13f0101 src/arch/arm/isa/templates/misc.isa
--- a/src/arch/arm/isa/templates/misc.isa       Sat Sep 27 09:08:36 2014 -0400
+++ b/src/arch/arm/isa/templates/misc.isa       Sat Sep 27 09:08:37 2014 -0400
@@ -433,6 +433,66 @@
     }
 }};
 
+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 -r 710ee116eb68 -r 7a76e13f0101 src/arch/arm/tlb.cc
--- a/src/arch/arm/tlb.cc       Sat Sep 27 09:08:36 2014 -0400
+++ b/src/arch/arm/tlb.cc       Sat Sep 27 09:08:37 2014 -0400
@@ -71,9 +71,10 @@
 
 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);
 
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to