From: Peter Zijlstra <peterz@infradead.org>
To: Indu Bhagat <indu.bhagat@oracle.com>
Cc: linux-toolchains@vger.kernel.org, daandemeyer@meta.com,
andrii@kernel.org, rostedt@goodmis.org, kris.van.hees@oracle.com,
elena.zannoni@oracle.com, nick.alcock@oracle.com
Subject: Re: [POC 3/5] sframe: add new SFrame library
Date: Tue, 2 May 2023 11:28:10 +0200 [thread overview]
Message-ID: <20230502092810.GK1597476@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20230501200410.3973453-4-indu.bhagat@oracle.com>
On Mon, May 01, 2023 at 01:04:08PM -0700, Indu Bhagat wrote:
> +int sframe_sec_find_fre(struct sframe_sec *sfsec, int32_t pc,
> + struct sframe_fre *frep)
> +{
> + struct sframe_func_desc_entry *fdep;
> + uint32_t start_address, i;
> + struct sframe_fre cur_fre, next_fre;
> + unsigned char *fres;
> + unsigned int fre_type, fde_type;
> + size_t esz;
> + int err = 0;
> + size_t size = 0;
> + /*
> + * For regular FDEs(i.e. fde_type SFRAME_FDE_TYPE_PCINC),
> + * where the start address in the FRE is an offset from start pc,
> + * use a bitmask with all bits set so that none of the address bits are
> + * ignored. In this case, we need to return the FRE where
> + * (PC >= FRE_START_ADDR).
> + */
> + uint64_t bitmask = 0xffffffff;
> +
> + if ((sfsec == NULL) || (frep == NULL))
> + return SFRAME_ERR_INVAL;
> +
> + /* Find the FDE which contains the PC, then scan its FRE entries. */
> + fdep = sframe_sec_find_fde(sfsec, pc, &err);
> + if (fdep == NULL || sfsec->fres == NULL)
> + return SFRAME_ERR_INIT_INVAL;
> +
> + fre_type = sframe_get_fre_type(fdep);
> + fde_type = sframe_get_fde_type(fdep);
> +
> + /*
> + * For FDEs for repetitive pattern of insns, we need to return the FRE
> + * such that(PC & FRE_START_ADDR_AS_MASK >= FRE_START_ADDR_AS_MASK).
> + * so, update the bitmask to the start address.
> + */
> + /* FIXME - the bitmask. */
> + if (fde_type == SFRAME_FDE_TYPE_PCMASK)
> + bitmask = 0xff;
> +
> + fres = (unsigned char *)sfsec->fres + fdep->func_start_fre_off;
> + for (i = 0; i < fdep->func_num_fres; i++) {
> + err = sframe_sec_read_fre((const char *)fres, &next_fre,
> + fre_type, &esz);
> + start_address = next_fre.start_ip_offset;
> +
> + if (((fdep->func_start_address
> + + (int32_t)start_address) & bitmask) <= (pc & bitmask)) {
What's the purpose of that int32_t cast?
> + sframe_fre_copy(&cur_fre, &next_fre);
> +
> + /* Get the next FRE in sequence. */
> + if (i < fdep->func_num_fres - 1) {
> + fres += esz;
> + err = sframe_sec_read_fre((const char *)fres,
> + &next_fre,
> + fre_type, &esz);
> +
> + /* Sanity check the next FRE. */
> + if (!sframe_fre_sanity_check_p(&next_fre))
> + return SFRAME_ERR_FRE_INVAL;
> +
> + size = next_fre.start_ip_offset;
> + } else {
> + size = fdep->func_size;
> + }
> +
> + if (((fdep->func_start_address
> + + (int32_t)size) & bitmask) > (pc & bitmask)) {
> + /* Cur FRE contains the PC, return it. */
> + sframe_fre_copy(frep, &cur_fre);
> + return 0;
> + }
> + } else {
> + return SFRAME_ERR_FRE_INVAL;
> + }
Or, you could've done:
start_address += fdep->func_start_address;
if ((start_address & bitmask) > (pc & bitmask))
return SFRAME_ERR_FRE_INVAL;
...and saved yourself an indent level and an unreadable
linebreak...
> + }
> + return SFRAME_ERR_FDE_INVAL;
> +}
next prev parent reply other threads:[~2023-05-02 9:28 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-01 20:04 [POC 0/5] SFrame based stack tracer for user space in the kernel Indu Bhagat
2023-05-01 20:04 ` [POC 1/5] Kconfig: x86: Add new config options for userspace unwinder Indu Bhagat
2023-05-01 20:04 ` [POC 2/5] task_struct : add additional member for sframe state Indu Bhagat
2023-05-01 20:04 ` [POC 3/5] sframe: add new SFrame library Indu Bhagat
2023-05-01 22:40 ` Steven Rostedt
2023-05-02 5:07 ` Indu Bhagat
2023-05-02 8:46 ` Peter Zijlstra
2023-05-02 9:09 ` Peter Zijlstra
2023-05-02 9:20 ` Peter Zijlstra
2023-05-02 9:28 ` Peter Zijlstra [this message]
2023-05-02 9:30 ` Peter Zijlstra
2023-05-03 6:03 ` Indu Bhagat
2023-05-02 10:31 ` Peter Zijlstra
2023-05-02 10:41 ` Peter Zijlstra
2023-05-02 15:22 ` Steven Rostedt
2023-05-01 20:04 ` [POC 4/5] sframe: add an SFrame format stack tracer Indu Bhagat
2023-05-01 23:00 ` Steven Rostedt
2023-05-02 6:16 ` Indu Bhagat
2023-05-02 8:53 ` Peter Zijlstra
2023-05-02 9:04 ` Peter Zijlstra
2023-05-01 20:04 ` [POC 5/5] x86_64: invoke SFrame based stack tracer for user space Indu Bhagat
2023-05-01 23:11 ` Steven Rostedt
2023-05-02 10:53 ` Peter Zijlstra
2023-05-02 15:27 ` Steven Rostedt
2023-05-16 17:25 ` Andrii Nakryiko
2023-05-16 17:38 ` Steven Rostedt
2023-05-16 17:51 ` Andrii Nakryiko
2024-03-13 14:37 ` Tatsuyuki Ishi
2024-03-13 14:52 ` Steven Rostedt
2024-03-13 14:58 ` Tatsuyuki Ishi
2024-03-13 15:04 ` Steven Rostedt
2023-05-01 22:15 ` [POC 0/5] SFrame based stack tracer for user space in the kernel Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230502092810.GK1597476@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=andrii@kernel.org \
--cc=daandemeyer@meta.com \
--cc=elena.zannoni@oracle.com \
--cc=indu.bhagat@oracle.com \
--cc=kris.van.hees@oracle.com \
--cc=linux-toolchains@vger.kernel.org \
--cc=nick.alcock@oracle.com \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).