changeset 47fe87b0cf97 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=47fe87b0cf97
description:
        arm: Fixes based on UBSan and static analysis

        Another churn to clean up undefined behaviour, mostly ARM, but some
        parts also touching the generic part of the code base.

        Most of the fixes are simply ensuring that proper intialisation. One
        of the more subtle changes is the return type of the sign-extension,
        which is changed to uint64_t. This is to avoid shifting negative
        values (undefined behaviour) in the ISA code.

diffstat:

 src/arch/arm/faults.hh                |  11 +++++++----
 src/arch/arm/insts/macromem.cc        |   9 +++++++--
 src/arch/arm/insts/mem64.hh           |   2 +-
 src/arch/arm/insts/misc.hh            |   1 -
 src/arch/arm/insts/pred_inst.hh       |   2 +-
 src/arch/arm/isa/insts/misc64.isa     |   3 ++-
 src/arch/arm/isa/insts/neon64_mem.isa |  12 ++++++++----
 src/arch/arm/linux/system.cc          |   4 ++--
 src/arch/arm/pmu.cc                   |   2 +-
 src/arch/arm/process.hh               |   2 --
 src/arch/arm/remote_gdb.cc            |   2 +-
 src/arch/arm/stage2_lookup.hh         |   4 ++--
 src/arch/arm/stage2_mmu.cc            |   2 +-
 src/arch/arm/system.cc                |   1 +
 src/arch/arm/table_walker.cc          |  10 +++++++++-
 src/arch/arm/table_walker.hh          |   7 ++++---
 src/arch/arm/tlb.cc                   |   2 ++
 src/arch/arm/types.hh                 |   6 ++++--
 src/arch/generic/types.hh             |   8 ++++----
 src/base/bitfield.hh                  |   2 +-
 src/cpu/base.cc                       |   2 ++
 src/cpu/minor/decode.cc               |   3 ++-
 src/cpu/minor/fetch1.hh               |   6 ------
 src/cpu/minor/fetch2.cc               |   3 ++-
 src/cpu/o3/lsq_unit.hh                |   5 +++--
 src/cpu/o3/rename_map.cc              |   2 +-
 src/cpu/o3/rename_map.hh              |   2 +-
 src/cpu/o3/thread_state.hh            |   3 ++-
 src/cpu/simple/atomic.cc              |   3 ++-
 src/cpu/simple/base.cc                |  15 ++-------------
 src/cpu/simple/base.hh                |   8 --------
 src/cpu/simple_thread.cc              |   3 ++-
 src/cpu/static_inst.hh                |   2 +-
 src/cpu/thread_state.cc               |   5 +++--
 src/dev/arm/realview.cc               |   2 +-
 35 files changed, 82 insertions(+), 74 deletions(-)

diffs (truncated from 585 to 300 lines):

diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/faults.hh
--- a/src/arch/arm/faults.hh    Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/faults.hh    Fri Nov 14 03:53:51 2014 -0500
@@ -172,7 +172,8 @@
     };
 
     ArmFault(ExtMachInst _machInst = 0, uint32_t _iss = 0) :
-        machInst(_machInst), issRaw(_iss), from64(false), to64(false) {}
+        machInst(_machInst), issRaw(_iss), from64(false), to64(false),
+        fromEL(EL0), toEL(EL0), fromMode(MODE_UNDEFINED) {}
 
     // Returns the actual syndrome register to use based on the target
     // exception level
@@ -395,9 +396,11 @@
     ArmFault::TranMethod tranMethod;
 
   public:
-    AbortFault(Addr _faultAddr, bool _write, TlbEntry::DomainType _domain, 
uint8_t _source,
-               bool _stage2, ArmFault::TranMethod _tranMethod = 
ArmFault::UnknownTran) :
-        faultAddr(_faultAddr), write(_write), domain(_domain), source(_source),
+    AbortFault(Addr _faultAddr, bool _write, TlbEntry::DomainType _domain,
+               uint8_t _source, bool _stage2,
+               ArmFault::TranMethod _tranMethod = ArmFault::UnknownTran) :
+        faultAddr(_faultAddr), OVAddr(0), write(_write),
+        domain(_domain), source(_source), srcEncoded(0),
         stage2(_stage2), s1ptw(false), tranMethod(_tranMethod)
     {}
 
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/insts/macromem.cc
--- a/src/arch/arm/insts/macromem.cc    Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/insts/macromem.cc    Fri Nov 14 03:53:51 2014 -0500
@@ -1281,7 +1281,10 @@
                              RegIndex rm, uint8_t eSize, uint8_t dataSize,
                              uint8_t numStructElems, uint8_t index, bool wb,
                              bool replicate) :
