Yu-hsin Wang has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/54063 )

Change subject: systemc: Allocate event members of scheduler on heap
......................................................................

systemc: Allocate event members of scheduler on heap

GCC11 introduces a new warning, free-nonheap-object, which would check
if the code potentially calls delete with a nonheap object. The
scheduler is a global object, and its events member full to this case.

Here's a simplified example.
https://godbolt.org/z/q6GqEfETa

There are two ways to fix it. One is allocate the global scheduler on
heap. The other is allocate the event members on heap. Here we choose
the second way. The reason is even we move the scheduler to heap. The
code is still potentially calls delete to a member of a class which
doesn't make sense.

This cl fixes the problem.

Change-Id: I6c5fca36b63fa416197a0a54d1425ff96540a9f6
---
M src/systemc/core/scheduler.cc
M src/systemc/core/scheduler.hh
2 files changed, 87 insertions(+), 47 deletions(-)



diff --git a/src/systemc/core/scheduler.cc b/src/systemc/core/scheduler.cc
index 42a2ca4..1398fa04 100644
--- a/src/systemc/core/scheduler.cc
+++ b/src/systemc/core/scheduler.cc
@@ -27,6 +27,8 @@

 #include "systemc/core/scheduler.hh"

+#include <memory>
+
 #include "base/fiber.hh"
 #include "base/logging.hh"
 #include "sim/eventq.hh"
@@ -44,14 +46,20 @@
 {

 Scheduler::Scheduler() :
-    eq(nullptr), readyEvent(this, false, ReadyPriority),
-    pauseEvent(this, false, PausePriority),
-    stopEvent(this, false, StopPriority), _throwUp(nullptr),
-    starvationEvent(this, false, StarvationPriority),
+    eq(nullptr),
+    readyEvent(std::make_unique<ReadyEvent>(this, false, ReadyPriority)),
+    pauseEvent(std::make_unique<PauseEvent>(this, false, PausePriority)),
+    stopEvent(std::make_unique<StopEvent>(this, false, StopPriority)),
+    _throwUp(nullptr),
+    starvationEvent(std::make_unique<StarvationEvent>(
+        this, false, StarvationPriority)),
     _elaborationDone(false), _started(false), _stopNow(false),
     _status(StatusOther), maxTick(gem5::MaxTick),
-    maxTickEvent(this, false, MaxTickPriority),
-    timeAdvancesEvent(this, false, TimeAdvancesPriority), _numCycles(0),
+    maxTickEvent(std::make_unique<MaxTickEvent>(
+        this, false, MaxTickPriority)),
+    timeAdvancesEvent(std::make_unique<TimeAdvancesEvent>(
+        this, false, TimeAdvancesPriority)),
+    _numCycles(0),
     _changeStamp(0), _current(nullptr), initDone(false), runToTime(true),
     runOnce(false)
 {}
@@ -79,18 +87,18 @@
     timeSlots.clear();

     // gem5 events.
-    if (readyEvent.scheduled())
-        deschedule(&readyEvent);
-    if (pauseEvent.scheduled())
-        deschedule(&pauseEvent);
-    if (stopEvent.scheduled())
-        deschedule(&stopEvent);
-    if (starvationEvent.scheduled())
-        deschedule(&starvationEvent);
-    if (maxTickEvent.scheduled())
-        deschedule(&maxTickEvent);
-    if (timeAdvancesEvent.scheduled())
-        deschedule(&timeAdvancesEvent);
+    if (readyEvent->scheduled())
+        deschedule(readyEvent.get());
+    if (pauseEvent->scheduled())
+        deschedule(pauseEvent.get());
+    if (stopEvent->scheduled())
+        deschedule(stopEvent.get());
+    if (starvationEvent->scheduled())
+        deschedule(starvationEvent.get());
+    if (maxTickEvent->scheduled())
+        deschedule(maxTickEvent.get());
+    if (timeAdvancesEvent->scheduled())
+        deschedule(timeAdvancesEvent.get());

     Process *p;
     while ((p = initList.getNext()))
@@ -266,20 +274,20 @@
 Scheduler::scheduleReadyEvent()
 {
     // Schedule the evaluate and update phases.
-    if (!readyEvent.scheduled()) {
-        schedule(&readyEvent);
-        if (starvationEvent.scheduled())
-            deschedule(&starvationEvent);
+    if (!readyEvent->scheduled()) {
+        schedule(readyEvent.get());
+        if (starvationEvent->scheduled())
+            deschedule(starvationEvent.get());
     }
 }

 void
 Scheduler::scheduleStarvationEvent()
 {
-    if (!starvationEvent.scheduled()) {
-        schedule(&starvationEvent);
-        if (readyEvent.scheduled())
-            deschedule(&readyEvent);
+    if (!starvationEvent->scheduled()) {
+        schedule(starvationEvent.get());
+        if (readyEvent->scheduled())
+            deschedule(readyEvent.get());
     }
 }

@@ -416,20 +424,20 @@
         kernel->status(::sc_core::SC_RUNNING);
     }

-    schedule(&maxTickEvent, maxTick);
+    schedule(maxTickEvent.get(), maxTick);
     scheduleTimeAdvancesEvent();

     // Return to gem5 to let it run events, etc.
     gem5::Fiber::primaryFiber()->run();

-    if (pauseEvent.scheduled())
-        deschedule(&pauseEvent);
-    if (stopEvent.scheduled())
-        deschedule(&stopEvent);
-    if (maxTickEvent.scheduled())
-        deschedule(&maxTickEvent);
-    if (starvationEvent.scheduled())
-        deschedule(&starvationEvent);
+    if (pauseEvent->scheduled())
+        deschedule(pauseEvent.get());
+    if (stopEvent->scheduled())
+        deschedule(stopEvent.get());
+    if (maxTickEvent->scheduled())
+        deschedule(maxTickEvent.get());
+    if (starvationEvent->scheduled())
+        deschedule(starvationEvent.get());

     if (_throwUp) {
         const ::sc_core::sc_report *to_throw = _throwUp;
@@ -449,10 +457,10 @@
 void
 Scheduler::schedulePause()
 {
-    if (pauseEvent.scheduled())
+    if (pauseEvent->scheduled())
         return;

-    schedule(&pauseEvent);
+    schedule(pauseEvent.get());
 }

 void
@@ -472,7 +480,7 @@
 void
 Scheduler::scheduleStop(bool finish_delta)
 {
-    if (stopEvent.scheduled())
+    if (stopEvent->scheduled())
         return;

     if (!finish_delta) {
@@ -481,7 +489,7 @@
         // pending activity.
         clear();
     }
-    schedule(&stopEvent);
+    schedule(stopEvent.get());
 }

 void
diff --git a/src/systemc/core/scheduler.hh b/src/systemc/core/scheduler.hh
index 6eabb56..c0a8602 100644
--- a/src/systemc/core/scheduler.hh
+++ b/src/systemc/core/scheduler.hh
@@ -32,6 +32,7 @@
 #include <functional>
 #include <list>
 #include <map>
+#include <memory>
 #include <mutex>
 #include <set>
 #include <vector>
@@ -465,13 +466,16 @@
     }

     void runReady();
-    gem5::EventWrapper<Scheduler, &Scheduler::runReady> readyEvent;
+    using ReadyEvent = gem5::EventWrapper<Scheduler, &Scheduler::runReady>;
+    std::unique_ptr<ReadyEvent> readyEvent;
     void scheduleReadyEvent();

     void pause();
     void stop();
-    gem5::EventWrapper<Scheduler, &Scheduler::pause> pauseEvent;
-    gem5::EventWrapper<Scheduler, &Scheduler::stop> stopEvent;
+    using PauseEvent = gem5::EventWrapper<Scheduler, &Scheduler::pause>;
+    std::unique_ptr<PauseEvent> pauseEvent;
+    using StopEvent = gem5::EventWrapper<Scheduler, &Scheduler::stop>;
+    std::unique_ptr<StopEvent> stopEvent;

     const ::sc_core::sc_report *_throwUp;

@@ -484,7 +488,8 @@
                  timeSlots.front()->targeted_when > maxTick) &&
                 initList.empty());
     }
