From fc856981d8da4ce0145f1d534c3d981f4bf954da Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Wed, 27 May 2020 03:46:07 -0700 Subject: [PATCH] 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 Reviewed-by: Jason Lowe-Power Tested-by: kokoro --- src/dev/hsa/hsa_packet_processor.hh | 5 ----- src/dev/hsa/hw_scheduler.cc | 17 +++++++++++++++-- 2 files changed, 15 insertions(+), 7 deletions(-) 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)", -- 2.30.2