-    PredMacroOp(mnem, machInst, __opClass)
+    PredMacroOp(mnem, machInst, __opClass),
+    eSize(0), dataSize(0), numStructElems(0), index(0),
+    wb(false), replicate(false)
+
 {
     RegIndex vx = NumFloatV8ArchRegs / 4;
     RegIndex rnsp = (RegIndex) makeSP((IntRegIndex) rn);
@@ -1352,7 +1355,9 @@
                              RegIndex rm, uint8_t eSize, uint8_t dataSize,
                              uint8_t numStructElems, uint8_t index, bool wb,
                              bool replicate) :
-    PredMacroOp(mnem, machInst, __opClass)
+    PredMacroOp(mnem, machInst, __opClass),
+    eSize(0), dataSize(0), numStructElems(0), index(0),
+    wb(false), replicate(false)
 {
     RegIndex vx = NumFloatV8ArchRegs / 4;
     RegIndex rnsp = (RegIndex) makeSP((IntRegIndex) rn);
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/insts/mem64.hh
--- a/src/arch/arm/insts/mem64.hh       Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/insts/mem64.hh       Fri Nov 14 03:53:51 2014 -0500
@@ -101,7 +101,7 @@
     Memory64(const char *mnem, ExtMachInst _machInst, OpClass __opClass,
              IntRegIndex _dest, IntRegIndex _base)
         : MightBeMicro64(mnem, _machInst, __opClass),
-          dest(_dest), base(_base), uops(NULL)
+          dest(_dest), base(_base), uops(NULL), memAccessFlags(0)
     {
         baseIsSP = isSP(_base);
     }
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/insts/misc.hh
--- a/src/arch/arm/insts/misc.hh        Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/insts/misc.hh        Fri Nov 14 03:53:51 2014 -0500
@@ -294,7 +294,6 @@
 {
   protected:
     IntRegIndex dest;
-    IntRegIndex op1;
     uint64_t imm1;
     uint64_t imm2;
 
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/insts/pred_inst.hh
--- a/src/arch/arm/insts/pred_inst.hh   Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/insts/pred_inst.hh   Fri Nov 14 03:53:51 2014 -0500
@@ -312,7 +312,7 @@
     /// Constructor
     PredMacroOp(const char *mnem, ExtMachInst _machInst, OpClass __opClass) :
                 PredOp(mnem, _machInst, __opClass),
-                numMicroops(0)
+                numMicroops(0), microOps(nullptr)
     {
         // We rely on the subclasses of this object to handle the
         // initialization of the micro-operations, since they are
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/isa/insts/misc64.isa
--- a/src/arch/arm/isa/insts/misc64.isa Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/isa/insts/misc64.isa Fri Nov 14 03:53:51 2014 -0500
@@ -84,7 +84,8 @@
         diff += intWidth;
     }
     uint64_t topBits M5_VAR_USED = ~mask(diff+1);
-    uint64_t result = (Op164 >> imm1) | (Op164 << (intWidth - imm1));
+    uint64_t result = imm1 == 0 ? Op164 :
+                      (Op164 >> imm1) | (Op164 << (intWidth - imm1));
     result &= bitMask;
     '''
 
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/isa/insts/neon64_mem.isa
--- a/src/arch/arm/isa/insts/neon64_mem.isa     Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/isa/insts/neon64_mem.isa     Fri Nov 14 03:53:51 2014 -0500
@@ -185,7 +185,8 @@
         if name == 'deint_neon_uop':
 
             eCode = '''
-                VReg input[4];  // input data from scratch area
+                // input data from scratch area
+                VReg input[4] = { {0, 0}, {0, 0}, {0, 0}, {0, 0} };
                 VReg output[2];  // output data to arch. SIMD regs
                 VReg temp;
                 temp.lo = 0;
@@ -270,7 +271,8 @@
         elif name == 'int_neon_uop':
 
             eCode = '''
-                VReg input[4];  // input data from arch. SIMD regs
+                // input data from arch. SIMD regs
+                VReg input[4] = { {0, 0}, {0, 0}, {0, 0}, {0, 0} };
                 VReg output[2];  // output data to scratch area
             '''
 
@@ -332,7 +334,8 @@
         elif name == 'unpack_neon_uop':
 
             eCode = '''
-                VReg input[4];  //input data from scratch area
+                //input data from scratch area
+                VReg input[4] = { {0, 0}, {0, 0}, {0, 0}, {0, 0} };
                 VReg output[2];  //output data to arch. SIMD regs
             '''
 
@@ -398,7 +401,8 @@
         elif name == 'pack_neon_uop':
 
             eCode = '''
-                VReg input[4];  // input data from arch. SIMD regs
+                // input data from arch. SIMD regs
+                VReg input[4] = { {0, 0}, {0, 0}, {0, 0}, {0, 0} };
                 VReg output[2];  // output data to scratch area
             '''
 
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/linux/system.cc
--- a/src/arch/arm/linux/system.cc      Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/linux/system.cc      Fri Nov 14 03:53:51 2014 -0500
@@ -61,9 +61,9 @@
 using namespace Linux;
 
 LinuxArmSystem::LinuxArmSystem(Params *p)
-    : ArmSystem(p),
+    : ArmSystem(p), dumpStatsPCEvent(nullptr),
       enableContextSwitchStatsDump(p->enable_context_switch_stats_dump),
-      kernelPanicEvent(NULL), kernelOopsEvent(NULL),
+      taskFile(nullptr), kernelPanicEvent(nullptr), kernelOopsEvent(nullptr),
       bootReleaseAddr(p->boot_release_addr)
 {
     if (p->panic_on_panic) {
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/pmu.cc
--- a/src/arch/arm/pmu.cc       Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/pmu.cc       Fri Nov 14 03:53:51 2014 -0500
@@ -95,7 +95,7 @@
     // Flag the event as available in the PMCEID register if it is an
     // architected event.
     if (id < 0x40)
-        reg_pmceid |= (1 << id);
+        reg_pmceid |= (ULL(1) << id);
 }
 
 void
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/process.hh
--- a/src/arch/arm/process.hh   Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/process.hh   Fri Nov 14 03:53:51 2014 -0500
@@ -67,7 +67,6 @@
 class ArmLiveProcess32 : public ArmLiveProcess
 {
   protected:
-    ObjectFile::Arch arch;
     ArmLiveProcess32(LiveProcessParams * params, ObjectFile *objFile,
                      ObjectFile::Arch _arch);
 
@@ -84,7 +83,6 @@
 class ArmLiveProcess64 : public ArmLiveProcess
 {
   protected:
-    ObjectFile::Arch arch;
     ArmLiveProcess64(LiveProcessParams * params, ObjectFile *objFile,
                      ObjectFile::Arch _arch);
 
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/remote_gdb.cc
--- a/src/arch/arm/remote_gdb.cc        Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/remote_gdb.cc        Fri Nov 14 03:53:51 2014 -0500
@@ -160,7 +160,7 @@
 using namespace ArmISA;
 
 RemoteGDB::RemoteGDB(System *_system, ThreadContext *tc)
-    : BaseRemoteGDB(_system, tc, MAX_NUMREGS)
+    : BaseRemoteGDB(_system, tc, MAX_NUMREGS), notTakenBkpt(0), takenBkpt(0)
 {
 }
 
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/stage2_lookup.hh
--- a/src/arch/arm/stage2_lookup.hh     Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/stage2_lookup.hh     Fri Nov 14 03:53:51 2014 -0500
@@ -80,8 +80,8 @@
         bool _functional, TLB::ArmTranslationType _tranType) :
         stage1Tlb(s1Tlb), stage2Tlb(s2Tlb), stage1Te(s1Te), s1Req(_req),
         transState(_transState), mode(_mode), timing(_timing),
-        functional(_functional), tranType(_tranType), fault(NoFault),
-        complete(false), selfDelete(false)
+        functional(_functional), tranType(_tranType), stage2Te(nullptr),
+        fault(NoFault), complete(false), selfDelete(false)
     {
         req.setVirt(0, s1Te.pAddr(s1Req->getVaddr()), s1Req->getSize(),
                     s1Req->getFlags(), s1Req->masterId(), 0);
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/stage2_mmu.cc
--- a/src/arch/arm/stage2_mmu.cc        Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/stage2_mmu.cc        Fri Nov 14 03:53:51 2014 -0500
@@ -108,7 +108,7 @@
 
 Stage2MMU::Stage2Translation::Stage2Translation(Stage2MMU &_parent,
         uint8_t *_data, Event *_event, Addr _oVAddr)
-    : data(_data), event(_event), parent(_parent), oVAddr(_oVAddr),
+    : data(_data), numBytes(0), event(_event), parent(_parent), 
oVAddr(_oVAddr),
     fault(NoFault)
 {
 }
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/system.cc
--- a/src/arch/arm/system.cc    Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/system.cc    Fri Nov 14 03:53:51 2014 -0500
@@ -58,6 +58,7 @@
       _haveLPAE(p->have_lpae),
       _haveVirtualization(p->have_virtualization),
       _haveGenericTimer(p->have_generic_timer),
+      _genericTimer(nullptr),
       _highestELIs64(p->highest_el_is_64),
       _resetAddr64(p->reset_addr_64),
       _physAddrRange64(p->phys_addr_range_64),
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/table_walker.cc
--- a/src/arch/arm/table_walker.cc      Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/table_walker.cc      Fri Nov 14 03:53:51 2014 -0500
@@ -90,7 +90,15 @@
     ;
 }
 
-TableWalker::WalkerState::WalkerState() : stage2Tran(NULL), l2Desc(l1Desc)
+TableWalker::WalkerState::WalkerState() :
+    tc(nullptr), aarch64(false), el(EL0), physAddrRange(0), req(nullptr),
+    asid(0), vmid(0), isHyp(false), transState(nullptr),
+    vaddr(0), vaddr_tainted(0), isWrite(false), isFetch(false), 
isSecure(false),
+    secureLookup(false), rwTable(false), userTable(false), xnTable(false),
+    pxnTable(false), stage2Req(false), doingStage2(false),
+    stage2Tran(nullptr), timing(false), functional(false),
+    mode(BaseTLB::Read), tranType(TLB::NormalTran), l2Desc(l1Desc),
+    delayed(false), tableWalker(nullptr)
 {
 }
 
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/table_walker.hh
--- a/src/arch/arm/table_walker.hh      Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/table_walker.hh      Fri Nov 14 03:53:51 2014 -0500
@@ -106,7 +106,7 @@
         bool _dirty;
 
         /** Default ctor */
-        L1Descriptor()
+        L1Descriptor() : data(0), _dirty(false)
         {
             lookupLevel = L1;
         }
@@ -250,12 +250,13 @@
         bool _dirty;
 
         /** Default ctor */
-        L2Descriptor()
+        L2Descriptor() : data(0), l1Parent(nullptr), _dirty(false)
         {
             lookupLevel = L2;
         }
 
-        L2Descriptor(L1Descriptor &parent) : l1Parent(&parent)
+        L2Descriptor(L1Descriptor &parent) : data(0), l1Parent(&parent),
+                                             _dirty(false)
         {
             lookupLevel = L2;
         }
diff -r aa97958ce2aa -r 47fe87b0cf97 src/arch/arm/tlb.cc
--- a/src/arch/arm/tlb.cc       Fri Nov 14 03:53:48 2014 -0500
+++ b/src/arch/arm/tlb.cc       Fri Nov 14 03:53:51 2014 -0500
@@ -75,6 +75,8 @@
       isStage2(p->is_stage2), stage2Req(false), _attr(0),
       directToStage2(false), tableWalker(p->walker), stage2Tlb(NULL),
       stage2Mmu(NULL), rangeMRU(1), bootUncacheability(false),
+      aarch64(false), aarch64EL(EL0), isPriv(false), isSecure(false),
+      isHyp(false), asid(0), vmid(0), dacr(0),
       miscRegValid(false), curTranType(NormalTran)
 {
     tableWalker->setTlb(this);
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to