Fix 32096 UBSAN issues in gprofng

Message ID 20240918044245.1509765-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. 18, 2024, 4:42 a.m. UTC
  From: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>

Fixed UBSAN runtime errors such as:
 - member call on address which does not point to an object of type 'Vector'
 - load of misaligned address 0x623e5a670173 for type 'int', which requires 4 byte alignment

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

	PR gprofng/32096
	* libcollector/unwind.c: Fix UBSAN runtime errors.
	* src/CallStack.cc (add_stack_java, add_stack_java_epilogue):
	Change argument type to Vector<Histable*>*.
	* src/Experiment.cc (update_ts_in_maps): Change variable type.
	* src/Experiment.h: Change field type to Vector<Histable*>*.
---
 gprofng/libcollector/unwind.c |  4 ++--
 gprofng/src/CallStack.cc      | 33 ++++++++++++++++++++-------------
 gprofng/src/Experiment.cc     |  6 +++---
 gprofng/src/Experiment.h      |  2 +-
 4 files changed, 26 insertions(+), 19 deletions(-)
  

Patch

diff --git a/gprofng/libcollector/unwind.c b/gprofng/libcollector/unwind.c
index 55fa2e9e9e7..952d26205b5 100644
--- a/gprofng/libcollector/unwind.c
+++ b/gprofng/libcollector/unwind.c
@@ -1555,8 +1555,8 @@  read_int (unsigned char *pc, int w)
   if (w == 1)
     return *((char *) pc);
   if (w == 2)
-    return *(short*) pc;
-  return *(int*) pc;
+    return pc[0] | (pc[1] << 8);
+  return pc[0] | (pc[1] << 8) | (pc[2] << 16) | (pc[3] << 24);
 }
 
 /* Return codes */
diff --git a/gprofng/src/CallStack.cc b/gprofng/src/CallStack.cc
index 6212b5bbe45..5bfafb7b2ca 100644
--- a/gprofng/src/CallStack.cc
+++ b/gprofng/src/CallStack.cc
@@ -146,13 +146,17 @@  private:
   CallStackNode *find_preg_stack (uint64_t);
   // objs are in the root..leaf order
   void *add_stack_d (Vector<Histable*> *objs);
-  void add_stack_java (DataDescriptor *dDscr, long idx, FramePacket *frp, hrtime_t tstamp, uint32_t thrid, Vector<DbeInstr*>* natpcs, bool natpc_added, cstk_ctx_chunk *cstCtxChunk);
-  void add_stack_java_epilogue (DataDescriptor *dDscr, long idx, FramePacket *frp, hrtime_t tstamp, uint32_t thrid, Vector<DbeInstr*>* natpcs, Vector<Histable*>* jpcs, bool natpc_added);
+  void add_stack_java (DataDescriptor *dDscr, long idx, FramePacket *frp,
+	hrtime_t tstamp, uint32_t thrid, Vector<Histable*>* natpcs,
+	bool natpc_added, cstk_ctx_chunk *cstCtxChunk);
+  void add_stack_java_epilogue (DataDescriptor *dDscr, long idx,
+	FramePacket *frp, hrtime_t tstamp, uint32_t thrid,
+	Vector<Histable*>* natpcs, Vector<Histable*>* jpcs, bool natpc_added);
 
   // Adjust HW counter event to find better trigger PC, etc.
   DbeInstr *adjustEvent (DbeInstr *leafPC, DbeInstr * candPC,
 			 Vaddr &eventEA, int abst_type);
-  Vector<DbeInstr*> *natpcsP;
+  Vector<Histable*> *natpcsP;
   Vector<Histable*> *jpcsP;
 };
 
