Bobby R. Bruce has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/29534 )
Change subject: misc: Fixed null-pointer arithmetic error
......................................................................
misc: Fixed null-pointer arithmetic error
Doing arithmetic on a null pointer is undefined behavior in C/C++. Clang
compilers complain when this occurs. As this MACRO is used twice, and
does nothing important, it has been removed in favor of a more simple
solution. A comment has been added explaining the MACRO's removal.
Change-Id: I42d9356179ee0fa5cb20f827af34bb11780ad1a9
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/29534
Maintainer: Bobby R. Bruce <[email protected]>
Reviewed-by: Jason Lowe-Power <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/dev/hsa/hsa_packet_processor.hh
M src/dev/hsa/hw_scheduler.cc
2 files changed, 15 insertions(+), 7 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Bobby R. Bruce: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/dev/hsa/hsa_packet_processor.hh
b/src/dev/hsa/hsa_packet_processor.hh
index 5558d4e..206d9ab 100644
--- a/src/dev/hsa/hsa_packet_processor.hh
+++ b/src/dev/hsa/hsa_packet_processor.hh
@@ -52,11 +52,6 @@
// HSA runtime supports only 5 signals per barrier packet
#define NumSignalsPerBarrier 5
-// This define is copied from hsa runtime (libhsakmt/src/libhsakmt.h)
-// This is the mapping function used by runtime for mapping
-// queueID to dooorbell address
-#define VOID_PTR_ADD32(ptr,n) (void*)((uint32_t*)(ptr) + n)/*ptr + offset*/
-
class HSADevice;
class HWScheduler;
diff --git a/src/dev/hsa/hw_scheduler.cc b/src/dev/hsa/hw_scheduler.cc
index 6b37155..57cf6d1 100644
--- a/src/dev/hsa/hw_scheduler.cc
+++ b/src/dev/hsa/hw_scheduler.cc
@@ -90,7 +90,12 @@
// Map queue ID to doorbell.
// We are only using offset to pio base address as doorbell
// We use the same mapping function used by hsa runtime to do this
mapping
- Addr db_offset = (Addr)(VOID_PTR_ADD32(0, queue_id));
+ //
+ // Originally
+ // #define VOID_PTR_ADD32(ptr,n) \
+ // (void*)((uint32_t*)(ptr) + n)/*ptr + offset*/
+ // (Addr)VOID_PTR_ADD32(0, queue_id)
+ Addr db_offset = queue_id;
if (dbMap.find(db_offset) != dbMap.end()) {
panic("Creating an already existing queue (queueID %d)", queue_id);
}
@@ -333,7 +338,15 @@
void
HWScheduler::unregisterQueue(uint64_t queue_id)
{
- Addr db_offset = (Addr)(VOID_PTR_ADD32(0, queue_id));
+ // Pointer arithmetic on a null pointer is undefined behavior. Clang
+ // compilers therefore complain if the following reads:
+ // `(Addr)(VOID_PRT_ADD32(0, queue_id))`
+ //
+ // Originally
+ // #define VOID_PTR_ADD32(ptr,n) \
+ // (void*)((uint32_t*)(ptr) + n)/*ptr + offset*/
+ // (Addr)VOID_PTR_ADD32(0, queue_id)
+ Addr db_offset = queue_id;
auto dbmap_iter = dbMap.find(db_offset);
if (dbmap_iter == dbMap.end()) {
panic("Destroying a non-existing queue (db_offset %x)",
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/29534
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v20.0.0.0
Gerrit-Change-Id: I42d9356179ee0fa5cb20f827af34bb11780ad1a9
Gerrit-Change-Number: 29534
Gerrit-PatchSet: 4
Gerrit-Owner: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Anthony Gutierrez <[email protected]>
Gerrit-Reviewer: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Matthew Poremba <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-CC: Sooraj Puthoor <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
