mcount: Fix lost diff event from read trigger with argument tracing#2048
Merged
Merged
Conversation
Contributor
Author
|
After apply this root@server:~/original/uftrace# uftrace -a -T a@read=proc/statm -F main tests/t-abc
# DURATION TID FUNCTION
[3137706] | main(1, 0x7fff47757a28) {
[3137706] | a() {
[3137706] | /* read:proc/statm (vmsize=13316KB vmrss=7344KB s>
[3137706] | b() {
[3137706] | c() {
0.327 us [3137706] | getpid() = 3137706;
3.936 us [3137706] | } = 37706; /* c */
4.403 us [3137706] | } = 37707; /* b */
[3137706] | /* diff:proc/statm (vmsize=+0KB vmrss=+4KB shared>
24.099 us [3137706] | } = 37706; /* a */
31.528 us [3137706] | } = 0; /* main */
root@server:~/original/uftrace# uftrace -T a@read=proc/statm -F main tests/t-abc
# DURATION TID FUNCTION
[3137743] | main() {
[3137743] | a() {
[3137743] | /* read:proc/statm (vmsize=13068KB vmrss=6888KB s>
[3137743] | b() {
[3137743] | c() {
0.324 us [3137743] | getpid();
1.297 us [3137743] | } /* c */
1.634 us [3137743] | } /* b */
[3137743] | /* diff:proc/statm (vmsize=+0KB vmrss=+0KB shared>
22.662 us [3137743] | } /* a */
28.724 us [3137743] | } /* main */diff is normally working. |
namhyung
approved these changes
May 23, 2026
namhyung
left a comment
Owner
There was a problem hiding this comment.
LGTM, thanks for fixing this.
Nit: the subject prefix would be "mcount" instead of "libmcount".
When `-A`, `-R`, or `-a` is combined with a function-level `@read`
trigger, the `diff` event at function exit is lost:
$ uftrace -A 'a@arg1/u64' -T a@read=proc/statm -F main tests/t-abc
# DURATION TID FUNCTION
[3162404] | main() {
[3162404] | a(1) {
[3162404] | /* read:proc/statm (vmsize=13192KB vmrss=7192KB ...) */
[3162404] | b() {
[3162404] | c() {
0.303 us [3162404] | getpid();
1.602 us [3162404] | } /* c */
1.902 us [3162404] | } /* b */
19.249 us [3162404] | } /* a */ <-- diff:proc/statm is missing
19.793 us [3162404] | } /* main */
In save_trigger_read(), arg_data is meant to point to the end of
the argument region so that an event written at the buffer tail does
not overwrite it. The argument size lives at the start of argbuf as
a uint32_t, but the code reads it from ptr (= argbuf + event_idx),
which on the exit-time call points to the read event header saved at
entry. Its first 4 bytes are the lower 32 bits of event->time, so
arg_data jumps far outside the buffer and the overwrite check
rejects the new diff event.
Fix it by reading the size from the start of the argbuf, and adding
the size header itself when computing the end of the region.
Signed-off-by: Jun Yeong Kim <junyeonggim5@gmail.com>
6aae0c4 to
6fde5ec
Compare
Contributor
Author
|
Thanks for the review. I just updated the prefix to mcount. |
Owner
|
Thanks, please do the same in the callsite PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
-A,-R, or-ais combined with a function-level@readtrigger, thediffevent at function exit is lost:In save_trigger_read(), arg_data is meant to point to the end of the argument region so that an event written at the buffer tail does not overwrite it. The argument size lives at the start of argbuf as a uint32_t, but the code reads it from ptr (= argbuf + event_idx), which on the exit-time call points to the read event header saved at entry. Its first 4 bytes are the lower 32 bits of event->time, so arg_data jumps far outside the buffer and the overwrite check rejects the new diff event.
Fix it by reading the size from the start of the argbuf, and adding the size header itself when computing the end of the region.