Skip to content

Commit ad0751b

Browse files
author
Vladimir Mezentsev
committed
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.
1 parent e20c1e4 commit ad0751b

File tree

5 files changed

+40
-24
lines changed

5 files changed

+40
-24
lines changed

‎gprofng/src/BaseMetric.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ BaseMetric::BaseMetric (const char *_cmd, const char *_username,
212212
clock_unit = CUNIT_NULL; // should it be CUNIT_TIME or 0 or something?
213213

214214
/* we're not going to process packets for derived metrics */
215-
packet_type = (ProfData_type) (-1);
215+
packet_type = DATA_NONE;
216216
value_styles = VAL_VALUE;
217217
valtype = VT_DOUBLE;
218218
precision = 1000;
@@ -443,7 +443,7 @@ BaseMetric::specify ()
443443

444444
char buf[256];
445445
char buf2[256];
446-
packet_type = (ProfData_type) - 1; // illegal value
446+
packet_type = DATA_NONE;
447447
clock_unit = CUNIT_TIME;
448448
switch (type)
449449
{

‎gprofng/src/BaseMetric.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ class BaseMetric
194194
ValueTag valtype; // e.g. VT_LLONG
195195
long long precision; // e.g. METRIC_SIG_PRECISION, 1, etc.
196196
Hwcentry *hw_ctr; // HWC definition
197-
ProfData_type packet_type; // e.g. DATA_HWC, or -1 for N/A
197+
ProfData_type packet_type; // e.g. DATA_HWC, or DATA_NONE for N/A
198198
bool zeroThreshold; // deadlock stuff
199199
Presentation_clock_unit clock_unit;
200200

‎gprofng/src/Emsg.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ class StringBuilder;
3838

3939
typedef enum
4040
{
41+
CMSG_NONE = -1,
4142
CMSG_WARN = 0,
4243
CMSG_ERROR,
4344
CMSG_FATAL,

‎gprofng/src/Experiment.cc‎

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ Experiment::ExperimentHandler::ExperimentHandler (Experiment *_exp)
315315
pDscr = NULL;
316316
propDscr = NULL;
317317
text = NULL;
318-
mkind = (Cmsg_warn) - 1; // CMSG_NONE
318+
mkind = CMSG_NONE;
319319
mnum = -1;
320320
mec = -1;
321321
}
@@ -368,8 +368,7 @@ Experiment::ExperimentHandler::pushElem (Element elem)
368368
void
369369
Experiment::ExperimentHandler::popElem ()
370370
{
371-
stack->remove (stack->size () - 1);
372-
curElem = stack->fetch (stack->size () - 1);
371+
curElem = stack->remove (stack->size () - 1);
373372
}
374373

375374
void
@@ -1240,7 +1239,7 @@ Experiment::ExperimentHandler::characters (char *ch, int start, int length)
12401239
void
12411240
Experiment::ExperimentHandler::endElement (char*, char*, char*)
12421241
{
1243-
if (curElem == EL_EVENT && mkind >= 0 && mnum >= 0)
1242+
if (curElem == EL_EVENT && mkind != CMSG_NONE && mnum >= 0)
12441243
{
12451244
char *str;
12461245
if (mec > 0)
@@ -1262,7 +1261,7 @@ Experiment::ExperimentHandler::endElement (char*, char*, char*)
12621261
exp->commentq->append (msg);
12631262
else
12641263
delete msg;
1265-
mkind = (Cmsg_warn) - 1;
1264+
mkind = CMSG_NONE;
12661265
mnum = -1;
12671266
mec = -1;
12681267
}
@@ -1398,7 +1397,7 @@ Experiment::Experiment ()
13981397
archiveMap = NULL;
13991398
nnodes = 0;
14001399
nchunks = 0;
1401-
chunks = 0;
1400+
chunks = NULL;
14021401
uidHTable = NULL;
14031402
uidnodes = new Vector<UIDnode*>;
14041403
mrecs = new Vector<MapRecord*>;
@@ -4688,26 +4687,36 @@ Experiment::readPacket (Data_window *dwin, Data_window::Span *span)
46884687
return size;
46894688
}
46904689

4690+
static uint32_t get_v32(char *p)
4691+
{
4692+
uint32_t v;
4693+
memcpy (&v, p, sizeof(uint32_t));
4694+
return v;
4695+
}
4696+
4697+
static uint64_t get_v64(char *p)
4698+
{
4699+
uint64_t v;
4700+
memcpy (&v, p, sizeof(uint64_t));
4701+
return v;
4702+
}
4703+
46914704
void
46924705
Experiment::readPacket (Data_window *dwin, char *ptr, PacketDescriptor *pDscr,
46934706
DataDescriptor *dDscr, int arg, uint64_t pktsz)
46944707
{
4695-
union Value
4696-
{
4697-
uint32_t val32;
4698-
uint64_t val64;
4699-
} *v;
4700-
47014708
long recn = dDscr->addRecord ();
47024709
Vector<FieldDescr*> *fields = pDscr->getFields ();
4710+
uint32_t v32;
4711+
uint64_t v64;
47034712
int sz = fields->size ();
47044713
for (int i = 0; i < sz; i++)
47054714
{
47064715
FieldDescr *field = fields->fetch (i);
4707-
v = (Value*) (ptr + field->offset);
47084716
if (field->propID == arg)
47094717
{
4710-
dDscr->setValue (PROP_NTICK, recn, dwin->decode (v->val32));
4718+
v32 = get_v32(ptr + field->offset);
4719+
dDscr->setValue (PROP_NTICK, recn, dwin->decode (v32));
47114720
dDscr->setValue (PROP_MSTATE, recn, (uint32_t) (field->propID - PROP_UCPU));
47124721
}
47134722
if (field->propID == PROP_THRID || field->propID == PROP_LWPID
@@ -4718,11 +4727,13 @@ Experiment::readPacket (Data_window *dwin, char *ptr, PacketDescriptor *pDscr,
47184727
{
47194728
case TYPE_INT32:
47204729
case TYPE_UINT32:
4721-
tmp64 = dwin->decode (v->val32);
4730+
v32 = get_v32 (ptr + field->offset);
4731+
tmp64 = dwin->decode (v32);
47224732
break;
47234733
case TYPE_INT64:
47244734
case TYPE_UINT64:
4725-
tmp64 = dwin->decode (v->val64);
4735+
v64 = get_v64 (ptr + field->offset);
4736+
tmp64 = dwin->decode (v64);
47264737
break;
47274738
case TYPE_STRING:
47284739
case TYPE_DOUBLE:
@@ -4743,11 +4754,13 @@ Experiment::readPacket (Data_window *dwin, char *ptr, PacketDescriptor *pDscr,
47434754
{
47444755
case TYPE_INT32:
47454756
case TYPE_UINT32:
4746-
dDscr->setValue (field->propID, recn, dwin->decode (v->val32));
4757+
v32 = get_v32 (ptr + field->offset);
4758+
dDscr->setValue (field->propID, recn, dwin->decode (v32));
47474759
break;
47484760
case TYPE_INT64:
47494761
case TYPE_UINT64:
4750-
dDscr->setValue (field->propID, recn, dwin->decode (v->val64));
4762+
v64 = get_v64 (ptr + field->offset);
4763+
dDscr->setValue (field->propID, recn, dwin->decode (v64));
47514764
break;
47524765
case TYPE_STRING:
47534766
{
@@ -5039,7 +5052,8 @@ Experiment::new_uid_node (uint64_t uid, uint64_t val)
50395052
// Reallocate Node chunk array
50405053
UIDnode** old_chunks = chunks;
50415054
chunks = new UIDnode*[nchunks + NCHUNKSTEP];
5042-
memcpy (chunks, old_chunks, nchunks * sizeof (UIDnode*));
5055+
if (old_chunks)
5056+
memcpy (chunks, old_chunks, nchunks * sizeof (UIDnode*));
50435057
nchunks += NCHUNKSTEP;
50445058
delete[] old_chunks;
50455059
// Clean future pointers
@@ -5855,7 +5869,7 @@ SegMem*
58555869
Experiment::update_ts_in_maps (Vaddr addr, hrtime_t ts)
58565870
{
58575871
Vector<SegMem *> *segMems = (Vector<SegMem *> *) maps->values ();
5858-
if (!segMems->is_sorted ())
5872+
if (segMems && !segMems->is_sorted ())
58595873
{
58605874
Dprintf (DEBUG_MAPS, NTXT ("update_ts_in_maps: segMems.size=%lld\n"), (long long) segMems->size ());
58615875
segMems->sort (SegMemCmp);

‎gprofng/src/Table.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,8 @@ enum VType_type
7171

7272
enum ProfData_type
7373
{ // a.k.a "data_id" (not the same as Pckt_type "kind")
74-
DATA_SAMPLE, // Traditional collect "Samples"
74+
DATA_NONE = -1,
75+
DATA_SAMPLE = 0, // Traditional collect "Samples"
7576
DATA_GCEVENT, // Java Garbage Collection events
7677
DATA_HEAPSZ, // heap size tracking based on heap tracing data
7778
DATA_CLOCK, // clock profiling data

0 commit comments

Comments
 (0)