From c499d1f483a99e86a5f712277de7c8fa33a9ec0a Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Sat, 6 Jul 2019 14:26:40 -0300 Subject: tools arch kvm: Sync kvm headers with the kernel sources To pick up the changes from: 41040cf7c5f0 ("arm64/sve: Fix missing SVE/FPSIMD endianness conversions") 6ca00dfafda7 ("KVM: x86: Modify struct kvm_nested_state to have explicit fields for data") None entail changes in tooling. This silences these tools/perf build warnings: Warning: Kernel ABI header at 'tools/arch/x86/include/uapi/asm/kvm.h' differs from latest version at 'arch/x86/include/uapi/asm/kvm.h' diff -u tools/arch/x86/include/uapi/asm/kvm.h arch/x86/include/uapi/asm/kvm.h Warning: Kernel ABI header at 'tools/arch/arm64/include/uapi/asm/kvm.h' differs from latest version at 'arch/arm64/include/uapi/asm/kvm.h' diff -u tools/arch/arm64/include/uapi/asm/kvm.h arch/arm64/include/uapi/asm/kvm.h Cc: Adrian Hunter Cc: Dave Martin Cc: Jiri Olsa Cc: Liran Alon Cc: Namhyung Kim Cc: Paolo Bonzini Cc: Will Deacon Link: https://lkml.kernel.org/n/tip-1cdbq5ulr4d6cx3iv2ye5wdv@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/arch/arm64/include/uapi/asm/kvm.h | 7 +++++++ tools/arch/x86/include/uapi/asm/kvm.h | 31 +++++++++++++++++++++---------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/tools/arch/arm64/include/uapi/asm/kvm.h b/tools/arch/arm64/include/uapi/asm/kvm.h index 7b7ac0f6cec9..d819a3e8b552 100644 --- a/tools/arch/arm64/include/uapi/asm/kvm.h +++ b/tools/arch/arm64/include/uapi/asm/kvm.h @@ -260,6 +260,13 @@ struct kvm_vcpu_events { KVM_REG_SIZE_U256 | \ ((i) & (KVM_ARM64_SVE_MAX_SLICES - 1))) +/* + * Register values for KVM_REG_ARM64_SVE_ZREG(), KVM_REG_ARM64_SVE_PREG() and + * KVM_REG_ARM64_SVE_FFR() are represented in memory in an endianness- + * invariant layout which differs from the layout used for the FPSIMD + * V-registers on big-endian systems: see sigcontext.h for more explanation. + */ + #define KVM_ARM64_SVE_VQ_MIN __SVE_VQ_MIN #define KVM_ARM64_SVE_VQ_MAX __SVE_VQ_MAX diff --git a/tools/arch/x86/include/uapi/asm/kvm.h b/tools/arch/x86/include/uapi/asm/kvm.h index 24a8cd229df6..d6ab5b4d15e5 100644 --- a/tools/arch/x86/include/uapi/asm/kvm.h +++ b/tools/arch/x86/include/uapi/asm/kvm.h @@ -383,6 +383,9 @@ struct kvm_sync_regs { #define KVM_X86_QUIRK_LAPIC_MMIO_HOLE (1 << 2) #define KVM_X86_QUIRK_OUT_7E_INC_RIP (1 << 3) +#define KVM_STATE_NESTED_FORMAT_VMX 0 +#define KVM_STATE_NESTED_FORMAT_SVM 1 /* unused */ + #define KVM_STATE_NESTED_GUEST_MODE 0x00000001 #define KVM_STATE_NESTED_RUN_PENDING 0x00000002 #define KVM_STATE_NESTED_EVMCS 0x00000004 @@ -390,7 +393,14 @@ struct kvm_sync_regs { #define KVM_STATE_NESTED_SMM_GUEST_MODE 0x00000001 #define KVM_STATE_NESTED_SMM_VMXON 0x00000002 -struct kvm_vmx_nested_state { +#define KVM_STATE_NESTED_VMX_VMCS_SIZE 0x1000 + +struct kvm_vmx_nested_state_data { + __u8 vmcs12[KVM_STATE_NESTED_VMX_VMCS_SIZE]; + __u8 shadow_vmcs12[KVM_STATE_NESTED_VMX_VMCS_SIZE]; +}; + +struct kvm_vmx_nested_state_hdr { __u64 vmxon_pa; __u64 vmcs12_pa; @@ -401,24 +411,25 @@ struct kvm_vmx_nested_state { /* for KVM_CAP_NESTED_STATE */ struct kvm_nested_state { - /* KVM_STATE_* flags */ __u16 flags; - - /* 0 for VMX, 1 for SVM. */ __u16 format; - - /* 128 for SVM, 128 + VMCS size for VMX. */ __u32 size; union { - /* VMXON, VMCS */ - struct kvm_vmx_nested_state vmx; + struct kvm_vmx_nested_state_hdr vmx; /* Pad the header to 128 bytes. */ __u8 pad[120]; - }; + } hdr; - __u8 data[0]; + /* + * Define data region as 0 bytes to preserve backwards-compatability + * to old definition of kvm_nested_state in order to avoid changing + * KVM_{GET,PUT}_NESTED_STATE ioctl values. + */ + union { + struct kvm_vmx_nested_state_data vmx[0]; + } data; }; #endif /* _ASM_X86_KVM_H */ -- cgit v1.2.3 From c952b35f4b15dd1b83e952718dec3307256383ef Mon Sep 17 00:00:00 2001 From: Song Liu Date: Wed, 19 Jun 2019 18:04:53 -0700 Subject: perf header: Assign proper ff->ph in perf_event__synthesize_features() bpf/btf write_* functions need ff->ph->env. With this missing, pipe-mode (perf record -o -) would crash like: Program terminated with signal SIGSEGV, Segmentation fault. This patch assign proper ph value to ff. Committer testing: (gdb) run record -o - Starting program: /root/bin/perf record -o - PERFILE2 Thread 1 "perf" received signal SIGSEGV, Segmentation fault. __do_write_buf (size=4, buf=0x160, ff=0x7fffffff8f80) at util/header.c:126 126 memcpy(ff->buf + ff->offset, buf, size); (gdb) bt #0 __do_write_buf (size=4, buf=0x160, ff=0x7fffffff8f80) at util/header.c:126 #1 do_write (ff=ff@entry=0x7fffffff8f80, buf=buf@entry=0x160, size=4) at util/header.c:137 #2 0x00000000004eddba in write_bpf_prog_info (ff=0x7fffffff8f80, evlist=) at util/header.c:912 #3 0x00000000004f69d7 in perf_event__synthesize_features (tool=tool@entry=0x97cc00 , session=session@entry=0x7fffe9c6d010, evlist=0x7fffe9cae010, process=process@entry=0x4435d0 ) at util/header.c:3695 #4 0x0000000000443c79 in record__synthesize (tail=tail@entry=false, rec=0x97cc00 ) at builtin-record.c:1214 #5 0x0000000000444ec9 in __cmd_record (rec=0x97cc00 , argv=, argc=0) at builtin-record.c:1435 #6 cmd_record (argc=0, argv=) at builtin-record.c:2450 #7 0x00000000004ae3e9 in run_builtin (p=p@entry=0x98e058 , argc=argc@entry=3, argv=0x7fffffffd670) at perf.c:304 #8 0x000000000042eded in handle_internal_command (argv=, argc=) at perf.c:356 #9 run_argv (argcp=, argv=) at perf.c:400 #10 main (argc=3, argv=) at perf.c:522 (gdb) After the patch the SEGSEGV is gone. Reported-by: David Carrillo Cisneros Signed-off-by: Song Liu Tested-by: Arnaldo Carvalho de Melo Cc: Jiri Olsa Cc: Namhyung Kim Cc: kernel-team@fb.com Cc: stable@vger.kernel.org # v5.1+ Fixes: 606f972b1361 ("perf bpf: Save bpf_prog_info information as headers to perf.data") Link: http://lkml.kernel.org/r/20190620010453.4118689-1-songliubraving@fb.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/header.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index 847ae51a524b..fb0aa661644b 100644 --- a/tools/perf/util/header.c +++ b/tools/perf/util/header.c @@ -3602,6 +3602,7 @@ int perf_event__synthesize_features(struct perf_tool *tool, return -ENOMEM; ff.size = sz - sz_hdr; + ff.ph = &session->header; for_each_set_bit(feat, header->adds_features, HEADER_FEAT_BITS) { if (!feat_ops[feat].synthesize) { -- cgit v1.2.3 From 4c00af0e94cd01b8c5a5e6b3323d34677b04e192 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 5 Jul 2019 12:11:35 -0300 Subject: perf thread: Allow references to thread objects after machine__exit() Threads are created when we either synthesize PERF_RECORD_FORK events for pre-existing threads or when we receive PERF_RECORD_FORK events from the kernel as new threads get created. We then keep them in machine->threads[].entries rb trees till when we receive a PERF_RECORD_EXIT, i.e. that thread terminated. The thread object has a reference count that is grabbed when, for instance, we keep that thread referenced in struct hist_entry, in 'perf report' and 'perf top'. When we receive a PERF_RECORD_EXIT we remove the thread object from the rb tree and move it to the corresponding machine->threads[].dead list, then we do a thread__put(), dropping the reference we had for keeping it in the rb tree. In thread__put() we were assuming that when the reference count hit zero we should remove it from the dead list by simply doing a list_del_init(&thread->node). That works well when all the thread lifetime is during the machine that has the list heads lifetime, since we know that we can do the list_del_init() and it will update the 'dead' list_head. But in 'perf sched lat' we were doing: machine__new() (via perf_session__new) process events, grabbing refcounts to keep those thread objects in 'perf sched' local data structures. machine__exit() (via perf_session__delete) which would delete the 'dead' list heads. And then doing the final thread__put() for the refcounts 'perf sched' rightfully obtained for keeping those thread object references. b00m, since thread__put() would do the list_del_init() touching a dead dead list head. Fix it by removing all the dead threads from machine->threads[].dead at machine__exit(), since whatever is there should have refcounts taken by things like 'perf sched lat', and make thread__put() check if the thread is in a linked list before removing it from that list. Reported-by: Wei Li Link: https://lkml.kernel.org/r/20190508143648.8153-1-liwei391@huawei.com Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Zhipeng Xie Link: https://lkml.kernel.org/r/20190704194355.GI10740@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/machine.c | 25 +++++++++++++++++++++++-- tools/perf/util/thread.c | 23 ++++++++++++++++++++--- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index dc7aafe45a2b..e00dc413652d 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -209,6 +209,18 @@ void machine__exit(struct machine *machine) for (i = 0; i < THREADS__TABLE_SIZE; i++) { struct threads *threads = &machine->threads[i]; + struct thread *thread, *n; + /* + * Forget about the dead, at this point whatever threads were + * left in the dead lists better have a reference count taken + * by who is using them, and then, when they drop those references + * and it finally hits zero, thread__put() will check and see that + * its not in the dead threads list and will not try to remove it + * from there, just calling thread__delete() straight away. + */ + list_for_each_entry_safe(thread, n, &threads->dead, node) + list_del_init(&thread->node); + exit_rwsem(&threads->lock); } } @@ -1758,9 +1770,11 @@ static void __machine__remove_thread(struct machine *machine, struct thread *th, if (threads->last_match == th) threads__set_last_match(threads, NULL); - BUG_ON(refcount_read(&th->refcnt) == 0); if (lock) down_write(&threads->lock); + + BUG_ON(refcount_read(&th->refcnt) == 0); + rb_erase_cached(&th->rb_node, &threads->entries); RB_CLEAR_NODE(&th->rb_node); --threads->nr; @@ -1770,9 +1784,16 @@ static void __machine__remove_thread(struct machine *machine, struct thread *th, * will be called and we will remove it from the dead_threads list. */ list_add_tail(&th->node, &threads->dead); + + /* + * We need to do the put here because if this is the last refcount, + * then we will be touching the threads->dead head when removing the + * thread. + */ + thread__put(th); + if (lock) up_write(&threads->lock); - thread__put(th); } void machine__remove_thread(struct machine *machine, struct thread *th) diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index b413ba5b9835..7bfb740d2ede 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -125,10 +125,27 @@ void thread__put(struct thread *thread) { if (thread && refcount_dec_and_test(&thread->refcnt)) { /* - * Remove it from the dead_threads list, as last reference - * is gone. + * Remove it from the dead threads list, as last reference is + * gone, if it is in a dead threads list. + * + * We may not be there anymore if say, the machine where it was + * stored was already deleted, so we already removed it from + * the dead threads and some other piece of code still keeps a + * reference. + * + * This is what 'perf sched' does and finally drops it in + * perf_sched__lat(), where it calls perf_sched__read_events(), + * that processes the events by creating a session and deleting + * it, which ends up destroying the list heads for the dead + * threads, but before it does that it removes all threads from + * it using list_del_init(). + * + * So we need to check here if it is in a dead threads list and + * if so, remove it before finally deleting the thread, to avoid + * an use after free situation. */ - list_del_init(&thread->node); + if (!list_empty(&thread->node)) + list_del_init(&thread->node); thread__delete(thread); } } -- cgit v1.2.3 From cd136189370cc8a5aec0ea4b4ec517e5ee38d8a0 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Wed, 3 Jul 2019 10:09:49 +0200 Subject: perf evsel: Do not rely on errno values for precise_ip fallback Konstantin reported problem with default perf record command, which fails on some AMD servers, because of the default maximum precise config. The current fallback mechanism counts on getting ENOTSUP errno for precise_ip fails, but that's not the case on some AMD servers. We can fix this by removing the errno check completely, because the precise_ip fallback is separated. We can just try (if requested by evsel->precise_max) all possible precise_ip, and if one succeeds we win, if not, we continue with standard fallback. Reported-by: Konstantin Kharlamov Signed-off-by: Jiri Olsa Cc: Alexander Shishkin Cc: Andi Kleen Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Quentin Monnet Cc: Kim Phillips Link: http://lkml.kernel.org/r/20190703080949.10356-1-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evsel.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 4a5947625c5c..69beb9f80f07 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -1785,14 +1785,8 @@ static int perf_event_open(struct perf_evsel *evsel, if (fd >= 0) break; - /* - * Do quick precise_ip fallback if: - * - there is precise_ip set in perf_event_attr - * - maximum precise is requested - * - sys_perf_event_open failed with ENOTSUP error, - * which is associated with wrong precise_ip - */ - if (!precise_ip || !evsel->precise_max || (errno != ENOTSUP)) + /* Do not try less precise if not requested. */ + if (!evsel->precise_max) break; /* -- cgit v1.2.3 From bff5a556c149804de29347a88a884d25e4e4e3a2 Mon Sep 17 00:00:00 2001 From: Seeteena Thoufeek Date: Thu, 27 Jun 2019 15:46:54 +0530 Subject: perf tests: Fix record+probe_libc_inet_pton.sh for powerpc64 'probe libc's inet_pton & backtrace it with ping' testcase sometimes fails on powerpc because distro ping binary does not have symbol information and thus it prints "[unknown]" function name in the backtrace. Accept "[unknown]" as valid function name for powerpc as well. # perf test -v "probe libc's inet_pton & backtrace it with ping" Before: 59: probe libc's inet_pton & backtrace it with ping : --- start --- test child forked, pid 79695 ping 79718 [077] 96483.787025: probe_libc:inet_pton: (7fff83a754c8) 7fff83a754c8 __GI___inet_pton+0x8 (/usr/lib64/power9/libc-2.28.so) 7fff83a2b7a0 gaih_inet.constprop.7+0x1020 (/usr/lib64/power9/libc-2.28.so) 7fff83a2c170 getaddrinfo+0x160 (/usr/lib64/power9/libc-2.28.so) 1171830f4 [unknown] (/usr/bin/ping) FAIL: expected backtrace entry ".*\+0x[[:xdigit:]]+[[:space:]]\(.*/bin/ping.*\)$" got "1171830f4 [unknown] (/usr/bin/ping)" test child finished with -1 ---- end ---- probe libc's inet_pton & backtrace it with ping: FAILED! After: 59: probe libc's inet_pton & backtrace it with ping : --- start --- test child forked, pid 79085 ping 79108 [045] 96400.214177: probe_libc:inet_pton: (7fffbb9654c8) 7fffbb9654c8 __GI___inet_pton+0x8 (/usr/lib64/power9/libc-2.28.so) 7fffbb91b7a0 gaih_inet.constprop.7+0x1020 (/usr/lib64/power9/libc-2.28.so) 7fffbb91c170 getaddrinfo+0x160 (/usr/lib64/power9/libc-2.28.so) 132e830f4 [unknown] (/usr/bin/ping) test child finished with 0 ---- end ---- probe libc's inet_pton & backtrace it with ping: Ok Signed-off-by: Seeteena Thoufeek Reviewed-by: Kim Phillips Cc: Alexander Shishkin Cc: Hendrik Brueckner Cc: Jiri Olsa Cc: Michael Petlan Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Sandipan Das Fixes: 1632936480a5 ("perf tests: Fix record+probe_libc_inet_pton.sh without ping's debuginfo") Link: http://lkml.kernel.org/r/1561630614-3216-1-git-send-email-s1seetee@linux.vnet.ibm.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/shell/record+probe_libc_inet_pton.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh index 61c9f8fc6fa1..58a99a292930 100755 --- a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh +++ b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh @@ -44,7 +44,7 @@ trace_libc_inet_pton_backtrace() { eventattr='max-stack=4' echo "gaih_inet.*\+0x[[:xdigit:]]+[[:space:]]\($libc\)$" >> $expected echo "getaddrinfo\+0x[[:xdigit:]]+[[:space:]]\($libc\)$" >> $expected - echo ".*\+0x[[:xdigit:]]+[[:space:]]\(.*/bin/ping.*\)$" >> $expected + echo ".*(\+0x[[:xdigit:]]+|\[unknown\])[[:space:]]\(.*/bin/ping.*\)$" >> $expected ;; *) eventattr='max-stack=3' -- cgit v1.2.3 From d5b2179d6a675ee8cdbd3250d42f1e32d5a45fb1 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 28 May 2019 16:02:56 -0300 Subject: perf annotate TUI browser: Do not use member from variable within its own initialization Some compilers will complain when using a member of a struct to initialize another member, in the same struct initialization. For instance: debian:8 Debian clang version 3.5.0-10 (tags/RELEASE_350/final) (based on LLVM 3.5.0) oraclelinux:7 clang version 3.4.2 (tags/RELEASE_34/dot2-final) Produce: ui/browsers/annotate.c:104:12: error: variable 'ops' is uninitialized when used within its own initialization [-Werror,-Wuninitialized] (!ops.current_entry || ^~~ 1 error generated. So use an extra variable, initialized just before that struct, to have the value used in the expressions used to init two of the struct members. Cc: Adrian Hunter Cc: Jiri Olsa Cc: Namhyung Kim Fixes: c298304bd747 ("perf annotate: Use a ops table for annotation_line__write()") Link: https://lkml.kernel.org/n/tip-f9nexro58q62l3o9hez8hr0i@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/ui/browsers/annotate.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 98d934a36d86..b0d089a95dac 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -97,11 +97,12 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int struct annotate_browser *ab = container_of(browser, struct annotate_browser, b); struct annotation *notes = browser__annotation(browser); struct annotation_line *al = list_entry(entry, struct annotation_line, node); + const bool is_current_entry = ui_browser__is_current_entry(browser, row); struct annotation_write_ops ops = { .first_line = row == 0, - .current_entry = ui_browser__is_current_entry(browser, row), + .current_entry = is_current_entry, .change_color = (!notes->options->hide_src_code && - (!ops.current_entry || + (!is_current_entry || (browser->use_navkeypressed && !browser->navkeypressed))), .width = browser->width, -- cgit v1.2.3 From c18ae6327a13a6d707f6139c31597057103aa85b Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 28 May 2019 17:36:46 -0300 Subject: perf python: Remove -fstack-protector-strong if clang doesn't have it Some distros put -fstack-protector-strong in the compiler flags to be used to build python extensions, but then, the clang version in that distro doesn't know about that, only gcc does. Check if that is the case and remove it from the set of options used to build the python binding with clang. Case at hand: oraclelinux:7 $ head -2 /etc/os-release NAME="Oracle Linux Server" VERSION="7.6" $ grep stack-protector /usr/lib64/python2.7/_sysconfigdata.py | head -1 | cut -c-120 'CFLAGS': '-fno-strict-aliasing -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --para $ gcc version 4.8.5 20150623 (Red Hat 4.8.5-36.0.1) (GCC) clang version 3.4.2 (tags/RELEASE_34/dot2-final) clang: error: unknown argument: '-fstack-protector-strong' clang: error: unknown argument: '-fstack-protector-strong' error: command 'clang' failed with exit status 1 cp: cannot stat '/tmp/build/perf/python_ext_build/lib/perf*.so': No such file or directory make[2]: *** [/tmp/build/perf/python/perf.so] Error 1 Cc: Adrian Hunter Cc: Jiri Olsa Cc: Namhyung Kim Link: https://lkml.kernel.org/n/tip-brmp2415zxpbhz45etkgjoma@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/setup.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py index 5b5a167b43ce..a1a68a2fa917 100644 --- a/tools/perf/util/setup.py +++ b/tools/perf/util/setup.py @@ -17,6 +17,8 @@ if cc == "clang": vars[var] = sub("-fcf-protection", "", vars[var]) if not clang_has_option("-fstack-clash-protection"): vars[var] = sub("-fstack-clash-protection", "", vars[var]) + if not clang_has_option("-fstack-protector-strong"): + vars[var] = sub("-fstack-protector-strong", "", vars[var]) from distutils.core import setup, Extension -- cgit v1.2.3 From dab0f4ebb22ee6d16051011d624cff79a99baa8a Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Fri, 31 May 2019 15:13:21 +0200 Subject: perf jvmti: Address gcc string overflow warning for strncpy() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We are getting false positive gcc warning when we compile with gcc9 (9.1.1): CC jvmti/libjvmti.o In file included from /usr/include/string.h:494, from jvmti/libjvmti.c:5: In function ‘strncpy’, inlined from ‘copy_class_filename.constprop’ at jvmti/libjvmti.c:166:3: /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=] 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ jvmti/libjvmti.c: In function ‘copy_class_filename.constprop’: jvmti/libjvmti.c:165:26: note: length computed here 165 | size_t file_name_len = strlen(file_name); | ^~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors As per Arnaldo's suggestion use strlcpy(), which does the same thing and keeps gcc silent. Suggested-by: Arnaldo Carvalho de Melo Signed-off-by: Jiri Olsa Cc: Alexander Shishkin Cc: Ben Gainey Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/20190531131321.GB1281@krava Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/jvmti/libjvmti.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c index aea7b1fe85aa..c441a34cb1c0 100644 --- a/tools/perf/jvmti/libjvmti.c +++ b/tools/perf/jvmti/libjvmti.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 #include +#include #include #include #include @@ -162,8 +163,7 @@ copy_class_filename(const char * class_sign, const char * file_name, char * resu result[i] = '\0'; } else { /* fallback case */ - size_t file_name_len = strlen(file_name); - strncpy(result, file_name, file_name_len < max_length ? file_name_len : max_length); + strlcpy(result, file_name, max_length); } } -- cgit v1.2.3 From 05c78468a60f2fd961cd0a0c01c27f288bf81204 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 13 Jun 2019 12:04:19 -0300 Subject: tools build: Check if gettid() is available before providing helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Laura reported that the perf build failed in fedora when we got a glibc that provides gettid(), which I reproduced using fedora rawhide with the glibc-devel-2.29.9000-26.fc31.x86_64 package. Add a feature check to avoid providing a gettid() helper in such systems. On a fedora rawhide system with this patch applied we now get: [root@7a5f55352234 perf]# grep gettid /tmp/build/perf/FEATURE-DUMP feature-gettid=1 [root@7a5f55352234 perf]# cat /tmp/build/perf/feature/test-gettid.make.output [root@7a5f55352234 perf]# ldd /tmp/build/perf/feature/test-gettid.bin linux-vdso.so.1 (0x00007ffc6b1f6000) libc.so.6 => /lib64/libc.so.6 (0x00007f04e0a74000) /lib64/ld-linux-x86-64.so.2 (0x00007f04e0c47000) [root@7a5f55352234 perf]# nm /tmp/build/perf/feature/test-gettid.bin | grep -w gettid U gettid@@GLIBC_2.30 [root@7a5f55352234 perf]# While on a fedora:29 system: [acme@quaco perf]$ grep gettid /tmp/build/perf/FEATURE-DUMP feature-gettid=0 [acme@quaco perf]$ cat /tmp/build/perf/feature/test-gettid.make.output test-gettid.c: In function ‘main’: test-gettid.c:8:9: error: implicit declaration of function ‘gettid’; did you mean ‘getgid’? [-Werror=implicit-function-declaration] return gettid(); ^~~~~~ getgid cc1: all warnings being treated as errors [acme@quaco perf]$ Reported-by: Laura Abbott Tested-by: Laura Abbott Acked-by: Jiri Olsa Cc: Adrian Hunter Cc: Florian Weimer Cc: Namhyung Kim Cc: Stephane Eranian Link: https://lkml.kernel.org/n/tip-yfy3ch53agmklwu9o7rlgf9c@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/build/Makefile.feature | 1 + tools/build/feature/Makefile | 4 ++++ tools/build/feature/test-all.c | 5 +++++ tools/build/feature/test-gettid.c | 11 +++++++++++ tools/perf/Makefile.config | 4 ++++ tools/perf/jvmti/jvmti_agent.c | 2 ++ 6 files changed, 27 insertions(+) create mode 100644 tools/build/feature/test-gettid.c diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature index 3b24231c58a2..50377cc2f5f9 100644 --- a/tools/build/Makefile.feature +++ b/tools/build/Makefile.feature @@ -36,6 +36,7 @@ FEATURE_TESTS_BASIC := \ fortify-source \ sync-compare-and-swap \ get_current_dir_name \ + gettid \ glibc \ gtk2 \ gtk2-infobar \ diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile index 4b8244ee65ce..523ee42db0c8 100644 --- a/tools/build/feature/Makefile +++ b/tools/build/feature/Makefile @@ -54,6 +54,7 @@ FILES= \ test-get_cpuid.bin \ test-sdt.bin \ test-cxx.bin \ + test-gettid.bin \ test-jvmti.bin \ test-jvmti-cmlr.bin \ test-sched_getcpu.bin \ @@ -267,6 +268,9 @@ $(OUTPUT)test-sdt.bin: $(OUTPUT)test-cxx.bin: $(BUILDXX) -std=gnu++11 +$(OUTPUT)test-gettid.bin: + $(BUILD) + $(OUTPUT)test-jvmti.bin: $(BUILD) diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c index a59c53705093..3b3d5d72124a 100644 --- a/tools/build/feature/test-all.c +++ b/tools/build/feature/test-all.c @@ -38,6 +38,10 @@ # include "test-get_current_dir_name.c" #undef main +#define main main_test_gettid +# include "test-gettid.c" +#undef main + #define main main_test_glibc # include "test-glibc.c" #undef main @@ -195,6 +199,7 @@ int main(int argc, char *argv[]) main_test_libelf(); main_test_libelf_mmap(); main_test_get_current_dir_name(); + main_test_gettid(); main_test_glibc(); main_test_dwarf(); main_test_dwarf_getlocations(); diff --git a/tools/build/feature/test-gettid.c b/tools/build/feature/test-gettid.c new file mode 100644 index 000000000000..ef24e42d3f1b --- /dev/null +++ b/tools/build/feature/test-gettid.c @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2019, Red Hat Inc, Arnaldo Carvalho de Melo +#define _GNU_SOURCE +#include + +int main(void) +{ + return gettid(); +} + +#undef _GNU_SOURCE diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config index 85fbcd265351..a6251e622c7e 100644 --- a/tools/perf/Makefile.config +++ b/tools/perf/Makefile.config @@ -332,6 +332,10 @@ ifeq ($(feature-get_current_dir_name), 1) CFLAGS += -DHAVE_GET_CURRENT_DIR_NAME endif +ifeq ($(feature-gettid), 1) + CFLAGS += -DHAVE_GETTID +endif + ifdef NO_LIBELF NO_DWARF := 1 NO_DEMANGLE := 1 diff --git a/tools/perf/jvmti/jvmti_agent.c b/tools/perf/jvmti/jvmti_agent.c index f7eb63cbbc65..88108598d6e9 100644 --- a/tools/perf/jvmti/jvmti_agent.c +++ b/tools/perf/jvmti/jvmti_agent.c @@ -45,10 +45,12 @@ static char jit_path[PATH_MAX]; static void *marker_addr; +#ifndef HAVE_GETTID static inline pid_t gettid(void) { return (pid_t)syscall(__NR_gettid); } +#endif static int get_e_machine(struct jitheader *hdr) { -- cgit v1.2.3 From 686cbe9e5d88ad639bbe26d963e7d5dafa1c1c28 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 8 Jul 2019 13:47:14 -0300 Subject: tools arch x86: Sync asm/cpufeatures.h with the with the kernel To pick up the changes in: 6dbbf5ec9e1e ("x86/cpufeatures: Enumerate user wait instructions") b302e4b176d0 ("x86/cpufeatures: Enumerate the new AVX512 BFLOAT16 instructions") acec0ce081de ("x86/cpufeatures: Combine word 11 and 12 into a new scattered features word") cbb99c0f5887 ("x86/cpufeatures: Add FDP_EXCPTN_ONLY and ZERO_FCS_FDS") That don't affect anything in tools/. This silences this perf build warning: Warning: Kernel ABI header at 'tools/arch/x86/include/asm/cpufeatures.h' differs from latest version at 'arch/x86/include/asm/cpufeatures.h' diff -u tools/arch/x86/include/asm/cpufeatures.h arch/x86/include/asm/cpufeatures.h Cc: Aaron Lewis Cc: Adrian Hunter Cc: Borislav Petkov Cc: Fenghua Yu Cc: Jiri Olsa Cc: Namhyung Kim Cc: Thomas Gleixner Link: https://lkml.kernel.org/n/tip-y60wnyg2fuxi0hx7icruo9po@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/arch/x86/include/asm/cpufeatures.h | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tools/arch/x86/include/asm/cpufeatures.h b/tools/arch/x86/include/asm/cpufeatures.h index 75f27ee2c263..998c2cc08363 100644 --- a/tools/arch/x86/include/asm/cpufeatures.h +++ b/tools/arch/x86/include/asm/cpufeatures.h @@ -239,12 +239,14 @@ #define X86_FEATURE_BMI1 ( 9*32+ 3) /* 1st group bit manipulation extensions */ #define X86_FEATURE_HLE ( 9*32+ 4) /* Hardware Lock Elision */ #define X86_FEATURE_AVX2 ( 9*32+ 5) /* AVX2 instructions */ +#define X86_FEATURE_FDP_EXCPTN_ONLY ( 9*32+ 6) /* "" FPU data pointer updated only on x87 exceptions */ #define X86_FEATURE_SMEP ( 9*32+ 7) /* Supervisor Mode Execution Protection */ #define X86_FEATURE_BMI2 ( 9*32+ 8) /* 2nd group bit manipulation extensions */ #define X86_FEATURE_ERMS ( 9*32+ 9) /* Enhanced REP MOVSB/STOSB instructions */ #define X86_FEATURE_INVPCID ( 9*32+10) /* Invalidate Processor Context ID */ #define X86_FEATURE_RTM ( 9*32+11) /* Restricted Transactional Memory */ #define X86_FEATURE_CQM ( 9*32+12) /* Cache QoS Monitoring */ +#define X86_FEATURE_ZERO_FCS_FDS ( 9*32+13) /* "" Zero out FPU CS and FPU DS */ #define X86_FEATURE_MPX ( 9*32+14) /* Memory Protection Extension */ #define X86_FEATURE_RDT_A ( 9*32+15) /* Resource Director Technology Allocation */ #define X86_FEATURE_AVX512F ( 9*32+16) /* AVX-512 Foundation */ @@ -269,13 +271,19 @@ #define X86_FEATURE_XGETBV1 (10*32+ 2) /* XGETBV with ECX = 1 instruction */ #define X86_FEATURE_XSAVES (10*32+ 3) /* XSAVES/XRSTORS instructions */ -/* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:0 (EDX), word 11 */ -#define X86_FEATURE_CQM_LLC (11*32+ 1) /* LLC QoS if 1 */ +/* + * Extended auxiliary flags: Linux defined - for features scattered in various + * CPUID levels like 0xf, etc. + * + * Reuse free bits when adding new feature flags! + */ +#define X86_FEATURE_CQM_LLC (11*32+ 0) /* LLC QoS if 1 */ +#define X86_FEATURE_CQM_OCCUP_LLC (11*32+ 1) /* LLC occupancy monitoring */ +#define X86_FEATURE_CQM_MBM_TOTAL (11*32+ 2) /* LLC Total MBM monitoring */ +#define X86_FEATURE_CQM_MBM_LOCAL (11*32+ 3) /* LLC Local MBM monitoring */ -/* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:1 (EDX), word 12 */ -#define X86_FEATURE_CQM_OCCUP_LLC (12*32+ 0) /* LLC occupancy monitoring */ -#define X86_FEATURE_CQM_MBM_TOTAL (12*32+ 1) /* LLC Total MBM monitoring */ -#define X86_FEATURE_CQM_MBM_LOCAL (12*32+ 2) /* LLC Local MBM monitoring */ +/* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */ +#define X86_FEATURE_AVX512_BF16 (12*32+ 5) /* AVX512 BFLOAT16 instructions */ /* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */ #define X86_FEATURE_CLZERO (13*32+ 0) /* CLZERO instruction */ @@ -322,6 +330,7 @@ #define X86_FEATURE_UMIP (16*32+ 2) /* User Mode Instruction Protection */ #define X86_FEATURE_PKU (16*32+ 3) /* Protection Keys for Userspace */ #define X86_FEATURE_OSPKE (16*32+ 4) /* OS Protection Keys Enable */ +#define X86_FEATURE_WAITPKG (16*32+ 5) /* UMONITOR/UMWAIT/TPAUSE Instructions */ #define X86_FEATURE_AVX512_VBMI2 (16*32+ 6) /* Additional AVX512 Vector Bit Manipulation Instructions */ #define X86_FEATURE_GFNI (16*32+ 8) /* Galois Field New Instructions */ #define X86_FEATURE_VAES (16*32+ 9) /* Vector AES */ -- cgit v1.2.3