diff mbox series

Fix 32096 UBSAN issues in gprofng

Message ID 20240911041656.626072-1-vladimir.mezentsev@oracle.com
State New
Headers
Series Fix 32096 UBSAN issues in gprofng |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Vladimir Mezentsev Sept. 11, 2024, 4:16 a.m. UTC 
  From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>

Fixed UBSAN runtime errors such as:
 - load of value 4294967295, which is not a valid value for type 'Cmsg_warn'
 - null pointer passed as argument 2, which is declared to never be null
 - load of value 4294967295, which is not a valid value for type 'ProfData_type'
 - reference binding to misaligned address 0x00000357583c for type 'long unsigned int', which requires 8 byte alignment

gprofng/ChangeLog
2024-09-09  Vladimir Mezentsev  <vladimir.mezentsev@oracle.com>.

	PR gprofng/32096
	* src/BaseMetric.cc: Fix UBSAN runtime errors.
	* src/BaseMetric.h: Likewise.
	* src/Emsg.h: Likewise.
	* src/Experiment.cc: Likewise.
	* src/Table.h: Likewise.
---
 gprofng/src/BaseMetric.cc |  4 +--
 gprofng/src/BaseMetric.h  |  2 +-
 gprofng/src/Emsg.h        |  1 +
 gprofng/src/Experiment.cc | 54 ++++++++++++++++++++++++---------------
 gprofng/src/Table.h       |  3 ++-
 5 files changed, 40 insertions(+), 24 deletions(-)
diff mbox series

Patch

