Remove all global sc_events #9
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The sc_event::none variable was introduced in 55da81d. The problem with the introduced implementation is that the created event contains a reference to the simulation context. This simulation context is created by the sc_event constructor through the invocation of sc_get_curr_simcontext() on program startup. This commit proposes a change which tracks the none event as part of the simulation context. The change converts sc_event::none to a static member function which returns a reference to the m_none member of the current simulation context. This eases reset of the simulation context as discussed in accellera-official#8. Neither sc_event::none nor sc_get_curr_simcontext() are currently mandated by IEEE Std 1666-2011. For this reason, this change should not affected existing standard-conforming SystemC programs. Signed-off-by: Sören Tempel <[email protected]>
Similar to sc_event::none, sc_process_handle::non_event is a global sc_event which holds a reference to the simulation context. Thereby complicating reset of this context as discussed in accellera-official#8. This commit couples the sc_process_handle::non_event with the simulation context, thereby easing resets of the latter. At the time of writing, it is unclear to me whether it is strictly necessary to use different sc_event members in sc_simcontext for sc_event::none and sc_process_handle::non_event. Signed-off-by: Sören Tempel <[email protected]>
|
This change seems reasonable, but I would have implemented it a bit differently. I am not a big fan of having references to internal fields in sc_simcontext by methods in other classes, and I certainly would prefer those methods did not change sc_simcontext fields. My preference would be methods in sc_simcontext that return the events, creating them if necessary. An additional question is whether you want the methods in sc_event and sc_process_handle if you can just call the sc_simcontext methods. Also, as far as I can tell a single event for both none and non_event would be fine. |
|
I will close this pull request, as @AndrewGoodrich will pick this up and rework it within the LWG. |
Has there been any progress on this? Will the resulting changes be made open source again? |
|
Yes, and yes. It will appear in the next release.
Andy
… On Nov 11, 2021, at 8:36 AM, nmeum ***@***.***> wrote:
I will close this pull request, as @AndrewGoodrich <https://github.com/AndrewGoodrich> will pick this up and rework it within the LWG.
Has there been any progress on this? Will the resulting changes be made open source again?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#9 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGQR2K3EAPJ3B4Y7TJBYSLULPBHLANCNFSM4RX6XFPA>.
Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
lmailletcontoz
pushed a commit
that referenced
this pull request
Dec 1, 2022
==33083==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7f36df113d73 in sc_dt::vec_cmp(int, unsigned int const*, int,
unsigned int const*)
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/datatypes/int/sc_nbutils.h:427:3
#1 0x7f36df113d73 in sc_dt::vec_skip_and_cmp(int, unsigned int
const*, int, unsigned int const*)
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/datatypes/int/sc_nbutils.h:500:10
#2 0x7f36df113d73 in sc_dt::compare_unsigned(int, int, int, unsigned
int const*, int, int, int, unsigned int const*, int, int)
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/datatypes/int/sc_unsigned.cpp:2090:21
#3 0x7f36df13e1ab in sc_dt::operator==(sc_dt::sc_unsigned const&,
sc_dt::sc_unsigned const&)
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/datatypes/int/sc_unsigned.cpp:1722:7
#4 0x7f36df13e1ab in sc_dt::operator!=(sc_dt::sc_unsigned const&,
sc_dt::sc_unsigned const&)
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/datatypes/int/sc_nbcommon.inc:1930:13
#5 0x4ab2d4 in sc_main
/builds/coseda/systemc/systemc-regressions/scripts/systemc/datatypes/misc/concat/test07/./test07/test07.cpp:119:5
#6 0x7f36dec273e5 in sc_elab_and_sim
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/kernel/sc_main_main.cpp:89:18
#7 0x7f36dec26f38 in main
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/kernel/sc_main.cpp:36:9
#8 0x7f36dd41a492 in __libc_start_main (/lib64/libc.so.6+0x23492)
#9 0x42387d in _start
(/builds/coseda/systemc/systemc-regressions/scripts/systemc/datatypes/misc/concat/test07/systemc.exe+0x42387d)
Uninitialized value was stored to memory at
#0 0x7f36def86e38 in sc_dt::sc_signed::concat_get_data(unsigned
int*, int) const
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/datatypes/int/sc_signed.cpp:337:26
#1 0x4b9fae in sc_dt::sc_concatref::value() const
/opt/systemc-latest/include/sysc/datatypes/misc/sc_concatref.h:265:41
#2 0x4ab087 in sc_main
/builds/coseda/systemc/systemc-regressions/scripts/systemc/datatypes/misc/concat/test07/./test07/test07.cpp:119:5
#3 0x7f36dec273e5 in sc_elab_and_sim
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/kernel/sc_main_main.cpp:89:18
#4 0x7f36dec26f38 in main
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/kernel/sc_main.cpp:36:9
#5 0x7f36dd41a492 in __libc_start_main (/lib64/libc.so.6+0x23492)
Uninitialized value was created by a heap allocation
#0 0x4a8abb in operator new(unsigned long)
(/builds/coseda/systemc/systemc-regressions/scripts/systemc/datatypes/misc/concat/test07/systemc.exe+0x4a8abb)
#1 0x7f36debf6f3b in sc_core::sc_byte_heap::initialize(unsigned
long) /builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/utils/sc_temporary.h:96:19
#2 0x7f36debf6f3b in sc_core::sc_byte_heap::sc_byte_heap(unsigned
long) /builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/utils/sc_temporary.h:114:3
#3 0x7f36debf6f3b in __cxx_global_var_init
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/datatypes/misc/sc_concatref.cpp:55:30
#4 0x7f36debf6f3b in _GLOBAL__sub_I_sc_concatref.cpp
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/datatypes/misc/sc_concatref.cpp
#5 0x7f36df4ae8b9 in call_init.part.0
(/lib64/ld-linux-x86-64.so.2+0xf8b9)
SUMMARY: MemorySanitizer: use-of-uninitialized-value
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/datatypes/int/sc_nbutils.h:427:3
in sc_dt::vec_cmp(int, unsigned int const*, int, unsigned int const*)
Exiting
==39132==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x6ba6ac in tlm_utils::simple_target_socket_b<ExplicitATTarget,
32u, tlm::tlm_base_protocol_types,
(sc_core::sc_port_policy)0>::bw_process::nb_transport_bw(tlm::tlm_generic_payload&,
tlm::tlm_phase&, sc_core::sc_time&)
/opt/systemc-latest/include/tlm_utils/simple_target_socket.h:157:13
#1 0x69bf2b in ExplicitATTarget::beginResponse()
/builds/coseda/systemc/systemc-regressions/include/tlm/ExplicitATTarget.h:133:19
#2 0x7ff92cf43a94 in sc_core::sc_process_b::semantics()
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/kernel/sc_process.h:685:9
#3 0x7ff92cf43a94 in sc_core::sc_thread_cor_fn(void*)
/builds/coseda/systemc/systemc-latest/objdir/src/sysc/../../../src/sysc/kernel/sc_thread_process.cpp:117:23
#4 0x7ff92d4be4bc
(/opt/systemc-latest/lib-linux64/libsystemc-2.3.4.so+0x6b74bc)
Uninitialized value was created by an allocation of 'target7' in the
stack frame of function 'sc_main'
#0 0x4f7fe0 in sc_main
/builds/coseda/systemc/systemc-regressions/scripts/tlm/bus_dmi/./bus_dmi/bus_dmi.cpp:37
SUMMARY: MemorySanitizer: use-of-uninitialized-value
/opt/systemc-latest/include/tlm_utils/simple_target_socket.h:157:13 in
tlm_utils::simple_target_socket_b<ExplicitATTarget, 32u,
tlm::tlm_base_protocol_types,
(sc_core::sc_port_policy)0>::bw_process::nb_transport_bw(tlm::tlm_generic_payload&,
tlm::tlm_phase&, sc_core::sc_time&)
Exiting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The changes proposed here remove all(?) global
sc_events (sc_event::noneandsc_process_handle::non_event) by converting them to functions returning a reference to ansc_eventin the current simulation context. This eases reset of the simulation context, as discussed in #8, as thesesc_events no longer contain any reference to the initial simulation context.A useful side effect of these changes: Previously the simulation context was created on program startup by the
sc_eventconstructor (through the invocation ofsc_get_curr_simcontext()) by the aforementioned globalsc_events. With these changes applied it is created on the first invocation ofsc_get_curr_simcontext().It is currently unclear to me if it would be possible to return the same reference for
sc_event::none()andsc_process_handle::non_event(), currently two different class members in thesc_simcontextare used.To the best of my knowledge neither
sc_event::nonenorsc_process_handle::non_eventare mandated by IEEE Std 1666-2011. For this reason, the proposed changes should not affect existing standard-conforming SystemC programs.