[PATCH v12 10/10] btrace: Extend ptwrite event decoding.

[Metzger, Markus T]

Hello Felix,

>diff --git a/gdb/btrace.c b/gdb/btrace.c
>index 1835e1a26a7..6757ecbf6f3 100644
>--- a/gdb/btrace.c
>+++ b/gdb/btrace.c
>@@ -1277,20 +1277,37 @@ handle_pt_insn_events (struct
>btrace_thread_info *btinfo,
> 	    uint64_t pc = 0;
> 	    std::optional<std::string> ptw_string;
>
>-	    /* Lookup the PC if available.  */
>+	    /* Lookup the PC if available.  The event often doesn't provide
>+	       one, so we look into the last function segment as well.
>+	       Looking further back makes limited sense for ptwrite.  */
> 	    if (event.ip_suppressed == 0)
> 	      pc = event.variant.ptwrite.ip;
>-	    else if (!btinfo->functions.empty ()
>-		     && !btinfo->functions.back ().insn.empty ())
>+	    else if (!btinfo->functions.empty ())
> 	      {
>-		btrace_insn* last_insn
>-		  = &btinfo->functions.back ().insn.back ();
>-		if (last_insn->iclass != BTRACE_INSN_AUX)
>-		  pc = last_insn->pc;
>-		else
>-		  warning (_("Failed to find the PC for ptwrite."));
>+		std::vector<btrace_insn> &insns
>+		  = btinfo->functions.back ().insn;
>+		for (auto insn = insns.rbegin (); insn != insns.rend ();
>+		     ++insn)
>+		  {
>+		    switch (insn->iclass)
>+		    {
>+		    case BTRACE_INSN_AUX:
>+		      continue;

Let's add an empty line to separate the cases more clearly.

>+		    case BTRACE_INSN_OTHER:
>+		    case BTRACE_INSN_CALL:
>+		    case BTRACE_INSN_RETURN:
>+		    case BTRACE_INSN_JUMP:
>+		      pc = insn->pc;
>+		      break;
>+		    /* No default to rely on compiler warnings.  */
>+		    }
>+		    break;
>+		  }
> 	      }
>
>+	    if (pc == 0)
>+	      warning (_("Failed to determine the PC for ptwrite."));
>+
> 	    if (btinfo->ptw_callback_fun != nullptr)
> 	      ptw_string
> 		= btinfo->ptw_callback_fun (event.variant.ptwrite.payload,
>
>Is this ok? Or should I post v13 to make it clearer?

That looks good to me.  I don't think we need v13.  That was the only patch
in the series that needed a bit more discussion.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928