From nobody Wed Nov 5 18:54:54 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DFA1BEB64DB for ; Thu, 15 Jun 2023 02:51:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237736AbjFOCvT (ORCPT ); Wed, 14 Jun 2023 22:51:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38814 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231177AbjFOCvR (ORCPT ); Wed, 14 Jun 2023 22:51:17 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1FA46E3 for ; Wed, 14 Jun 2023 19:51:16 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id 3f1490d57ef6-bd7f0e5df8eso1555744276.1 for ; Wed, 14 Jun 2023 19:51:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1686797475; x=1689389475; h=to:from:subject:mime-version:message-id:date:from:to:cc:subject :date:message-id:reply-to; bh=gVuSMQJnyJqCPMn5wsYe5ruCFs+oyn3bf7+bzMc2VbA=; b=t2ELNQ3GM7CTdlb9230ffQCHUNkSOyG6FHs6z2PtiQ+8pdVl1Y1TrKhOxbvQZshNZf LtAOSjoNVtIHtBfpCm4ZguKPFjf7guyQxPtgAX9giEABWtSPmlT4bru+bc3gtLmgLWbR qlRtYKTjjhESmK0fi9jiFBW/duQknAYD01Xo6+EAcFimMWyWZrM3+Olkd2HJWKEbaeoZ n1VqyJjhOiewyHytbzJWCdHbnHVOltdzrM1uddprBM7pDH4lhgv68kurNc9eC/LQhGLl k7wiSBmQGVfoQ3bQkJ1rZe6iVTDIX808nOcJuxtp6yXAGB2zBvnAioVtpWsnuGg7hw7T GdyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686797475; x=1689389475; h=to:from:subject:mime-version:message-id:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=gVuSMQJnyJqCPMn5wsYe5ruCFs+oyn3bf7+bzMc2VbA=; b=KY0yzoHHQbMxnvdEZX85syQi/NGySdtW0RNxrnuIlwVoyqG4amrjsY/WFgRwfHbGiP sVHfNqZdlVWIBpjsq/olc+7JTE85a3KgDRylnIPTjj0c/QDNc3/YqBamShCkgZCDalGy qQCWrzkDJj4WBK1h1WjZFj8L2bmjA/qqsfpowYmHYAGu5lS3SduPVd2lTt7Zc4F83zqM 9Nmk9vfido63wyFEy4MOyPJ0v6yD2EG1coQVo62HlYw7QZi3DndpMFZfvPCjfbDx6qwB 18G+tr1zWc5f4Xt5nLvVSE6xC5LgYDT8PvpK4MJDNKkqPUBiP2He2he0eR2krQOELApx NaBA== X-Gm-Message-State: AC+VfDxUouz70ytk+Se9TNAhTTdXrMQNa5KbizZbUhUiIRG0yTWQzKb5 1jXJ3vc/ctre+h1PQuhJNEYHX4TNYmX3 X-Google-Smtp-Source: ACHHUZ7AvmRKIAMspzKtWYz8IrakoMh8S7Qs/SZ2WaA8e/Ns75rUCQVbSCgQvZCym0Ej7MoGX76mbAUvfkhi X-Received: from irogers.svl.corp.google.com ([2620:15c:2d4:203:768d:713f:8ce8:1d35]) (user=irogers job=sendgmr) by 2002:a25:20a:0:b0:bc6:3354:e65e with SMTP id 10-20020a25020a000000b00bc63354e65emr579424ybc.7.1686797475352; Wed, 14 Jun 2023 19:51:15 -0700 (PDT) Date: Wed, 14 Jun 2023 19:50:41 -0700 Message-Id: <20230615025041.1982072-1-irogers@google.com> Mime-Version: 1.0 X-Mailer: git-send-email 2.41.0.162.gfafddb0af9-goog Subject: [PATCH v1] perf srcline: Fix handling of inline functions From: Ian Rogers To: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Ian Rogers , Adrian Hunter , Nathan Chancellor , Nick Desaulniers , Tom Rix , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, llvm@lists.linux.dev Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" We write an address then a ',' to addr2line. With inline data we generally get back (// are my comments): 0x1234 // address foo // function name foo.c:123 // filename:line bar // function name bar.c:123 // filename:line 0x000000000000000 // sentinel address created by ',' ?? // unknown function name ??:0 // unknown filename:line The code was assuming the inline data also had the address, which is incorrect. This means the first inline function name (bar above) needs to be checked to see if it is the sentinel, otherwise to be treated as a function name. The regression was caused by the addition of addresses as the kernel is reporting a symbol at address 0 (used by GNU binutils when it interprets ','). Fixes: 8dc26b6f718a ("perf srcline: Make sentinel reading for binutils addr= 2line more robust") Reported-by: Arnaldo Carvalho de Melo Signed-off-by: Ian Rogers --- tools/perf/util/srcline.c | 136 ++++++++++++++++++++++---------------- 1 file changed, 80 insertions(+), 56 deletions(-) diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c index c013bcbdfd42..034b496df297 100644 --- a/tools/perf/util/srcline.c +++ b/tools/perf/util/srcline.c @@ -389,7 +389,7 @@ static int filename_split(char *filename, unsigned int = *line_nr) *line_nr =3D strtoul(sep, NULL, 0); return 1; } - + pr_debug("addr2line missing ':' in filename split\n"); return 0; } =20 @@ -465,10 +465,12 @@ static enum a2l_style addr2line_configure(struct chil= d_process *a2l, const char style =3D LLVM; cached =3D true; lines =3D 1; + pr_debug("Detected LLVM addr2line style\n"); } else if (ch =3D=3D '0') { style =3D GNU_BINUTILS; cached =3D true; lines =3D 3; + pr_debug("Detected binutils addr2line style\n"); } else { if (!symbol_conf.disable_add2line_warn) { char *output =3D NULL; @@ -479,6 +481,7 @@ static enum a2l_style addr2line_configure(struct child_= process *a2l, const char __func__, dso_name); pr_warning("\t%c%s", ch, output); } + pr_debug("Unknown/broken addr2line style\n"); return BROKEN; } while (lines) { @@ -496,6 +499,9 @@ static enum a2l_style addr2line_configure(struct child_= process *a2l, const char =20 static int read_addr2line_record(struct io *io, enum a2l_style style, + const char *dso_name, + u64 addr, + bool first, char **function, char **filename, unsigned int *line_nr) @@ -521,56 +527,62 @@ static int read_addr2line_record(struct io *io, *line_nr =3D 0; =20 /* - * Read the first line. Without an error this will be either an address - * like 0x1234 or for llvm-addr2line the sentinal ',' character. + * Read the first line. Without an error this will be: + * - for the first line an address like 0x1234, + * - the binutils sentinel 0x0000000000000000, + * - the llvm-addr2line the sentinel ',' character, + * - the function name line for an inlined function. */ if (io__getline(io, &line, &line_len) < 0 || !line_len) goto error; =20 - if (style =3D=3D LLVM) { - if (line_len =3D=3D 2 && line[0] =3D=3D ',') { - zfree(&line); - return 0; - } - } else { + pr_debug("%s %s: addr2line read address for sentinel: %s", __func__, dso_= name, line); + if (style =3D=3D LLVM && line_len =3D=3D 2 && line[0] =3D=3D ',') { + /* Found the llvm-addr2line sentinel character. */ + zfree(&line); + return 0; + } else if (style =3D=3D GNU_BINUTILS && (!first || addr !=3D 0)) { int zero_count =3D 0, non_zero_count =3D 0; + /* + * Check for binutils sentinel ignoring it for the case the + * requested address is 0. + */ =20 - /* The address should always start 0x. */ - if (line_len < 2 || line[0] !=3D '0' || line[1] !=3D 'x') - goto error; - - for (size_t i =3D 2; i < line_len; i++) { - if (line[i] =3D=3D '0') - zero_count++; - else if (line[i] !=3D '\n') - non_zero_count++; - } - if (!non_zero_count) { - int ch; - - if (!zero_count) { - /* Line was erroneous just '0x'. */ - goto error; + /* A given address should always start 0x. */ + if (line_len >=3D 2 || line[0] !=3D '0' || line[1] !=3D 'x') { + for (size_t i =3D 2; i < line_len; i++) { + if (line[i] =3D=3D '0') + zero_count++; + else if (line[i] !=3D '\n') + non_zero_count++; + } + if (!non_zero_count) { + int ch; + + if (first && !zero_count) { + /* Line was erroneous just '0x'. */ + goto error; + } + /* + * Line was 0x0..0, the sentinel for binutils. Remove + * the function and filename lines. + */ + zfree(&line); + do { + ch =3D io__get_char(io); + } while (ch > 0 && ch !=3D '\n'); + do { + ch =3D io__get_char(io); + } while (ch > 0 && ch !=3D '\n'); + return 0; } - /* - * Line was 0x0..0, the sentinel for binutils. Remove - * the function and filename lines. - */ - zfree(&line); - do { - ch =3D io__get_char(io); - } while (ch > 0 && ch !=3D '\n'); - do { - ch =3D io__get_char(io); - } while (ch > 0 && ch !=3D '\n'); - return 0; } } - - /* Read the second function name line. */ - if (io__getline(io, &line, &line_len) < 0 || !line_len) + /* Read the second function name line (if inline data then this is the fi= rst line). */ + if (first && (io__getline(io, &line, &line_len) < 0 || !line_len)) goto error; =20 + pr_debug("%s %s: addr2line read line: %s", __func__, dso_name, line); if (function !=3D NULL) *function =3D strdup(strim(line)); =20 @@ -581,6 +593,7 @@ static int read_addr2line_record(struct io *io, if (io__getline(io, &line, &line_len) < 0 || !line_len) goto error; =20 + pr_debug("%s %s: addr2line filename:number : %s", __func__, dso_name, lin= e); if (filename_split(line, line_nr =3D=3D NULL ? &dummy_line_nr : line_nr) = =3D=3D 0 && style =3D=3D GNU_BINUTILS) { ret =3D 0; @@ -640,8 +653,7 @@ static int addr2line(const char *dso_name, u64 addr, if (!filename__has_section(dso_name, ".debug_line")) goto out; =20 - dso->a2l =3D addr2line_subprocess_init(symbol_conf.addr2line_path, - dso_name); + dso->a2l =3D addr2line_subprocess_init(symbol_conf.addr2line_path, dso_n= ame); a2l =3D dso->a2l; } =20 @@ -655,14 +667,13 @@ static int addr2line(const char *dso_name, u64 addr, goto out; =20 /* - * Send our request and then *deliberately* send something that can't be = interpreted as - * a valid address to ask addr2line about (namely, ","). This causes addr= 2line to first - * write out the answer to our request, in an unbounded/unknown number of= records, and - * then to write out the lines "??" and "??:0", for GNU binutils, or "," = for - * llvm-addr2line, so that we can detect when it has finished giving us a= nything - * useful. With GNU binutils, we have to be careful about the first recor= d, though, - * because it may be genuinely unknown, in which case we'll get two sets = of "??"/"??:0" - * lines. + * Send our request and then *deliberately* send something that can't be + * interpreted as a valid address to ask addr2line about (namely, + * ","). This causes addr2line to first write out the answer to our + * request, in an unbounded/unknown number of records, and then to write + * out the lines "0x0...0", "??" and "??:0", for GNU binutils, or "," + * for llvm-addr2line, so that we can detect when it has finished giving + * us anything useful. */ len =3D snprintf(buf, sizeof(buf), "%016"PRIx64"\n,\n", addr); written =3D len > 0 ? write(a2l->in, buf, len) : -1; @@ -673,7 +684,7 @@ static int addr2line(const char *dso_name, u64 addr, } io__init(&io, a2l->out, buf, sizeof(buf)); io.timeout_ms =3D addr2line_timeout_ms; - switch (read_addr2line_record(&io, a2l_style, + switch (read_addr2line_record(&io, a2l_style, dso_name, addr, /*first=3D*= /true, &record_function, &record_filename, &record_line_nr)) { case -1: if (!symbol_conf.disable_add2line_warn) @@ -683,16 +694,21 @@ static int addr2line(const char *dso_name, u64 addr, /* * The first record was invalid, so return failure, but first * read another record, since we sent a sentinel ',' for the - * sake of detected the last inlined function. + * sake of detected the last inlined function. Treat this as the + * first of a record as the ',' generates a new start with GNU + * binutils, also force a non-zero address as we're no longer + * reading that record. */ - switch (read_addr2line_record(&io, a2l_style, NULL, NULL, NULL)) { + switch (read_addr2line_record(&io, a2l_style, dso_name, + /*addr=3D*/1, /*first=3D*/true, + NULL, NULL, NULL)) { case -1: if (!symbol_conf.disable_add2line_warn) - pr_warning("%s %s: could not read delimiter record\n", + pr_warning("%s %s: could not read sentinel record\n", __func__, dso_name); break; case 0: - /* As expected. */ + /* The sentinel as expected. */ break; default: if (!symbol_conf.disable_add2line_warn) @@ -702,6 +718,7 @@ static int addr2line(const char *dso_name, u64 addr, } goto out; default: + /* First record as expected. */ break; } =20 @@ -722,9 +739,16 @@ static int addr2line(const char *dso_name, u64 addr, } } =20 - /* We have to read the records even if we don't care about the inline inf= o. */ + /* + * We have to read the records even if we don't care about the inline + * info. This isn't the first record and force the address to non-zero + * as we're reading records beyond the first. + */ while ((record_status =3D read_addr2line_record(&io, a2l_style, + dso_name, + /*addr=3D*/1, + /*first=3D*/false, &record_function, &record_filename, &record_line_nr)) =3D=3D 1) { --=20 2.41.0.162.gfafddb0af9-goog