From d9f4de63e9335d713b75e805ff500e79c5e8732f Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Sun, 17 Dec 2023 18:46:31 +0000 Subject: [PATCH 1/3] dwarf: Handle small-sized data properly Honggyu reported that enum with 2-byte encoding didn't work well. It seems that it might sign-extend the data when the highest bit is set to keep it negative. But for unsigned values in enum can have a problem since it doesn't match to the original value. Signed-off-by: Namhyung Kim --- utils/dwarf.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/utils/dwarf.c b/utils/dwarf.c index 1a796dd73..f49d600c2 100644 --- a/utils/dwarf.c +++ b/utils/dwarf.c @@ -240,6 +240,19 @@ static long int_attr(Dwarf_Die *die, int attr, bool follow) return 0; dwarf_formsdata(&da, &data); + switch (dwarf_whatform(&da)) { + case DW_FORM_data1: + data &= 0xff; + break; + case DW_FORM_data2: + data &= 0xffff; + break; + case DW_FORM_data4: + data &= 0xffffffff; + break; + default: + break; + } return data; } From b3e8313a4d58c6ed50aa3232fb11493a74e19bfc Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 19 Dec 2023 17:29:05 +0000 Subject: [PATCH 2/3] replay: Fix enum display It passed the value as an 32-bit (signed) integer which allows sign-extension. That makes uftrace replay cannot match values with MSB bit set. As it saves 8 byte data, it should pass the long type value to match enum data correctly. Signed-off-by: Namhyung Kim --- cmds/replay.c | 2 +- utils/argspec.h | 2 +- utils/auto-args.c | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cmds/replay.c b/cmds/replay.c index 653d86204..15752c372 100644 --- a/cmds/replay.c +++ b/cmds/replay.c @@ -639,7 +639,7 @@ void get_argspec_string(struct uftrace_task_reader *task, char *args, size_t len } dinfo = &map->mod->dinfo; - estr = get_enum_string(&dinfo->enums, spec->type_name, (int)val.i); + estr = get_enum_string(&dinfo->enums, spec->type_name, val.i); if (strstr(estr, "|") && strcmp("|", color_enum_or)) { struct strv enum_vals = STRV_INIT; diff --git a/utils/argspec.h b/utils/argspec.h index dea03d9e7..589e1d894 100644 --- a/utils/argspec.h +++ b/utils/argspec.h @@ -80,7 +80,7 @@ char *get_auto_retspec_str(void); char *get_auto_enum_str(void); int extract_trigger_args(char **pargs, char **prets, char *trigger); int parse_enum_string(char *enum_str, struct rb_root *root); -char *get_enum_string(struct rb_root *root, char *name, int val); +char *get_enum_string(struct rb_root *root, char *name, long val); void save_enum_def(struct rb_root *root, FILE *fp); void release_enum_def(struct rb_root *root); diff --git a/utils/auto-args.c b/utils/auto-args.c index 69b3848ac..c01b90c25 100644 --- a/utils/auto-args.c +++ b/utils/auto-args.c @@ -503,7 +503,7 @@ struct enum_def *find_enum_def(struct rb_root *root, char *name) return NULL; } -char *convert_enum_val(struct enum_def *e_def, int val) +char *convert_enum_val(struct enum_def *e_def, long val) { struct enum_val *e_val; char *str = NULL; @@ -529,22 +529,22 @@ char *convert_enum_val(struct enum_def *e_def, int val) if (str && val) { char *tmp; - xasprintf(&tmp, "%s+%#x", str, val); + xasprintf(&tmp, "%s+%#lx", str, val); free(str); str = tmp; } else if (unlikely(str == NULL)) { if (labs(val) > 100000) - xasprintf(&str, "%#x", val); + xasprintf(&str, "%#lx", val); else - xasprintf(&str, "%d", val); + xasprintf(&str, "%ld", val); } return str; } /* caller should free the return value */ -char *get_enum_string(struct rb_root *root, char *name, int val) +char *get_enum_string(struct rb_root *root, char *name, long val) { struct enum_def *e_def; char *ret; @@ -554,7 +554,7 @@ char *get_enum_string(struct rb_root *root, char *name, int val) e_def = find_enum_def(&auto_enum, name); if (e_def == NULL) - xasprintf(&ret, "%d", val); + xasprintf(&ret, "%ld", val); else ret = convert_enum_val(e_def, val); From f73febfc2d17efd5b5586ca4c68e32b6a102c013 Mon Sep 17 00:00:00 2001 From: Honggyu Kim Date: Wed, 20 Dec 2023 10:18:04 +0900 Subject: [PATCH 3/3] tests: Add t287_arg_enum3.py tests This test is to verify enum types with corner case values at s-enum2.c as follows. enum memory_order_modifier { zero = 0, memory_order_mask = 0x0ffff, memory_order_modifier_mask = 0xffff0000, memory_order_hle_acquire = 0x10000, memory_order_hle_release = 0x20000 }; A single test result is as follows. $ ./runtest.py -vdp -O0 -c gcc 287 ... # DURATION TID FUNCTION [ 63153] | main() { 0.471 us [ 63153] | foo(memory_order_mask); 0.081 us [ 63153] | foo(memory_order_modifier_mask); 0.068 us [ 63153] | foo(memory_order_hle_acquire); 0.067 us [ 63153] | foo(memory_order_hle_release); 1.794 us [ 63153] | } = 0; /* main */ ... 287 arg_enum3 : OK The test result shows it works fine as expected. $ ./runtest.py 287 Start 1 tests without worker pool Compiler gcc clang Runtime test case pg finstrument-fu fpatchable-fun pg finstrument-fu fpatchable-fun ------------------------: O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os 287 arg_enum3 : OK OK OK OK OK SK SK SK SK SK OK OK OK OK OK OK OK OK OK OK SK SK SK SK SK OK OK OK OK OK Signed-off-by: Honggyu Kim --- tests/s-enum2.c | 24 ++++++++++++++++++++++++ tests/t287_arg_enum3.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 tests/s-enum2.c create mode 100644 tests/t287_arg_enum3.py diff --git a/tests/s-enum2.c b/tests/s-enum2.c new file mode 100644 index 000000000..5f158cb7c --- /dev/null +++ b/tests/s-enum2.c @@ -0,0 +1,24 @@ +static int cnt; + +enum memory_order_modifier { + zero = 0, + memory_order_mask = 0x0ffff, + memory_order_modifier_mask = 0xffff0000, + memory_order_hle_acquire = 0x10000, + memory_order_hle_release = 0x20000 +}; + +__attribute__((noinline)) void foo(enum memory_order_modifier m) +{ + if (m != zero) + cnt++; +} + +int main() +{ + foo(memory_order_mask); + foo(memory_order_modifier_mask); + foo(memory_order_hle_acquire); + foo(memory_order_hle_release); + return cnt == 4 ? 0 : -1; +} diff --git a/tests/t287_arg_enum3.py b/tests/t287_arg_enum3.py new file mode 100644 index 000000000..9bdd8af1a --- /dev/null +++ b/tests/t287_arg_enum3.py @@ -0,0 +1,29 @@ +#!/usr/bin/env python3 + +import re + +from runtest import TestBase + +class TestCase(TestBase): + def __init__(self): + TestBase.__init__(self, 'enum2', result=""" +# DURATION TID FUNCTION + [ 57041] | main() { + 0.535 us [ 57041] | foo(memory_order_mask); + 0.109 us [ 57041] | foo(memory_order_modifier_mask); + 0.069 us [ 57041] | foo(memory_order_hle_acquire); + 0.068 us [ 57041] | foo(memory_order_hle_release); + 1.783 us [ 57041] | } = 0; /* main */ +""", cflags='-g') + + def build(self, name, cflags='', ldflags=''): + if not "dwarf" in self.feature: + return TestBase.TEST_SKIP + # cygprof doesn't support arguments now + if cflags.find('-finstrument-functions') >= 0: + return TestBase.TEST_SKIP + + return TestBase.build(self, name, cflags, ldflags) + + def setup(self): + self.option = '-F main -a'