[Bug translator/6702] New: combination of "probe ... if()" and argument refering at return probe causes array overflow error.

[Bug translator/6702] New: combination of "probe ... if()" and argument refering at return probe causes array overflow error.

[mhiramat at redhat dot com]

When I executed below script, stap caused an Array overflow error.
---
global flag
probe syscall.read.return if (flag) {
	printf("read count = %d\n", $count)
}
probe begin {
	flag = 0
}
probe procfs.write {
	flag = strtol($value,10)
}
---

$ stap -v ./script
Pass 1: parsed user script and 42 library script(s) in 280usr/10sys/304real ms.
Pass 2: analyzed script: 4 probe(s), 4 function(s), 2 embed(s), 3 global(s) in
270usr/80sys/361real ms.
Pass 3: translated to C into
"/tmp/stapo8bkMe/stap_d4b424cfc8957f11a460f1059d6d7f5d_7192.c" in
20usr/10sys/53real ms.
/home/mhiramat/.systemtap/cache/d4/stap_d4b424cfc8957f11a460f1059d6d7f5d_7192.ko
Pass 4: compiled C into "stap_d4b424cfc8957f11a460f1059d6d7f5d_7192.ko" in
3620usr/470sys/7321real ms.
Pass 5: starting run.
ERROR: Array overflow, check MAXMAPENTRIES near identifier '$count' at ./script:3:30
WARNING: Removal of /proc/systemtap/stap_d4b424cfc8957f11a460f1059d6d7f5d_7192
is deferred until it is no longer in use.
Systemtap module removal will block.
WARNING: Number of errors: 1, skipped probes: 0
---

This was happened because probe "if(flag)" statement will be executed 
before deleting unused array element.

Here is the result of '-p2' option.
---
# probes
kernel.function("sys_read@fs/read_write.c:354").return if (flag) /* pc=0x807e6
*/ /* <- syscall.read.return = kernel.function("sys_read").return if (flag) <-
syscall.read.return if (flag) */
  # locals
  _dwarf_tvar_tid:long
  _dwarf_tvar_count_0_tmp:long
{
if (!(flag)) next

{
{
(_dwarf_tvar_tid) = (tid())
(_dwarf_tvar_count_0_tmp) = (_dwarf_tvar_count_0[_dwarf_tvar_tid,
_dwarf_tvar_count_0_ctr[_dwarf_tvar_tid]])
delete _dwarf_tvar_count_0[_dwarf_tvar_tid,
(_dwarf_tvar_count_0_ctr[_dwarf_tvar_tid])--]
if (!(_dwarf_tvar_count_0_ctr[_dwarf_tvar_tid])) delete
_dwarf_tvar_count_0_ctr[_dwarf_tvar_tid]

}
printf("read count = %d\\n", _dwarf_tvar_count_0_tmp)
}
}
---

When I wrote it as 'probe syscall.read.return { if (!flag) next; ... }',
stap showed below result;
---
# probes
kernel.function("sys_read@fs/read_write.c:354").return /* pc=0x807e6 */ /* <-
syscall.read.return = kernel.function("sys_read").return <- syscall.read.return */
  # locals
  _dwarf_tvar_tid:long
  _dwarf_tvar_count_0_tmp:long
{
{
(_dwarf_tvar_tid) = (tid())
(_dwarf_tvar_count_0_tmp) = (_dwarf_tvar_count_0[_dwarf_tvar_tid,
_dwarf_tvar_count_0_ctr[_dwarf_tvar_tid]])
delete _dwarf_tvar_count_0[_dwarf_tvar_tid,
(_dwarf_tvar_count_0_ctr[_dwarf_tvar_tid])--]
if (!(_dwarf_tvar_count_0_ctr[_dwarf_tvar_tid])) delete
_dwarf_tvar_count_0_ctr[_dwarf_tvar_tid]

}
{
if (!(flag)) next

printf("read count = %d\\n", _dwarf_tvar_count_0_tmp)
}
}
---

the place of if() statement was changed after deleting unused array element.

-- 
           Summary: combination of "probe ... if()" and argument refering at
                    return probe causes array overflow error.
           Product: systemtap
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: translator
        AssignedTo: systemtap at sources dot redhat dot com
        ReportedBy: mhiramat at redhat dot com


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

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

[Bug translator/6702] combination of "probe ... if()" and argument refering at return probe causes array overflow error.

[fche at redhat dot com]

