[Bug translator/1133] support .callees probe pattern extension

[Bug translator/1133] support .callees probe pattern extension

[mjw at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=1133

Mark Wielaard <mjw at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |mjw at redhat dot com

--- Comment #2 from Mark Wielaard <mjw at redhat dot com> 2012-01-14 17:12:03 UTC ---
gcc 4.7 will output dwarf call site information
http://www.dwarfstd.org/ShowIssue.php?issue=100909.2

That should make it possible to support something like function("").callees, or
the other way around function("").calls("").

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/1133] support .callees probe pattern extension

[brolley at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=1133

Dave Brolley <brolley at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |brolley at redhat dot com

--- Comment #3 from Dave Brolley <brolley at redhat dot com> 2012-01-17 17:02:25 UTC ---
For --privilege=stapusr, these probes in user space (e.g.
process.function("x").callees) should be treated the same as other user space
.function variants.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

[Bug translator/1133] support .callees probe pattern extension

[jlebon at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=1133

Jonathan Lebon <jlebon at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jlebon at redhat dot com

--- Comment #4 from Jonathan Lebon <jlebon at redhat dot com> ---
Unfortunately, it seems like the new DWARF extensions added by GCC don't allow
us (at least for now) to support a .callsites probe because there is no
attribute from which we can retrieve the address of the call site. Although the
DWARF issue[1] mentions a DW_TAG_call_site attribute for it
(DW_AT_call_site_pc), this hasn't been implemented in GCC[2]. If/when it is
implemented, then we could add a syntax along those lines.

GCC does set the DW_AT_low_pc attribute of DW_TAG_call_site, but it seems like
it points to the instruction after the call site (at first I thought it was the
return addr, but it points to the next addr even when it's a tail call).

What the new DW_TAG_call_site DIE does give us however is a way of listing
callees of a DW_TAG_subprogram. So, we could use them to support .callees("")
by retrieving the address from the DIEs of all target functions.

Note however that call site DIEs are not guaranteed to be emitted for all call
sites (DW_TAG_subprogram attributes such as DW_AT_all_call_sites could be
examined to determine if they're all there, if it matters to us). Also note
that call site DIEs are not emitted when optimization is turned off.

BTW, for those looking for more details in the reasoning behind the addition of
DW_TAG_call_site, see the excellent paper[3].

[1] DWARF issue for DW_TAG_call_site
    http://www.dwarfstd.org/ShowIssue.php?issue=100909.2

[2] GCC patch adding support for DW_TAG_call_site and DW_OP_entry_value
    http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00814.html

[3] Improving debug info for optimized away parameters
   
http://gcc.gnu.org/wiki/summit2010?action=AttachFile&do=get&target=jelinek.pdf

[4] DWARF issue for DW_OP_entry_value
    http://www.dwarfstd.org/ShowIssue.php?issue=100909.1

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug translator/1133] support .callees probe pattern extension

[jlebon at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=1133

--- Comment #5 from Jonathan Lebon <jlebon at redhat dot com> ---
(In reply to Jonathan Lebon from comment #4)
> What the new DW_TAG_call_site DIE does give us however is a way of listing
> callees of a DW_TAG_subprogram. So, we could use them to support
> .callees("") by retrieving the address from the DIEs of all target functions.

I've implemented this feature on my branch jlebon/callee. I've opted for the
'.callee' suffix instead of callees to be more in line with other components
that allow a string arg to filter results, such as 'function', 'label', etc...
If desired, we can also add a .callees as a shorthand for .callee("*").

The branch also includes a 'temp' dir with some sample programs on which to
test it. For example:

$ cd temp
$ cat simple.c
// gcc -o simple simple.c -g -O

__attribute__((noinline))
int func(int a, int b) {
   return a + b;
}

int main(void) {
   int a = 1;
   a = func(a, a);
   return a;
}

$ gcc -o simple simple.c -g -O
$ stap -L 'process("simple").function("main").callee("*")'
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/simple").function("func@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/simple.c:4")
$a:int $b:int
$ 
$ 
$ 
$ cat inlined.c
// gcc -o inlined inlined.c -g -O

__attribute__((noinline))
int foo(int a, int b) {
   return a % b;
}

__attribute__((always_inline))
int func(int a, int b) {
   int c, d, e;
   c = a + b;
   d = a * b;
   e = foo(c, d);
   return e - 1;
}

int main(void) {
   int a = 1;
   a = func(a, a);
   return a;
}

$ gcc -o inlined inlined.c -g -O -w
$ stap -L 'process("inlined").function("main").callee("*")'
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/inlined").function("func@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/inlined.c:9")
$b:int $a:int
$ stap -L 'process("inlined").function("func").callee("*")'
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/inlined").function("foo@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/inlined.c:4")
$a:int $b:int
$ 

Callee functions defined externally are also supported:

$ cat extern.c
// gcc -o extern extern.c extern2.c -g -O

long int fn2 (long int, long int, long int);
long int fn4 (long int *i, long int j);

long int
fn3 (long int x, long int (*fn4) (long int *, long int))
{
  long int v, w, w2, z;
  w = (*fn4) (&w2, x);
  v = (*fn4) (&w2, x);
  z = fn2 (1, v + 1, w);
  {
    int v1 = v + 4;
    z += fn2 (w, v * 2, x);
  }
  return z;
}

int main(void) {
  long int a = 1;
  a = fn3(a, &fn4);
  return a - 5;
}

$ cat extern2.c
// gcc -o extern extern.c extern2.c -g -O

long int
fn2 (long int a, long int b, long int c)
{
  long int q = 2 * a;
  return q + 5;
}

long int fn4 (long int *i, long int j) {
  return *i << j;
}

$ gcc -o extern extern.c extern2.c -g -O
$ stap -L 'process("extern").function("main").callee("*")'
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/extern").function("fn3@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/extern.c:7")
$x:long int $fn4:void* $w2:long int
$ 
$ stap -L 'process("extern").function("fn3").callee("*")'
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/extern").function("fn2@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/extern2.c:4")
$a:long int $b:long int $c:long int $q:long int
$ 

> Note however that call site DIEs are not guaranteed to be emitted for all
> call sites (DW_TAG_subprogram attributes such as DW_AT_all_call_sites could
> be examined to determine if they're all there, if it matters to us).

Unfortunately, there is a slight issue here as well. DW_AT_all_call_sites only
guarantees that all tail calls and normal calls have a corresponding
DW_TAG_call_site DIE, but it makes no guarantee regarding inlined functions. In
other words, we have no way of determining whether callee("*") truly caught
every call possible.

The DWARF issue (see [1] in comment 4) does have a new flag to that effect,
DW_AT_all_source_call_sites, which, if present, indicates that all inlines,
normal, and tail calls are present. Until this is implemented in gcc, we can
make no guarantee on whether callee("*") is completely thorough.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug translator/1133] support .callees probe pattern extension

[jlebon at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=1133

--- Comment #6 from Jonathan Lebon <jlebon at redhat dot com> ---
I forgot to add also that for now, .callee simply places probes on callees'
entry points, which means that they will trigger regardless of who the caller
actually is. The future DW_AT_call_site_pc attribute should allow us to
painlessly place probe points on the call site itself rather than at entry.

As a workaround for now, we could implement a tapset function e.g.
is_caller("wanted_caller") to do this. Not sure how precise this would be, or
whether it needs to be built into the probe, or something users would add
explicitly (maybe we could add a more general .function("func").from("parent")
form to which .callee would degenerate).

Also, I haven't implemented a possible .callee(N) yet, which would recursively
probe callees of callees.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug translator/1133] support .callees probe pattern extension

[jlebon at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=1133

--- Comment #7 from Jonathan Lebon <jlebon at redhat dot com> ---
(In reply to Jonathan Lebon from comment #6)
> Also, I haven't implemented a possible .callee(N) yet, which would
> recursively probe callees of callees.

Added .callees(N), which does this. I also added .callees, which is equivalent
to .callees(1) (also equivalent to .callee("*")):

$ cd temp
$ cat nested.c
// gcc -o nested nested.c -g -O

__attribute__((noinline))
int level3(int a, int b) {
   return a + b;
}

__attribute__((noinline))
int level2(int a, int b) {
   return level3(a-b, a+b);
}

__attribute__((noinline))
int level1(int a, int b) {
   return level2(a/b, a%b);
}

int main(void) {
   int a = 1;
   a = level1(a, a);
   return a;
}

$ gcc -o nested nested.c -g -O
$ stap -L 'process("nested").function("main").callees'
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested").function("level1@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested.c:14")
$a:int $b:int
$ stap -L 'process("nested").function("main").callees(1)'
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested").function("level1@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested.c:14")
$a:int $b:int
$ stap -L 'process("nested").function("main").callees(2)'
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested").function("level1@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested.c:14")
$a:int $b:int
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested").function("level2@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested.c:9")
$a:int $b:int
$ stap -L 'process("nested").function("main").callees(3)'
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested").function("level1@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested.c:14")
$a:int $b:int
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested").function("level2@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested.c:9")
$a:int $b:int
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested").function("level3@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/nested.c:4")
$a:int $b:int
$

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug translator/1133] support .callees probe pattern extension

[jlebon at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=1133

--- Comment #8 from Jonathan Lebon <jlebon at redhat dot com> ---
(In reply to Jonathan Lebon from comment #6)
> I forgot to add also that for now, .callee simply places probes on callees'
> entry points, which means that they will trigger regardless of who the
> caller actually is. The future DW_AT_call_site_pc attribute should allow us
> to painlessly place probe points on the call site itself rather than at
> entry.
> 
> As a workaround for now, we could implement a tapset function e.g.
> is_caller("wanted_caller") to do this. Not sure how precise this would be,
> or whether it needs to be built into the probe, or something users would add
> explicitly (maybe we could add a more general
> .function("func").from("parent") form to which .callee would degenerate).

I've implemented this by adding a condition expression to the probe points
created which check if it's the right caller:

$ cd temp
$ cat truecallee.c 
// gcc -o truecallee truecallee.c -g -O

__attribute__((noinline))
int foo(int a, int b) {
   return a + b;
}

__attribute__((noinline))
int bar(int a, int b) {
   return foo(a*b, a-b);
}

int main(int argc, char** argv) {
   int a = argc;
   a = foo(a, a);
   a = bar(a, a);
   return a;
}

$ gcc -o truecallee truecallee.c -g -O
$ stap -L 'process("truecallee").function("main").callee("foo")'
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/truecallee").function("foo@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/truecallee.c:4")
if ((ustack(1)) == (0x40050b)) $a:int $b:int
$ stap -L 'process("truecallee").function("bar").callee("foo")'
process("/home/yyz/jlebon/codebase/systemtap/systemtap/temp/truecallee").function("foo@/home/yyz/jlebon/codebase/systemtap/systemtap/temp/truecallee.c:4")
if ((ustack(1)) == (0x400502)) $a:int $b:int
$ 
$ stap -we 'probe process.function("bar").callee("foo") { println("This is the
foo() call from bar(), not main()"); printf("My parent function is: %s\n",
usymname(ustack(1))); exit() }' -c ./truecallee
This is the foo() call from bar(), not main()
My parent function is: bar
$ 

It seems to work OK for now, but further testing is needed (esp. for inlines
and tail calls).

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug translator/1133] support .callees probe pattern extension

[jlebon at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=1133

--- Comment #9 from Jonathan Lebon <jlebon at redhat dot com> ---
(In reply to Jonathan Lebon from comment #8)
> I've implemented this by adding a condition expression to the probe points
> created which check if it's the right caller
(In reply to Jonathan Lebon from comment #8)
> It seems to work OK for now, but further testing is needed (esp. for inlines
> and tail calls).

This technique now adds a check in the probe's body rather than using probe
point conditions. It also account for relocation by using
_stp_[uk]module_relocate() at runtime. Finally, inlined callees are properly
triggered now.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug translator/1133] support .callees probe pattern extension

[jlebon at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=1133

--- Comment #10 from Jonathan Lebon <jlebon at redhat dot com> ---
I've merged the jlebon/callee branch into master. A new testcase callee.exp was
also added to test the new feature.

As mentioned in previous comments, once DWARF5 is finalized and implemented,
more features can be added on, notably
- giving guarantees regarding the completeness of the callee set, and
- probing at the call sites rather than at callee entry.

I'll leave this PR open to keep track those issues.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug translator/1133] support .callees probe pattern extension

[jlebon at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=1133

Jonathan Lebon <jlebon at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #11 from Jonathan Lebon <jlebon at redhat dot com> ---
(In reply to Jonathan Lebon from comment #10)
> I'll leave this PR open to keep track those issues.

Moved off those issues to bug 16607.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug translator/1133] support .callees probe pattern extension

[fche at redhat dot com]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|fche at redhat dot com      |systemtap at sources dot
                   |                            |redhat dot com
             Status|ASSIGNED                    |NEW


http://sourceware.org/bugzilla/show_bug.cgi?id=1133

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.