Modify stap-probe.h to identify SystemTap probes

Modify stap-probe.h to identify SystemTap probes

[Sergio Durigan Junior]

[Moving to @gdb...]

On Saturday, May 05 2012, Jan Kratochvil wrote:

> On Sat, 05 May 2012 08:10:55 +0200, Sergio Durigan Junior wrote:
>> On Saturday, May 05 2012, Jan Kratochvil wrote:
>> > There is that
>> > 	gdb_assert (probe_generic->pops == &stap_probe_ops);
>> >
>> > for this purpose.
>> 
>> Which can only be used inside stap-probe.c.
>
> Why?  stap_probe_ops can be made extern-public in stap-probe.h if there is now
> a need for it.

Which is equivalent to create the method that I propose, that would only
validate a probe according to its ->probe_ops field, right?

TBH, I think it's more clear to declare a method responsible for this
"validation" instead of exporting stap_probe_ops to the public.

> But in general there should never be such backend-specific conditional, the
> virtualization makes no sense in such case at all.

I don't think such conditional would invalidate the virtualization we
made.  Sometimes it is necessary to know which probe we have, for
example in the case of exception/longjmp probes, which require SystemTap
specific probes.  Today, those cases are implemented because (a) we only
support stap probes, so there's no way we would catch another probe type
when filtering, and (b) because glibc/libgcc only have SystemTap
probes.  But it would be more correct if we checked the type of the
probe(s) extracted from those objfiles to make sure we're dealing with
SystemTap probes, and not with another probe type.

> If the current probe.h interface is insufficient then it should be extended.

I agree, but that's not the case.  `probe.h' is sufficient, but it is
not its responsibility to tell if a probe foo is a SystemTap probe.  I
see that it is a stap-probe.c-specific task.

Sorry for keep pushing this subject, but I still think we should have a
way of differentiating between probes' types.  BTW, I already have a
patch for that, but I won't send it yet because I want to see how this
discussion goes...

Thanks,

-- 
Sergio

Re: Modify stap-probe.h to identify SystemTap probes

[Jan Kratochvil]

On Sat, 05 May 2012 08:41:49 +0200, Sergio Durigan Junior wrote:
> On Saturday, May 05 2012, Jan Kratochvil wrote:
> > If the current probe.h interface is insufficient then it should be extended.
> 
> I agree, but that's not the case.  `probe.h' is sufficient, but it is
> not its responsibility to tell if a probe foo is a SystemTap probe.

Why cannot probe XYZ serve the task a SystemTap probe does?  The name
"SystemTap" makes the probe somehow unique no other probe kind can be placed
at the same address providing the same information?


Regards,
Jan

Re: Modify stap-probe.h to identify SystemTap probes

[Sergio Durigan Junior]

On Saturday, May 05 2012, Jan Kratochvil wrote:

> On Sat, 05 May 2012 08:41:49 +0200, Sergio Durigan Junior wrote:
>> On Saturday, May 05 2012, Jan Kratochvil wrote:
>> > If the current probe.h interface is insufficient then it should be extended.
>> 
>> I agree, but that's not the case.  `probe.h' is sufficient, but it is
>> not its responsibility to tell if a probe foo is a SystemTap probe.
>
> Why cannot probe XYZ serve the task a SystemTap probe does?  The name
> "SystemTap" makes the probe somehow unique no other probe kind can be placed
> at the same address providing the same information?

Currently only SystemTap is implemented, and SystemTap userspace probes
can have arguments, but this may not be true for all types of probes
that will eventually be implemented.  Also, and maybe most important,
only SystemTap probes are used inside glibc/libgcc for this purpose, so
allowing any probe type to be accepted in this case is not conceptually
right IMO.

If any other probe of any other type is included in glibc/libgcc for the
same purpose, it is easy to allow this type of probe in the code too.

-- 
Sergio

Re: Modify stap-probe.h to identify SystemTap probes

[Jan Kratochvil]

On Sat, 05 May 2012 09:04:56 +0200, Sergio Durigan Junior wrote:
> If any other probe of any other type is included in glibc/libgcc for the
> same purpose,

I can include any other type of probe right now into glibc/libgcc.
Even the SystemTap probe isn't upstreamed anyway.


> it is easy to allow this type of probe in the code too.

If you want to place exceptions into code here and there then the
virtualization really has no use and it did not have to be there.


Regards,
Jan

Re: Modify stap-probe.h to identify SystemTap probes

[Sergio Durigan Junior]

On Saturday, May 05 2012, Jan Kratochvil wrote:

> On Sat, 05 May 2012 09:04:56 +0200, Sergio Durigan Junior wrote:
>> If any other probe of any other type is included in glibc/libgcc for the
>> same purpose,
>
> I can include any other type of probe right now into glibc/libgcc.
> Even the SystemTap probe isn't upstreamed anyway.

libgcc's SystemTap probes are upstreamed.

    http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01016.html
    http://gcc.gnu.org/ml/gcc-patches/2011-11/msg02246.html

>> it is easy to allow this type of probe in the code too.
>
> If you want to place exceptions into code here and there then the
> virtualization really has no use and it did not have to be there.

My goal was not to overcomplicate the code, nor to insert exceptions
"here and there" as you say, nor to break the virtualization layer that
we implemented (and I totally agreed with), but to guarantee correctness
of the code.  But I won't push anymore, I understand your reasons and I
think it's not worth to spend more time discussing this design change.

Thanks,

-- 
Sergio