Fix 32096 UBSAN issues in gprofng

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.

diff --git a/gprofng/src/BaseMetric.cc b/gprofng/src/BaseMetric.cc
index 08ab883e1ebe5d8787674e19ef5e9f7a15a98907..ae0ee32adf8e6e95ac5a0c13446711257440ade7 100644 (file)
--- 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 4a8668720788fc39ca2487cb602215027068afa9..2d7d2f7b7ac22da91a5d70a471eb688baa00816f 100644 (file)
--- 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 2bd40f911f54807d91fcb863b61bbe9b479fb7d0..bc562273767807fe5c1a511abf77364f90ae0099 100644 (file)
--- 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 627a755c88cff265c3a5ec74adc96f01ace49543..eee4eb85a5855fa068516564cc595fa61ce737b1 100644 (file)
--- 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 e72034309c3e83afbb9640964705748c4e95b911..92f1e78eea6fd8b74da8fe17834cc12c910ed7bd 100644 (file)
--- 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