Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756528AbcLTAQo (ORCPT ); Mon, 19 Dec 2016 19:16:44 -0500 Received: from mail-wm0-f66.google.com ([74.125.82.66]:36401 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755962AbcLTAQi (ORCPT ); Mon, 19 Dec 2016 19:16:38 -0500 From: Alexis Berlemont To: linux-kernel@vger.kernel.org Cc: Alexis Berlemont , peterz@infradead.org, mingo@redhat.com, acme@kernel.org, alexander.shishkin@linux.intel.com Subject: [PATCH v2 1/1] perf annotate: check that objdump correctly works Date: Tue, 20 Dec 2016 01:11:37 +0100 Message-Id: <20161220001137.26773-2-alexis.berlemont@gmail.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20161220001137.26773-1-alexis.berlemont@gmail.com> References: <20161220001137.26773-1-alexis.berlemont@gmail.com> In-Reply-To: <20161206193805.GB8257@kernel.org> References: <20161206193805.GB8257@kernel.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4710 Lines: 157 Before disassembling, the tool objdump is called just to be sure: * objdump is available in the path; * objdump is an executable binary; * objdump has no dependency issue or anything else. This objdump "pre-"command is only necessary because the real objdump command is followed by some " | grep ..."; this prevents the shell from returning the exit code of objdump execution. Signed-off-by: Alexis Berlemont --- tools/perf/util/annotate.c | 69 +++++++++++++++++++++++++++++++++++++++++++--- tools/perf/util/annotate.h | 3 ++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index ea7e0de4b9c1..6fbaabbc9d2a 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -20,9 +20,12 @@ #include "block-range.h" #include "arch/common.h" #include +#include #include #include #include +#include +#include const char *disassembler_style; const char *objdump_path; @@ -1286,6 +1289,21 @@ int symbol__strerror_disassemble(struct symbol *sym __maybe_unused, struct map * " --vmlinux vmlinux\n", build_id_msg ?: ""); } break; + + case SYMBOL_ANNOTATE_ERRNO__NO_OBJDUMP: + scnprintf(buf, buflen, "No objdump tool available in $PATH\n"); + break; + + case SYMBOL_ANNOTATE_ERRNO__NO_EXEC_OBJDUMP: + scnprintf(buf, buflen, + "The objdump tool found in $PATH cannot be executed\n"); + break; + + case SYMBOL_ANNOTATE_ERRNO__NO_OUTPUT: + scnprintf(buf, buflen, + "The objdump tool returned no disassembled code\n"); + break; + default: scnprintf(buf, buflen, "Internal error: Invalid %d error code\n", errnum); break; @@ -1329,6 +1347,44 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil return 0; } +static int annotate__check_exec(int pid, const char *command) +{ + int wstatus, err; + + err = waitpid(pid, &wstatus, 0); + if (err < 0) { + pr_err("Failure calling waitpid: %s: (%s)\n", + strerror(errno), command); + return -1; + } + + pr_debug("%s: %d %d\n", command, pid, WEXITSTATUS(wstatus)); + + switch (WEXITSTATUS(wstatus)) { + case 0: + /* Success */ + err = 0; + break; + case 127: + /* The shell did not find objdump in the path */ + err = SYMBOL_ANNOTATE_ERRNO__NO_OBJDUMP; + break; + default: + /* + * In the default case, we consider that objdump + * cannot be executed; so it gathers many fault + * scenarii: + * - objdump is not an executable (126); + * - objdump has some dependency issue; + * - ... + */ + err = SYMBOL_ANNOTATE_ERRNO__NO_EXEC_OBJDUMP; + break; + } + + return err; +} + static const char *annotate__norm_arch(const char *arch_name) { struct utsname uts; @@ -1426,7 +1482,8 @@ int symbol__disassemble(struct symbol *sym, struct map *map, const char *arch_na snprintf(command, sizeof(command), "%s %s%s --start-address=0x%016" PRIx64 " --stop-address=0x%016" PRIx64 - " -l -d %s %s -C %s 2>/dev/null|grep -v %s|expand", + " -l -d %s %s -C %s 2>/dev/null | " + "{ grep -v %s || true; } | expand", objdump_path ? objdump_path : "objdump", disassembler_style ? "-M " : "", disassembler_style ? disassembler_style : "", @@ -1454,7 +1511,7 @@ int symbol__disassemble(struct symbol *sym, struct map *map, const char *arch_na close(stdout_fd[0]); dup2(stdout_fd[1], 1); close(stdout_fd[1]); - execl("/bin/sh", "sh", "-c", command, NULL); + execl("/bin/sh", "sh", "-o", "pipefail", "-c", command, NULL); perror(command); exit(-1); } @@ -1479,8 +1536,12 @@ int symbol__disassemble(struct symbol *sym, struct map *map, const char *arch_na nline++; } - if (nline == 0) + err = annotate__check_exec(pid, command); + + if (err == 0 && nline == 0) { pr_err("No output from %s\n", command); + err = SYMBOL_ANNOTATE_ERRNO__NO_OUTPUT; + } /* * kallsyms does not have symbol sizes so there may a nop at the end. @@ -1490,7 +1551,7 @@ int symbol__disassemble(struct symbol *sym, struct map *map, const char *arch_na delete_last_nop(sym); fclose(file); - err = 0; + out_remove_tmp: close(stdout_fd[0]); diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index 87e4cadc5d27..123f60c509a9 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -172,6 +172,9 @@ enum symbol_disassemble_errno { __SYMBOL_ANNOTATE_ERRNO__START = -10000, SYMBOL_ANNOTATE_ERRNO__NO_VMLINUX = __SYMBOL_ANNOTATE_ERRNO__START, + SYMBOL_ANNOTATE_ERRNO__NO_EXEC_OBJDUMP, + SYMBOL_ANNOTATE_ERRNO__NO_OBJDUMP, + SYMBOL_ANNOTATE_ERRNO__NO_OUTPUT, __SYMBOL_ANNOTATE_ERRNO__END, }; -- 2.11.0