diff options
author | Vladimir Mezentsev <vladimir.mezentsev@oracle.com> | 2024-09-10 21:05:19 -0700 |
---|---|---|
committer | Vladimir Mezentsev <vladimir.mezentsev@oracle.com> | 2024-09-11 12:08:16 -0700 |
commit | ad0751b6ecf40cb94ecb693d1acfc38fd223f408 (patch) | |
tree | c8047ebfc282c882fa3d0cf22cfda88e67c26d3d | |
parent | e20c1e410ba1e689cdc0aaf6becc8fd041608aa5 (diff) | |
download | gdb-ad0751b6ecf40cb94ecb693d1acfc38fd223f408.zip gdb-ad0751b6ecf40cb94ecb693d1acfc38fd223f408.tar.gz gdb-ad0751b6ecf40cb94ecb693d1acfc38fd223f408.tar.bz2 |
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.
-rw-r--r-- | gprofng/src/BaseMetric.cc | 4 | ||||
-rw-r--r-- | gprofng/src/BaseMetric.h | 2 | ||||
-rw-r--r-- | gprofng/src/Emsg.h | 1 | ||||
-rw-r--r-- | gprofng/src/Experiment.cc | 54 | ||||
-rw-r--r-- | gprofng/src/Table.h | 3 |
5 files changed, 40 insertions, 24 deletions
diff --git a/gprofng/src/BaseMetric.cc b/gprofng/src/BaseMetric.cc index 08ab883..ae0ee32 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 4a86687..2d7d2f7 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 2bd40f9..bc56227 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 627a755c..eee4eb8 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 e720343..92f1e78 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 |