• Thomas Richter's avatar
    perf test: Set NULL sentinel in pmu_events table in "Parse and process metrics" test · 492d4d87
    Thomas Richter authored
    Linux 5.9 introduced perf test case "Parse and process metrics" and
    on s390 this test case always dumps core:
    
      [root@t35lp67 perf]# ./perf test -vvvv -F 67
      67: Parse and process metrics                             :
      --- start ---
      metric expr inst_retired.any / cpu_clk_unhalted.thread for IPC
      parsing metric: inst_retired.any / cpu_clk_unhalted.thread
      Segmentation fault (core dumped)
      [root@t35lp67 perf]#
    
    I debugged this core dump and gdb shows this call chain:
    
      (gdb) where
       #0  0x000003ffabc3192a in __strnlen_c_1 () from /lib64/libc.so.6
       #1  0x000003ffabc293de in strcasestr () from /lib64/libc.so.6
       #2  0x0000000001102ba2 in match_metric(list=0x1e6ea20 "inst_retired.any",
                n=<optimized out>)
           at util/metricgroup.c:368
       #3  find_metric (map=<optimized out>, map=<optimized out>,
               metric=0x1e6ea20 "inst_retired.any")
          at util/metricgroup.c:765
       #4  __resolve_metric (ids=0x0, map=<optimized out>, metric_list=0x0,
               metric_no_group=<optimized out>, m=<optimized out>)
          at util/metricgroup.c:844
       #5  resolve_metric (ids=0x0, map=0x0, metric_list=0x0,
              metric_no_group=<optimized out>)
          at util/metricgroup.c:881
       #6  metricgroup__add_metric (metric=<optimized out>,
            metric_no_group=metric_no_group@entry=false, events=<optimized out>,
            events@entry=0x3ffd84fb878, metric_list=0x0,
            metric_list@entry=0x3ffd84fb868, map=0x0)
          at util/metricgroup.c:943
       #7  0x00000000011034ae in metricgroup__add_metric_list (map=0x13f9828 <map>,
            metric_list=0x3ffd84fb868, events=0x3ffd84fb878,
            metric_no_group=<optimized out>, list=<optimized out>)
          at util/metricgroup.c:988
       #8  parse_groups (perf_evlist=perf_evlist@entry=0x1e70260,
              str=str@entry=0x12f34b2 "IPC", metric_no_group=<optimized out>,
              metric_no_merge=<optimized out>,
              fake_pmu=fake_pmu@entry=0x1462f18 <perf_pmu.fake>,
              metric_events=0x3ffd84fba58, map=0x1)
          at util/metricgroup.c:1040
       #9  0x0000000001103eb2 in metricgroup__parse_groups_test(
      	evlist=evlist@entry=0x1e70260, map=map@entry=0x13f9828 <map>,
      	str=str@entry=0x12f34b2 "IPC",
      	metric_no_group=metric_no_group@entry=false,
      	metric_no_merge=metric_no_merge@entry=false,
      	metric_events=0x3ffd84fba58)
          at util/metricgroup.c:1082
       #10 0x00000000010c84d8 in __compute_metric (ratio2=0x0, name2=0x0,
              ratio1=<synthetic pointer>, name1=0x12f34b2 "IPC",
      	vals=0x3ffd84fbad8, name=0x12f34b2 "IPC")
          at tests/parse-metric.c:159
       #11 compute_metric (ratio=<synthetic pointer>, vals=0x3ffd84fbad8,
      	name=0x12f34b2 "IPC")
          at tests/parse-metric.c:189
       #12 test_ipc () at tests/parse-metric.c:208
    .....
    ..... omitted many more lines
    
    This test case was added with
    commit 218ca91d ("perf tests: Add parse metric test for frontend metric").
    
    When I compile with make DEBUG=y it works fine and I do not get a core dump.
    
    It turned out that the above listed function call chain worked on a struct
    pmu_event array which requires a trailing element with zeroes which was
    missing. The marco map_for_each_event() loops over that array tests for members
    metric_expr/metric_name/metric_group being non-NULL. Adding this element fixes
    the issue.
    
    Output after:
    
      [root@t35lp46 perf]# ./perf test 67
      67: Parse and process metrics                             : Ok
      [root@t35lp46 perf]#
    
    Committer notes:
    
    As Ian remarks, this is not s390 specific:
    
    <quote Ian>
      This also shows up with address sanitizer on all architectures
      (perhaps change the patch title) and perhaps add a "Fixes: <commit>"
      tag.
    
      =================================================================
      ==4718==ERROR: AddressSanitizer: global-buffer-overflow on address
      0x55c93b4d59e8 at pc 0x55c93a1541e2 bp 0x7ffd24327c60 sp
      0x7ffd24327c58
      READ of size 8 at 0x55c93b4d59e8 thread T0
          #0 0x55c93a1541e1 in find_metric tools/perf/util/metricgroup.c:764:2
          #1 0x55c93a153e6c in __resolve_metric tools/perf/util/metricgroup.c:844:9
          #2 0x55c93a152f18 in resolve_metric tools/perf/util/metricgroup.c:881:9
          #3 0x55c93a1528db in metricgroup__add_metric
      tools/perf/util/metricgroup.c:943:9
          #4 0x55c93a151996 in metricgroup__add_metric_list
      tools/perf/util/metricgroup.c:988:9
          #5 0x55c93a1511b9 in parse_groups tools/perf/util/metricgroup.c:1040:8
          #6 0x55c93a1513e1 in metricgroup__parse_groups_test
      tools/perf/util/metricgroup.c:1082:9
          #7 0x55c93a0108ae in __compute_metric tools/perf/tests/parse-metric.c:159:8
          #8 0x55c93a010744 in compute_metric tools/perf/tests/parse-metric.c:189:9
          #9 0x55c93a00f5ee in test_ipc tools/perf/tests/parse-metric.c:208:2
          #10 0x55c93a00f1e8 in test__parse_metric
      tools/perf/tests/parse-metric.c:345:2
          #11 0x55c939fd7202 in run_test tools/perf/tests/builtin-test.c:410:9
          #12 0x55c939fd6736 in test_and_print tools/perf/tests/builtin-test.c:440:9
          #13 0x55c939fd58c3 in __cmd_test tools/perf/tests/builtin-test.c:661:4
          #14 0x55c939fd4e02 in cmd_test tools/perf/tests/builtin-test.c:807:9
          #15 0x55c939e4763d in run_builtin tools/perf/perf.c:313:11
          #16 0x55c939e46475 in handle_internal_command tools/perf/perf.c:365:8
          #17 0x55c939e4737e in run_argv tools/perf/perf.c:409:2
          #18 0x55c939e45f7e in main tools/perf/perf.c:539:3
    
      0x55c93b4d59e8 is located 0 bytes to the right of global variable
      'pme_test' defined in 'tools/perf/tests/parse-metric.c:17:25'
      (0x55c93b4d54a0) of size 1352
      SUMMARY: AddressSanitizer: global-buffer-overflow
      tools/perf/util/metricgroup.c:764:2 in find_metric
      Shadow bytes around the buggy address:
        0x0ab9a7692ae0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        0x0ab9a7692af0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        0x0ab9a7692b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        0x0ab9a7692b10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        0x0ab9a7692b20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
      =>0x0ab9a7692b30: 00 00 00 00 00 00 00 00 00 00 00 00 00[f9]f9 f9
        0x0ab9a7692b40: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
        0x0ab9a7692b50: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
        0x0ab9a7692b60: f9 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
        0x0ab9a7692b70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
        0x0ab9a7692b80: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
      Shadow byte legend (one shadow byte represents 8 application bytes):
        Addressable:           00
        Partially addressable: 01 02 03 04 05 06 07
        Heap left redzone:	   fa
        Freed heap region:	   fd
        Stack left redzone:	   f1
        Stack mid redzone:	   f2
        Stack right redzone:     f3
        Stack after return:	   f5
        Stack use after scope:   f8
        Global redzone:          f9
        Global init order:	   f6
        Poisoned by user:        f7
        Container overflow:	   fc
        Array cookie:            ac
        Intra object redzone:    bb
        ASan internal:           fe
        Left alloca redzone:     ca
        Right alloca redzone:    cb
        Shadow gap:              cc
    </quote>
    
    I'm also adding the missing "Fixes" tag and setting just .name to NULL,
    as doing it that way is more compact (the compiler will zero out
    everything else) and the table iterators look for .name being NULL as
    the sentinel marking the end of the table.
    
    Fixes: 0a507af9 ("perf tests: Add parse metric test for ipc metric")
    Signed-off-by: default avatarThomas Richter <tmricht@linux.ibm.com>
    Reviewed-by: default avatarSumanth Korikkar <sumanthk@linux.ibm.com>
    Acked-by: default avatarIan Rogers <irogers@google.com>
    Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Sven Schnelle <svens@linux.ibm.com>
    Cc: Vasily Gorbik <gor@linux.ibm.com>
    Link: http://lore.kernel.org/lkml/20200825071211.16959-1-tmricht@linux.ibm.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
    492d4d87
parse-metric.c 9.14 KB