Skip to content

mcount: Fix lost diff event from read trigger with argument tracing#2048

Merged
namhyung merged 1 commit into
namhyung:masterfrom
junyeong0619:fix-read-trigger-arg-data
May 23, 2026
Merged

mcount: Fix lost diff event from read trigger with argument tracing#2048
namhyung merged 1 commit into
namhyung:masterfrom
junyeong0619:fix-read-trigger-arg-data

Conversation

@junyeong0619

Copy link
Copy Markdown
Contributor

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.

@junyeong0619

Copy link
Copy Markdown
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 namhyung left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@junyeong0619 junyeong0619 force-pushed the fix-read-trigger-arg-data branch from 6aae0c4 to 6fde5ec Compare May 23, 2026 06:53
@junyeong0619

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I just updated the prefix to mcount.

@junyeong0619 junyeong0619 changed the title libmcount: Fix lost diff event from read trigger with argument tracing mcount: Fix lost diff event from read trigger with argument tracing May 23, 2026
@namhyung

Copy link
Copy Markdown
Owner

Thanks, please do the same in the callsite PR.

@namhyung namhyung merged commit 50d2284 into namhyung:master May 23, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants