Patch "perf annotate bpf: Don't enclose non-debug code with an assert()" has been added to the 5.10-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    perf annotate bpf: Don't enclose non-debug code with an assert()

to the 5.10-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     perf-annotate-bpf-don-t-enclose-non-debug-code-with-.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 609366315a3590a8b48720d42b2e6dc66434f2f7
Author: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
Date:   Wed Aug 2 18:22:14 2023 -0300

    perf annotate bpf: Don't enclose non-debug code with an assert()
    
    [ Upstream commit 979e9c9fc9c2a761303585e07fe2699bdd88182f ]
    
    In 616b14b47a86d880 ("perf build: Conditionally define NDEBUG") we
    started using NDEBUG=1 when DEBUG=1 isn't present, so code that is
    enclosed with assert() is not called.
    
    In dd317df072071903 ("perf build: Make binutil libraries opt in") we
    stopped linking against binutils-devel, for licensing reasons.
    
    Recently people asked me why annotation of BPF programs wasn't working,
    i.e. this:
    
      $ perf annotate bpf_prog_5280546344e3f45c_kfree_skb
    
    was returning:
    
      case SYMBOL_ANNOTATE_ERRNO__NO_LIBOPCODES_FOR_BPF:
         scnprintf(buf, buflen, "Please link with binutils's libopcode to enable BPF annotation");
    
    This was on a fedora rpm, so its new enough that I had to try to test by
    rebuilding using BUILD_NONDISTRO=1, only to get it segfaulting on me.
    
    This combination made this libopcode function not to be called:
    
            assert(bfd_check_format(bfdf, bfd_object));
    
    Changing it to:
    
            if (!bfd_check_format(bfdf, bfd_object))
                    abort();
    
    Made it work, looking at this "check" function made me realize it
    changes the 'bfdf' internal state, i.e. we better call it.
    
    So stop using assert() on it, just call it and abort if it fails.
    
    Probably it is better to propagate the error, etc, but it seems it is
    unlikely to fail from the usage done so far and we really need to stop
    using libopcodes, so do the quick fix above and move on.
    
    With it we have BPF annotation back working when built with
    BUILD_NONDISTRO=1:
    
      ⬢[acme@toolbox perf-tools-next]$ perf annotate --stdio2 bpf_prog_5280546344e3f45c_kfree_skb   | head
      No kallsyms or vmlinux with build-id 939bc71a1a51cdc434e60af93c7e734f7d5c0e7e was found
      Samples: 12  of event 'cpu-clock:ppp', 4000 Hz, Event count (approx.): 3000000, [percent: local period]
      bpf_prog_5280546344e3f45c_kfree_skb() bpf_prog_5280546344e3f45c_kfree_skb
      Percent      int kfree_skb(struct trace_event_raw_kfree_skb *args) {
                     nop
       33.33         xchg   %ax,%ax
                     push   %rbp
                     mov    %rsp,%rbp
                     sub    $0x180,%rsp
                     push   %rbx
                     push   %r13
      ⬢[acme@toolbox perf-tools-next]$
    
    Fixes: 6987561c9e86eace ("perf annotate: Enable annotation of BPF programs")
    Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
    Cc: Ian Rogers <irogers@xxxxxxxxxx>
    Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
    Cc: Mohamed Mahmoud <mmahmoud@xxxxxxxxxx>
    Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
    Cc: Dave Tucker <datucker@xxxxxxxxxx>
    Cc: Derek Barbosa <debarbos@xxxxxxxxxx>
    Cc: Song Liu <songliubraving@xxxxxx>
    Link: https://lore.kernel.org/lkml/ZMrMzoQBe0yqMek1@xxxxxxxxxx
    Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 3081894547883..c9078cee6be01 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1718,8 +1718,11 @@ static int symbol__disassemble_bpf(struct symbol *sym,
 	perf_exe(tpath, sizeof(tpath));
 
 	bfdf = bfd_openr(tpath, NULL);
-	assert(bfdf);
-	assert(bfd_check_format(bfdf, bfd_object));
+	if (bfdf == NULL)
+		abort();
+
+	if (!bfd_check_format(bfdf, bfd_object))
+		abort();
 
 	s = open_memstream(&buf, &buf_size);
 	if (!s) {
@@ -1767,7 +1770,8 @@ static int symbol__disassemble_bpf(struct symbol *sym,
 #else
 	disassemble = disassembler(bfdf);
 #endif
-	assert(disassemble);
+	if (disassemble == NULL)
+		abort();
 
 	fflush(s);
 	do {



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux