linux-kernel - Re: [PATCH v5 6/6] perf arm64: inject missing frames if perf-record used "--call-graph=fp"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 10 Jan 2022 10:48:57 +0000
From:   German Gomez <german.gomez@....com>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        acme@...nel.org, Alexandre Truong <alexandre.truong@....com>,
        John Garry <john.garry@...wei.com>,
        Will Deacon <will@...nel.org>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Leo Yan <leo.yan@...aro.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v5 6/6] perf arm64: inject missing frames if perf-record
 used "--call-graph=fp"

Hi Jiri,

On 21/12/2021 09:32, Jiri Olsa wrote:
> On Fri, Dec 17, 2021 at 03:45:20PM +0000, German Gomez wrote:
>
> SNIP
>
>> +}
>> +
>> +u64 get_leaf_frame_caller_aarch64(struct perf_sample *sample, struct thread *thread, int usr_idx)
>> +{
>> +	int ret;
>> +	struct entries entries = {};
>> +	struct regs_dump old_regs = sample->user_regs;
>> +
>> +	if (!get_leaf_frame_caller_enabled(sample))
>> +		return 0;
>> +
>> +	/*
>> +	 * If PC and SP are not recorded, get the value of PC from the stack
>> +	 * and set its mask. SP is not used when doing the unwinding but it
>> +	 * still needs to be set to prevent failures.
>> +	 */
>> +
>> +	if (!(sample->user_regs.mask & SMPL_REG_MASK(PERF_REG_ARM64_PC))) {
>> +		sample->user_regs.cache_mask |= SMPL_REG_MASK(PERF_REG_ARM64_PC);
>> +		sample->user_regs.cache_regs[PERF_REG_ARM64_PC] = sample->callchain->ips[usr_idx+1];
>> +	}
>> +
>> +	if (!(sample->user_regs.mask & SMPL_REG_MASK(PERF_REG_ARM64_SP))) {
>> +		sample->user_regs.cache_mask |= SMPL_REG_MASK(PERF_REG_ARM64_SP);
>> +		sample->user_regs.cache_regs[PERF_REG_ARM64_SP] = 0;
>> +	}
>> +
>> +	ret = unwind__get_entries(add_entry, &entries, thread, sample, 2);
> just curious, did you try this with both unwinders libunwind/libdw?

Yes I did,

This is the program I was using:

  int a = 0;

  void leaf() {
    for (int i = 0; i < 10000000; i++)
      a *= a;
  }

  void parent() {
    leaf();
  }

  int main() {
    parent();
  }

  ... compiled with "gcc -O0 -fno-omit-frame-pointer -fno-inline program.c"

>
> any chance you could add arm specific test for this?

I don't see a reason not to. I'll make a note for a separate patch.

>
> otherwise it looks good to me
>
> Acked-by: Jiri Olsa <jolsa@...nel.org>

Thanks for the review,
German