commit fc856981d8da4ce0145f1d534c3d981f4bf954da
author Bobby R. Bruce <bbruce@ucdavis.edu> Wed May 27 03:46:07 2020 -0700
committer Bobby R. Bruce <bbruce@ucdavis.edu> Thu May 28 04:48:54 2020 +0000
tree ffdfec50e498b424df8b204d0561f3b1645e4344
parent a8fb7a0c1d42aa335643bc2cf1b80ac36ea715ec

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 <bbruce@ucdavis.edu>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>

src/dev/hsa/hsa_packet_processor.hh

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;
 

src/dev/hsa/hw_scheduler.cc

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)",