Hello Andreas Sandberg,

I'd like you to do a code review. Please visit

    https://gem5-review.googlesource.com/6161

to review the following change.


Change subject: arch-arm: Add assertions when extracting an ArmSystem from a TC
......................................................................

arch-arm: Add assertions when extracting an ArmSystem from a TC

If ArmSystem utility functions are used for non-ARM system, we want a
predictable simulation failure.  Previously relying on deferencing
nullptr.

Change-Id: Id382d0c1dceefee8f74d070c205c7b43b83ab215
Signed-off-by: Giacomo Travaglini <giacomo.travagl...@arm.com>
Reviewed-by: Andreas Sandberg <andreas.sandb...@arm.com>
---
M src/arch/arm/system.cc
M src/arch/arm/system.hh
2 files changed, 25 insertions(+), 30 deletions(-)



diff --git a/src/arch/arm/system.cc b/src/arch/arm/system.cc
index 50d0af9..1fe7060 100644
--- a/src/arch/arm/system.cc
+++ b/src/arch/arm/system.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2012-2013, 2015 ARM Limited
+ * Copyright (c) 2010, 2012-2013, 2015,2017 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -169,15 +169,18 @@
     }
 }

+ArmSystem*
+ArmSystem::getArmSystem(ThreadContext *tc)
+{
+    ArmSystem *a_sys = dynamic_cast<ArmSystem *>(tc->getSystemPtr());
+    assert(a_sys);
+    return a_sys;
+}
+
 bool
 ArmSystem::haveSecurity(ThreadContext *tc)
 {
-    if (!FullSystem)
-        return false;
-
-    ArmSystem *a_sys = dynamic_cast<ArmSystem *>(tc->getSystemPtr());
-    assert(a_sys);
-    return a_sys->haveSecurity();
+    return FullSystem? getArmSystem(tc)->haveSecurity() : false;
 }


@@ -201,63 +204,49 @@
 bool
 ArmSystem::haveLPAE(ThreadContext *tc)
 {
-    if (!FullSystem)
-        return false;
-
-    ArmSystem *a_sys = dynamic_cast<ArmSystem *>(tc->getSystemPtr());
-    assert(a_sys);
-    return a_sys->haveLPAE();
+    return FullSystem? getArmSystem(tc)->haveLPAE() : false;
 }

 bool
 ArmSystem::haveVirtualization(ThreadContext *tc)
 {
-    if (!FullSystem)
-        return false;
-
-    ArmSystem *a_sys = dynamic_cast<ArmSystem *>(tc->getSystemPtr());
-    assert(a_sys);
-    return a_sys->haveVirtualization();
+    return FullSystem? getArmSystem(tc)->haveVirtualization() : false;
 }

 bool
 ArmSystem::highestELIs64(ThreadContext *tc)
 {
-    return FullSystem ?
-        dynamic_cast<ArmSystem *>(tc->getSystemPtr())->highestELIs64() :
-        true;
+    return FullSystem? getArmSystem(tc)->highestELIs64() : true;
 }

 ExceptionLevel
 ArmSystem::highestEL(ThreadContext *tc)
 {
-    return FullSystem ?
-        dynamic_cast<ArmSystem *>(tc->getSystemPtr())->highestEL() :
-        EL1;
+    return FullSystem? getArmSystem(tc)->highestEL() : EL1;
 }

 Addr
 ArmSystem::resetAddr64(ThreadContext *tc)
 {
-    return dynamic_cast<ArmSystem *>(tc->getSystemPtr())->resetAddr64();
+    return getArmSystem(tc)->resetAddr64();
 }

 uint8_t
 ArmSystem::physAddrRange(ThreadContext *tc)
 {
-    return dynamic_cast<ArmSystem *>(tc->getSystemPtr())->physAddrRange();
+    return getArmSystem(tc)->physAddrRange();
 }

 Addr
 ArmSystem::physAddrMask(ThreadContext *tc)
 {
-    return dynamic_cast<ArmSystem *>(tc->getSystemPtr())->physAddrMask();
+    return getArmSystem(tc)->physAddrMask();
 }

 bool
 ArmSystem::haveLargeAsid64(ThreadContext *tc)
 {
- return dynamic_cast<ArmSystem *>(tc->getSystemPtr())->haveLargeAsid64();
+    return getArmSystem(tc)->haveLargeAsid64();
 }

 ArmSystem *
diff --git a/src/arch/arm/system.hh b/src/arch/arm/system.hh
index 9776933..b81ec05 100644
--- a/src/arch/arm/system.hh
+++ b/src/arch/arm/system.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2012-2013, 2015-2016 ARM Limited
+ * Copyright (c) 2010, 2012-2013, 2015-2017 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -228,6 +228,12 @@
      */
     const AddrRange &m5opRange() const { return _m5opRange; }

+    /**
+     * Returns a valid ArmSystem pointer if using ARM ISA, it fails
+     * otherwise.
+     */
+    static ArmSystem* getArmSystem(ThreadContext *tc);
+
/** Returns true if the system of a specific thread context implements the
      * Security Extensions
      */

--
To view, visit https://gem5-review.googlesource.com/6161
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id382d0c1dceefee8f74d070c205c7b43b83ab215
Gerrit-Change-Number: 6161
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to