From: Bobby R. Bruce <bbruce@ucdavis.edu>
Date: Wed, 27 May 2020 10:46:07 +0000 (-0700)
Subject: misc: Fixed null-pointer arithmetic error
X-Git-Tag: v20.0.0.0~3
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=fc856981d8da4ce0145f1d534c3d981f4bf954da;p=gem5.git

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>
---

diff --git a/src/dev/hsa/hsa_packet_processor.hh b/src/dev/hsa/hsa_packet_processor.hh
index 5558d4eb6..206d9ab84 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 6b371551b..57cf6d1b1 100644
--- a/src/dev/hsa/hw_scheduler.cc
+++ b/src/dev/hsa/hw_scheduler.cc
@@ -90,7 +90,12 @@ HWScheduler::registerNewQueue(uint64_t hostReadIndexPointer,
     // 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 @@ HWScheduler::write(Addr db_addr, uint32_t doorbell_reg)
 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)",