diff --git a/gprofng/src/BaseMetric.cc b/gprofng/src/BaseMetric.cc
index 08ab883e1eb..ae0ee32adf8 100644
--- a/gprofng/src/BaseMetric.cc
+++ b/gprofng/src/BaseMetric.cc
@@ -212,7 +212,7 @@  BaseMetric::BaseMetric (const char *_cmd, const char *_username,
   clock_unit = CUNIT_NULL; // should it be CUNIT_TIME or 0 or something?
 
   /* we're not going to process packets for derived metrics */
-  packet_type = (ProfData_type) (-1);
+  packet_type = DATA_NONE;
   value_styles = VAL_VALUE;
   valtype = VT_DOUBLE;
   precision = 1000;
@@ -443,7 +443,7 @@  BaseMetric::specify ()
 
   char buf[256];
   char buf2[256];
-  packet_type = (ProfData_type) - 1; // illegal value
+  packet_type = DATA_NONE;
   clock_unit = CUNIT_TIME;
   switch (type)
     {
diff --git a/gprofng/src/BaseMetric.h b/gprofng/src/BaseMetric.h
index 4a866872078..2d7d2f7b7ac 100644
--- a/gprofng/src/BaseMetric.h
+++ b/gprofng/src/BaseMetric.h
@@ -194,7 +194,7 @@  private:
   ValueTag valtype;                 // e.g. VT_LLONG
   long long precision;              // e.g. METRIC_SIG_PRECISION, 1, etc.
   Hwcentry *hw_ctr;                 // HWC definition
-  ProfData_type packet_type;        // e.g. DATA_HWC, or -1 for N/A
+  ProfData_type packet_type;        // e.g. DATA_HWC, or DATA_NONE for N/A
   bool zeroThreshold;               // deadlock stuff
   Presentation_clock_unit clock_unit;
 
diff --git a/gprofng/src/Emsg.h b/gprofng/src/Emsg.h
index 2bd40f911f5..bc562273767 100644
--- a/gprofng/src/Emsg.h
+++ b/gprofng/src/Emsg.h
@@ -38,6 +38,7 @@  class StringBuilder;
 
 typedef enum
 {
+  CMSG_NONE = -1,
   CMSG_WARN = 0,
   CMSG_ERROR,
   CMSG_FATAL,
diff --git a/gprofng/src/Experiment.cc b/gprofng/src/Experiment.cc
index 627a755c88c..eee4eb85a58 100644
--- a/gprofng/src/Experiment.cc
+++ b/gprofng/src/Experiment.cc
@@ -315,7 +315,7 @@  Experiment::ExperimentHandler::ExperimentHandler (Experiment *_exp)
   pDscr = NULL;
   propDscr = NULL;
   text = NULL;
-  mkind = (Cmsg_warn) - 1; // CMSG_NONE
+  mkind = CMSG_NONE;
   mnum = -1;
   mec = -1;
 }
@@ -368,8 +368,7 @@  Experiment::ExperimentHandler::pushElem (Element elem)
 void
 Experiment::ExperimentHandler::popElem ()
 {
-  stack->remove (stack->size () - 1);
-  curElem = stack->fetch (stack->size () - 1);
+  curElem = stack->remove (stack->size () - 1);
 }
 
 void
@@ -1240,7 +1239,7 @@  Experiment::ExperimentHandler::characters (char *ch, int start, int length)
 void
 Experiment::ExperimentHandler::endElement (char*, char*, char*)
 {
-  if (curElem == EL_EVENT && mkind >= 0 && mnum >= 0)
+  if (curElem == EL_EVENT && mkind != CMSG_NONE && mnum >= 0)
     {
       char *str;
       if (mec > 0)
@@ -1262,7 +1261,7 @@  Experiment::ExperimentHandler::endElement (char*, char*, char*)
 	exp->commentq->append (msg);
       else
 	delete msg;
-      mkind = (Cmsg_warn) - 1;
+      mkind = CMSG_NONE;
       mnum = -1;
       mec = -1;
     }
@@ -1398,7 +1397,7 @@  Experiment::Experiment ()
   archiveMap = NULL;
   nnodes = 0;
   nchunks = 0;
-  chunks = 0;
+  chunks = NULL;
   uidHTable = NULL;
   uidnodes = new Vector<UIDnode*>;
   mrecs = new Vector<MapRecord*>;
@@ -4688,26 +4687,36 @@  Experiment::readPacket (Data_window *dwin, Data_window::Span *span)
   return size;
 }
 
+static uint32_t get_v32(char *p)
+{
+  uint32_t v;
+  memcpy (&v, p, sizeof(uint32_t));
+  return v;
+}
+
+static uint64_t get_v64(char *p)
+{
+  uint64_t v;
+  memcpy (&v, p, sizeof(uint64_t));
+  return v;
+}
+
 void
 Experiment::readPacket (Data_window *dwin, char *ptr, PacketDescriptor *pDscr,
 			DataDescriptor *dDscr, int arg, uint64_t pktsz)
 {
-  union Value
-  {
-    uint32_t val32;
-    uint64_t val64;
-  } *v;
-
   long recn = dDscr->addRecord ();
   Vector<FieldDescr*> *fields = pDscr->getFields ();
+  uint32_t v32;
+  uint64_t v64;
   int sz = fields->size ();
   for (int i = 0; i < sz; i++)
     {
       FieldDescr *field = fields->fetch (i);
-      v = (Value*) (ptr + field->offset);
       if (field->propID == arg)
 	{
-	  dDscr->setValue (PROP_NTICK, recn, dwin->decode (v->val32));
+	  v32 = get_v32(ptr + field->offset);
+	  dDscr->setValue (PROP_NTICK, recn, dwin->decode (v32));
 	  dDscr->setValue (PROP_MSTATE, recn, (uint32_t) (field->propID - PROP_UCPU));
 	}
       if (field->propID == PROP_THRID || field->propID == PROP_LWPID
@@ -4718,11 +4727,13 @@  Experiment::readPacket (Data_window *dwin, char *ptr, PacketDescriptor *pDscr,
 	    {
 	    case TYPE_INT32:
 	    case TYPE_UINT32:
-	      tmp64 = dwin->decode (v->val32);
+	      v32 = get_v32 (ptr + field->offset);
+	      tmp64 = dwin->decode (v32);
 	      break;
 	    case TYPE_INT64:
 	    case TYPE_UINT64:
-	      tmp64 = dwin->decode (v->val64);
+	      v64 = get_v64 (ptr + field->offset);
+	      tmp64 = dwin->decode (v64);
 	      break;
 	    case TYPE_STRING:
 	    case TYPE_DOUBLE:
@@ -4743,11 +4754,13 @@  Experiment::readPacket (Data_window *dwin, char *ptr, PacketDescriptor *pDscr,
 	    {
 	    case TYPE_INT32:
 	    case TYPE_UINT32:
-	      dDscr->setValue (field->propID, recn, dwin->decode (v->val32));
+	      v32 = get_v32 (ptr + field->offset);
+	      dDscr->setValue (field->propID, recn, dwin->decode (v32));
 	      break;
 	    case TYPE_INT64:
 	    case TYPE_UINT64:
-	      dDscr->setValue (field->propID, recn, dwin->decode (v->val64));
+	      v64 = get_v64 (ptr + field->offset);
+	      dDscr->setValue (field->propID, recn, dwin->decode (v64));
 	      break;
 	    case TYPE_STRING:
 	      {
@@ -5039,7 +5052,8 @@  Experiment::new_uid_node (uint64_t uid, uint64_t val)
       // Reallocate Node chunk array
       UIDnode** old_chunks = chunks;
       chunks = new UIDnode*[nchunks + NCHUNKSTEP];
-      memcpy (chunks, old_chunks, nchunks * sizeof (UIDnode*));
+      if (old_chunks)
+	memcpy (chunks, old_chunks, nchunks * sizeof (UIDnode*));
       nchunks += NCHUNKSTEP;
       delete[] old_chunks;
       // Clean future pointers
@@ -5855,7 +5869,7 @@  SegMem*
 Experiment::update_ts_in_maps (Vaddr addr, hrtime_t ts)
 {
   Vector<SegMem *> *segMems = (Vector<SegMem *> *) maps->values ();
-  if (!segMems->is_sorted ())
+  if (segMems && !segMems->is_sorted ())
     {
       Dprintf (DEBUG_MAPS, NTXT ("update_ts_in_maps: segMems.size=%lld\n"), (long long) segMems->size ());
       segMems->sort (SegMemCmp);
diff --git a/gprofng/src/Table.h b/gprofng/src/Table.h
index e72034309c3..92f1e78eea6 100644
--- a/gprofng/src/Table.h
+++ b/gprofng/src/Table.h
@@ -71,7 +71,8 @@  enum VType_type
 
 enum ProfData_type
 { // a.k.a "data_id" (not the same as Pckt_type "kind")
-  DATA_SAMPLE,      // Traditional collect "Samples"
+  DATA_NONE = -1,
+  DATA_SAMPLE = 0,  // Traditional collect "Samples"
   DATA_GCEVENT,     // Java Garbage Collection events
   DATA_HEAPSZ,      // heap size tracking based on heap tracing data
   DATA_CLOCK,       // clock profiling data