------- Additional Comments From fche at redhat dot com  2008-06-27 16:10 -------
What we have here is a conflict between the rewriting of the 
  probe ... if () {}
block and the 
  probe foo.*.return { $saved_variable }
block.

One possible fix would be to change the latter, so that the synthetic
  probe foo.*.call { save_that_variable }
probe also gets the same "if ()" condition prepended.  That way, the
data gathering probe is also skipped if the condition is false.  That
sounds rather natural.

The code in tapsets.cxx:4070ish taht creates the new probe_point for
the synthetic probe should thusly:
- add ".call" to replace ".return" (to be a proper dual, considering
  the possibility of inline functions)
- copy the incoming probe-point's condition expression over


-- 


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

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

[Bug translator/6702] combination of "probe ... if()" and argument refering at return probe causes array overflow error.

[mhiramat at redhat dot com]

------- Additional Comments From mhiramat at redhat dot com  2008-06-27 16:36 -------
(In reply to comment #1)
> What we have here is a conflict between the rewriting of the 
>   probe ... if () {}
> block and the 
>   probe foo.*.return { $saved_variable }
> block.
> 
> One possible fix would be to change the latter, so that the synthetic
>   probe foo.*.call { save_that_variable }
> probe also gets the same "if ()" condition prepended.  That way, the
> data gathering probe is also skipped if the condition is false.  That
> sounds rather natural.

In most cases, especially at the short function, it can work.
However, a process which calls some function like syscall functions 
will be preempted while executing it, and when the process sleeping,
another process can change the flag status. In that case, saved 
variables still remain on the array. The opposite case is worse...
return probe can't get the variable because entrance probe didn't save it.

I think pr #5916 could solve this issue.

Anyway, IMHO, now what we can do it on older kernels is swapping
the order of probe's if() condition and releasing saved variable block.


-- 


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

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

[Bug translator/6702] combination of "probe ... if()" and argument refering at return probe causes array overflow error.

[fche at redhat dot com]

------- Additional Comments From fche at redhat dot com  2008-06-27 16:48 -------
> > One possible fix would be to change the latter, so that the synthetic
> >   probe foo.*.call { save_that_variable }
> > probe also gets the same "if ()" condition prepended.  That way, the
> > data gathering probe is also skipped if the condition is false.  That
> > sounds rather natural.
> 
> [...]
> However, a process which calls some function like syscall functions 
> will be preempted while executing it, and when the process sleeping,
> another process can change the flag status. In that case, saved 
> variables still remain on the array. 

Yup, we don't want to build up garbage for this scenario.  It seems
to me that the synthetic global arrays that store these values should
be set up in overwrite mode, with no more than the .return.maxactive()
slots allocated.

> The opposite case is worse...
> return probe can't get the variable because entrance probe didn't save it.

OTOH I think that's a reasonable price to pay.  We shouldn't crash of
course, but the .return-side generated code could *skip* the return
handler if the incoming parameters weren't saved for whatever reason.

In any case, once we properly implement probe-point-conditions by disarming
probes on the fly, we won't want to enable the synthetic function-entry
kprobe constantly while the real kretprobe is disabled by condition.


-- 


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

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

[Bug translator/6702] combination of "probe ... if()" and argument refering at return probe causes array overflow error.

[mhiramat at redhat dot com]

------- Additional Comments From mhiramat at redhat dot com  2008-06-28 00:05 -------
(In reply to comment #3)
[...]
> > However, a process which calls some function like syscall functions 
> > will be preempted while executing it, and when the process sleeping,
> > another process can change the flag status. In that case, saved 
> > variables still remain on the array. 
> 
> Yup, we don't want to build up garbage for this scenario.  It seems
> to me that the synthetic global arrays that store these values should
> be set up in overwrite mode, with no more than the .return.maxactive()
> slots allocated.

I think overwrite mode can't help us in this situation, because we
can't decide which element can be overwritten except for hooking
thread termination (that is what kretprobe does).

> > The opposite case is worse...
> > return probe can't get the variable because entrance probe didn't save it.
> 
> OTOH I think that's a reasonable price to pay.  We shouldn't crash of
> course, but the .return-side generated code could *skip* the return
> handler if the incoming parameters weren't saved for whatever reason.
> 
> In any case, once we properly implement probe-point-conditions by disarming
> probes on the fly, we won't want to enable the synthetic function-entry
> kprobe constantly while the real kretprobe is disabled by condition.

Sure, until it, I hope pr #5916 will solve this...


-- 


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

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