From: Andreas Hansson Date: Sat, 27 Sep 2014 13:08:37 +0000 (-0400) Subject: arm: Fixed undefined behaviours identified by gcc X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=ec41000dadd5256fd90f0bfdc97264946e50a3aa;p=gem5.git 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 efc334c4b..7432f436e 100644 --- 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 3d947a272..4217dc6f1 100644 --- 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 c5e2ab386..8a335879b 100644 --- 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 0ba114e86..7d3865104 100644 --- 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 76fc1fbed..00c907acd 100644 --- 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 c3866a51f..5cd4637a6 100644 --- 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 ece4e5a1c..94343c1c2 100644 --- 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);