-    gem5::EventWrapper<Scheduler, &Scheduler::pause> starvationEvent;
+ using StarvationEvent = gem5::EventWrapper<Scheduler, &Scheduler::pause>;
+    std::unique_ptr<StarvationEvent> starvationEvent;
     void scheduleStarvationEvent();

     bool _elaborationDone;
@@ -502,15 +507,18 @@
             _changeStamp++;
         pause();
     }
-    gem5::EventWrapper<Scheduler, &Scheduler::maxTickFunc> maxTickEvent;
+ using MaxTickEvent = gem5::EventWrapper<Scheduler, &Scheduler::maxTickFunc>;
+    std::unique_ptr<MaxTickEvent> maxTickEvent;

     void timeAdvances() { trace(false); }
- gem5::EventWrapper<Scheduler, &Scheduler::timeAdvances> timeAdvancesEvent;
+    using TimeAdvancesEvent = gem5::EventWrapper<Scheduler,
+                                                 &Scheduler::timeAdvances>;
+    std::unique_ptr<TimeAdvancesEvent> timeAdvancesEvent;
     void
     scheduleTimeAdvancesEvent()
     {
-        if (!traceFiles.empty() && !timeAdvancesEvent.scheduled())
-            schedule(&timeAdvancesEvent);
+        if (!traceFiles.empty() && !timeAdvancesEvent->scheduled())
+            schedule(timeAdvancesEvent.get());
     }

     uint64_t _numCycles;

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

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I6c5fca36b63fa416197a0a54d1425ff96540a9f6
Gerrit-Change-Number: 54063
Gerrit-PatchSet: 1
Gerrit-Owner: Yu-hsin Wang <yuhsi...@google.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to