@@ -335,7 +339,7 @@  CallStackP::find_preg_stack (uint64_t prid)
 void
 CallStackP::add_stack_java (DataDescriptor *dDscr, long idx, FramePacket *frp,
 			    hrtime_t tstamp, uint32_t thrid,
-			    Vector<DbeInstr*>* natpcs, bool natpc_added,
+			    Vector<Histable*>* natpcs, bool natpc_added,
 			    cstk_ctx_chunk *cstCtxChunk)
 {
   Vector<Histable*> *jpcs = NULL;
@@ -387,7 +391,7 @@  CallStackP::add_stack_java (DataDescriptor *dDscr, long idx, FramePacket *frp,
 	      bool found = false;
 	      for (; nind >= 0; nind--)
 		{
-		  DbeInstr *nat_addr = natpcs->fetch (nind);
+		  DbeInstr *nat_addr = (DbeInstr *) natpcs->fetch (nind);
 		  if (0 == nat_addr)
 		    continue;
 		  Function *nat_func = nat_addr->func;
@@ -415,12 +419,14 @@  CallStackP::add_stack_java (DataDescriptor *dDscr, long idx, FramePacket *frp,
 // It adds the native and java stacks to the stackmap
 
 void
-CallStackP::add_stack_java_epilogue (DataDescriptor *dDscr, long idx, FramePacket *frp, hrtime_t tstamp, uint32_t thrid, Vector<DbeInstr*>* natpcs, Vector<Histable*> *jpcs, bool natpc_added)
+CallStackP::add_stack_java_epilogue (DataDescriptor *dDscr, long idx,
+	FramePacket *frp, hrtime_t tstamp, uint32_t thrid,
+	Vector<Histable*>* natpcs, Vector<Histable*> *jpcs, bool natpc_added)
 {
   CallStackNode *node = NULL;
   if (!natpc_added)
     {
-      node = (CallStackNode *) add_stack ((Vector<Histable*>*)natpcs);
+      node = (CallStackNode *) add_stack (natpcs);
       dDscr->setObjValue (PROP_MSTACK, idx, node);
       dDscr->setObjValue (PROP_XSTACK, idx, node);
       dDscr->setObjValue (PROP_USTACK, idx, node);
@@ -469,7 +475,7 @@  void
 CallStackP::add_stack (DataDescriptor *dDscr, long idx, FramePacket *frp,
 		       cstk_ctx_chunk* cstCtxChunk)
 {
-  Vector<DbeInstr*> *natpcs = NULL;
+  Vector<Histable*> *natpcs = NULL;
   cstk_ctx *cstctx = NULL;
   int stack_size = frp->stackSize ();
   if (cstCtxChunk != NULL)
@@ -485,7 +491,7 @@  CallStackP::add_stack (DataDescriptor *dDscr, long idx, FramePacket *frp,
       // [leaf_pc .. root_pc] == [0..stack_size-1]
       // Leave room for a possible "truncated" frame
       if (natpcsP == NULL)
-	natpcsP = new Vector<DbeInstr*>;
+	natpcsP = new Vector<Histable*>;
       natpcs = natpcsP;
       natpcs->reset ();
     }
@@ -632,7 +638,7 @@  CallStackP::add_stack (DataDescriptor *dDscr, long idx, FramePacket *frp,
       natpcs->append (funwf->find_dbeinstr (0, 0));
     }
 
-  CallStackNode *node = (CallStackNode*) add_stack ((Vector<Histable*>*)natpcs);
+  CallStackNode *node = (CallStackNode*) add_stack (natpcs);
   dDscr->setObjValue (PROP_MSTACK, idx, node);
   dDscr->setObjValue (PROP_XSTACK, idx, node);
   dDscr->setObjValue (PROP_USTACK, idx, node);
@@ -813,7 +819,8 @@  CallStackP::add_stack (DataDescriptor *dDscr, long idx, FramePacket *frp,
 	    bool inOMP = false;
 	    for (btm = 0; btm < natpcs->size (); btm++)
 	      {
-		LoadObject *lo = natpcs->fetch (btm)->func->module->loadobject;
+		DbeInstr *instr = (DbeInstr *) natpcs->fetch (btm);
+		LoadObject *lo = instr->func->module->loadobject;
 		if (!inOMP)
 		  {
 		    if (lo->flags & SEG_FLAG_OMP)
@@ -854,7 +861,7 @@  CallStackP::add_stack (DataDescriptor *dDscr, long idx, FramePacket *frp,
 		    // Process the entire nat_stack. Skip libthread.
 		    for (top = natpcs->size () - 1; top >= 0; top--)
 		      {
-			DbeInstr *instr = natpcs->fetch (top);
+			DbeInstr *instr = (DbeInstr *) natpcs->fetch (top);
 			if (instr->func->module->loadobject->flags & SEG_FLAG_OMP)
 			  break;
 		      }
@@ -886,7 +893,7 @@  CallStackP::add_stack (DataDescriptor *dDscr, long idx, FramePacket *frp,
 	}
       for (int i = btm; i <= top; ++i)
 	{
-	  DbeInstr *instr = natpcs->fetch (i);
+	  DbeInstr *instr = (DbeInstr *) natpcs->fetch (i);
 	  if (instr->func->module->loadobject->flags & SEG_FLAG_OMP)
 	    continue; // Skip all frames from libmtsk
 	  omppcs->append (instr);
diff --git a/gprofng/src/Experiment.cc b/gprofng/src/Experiment.cc
index eee4eb85a58..3e1e1a7d39e 100644
--- a/gprofng/src/Experiment.cc
+++ b/gprofng/src/Experiment.cc
@@ -5868,7 +5868,7 @@  SegMemCmp (const void *a, const void *b)
 SegMem*
 Experiment::update_ts_in_maps (Vaddr addr, hrtime_t ts)
 {
-  Vector<SegMem *> *segMems = (Vector<SegMem *> *) maps->values ();
+  Vector<void *> *segMems = maps->values ();
   if (segMems && !segMems->is_sorted ())
     {
       Dprintf (DEBUG_MAPS, NTXT ("update_ts_in_maps: segMems.size=%lld\n"), (long long) segMems->size ());
@@ -5876,12 +5876,12 @@  Experiment::update_ts_in_maps (Vaddr addr, hrtime_t ts)
     }
   for (int i = 0, sz = segMems ? segMems->size () : 0; i < sz; i++)
     {
-      SegMem *sm = segMems->fetch (i);
+      SegMem *sm = (SegMem *) segMems->fetch (i);
       if (ts < sm->unload_time)
 	{
 	  for (; i < sz; i++)
 	    {
-	      sm = segMems->fetch (i);
+	      sm = (SegMem *) segMems->fetch (i);
 	      if ((addr >= sm->base) && (addr < sm->base + sm->size))
 		{
 		  Dprintf (DEBUG_MAPS,
diff --git a/gprofng/src/Experiment.h b/gprofng/src/Experiment.h
index e2fa30dc8d5..b98d373bc6d 100644
--- a/gprofng/src/Experiment.h
+++ b/gprofng/src/Experiment.h
@@ -65,7 +65,7 @@  template <class ITEM> class Vector;
 // operate on the next stage
 typedef struct
 {
-  Vector<DbeInstr*> *natpcs;
+  Vector<Histable*> *natpcs;
   Vector<Histable*> *jpcs;
   long idx;
   FramePacket *frp;