| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fXV6Jyd6pN7nWWy2mg1XUzbx4x=GNekyEWh9wgPDiF2kg@mail.gmail.com>
Date: Tue, 4 Feb 2025 21:17:32 -0800
From: Ian Rogers <irogers@...gle.com>
To: Howard Chu <howardchu95@...il.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>,
John Garry <john.g.garry@...cle.com>, Will Deacon <will@...nel.org>,
James Clark <james.clark@...aro.org>, Mike Leach <mike.leach@...aro.org>,
Leo Yan <leo.yan@...ux.dev>, Guo Ren <guoren@...nel.org>,
Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>, Charlie Jenkins <charlie@...osinc.com>,
Bibo Mao <maobibo@...ngson.cn>, Arnd Bergmann <arnd@...db.de>, Huacai Chen <chenhuacai@...nel.org>,
Catalin Marinas <catalin.marinas@....com>, Jiri Slaby <jirislaby@...nel.org>,
Björn Töpel <bjorn@...osinc.com>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-csky@...r.kernel.org,
linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v1 6/7] perf syscalltbl: Use lookup table containing
multiple architectures
On Tue, Feb 4, 2025 at 4:24 PM Howard Chu <howardchu95@...il.com> wrote:
>
> Hello,
>
> On Fri, Jan 31, 2025 at 11:15 PM Ian Rogers <irogers@...gle.com> wrote:
> >
> > Switch to use the lookup table containing all architectures rather
> > than tables matching the perf binary.
> >
> > This fixes perf trace when executed on a 32-bit i386 binary on an
> > x86-64 machine. Note in the following the system call names of the
> > 32-bit i386 binary as seen by an x86-64 perf.
> >
> > Before:
> > ```
> > ? ( ): a.out/447296 ... [continued]: munmap()) = 0
> > 0.024 ( 0.001 ms): a.out/447296 recvfrom(ubuf: 0x2, size: 4160585708, flags: DONTROUTE|CTRUNC|TRUNC|DONTWAIT|EOR|WAITALL|FIN|SYN|CONFIRM|RST|ERRQUEUE|NOSIGNAL|WAITFORONE|BATCH|SOCK_DEVMEM|ZEROCOPY|FASTOPEN|CMSG_CLOEXEC|0x91f80000, addr: 0xe30, addr_len: 0xffce438c) = 1475198976
> > 0.042 ( 0.003 ms): a.out/447296 lgetxattr(name: "", value: 0x3, size: 34) = 4160344064
> > 0.054 ( 0.003 ms): a.out/447296 dup2(oldfd: -134422744, newfd: 4) = -1 ENOENT (No such file or directory)
> > 0.060 ( 0.009 ms): a.out/447296 preadv(fd: 4294967196, vec: (struct iovec){.iov_base = (void *)0x2e646c2f6374652f,.iov_len = (__kernel_size_t)7307199665335594867,}, vlen: 557056, pos_h: 4160585708) = 3
> > 0.074 ( 0.004 ms): a.out/447296 lgetxattr(name: "", value: 0x1, size: 2) = 4160237568
> > 0.080 ( 0.001 ms): a.out/447296 lstat(filename: "", statbuf: 0x193f6) = 0
> > 0.089 ( 0.007 ms): a.out/447296 preadv(fd: 4294967196, vec: (struct iovec){.iov_base = (void *)0x3833692f62696c2f,.iov_len = (__kernel_size_t)3276497845987585334,}, vlen: 557056, pos_h: 4160585708) = 3
> > 0.097 ( 0.002 ms): a.out/447296 close(fd: 3</proc/447296/status>) = 512
> > 0.103 ( 0.002 ms): a.out/447296 lgetxattr(name: "", value: 0x1, size: 2050) = 4157935616
> > 0.107 ( 0.007 ms): a.out/447296 lgetxattr(pathname: "", name: "", value: 0x5, size: 2066) = 4158078976
> > 0.116 ( 0.003 ms): a.out/447296 lgetxattr(pathname: "", name: "", value: 0x1, size: 2066) = 4159639552
> > 0.121 ( 0.003 ms): a.out/447296 lgetxattr(pathname: "", name: "", value: 0x3, size: 2066) = 4160184320
> > 0.129 ( 0.002 ms): a.out/447296 lgetxattr(pathname: "", name: "", value: 0x3, size: 50) = 4160196608
> > 0.138 ( 0.001 ms): a.out/447296 lstat(filename: "") = 0
> > 0.145 ( 0.002 ms): a.out/447296 mq_timedreceive(mqdes: 4291706800, u_msg_ptr: 0xf7f9ea48, msg_len: 134616640, u_msg_prio: 0xf7fd7fec, u_abs_timeout: (struct __kernel_timespec){.tv_sec = (__kernel_time64_t)-578174027777317696,.tv_nsec = (long long int)4160349376,}) = 0
> > 0.148 ( 0.001 ms): a.out/447296 mkdirat(dfd: -134617816, pathname: " ��� ���▒���▒���", mode: IFREG|ISUID|IRUSR|IWGRP|0xf7fd0000) = 447296
> > 0.150 ( 0.001 ms): a.out/447296 process_vm_writev(pid: -134617812, lvec: (struct iovec){.iov_base = (void *)0xf7f9e9c8f7f9e4c0,.iov_len = (__kernel_size_t)4160349376,}, liovcnt: 4160588048, rvec: (struct iovec){}, riovcnt: 4160585708, flags: 4291707352) = 0
> > 0.197 ( 0.004 ms): a.out/447296 capget(header: 4160184320, dataptr: 8192) = 0
> > 0.202 ( 0.002 ms): a.out/447296 capget(header: 1448669184, dataptr: 4096) = 0
> > 0.208 ( 0.002 ms): a.out/447296 capget(header: 4160577536, dataptr: 8192) = 0
> > 0.220 ( 0.001 ms): a.out/447296 getxattr(pathname: "", name: "c������", value: 0xf7f77e34, size: 1) = 0
> > 0.228 ( 0.005 ms): a.out/447296 fchmod(fd: -134729728, mode: IRUGO|IWUGO|IFREG|IFIFO|ISVTX|IXUSR|0x10000) = 0
> > 0.240 ( 0.009 ms): a.out/447296 preadv(fd: 4294967196, vec: 0x5658e008, pos_h: 4160192052) = 3
> > 0.250 ( 0.008 ms): a.out/447296 close(fd: 3</proc/447296/status>) = 1436
> > 0.260 ( 0.018 ms): a.out/447296 stat(filename: "", statbuf: 0xffce32ac) = 1436
> > 0.288 (1000.213 ms): a.out/447296 readlinkat(buf: 0xffce31d4, bufsiz: 4291703244) = 0
> > ```
> >
> > After:
> > ```
> > ? ( ): a.out/442930 ... [continued]: execve()) = 0
> > 0.023 ( 0.002 ms): a.out/442930 brk() = 0x57760000
> > 0.052 ( 0.003 ms): a.out/442930 access(filename: 0xf7f5af28, mode: R) = -1 ENOENT (No such file or directory)
> > 0.059 ( 0.009 ms): a.out/442930 openat(dfd: CWD, filename: "/etc/ld.so.cache", flags: RDONLY|CLOEXEC|LARGEFILE) = 3
> > 0.078 ( 0.001 ms): a.out/442930 close(fd: 3</proc/442930/status>) = 0
> > 0.087 ( 0.007 ms): a.out/442930 openat(dfd: CWD, filename: "/lib/i386-linux-", flags: RDONLY|CLOEXEC|LARGEFILE) = 3
> > 0.095 ( 0.002 ms): a.out/442930 read(fd: 3</proc/442930/status>, buf: 0xffbdbb70, count: 512) = 512
> > 0.135 ( 0.001 ms): a.out/442930 close(fd: 3</proc/442930/status>) = 0
> > 0.148 ( 0.001 ms): a.out/442930 set_tid_address(tidptr: 0xf7f2b528) = 442930 (a.out)
> > 0.150 ( 0.001 ms): a.out/442930 set_robust_list(head: 0xf7f2b52c, len: 12) =
> > 0.196 ( 0.004 ms): a.out/442930 mprotect(start: 0xf7f03000, len: 8192, prot: READ) = 0
> > 0.202 ( 0.002 ms): a.out/442930 mprotect(start: 0x5658e000, len: 4096, prot: READ) = 0
> > 0.207 ( 0.002 ms): a.out/442930 mprotect(start: 0xf7f63000, len: 8192, prot: READ) = 0
> > 0.230 ( 0.005 ms): a.out/442930 munmap(addr: 0xf7f10000, len: 103414) = 0
> > 0.244 ( 0.010 ms): a.out/442930 openat(dfd: CWD, filename: 0x5658d008) = 3
> > 0.255 ( 0.007 ms): a.out/442930 read(fd: 3</proc/442930/status>, buf: 0xffbdb67c, count: 4096) = 1436
> > 0.264 ( 0.018 ms): a.out/442930 write(fd: 1</dev/pts/4>, buf: , count: 1436) = 1436
> > 0.292 (1000.173 ms): a.out/442930 clock_nanosleep(rqtp: { .tv_sec: 17866546940376776704, .tv_nsec: 4159878336 }, rmtp: 0xffbdb59c) = 0
> > 1000.478 ( ): a.out/442930 exit_group() = ?
> > ```
> >
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> > tools/perf/util/syscalltbl.c | 89 ++++++++++++++++++++++++++----------
> > 1 file changed, 64 insertions(+), 25 deletions(-)
> >
> > diff --git a/tools/perf/util/syscalltbl.c b/tools/perf/util/syscalltbl.c
> > index 760ac4d0869f..572083ba1efe 100644
> > --- a/tools/perf/util/syscalltbl.c
> > +++ b/tools/perf/util/syscalltbl.c
> > @@ -15,16 +15,39 @@
> > #include <string.h>
> > #include "string2.h"
> >
> > -#if __BITS_PER_LONG == 64
> > - #include <asm/syscalls_64.h>
> > -#else
> > - #include <asm/syscalls_32.h>
> > -#endif
> > +#include "trace/beauty/generated/syscalltbl.c"
> >
> > -const char *syscalltbl__name(int e_machine __maybe_unused, int id)
> > +static const struct syscalltbl *find_table(int e_machine)
> > {
> > - if (id >= 0 && id <= (int)ARRAY_SIZE(syscall_num_to_name))
> > - return syscall_num_to_name[id];
> > + static const struct syscalltbl *last_table;
> > + static int last_table_machine = EM_NONE;
> > +
> > + /* Tables only exist for EM_SPARC. */
> > + if (e_machine == EM_SPARCV9)
> > + e_machine = EM_SPARC;
> > +
> > + if (last_table_machine == e_machine && e_machine != EM_NONE)
>
> I don't think it should be && e_machine != EM_NONE. last_table_machine
> == e_machine == EM_NONE could mean last_table being uninitialized, but
> what if the called *is* trying to search for e_machine == EM_NONE? Now
> perf will need to traverse the whole syscalltbls array just to find
> the last EM_NONE table.
>
> My suggestion is:
>
> static const struct syscalltbl *last_table = NULL;
>
> and then:
>
> if (last_table_machine == e_machine && last_table)
> return last_table;
Agreed this is better. I'll add it in v2 with your tags :-)
Thanks,
Ian
> > + return last_table;
> > +
> > + for (size_t i = 0; i < ARRAY_SIZE(syscalltbls); i++) {
> > + const struct syscalltbl *entry = &syscalltbls[i];
> > +
> > + if (entry->e_machine != e_machine && entry->e_machine != EM_NONE)
> > + continue;
> > +
> > + last_table = entry;
> > + last_table_machine = e_machine;
> > + return entry;
> > + }
> > + return NULL;
> > +}
> > +
> > +const char *syscalltbl__name(int e_machine, int id)
> > +{
> > + const struct syscalltbl *table = find_table(e_machine);
> > +
> > + if (table && id >= 0 && id < table->num_to_name_len)
> > + return table->num_to_name[id];
> > return NULL;
> > }
> >
> > @@ -41,38 +64,54 @@ static int syscallcmpname(const void *vkey, const void *ventry)
> > return strcmp(key->name, key->tbl[*entry]);
> > }
> >
> > -int syscalltbl__id(int e_machine __maybe_unused, const char *name)
> > +int syscalltbl__id(int e_machine, const char *name)
> > {
> > - struct syscall_cmp_key key = {
> > - .name = name,
> > - .tbl = syscall_num_to_name,
> > - };
> > - const int *id = bsearch(&key, syscall_sorted_names,
> > - ARRAY_SIZE(syscall_sorted_names),
> > - sizeof(syscall_sorted_names[0]),
> > - syscallcmpname);
> > + const struct syscalltbl *table = find_table(e_machine);
> > + struct syscall_cmp_key key;
> > + const int *id;
> > +
> > + if (!table)
> > + return -1;
> > +
> > + key.name = name;
> > + key.tbl = table->num_to_name;
> > + id = bsearch(&key, table->sorted_names, table->sorted_names_len,
> > + sizeof(table->sorted_names[0]), syscallcmpname);
> >
> > return id ? *id : -1;
> > }
> >
> > -int syscalltbl__num_idx(int e_machine __maybe_unused)
> > +int syscalltbl__num_idx(int e_machine)
> > {
> > - return ARRAY_SIZE(syscall_sorted_names);
> > + const struct syscalltbl *table = find_table(e_machine);
> > +
> > + if (!table)
> > + return 0;
> > +
> > + return table->sorted_names_len;
> > }
> >
> > -int syscalltbl__id_at_idx(int e_machine __maybe_unused, int idx)
> > +int syscalltbl__id_at_idx(int e_machine, int idx)
> > {
> > - return syscall_sorted_names[idx];
> > + const struct syscalltbl *table = find_table(e_machine);
> > +
> > + if (!table)
> > + return -1;
> > +
> > + assert(idx >= 0 && idx < table->sorted_names_len);
> > + return table->sorted_names[idx];
> > }
> >
> > -int syscalltbl__strglobmatch_next(int e_machine __maybe_unused, const char *syscall_glob, int *idx)
> > +int syscalltbl__strglobmatch_next(int e_machine, const char *syscall_glob, int *idx)
> > {
> > - for (int i = *idx + 1; i < (int)ARRAY_SIZE(syscall_sorted_names); ++i) {
> > - const char *name = syscall_num_to_name[syscall_sorted_names[i]];
> > + const struct syscalltbl *table = find_table(e_machine);
> > +
> > + for (int i = *idx + 1; table && i < table->sorted_names_len; ++i) {
> > + const char *name = table->num_to_name[table->sorted_names[i]];
> >
> > if (strglobmatch(name, syscall_glob)) {
> > *idx = i;
> > - return syscall_sorted_names[i];
> > + return table->sorted_names[i];
> > }
> > }
> >
> > --
> > 2.48.1.362.g079036d154-goog
> >
Powered by blists - more mailing lists