[PATCH v6 3/7] start/stop btrace with coresight etm and parse etm buffer. nat independant

[Metzger, Markus T]

Hello Zied,


+

+  if (!bfun->insn.empty ())

+    bfun->insn.pop_back ();

+  else

+    {

+      /* A valid function must have at least one instruction.  */

+      internal_error (__FILE__, __LINE__,

+                 _("Attempt to remove last instruction"

+                   "from an empty function"));



We just removed an instruction in the then statement, which

could well result in BFUN->INSN to become empty().



The statement about valid functions above may be a bit too

generic.  Also, ftrace_new_* () may create empty function

segments.



Isn't it rather that in our use-case, the caller knows that the

function segment cannot be empty?
[Zied] yes, we are sure that we have at least the last executed instruction that raised the breakpoint execption.

Would it make more sense to simply do ‘bfun->insn.pop_back ()’ inside the caller?

Apparently, the caller knows that there must be an instruction so it could safely remove it.  If we turn this into a separate function, we need to handle the general case.

[Zied] what will happens in the case  of a gap proceeding current instruction, the function is still not created at this point, isn't it?

to be sure that none tries to use the function as a generic one, we can remove it and put the code in the caller function. It is called only once. will this be fine from your point of view?

Yes, I think that would be better.  We’re making too many assumptions for a generic function, here, and we can probably eliminate most of the checks when inlined into the sole call site.

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