From patchwork Thu Jan 22 00:34:10 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: vkamensky X-Patchwork-Id: 43482 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-we0-f200.google.com (mail-we0-f200.google.com [74.125.82.200]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 20A56218DB for ; Thu, 22 Jan 2015 00:34:30 +0000 (UTC) Received: by mail-we0-f200.google.com with SMTP id m14sf320152wev.3 for ; Wed, 21 Jan 2015 16:34:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mime-version:in-reply-to:references :date:message-id:subject:from:to:cc:content-type:sender:precedence :list-id:x-original-sender:x-original-authentication-results :mailing-list:list-post:list-help:list-archive:list-unsubscribe; bh=UhImCs8GDkxewWWpw18v2zTG1utr+8RAwAsOSVuEAv0=; b=aGXhVQiXZKQTqRHVcjWeJNQBueB3FSQqV2QdCVBnZrF4kVdviDf+DHW/Zyi2Pp2SWp kgXuxrwFL+/ZifmRehuhEiLT9RpyNwIbFR4OHkHul2aiS9gq8a+v2SesY11WCd8HCwO5 kV/NqfFodI4MNEAcArRMWDmPGzZ/kzNmeKPA4krOGaVJPa4CRbiqTC0SNp34X+2W/Ti3 YyDgf4ZKC8tkS8bfwOVMkupmh+dBF1Tfk5DQcGW9qQVbkCx2umaMswxyZVhKjoiryt8+ I5GmFP6fZUx7wNrcGUphXte5vMPq/AtUZVLbmVV8RUua8CMlW/9huwEF5WtBsxS3uWzs S4/g== X-Gm-Message-State: ALoCoQm9390lAM37/ePVrkfM3uvN9s5lw/FX6xv+Xdgwk0YKBGV5OVTEWNvvEbgpa7Rofl1HHN09 X-Received: by 10.112.114.162 with SMTP id jh2mr712314lbb.9.1421886869307; Wed, 21 Jan 2015 16:34:29 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.29.133 with SMTP id k5ls89611lah.48.gmail; Wed, 21 Jan 2015 16:34:29 -0800 (PST) X-Received: by 10.152.5.38 with SMTP id p6mr47128079lap.91.1421886869063; Wed, 21 Jan 2015 16:34:29 -0800 (PST) Received: from mail-la0-f50.google.com (mail-la0-f50.google.com. [209.85.215.50]) by mx.google.com with ESMTPS id t8si19259099lbb.115.2015.01.21.16.34.28 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 21 Jan 2015 16:34:28 -0800 (PST) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.50 as permitted sender) client-ip=209.85.215.50; Received: by mail-la0-f50.google.com with SMTP id pn19so43276487lab.9 for ; Wed, 21 Jan 2015 16:34:28 -0800 (PST) X-Received: by 10.112.84.225 with SMTP id c1mr46772198lbz.22.1421886868815; Wed, 21 Jan 2015 16:34:28 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.9.200 with SMTP id c8csp87313lbb; Wed, 21 Jan 2015 16:34:27 -0800 (PST) X-Received: by 10.70.33.177 with SMTP id s17mr18852456pdi.9.1421886866604; Wed, 21 Jan 2015 16:34:26 -0800 (PST) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id t5si5771525pdh.70.2015.01.21.16.34.25; Wed, 21 Jan 2015 16:34:26 -0800 (PST) Received-SPF: none (google.com: linux-kernel-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752090AbbAVAeR (ORCPT + 28 others); Wed, 21 Jan 2015 19:34:17 -0500 Received: from mail-qc0-f181.google.com ([209.85.216.181]:41934 "EHLO mail-qc0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750871AbbAVAeL (ORCPT ); Wed, 21 Jan 2015 19:34:11 -0500 Received: by mail-qc0-f181.google.com with SMTP id l6so30340016qcy.12 for ; Wed, 21 Jan 2015 16:34:10 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.140.108.74 with SMTP id i68mr59533574qgf.35.1421886850776; Wed, 21 Jan 2015 16:34:10 -0800 (PST) Received: by 10.229.68.197 with HTTP; Wed, 21 Jan 2015 16:34:10 -0800 (PST) In-Reply-To: <54C0216F.3080404@gmail.com> References: <1421168344-5363-1-git-send-email-victor.kamensky@linaro.org> <1421168344-5363-2-git-send-email-victor.kamensky@linaro.org> <54C0216F.3080404@gmail.com> Date: Wed, 21 Jan 2015 16:34:10 -0800 Message-ID: Subject: Re: [PATCH 2/2] perf symbols: debuglink should take symfs option into account From: Victor Kamensky To: David Ahern Cc: Arnaldo Carvalho de Melo , Namhyung Kim , Peter Zijlstra , Paul Mackerras , Ingo Molnar , Adrian Hunter , Jiri Olsa , Avi Kivity , Masami Hiramatsu , Anton Blanchard , Will Deacon , Dave Martin , open list , "linux-arm-kernel@lists.infradead.org" , Jiri Olsa , Waiman Long Sender: linux-kernel-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: victor.kamensky@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.50 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , David, Thank you for response! On 21 January 2015 at 14:00, David Ahern wrote: > On 1/19/15 10:50 AM, Victor Kamensky wrote: >>> >>> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c >>> index 45be944..6a2f663 100644 >>> --- a/tools/perf/util/dso.c >>> +++ b/tools/perf/util/dso.c >>> @@ -42,19 +42,30 @@ int dso__read_binary_type_filename(const struct dso >>> *dso, >>> size_t len; >>> >>> switch (type) { >>> - case DSO_BINARY_TYPE__DEBUGLINK: { >>> + case DSO_BINARY_TYPE__DEBUGLINK: >>> + { >>> char *debuglink; >>> - >>> - strncpy(filename, dso->long_name, size); >>> - debuglink = filename + dso->long_name_len; >>> - while (debuglink != filename && *debuglink != '/') >>> - debuglink--; >>> - if (*debuglink == '/') >>> - debuglink++; >>> - ret = filename__read_debuglink(dso->long_name, debuglink, >>> - size - (debuglink - >>> filename)); >>> - } >>> + char *filename_copy; >>> + >>> + filename_copy = malloc(PATH_MAX); >>> + if (filename_copy) { >>> + len = __symbol__join_symfs(filename, size, >>> + dso->long_name); >>> + strncpy(filename_copy, filename, PATH_MAX); >>> + debuglink = filename + len; >>> + while (debuglink != filename && *debuglink != >>> '/') >>> + debuglink--; >>> + if (*debuglink == '/') >>> + debuglink++; >>> + ret = filename__read_debuglink(filename_copy, >>> debuglink, >>> + size - (debuglink >>> - >>> + >>> filename)); >>> + free(filename_copy); >>> + } else >>> + ret = -1; >>> break; >>> + } >>> + > > > I do not believe the filename_copy is needed; just add the symfs path to > filename and pass filename to read_debuglink My first version of the fix did not create filename_copy and looked like as patch below. But then I noticed that filename__read_debuglink function receives two pointers: 'filename' first parameter pointer to executable path from which .gnu_debuglink sections content will be read 'debuglink' path to directory that will be updated by the function (note strncpy at line 531) to point to debug symbol file. Before my change offset within 'filename' parameter of dso__read_binary_type_filename function is passed as 'debuglink' parameter and it will be updated, and dso->long_name is separate, passed as 'filename' parameter to filename__read_debuglink function. When in the first version of patch, as below, I pass filename buffer to both (filename to open first and pointer within filename to be updated as debuglink) as in patch below, it will work with current implementation because open happens first but update happens after that. But that is specific dependency on current implementation of filename__read_debuglink function. It did not feel quite right to me, but practically it could be OK. Here is the first version of the without filename_copy. Practically I am OK with it. Please let me know if you prefer this version: >From cafc06d95886f1d82f7b127af58a51384c0fe931 Mon Sep 17 00:00:00 2001 From: Victor Kamensky Date: Mon, 12 Jan 2015 17:33:06 -0800 Subject: [PATCH 2/2] perf symbols: debuglink should take symfs option into account Currently code that tries to read corresponding debug symbol file from .gnu_debuglink section (DSO_BINARY_TYPE__DEBUGLINK) does not take in account symfs option, so filename__read_debuglink function cannot open ELF file, if symfs option is used. Fix is to add proper handling of symfs as it is done in other places: use __symbol__join_symfs function to get real file name of target ELF file. Signed-off-by: Victor Kamensky --- tools/perf/util/dso.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 45be944..ca8d8d5 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -45,13 +45,13 @@ int dso__read_binary_type_filename(const struct dso *dso, case DSO_BINARY_TYPE__DEBUGLINK: { char *debuglink; - strncpy(filename, dso->long_name, size); - debuglink = filename + dso->long_name_len; + len = __symbol__join_symfs(filename, size, dso->long_name); + debuglink = filename + len; while (debuglink != filename && *debuglink != '/') debuglink--; if (*debuglink == '/') debuglink++; - ret = filename__read_debuglink(dso->long_name, debuglink, + ret = filename__read_debuglink(filename, debuglink, size - (debuglink - filename)); } break;