RE: [PATCH v3 04/12] btrace: Handle stepping and goto for auxiliary instructions.

["Willgerodt, Felix" <felix.willgerodt@intel.com>]

> -----Original Message-----
> From: Metzger, Markus T <markus.t.metzger@intel.com>
> Sent: Donnerstag, 12. August 2021 13:07
> To: Willgerodt, Felix <felix.willgerodt@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: RE: [PATCH v3 04/12] btrace: Handle stepping and goto for auxiliary
> instructions.
> 
> Thanks, Felix,
> 
> The patch looks good with one issue.
> 
> >@@ -2400,12 +2405,25 @@ record_btrace_single_step_forward (struct
> >thread_info *tp)
> > 	 of the execution history.  */
> >       steps = btrace_insn_next (replay, 1);
> >       if (steps == 0)
> >+        {
> >+          *replay = start;
> >+          return btrace_step_no_history ();
> >+        }
> >+
> >+      const struct btrace_insn *insn = btrace_insn_get (replay);
> >+
> >+      /* If we're stepping a BTRACE_INSN_AUX instruction, print the auxiliary
> >+	 data and skip the instruction.  */
> >+      if (insn->iclass == BTRACE_INSN_AUX)
> 
> We need to check for nullptr.  Btrace_insn_get() return nullptr in case of
> trace errors, where we add a gap to the trace.

Done. Same for step_backwards.

> > 	{
> >-	  *replay = start;
> >-	  return btrace_step_no_history ();
> >+	  printf_unfiltered ("[%s]\n",
> >+			     btinfo->aux_data[insn->aux_data_index].c_str ());
> >+	  continue;
> > 	}
> >+
> >+      if (insn != NULL)
> >+	break;
> 
> Here you add the check.  Note that NULL is spelled nullptr nowadays.

I removed the if now that we have a nullptr check above, we can just break here.

> Regards,
> Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928