Re: [POC 3/5] sframe: add new SFrame library

[Steven Rostedt <rostedt@goodmis.org>]

On Mon,  1 May 2023 13:04:08 -0700
Indu Bhagat <indu.bhagat@oracle.com> wrote:

> This patch adds an implementation to read SFrame stack trace data from
> a .sframe section.  Some APIs are also provided to find stack tracing
> information per PC, e.g., given a PC, find the SFrame FRE.
> 
> These routines are provided in the sframe_read.h and sframe_read.c.
> 
> This implmentation is malloc-free.
> 
> Signed-off-by: Indu Bhagat <indu.bhagat@oracle.com>
> ---
>  lib/Makefile             |   1 +
>  lib/sframe/Makefile      |   5 +
>  lib/sframe/sframe.h      | 263 +++++++++++++++++++++
>  lib/sframe/sframe_read.c | 498 +++++++++++++++++++++++++++++++++++++++
>  lib/sframe/sframe_read.h |  75 ++++++
>  5 files changed, 842 insertions(+)
>  create mode 100644 lib/sframe/Makefile
>  create mode 100644 lib/sframe/sframe.h
>  create mode 100644 lib/sframe/sframe_read.c
>  create mode 100644 lib/sframe/sframe_read.h
> 
> diff --git a/lib/Makefile b/lib/Makefile
> index 876fcdeae34e..cb02d16dbffd 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -198,6 +198,7 @@ obj-$(CONFIG_ZSTD_COMPRESS) += zstd/
>  obj-$(CONFIG_ZSTD_DECOMPRESS) += zstd/
>  obj-$(CONFIG_XZ_DEC) += xz/
>  obj-$(CONFIG_RAID6_PQ) += raid6/
> +obj-$(CONFIG_USER_UNWINDER_SFRAME) += sframe/
>  
>  lib-$(CONFIG_DECOMPRESS_GZIP) += decompress_inflate.o
>  lib-$(CONFIG_DECOMPRESS_BZIP2) += decompress_bunzip2.o
> diff --git a/lib/sframe/Makefile b/lib/sframe/Makefile
> new file mode 100644
> index 000000000000..4e4291d9294f
> --- /dev/null
> +++ b/lib/sframe/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +##################################
> +obj-$(CONFIG_USER_UNWINDER_SFRAME) += sframe_read.o \

The ending backslash is only needed if there's a line wrap, which you don't
have here.

> +
> +CFLAGS_sframe_read.o += -I $(srctree)/lib/sframe/
> diff --git a/lib/sframe/sframe.h b/lib/sframe/sframe.h
> new file mode 100644
> index 000000000000..b1290e92839a
> --- /dev/null
> +++ b/lib/sframe/sframe.h
> @@ -0,0 +1,263 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2023, Oracle and/or its affiliates.
> + */
> +
> +#ifndef	SFRAME_H
> +#define	SFRAME_H
> +
> +#include <linux/types.h>
> +
> +/* This file contains definitions for the SFrame stack tracing format, which is
> + * documented at https://sourceware.org/binutils/docs */
> +
> +#define SFRAME_VERSION_1	1
> +#define SFRAME_MAGIC		0xdee2
> +#define SFRAME_VERSION	SFRAME_VERSION_1
> +
> +/* Function Descriptor Entries are sorted on PC. */
> +#define SFRAME_F_FDE_SORTED	0x1
> +/* Frame-pointer based stack tracing. Defined, but not set. */
> +#define SFRAME_F_FRAME_POINTER 0x2
> +
> +#define SFRAME_CFA_FIXED_FP_INVALID 0
> +#define SFRAME_CFA_FIXED_RA_INVALID 0
> +
> +/* Supported ABIs/Arch. */
> +#define SFRAME_ABI_AARCH64_ENDIAN_BIG      1 /* AARCH64 big endian. */
> +#define SFRAME_ABI_AARCH64_ENDIAN_LITTLE   2 /* AARCH64 little endian. */
> +#define SFRAME_ABI_AMD64_ENDIAN_LITTLE     3 /* AMD64 little endian. */
> +
> +/* SFrame FRE types. */
> +#define SFRAME_FRE_TYPE_ADDR1	0
> +#define SFRAME_FRE_TYPE_ADDR2	1
> +#define SFRAME_FRE_TYPE_ADDR4	2
> +
> +/*
> + * SFrame Function Descriptor Entry types.
> + *
> + * The SFrame format has two possible representations for functions.  The
> + * choice of which type to use is made according to the instruction patterns
> + * in the relevant program stub.
> + */
> +
> +/* Unwinders perform a (PC >= FRE_START_ADDR) to look up a matching FRE. */
> +#define SFRAME_FDE_TYPE_PCINC   0
> +/*
> + * Unwinders perform a (PC & FRE_START_ADDR_AS_MASK >= FRE_START_ADDR_AS_MASK)
> + * to look up a matching FRE.  Typical usecases are pltN entries, trampolines
> + * etc.
> + */
> +#define SFRAME_FDE_TYPE_PCMASK  1
> +
> +struct sframe_preamble
> +{

Note, for Linux coding style, structs should have the '{' on the same line:

struct sframe_preamble {

This is required because a common way to find a struct definition is to
search the name followed by a '{'.


> +	/* Magic number (SFRAME_MAGIC). */
> +	uint16_t magic;
> +	/* Data format version number (SFRAME_VERSION). */
> +	uint8_t version;
> +	/* Various flags. */
> +	uint8_t flags;
> +} __packed;
> +
> +struct sframe_header
> +{

Here too.

> +	struct sframe_preamble preamble;
> +	/* Information about the arch (endianness) and ABI. */
> +	uint8_t abi_arch;
> +	/*
> +	 * Offset for the Frame Pointer (FP) from CFA may be fixed for some
> +	 * ABIs (e.g, in AMD64 when -fno-omit-frame-pointer is used).  When fixed,
> +	 * this field specifies the fixed stack frame offset and the individual
> +	 * FREs do not need to track it.  When not fixed, it is set to
> +	 * SFRAME_CFA_FIXED_FP_INVALID, and the individual FREs may provide
> +	 * the applicable stack frame offset, if any.
> +	 */
> +	int8_t cfa_fixed_fp_offset;
> +	/*
> +	 * Offset for the Return Address from CFA is fixed for some ABIs
> +	 * (e.g., AMD64 has it as CFA-8).  When fixed, the header specifies the
> +	 * fixed stack frame offset and the individual FREs do not track it.  When
> +	 * not fixed, it is set to SFRAME_CFA_FIXED_RA_INVALID, and individual
> +	 * FREs provide the applicable stack frame offset, if any.
> +	 */
> +	int8_t cfa_fixed_ra_offset;
> +	/*
> +	 * Number of bytes making up the auxiliary header, if any.
> +	 * Some ABI/arch, in the future, may use this space for extending the
> +	 * information in SFrame header.  Auxiliary header is contained in
> +	 * bytes sequentially following the sframe_header.
> +	 */
> +	uint8_t auxhdr_len;
> +	/* Number of SFrame FDEs in this SFrame section. */
> +	uint32_t num_fdes;
> +	/* Number of SFrame Frame Row Entries. */
> +	uint32_t num_fres;
> +	/* Number of bytes in the SFrame Frame Row Entry section. */
> +	uint32_t fre_len;
> +	/* Offset of SFrame Function Descriptor Entry section. */
> +	uint32_t fdeoff;
> +	/* Offset of SFrame Frame Row Entry section. */
> +	uint32_t freoff;
> +} __packed;
> +
> +#define SFRAME_V1_HDR_SIZE(sframe_hdr)	\
> +  ((sizeof (struct sframe_header) + (sframe_hdr).auxhdr_len))
> +
> +/* Two possible keys for executable (instruction) pointers signing. */
> +#define SFRAME_AARCH64_PAUTH_KEY_A    0 /* Key A. */
> +#define SFRAME_AARCH64_PAUTH_KEY_B    1 /* Key B. */
> +
> +struct sframe_func_desc_entry
> +{

Basically all of them.

> +	/*
> +	 * Function start address.  Encoded as a signed offset, relative to the
> +	 * beginning of the current FDE.
> +	 */
> +	int32_t func_start_address;
> +	/* Size of the function in bytes. */
> +	uint32_t func_size;
> +	/*
> +	 * Offset of the first SFrame Frame Row Entry of the function, relative to the
> +	 * beginning of the SFrame Frame Row Entry sub-section.
> +	 */
> +	uint32_t func_start_fre_off;
> +	/* Number of frame row entries for the function. */
> +	uint32_t func_num_fres;
> +	/*
> +	 * Additional information for deciphering the unwind information for the
> +	 * function.
> +	 *   - 4-bits: Identify the FRE type used for the function.
> +	 *   - 1-bit: Identify the FDE type of the function - mask or inc.
> +	 *   - 1-bit: PAC authorization A/B key (aarch64).
> +	 *   - 2-bits: Unused.
> +	 * --------------------------------------------------------------------------
> +	 * |     Unused    |  PAC auth A/B key (aarch64) |  FDE type |   FRE type   |
> +	 * |               |        Unused (amd64)       |           |              |
> +	 * --------------------------------------------------------------------------
> +	 * 8               6                             5           4              0
> +	 */
> +	uint8_t func_info;
> +} __packed;
> +
> +/* Note: Set PAC auth key to SFRAME_AARCH64_PAUTH_KEY_A by default.  */
> +#define SFRAME_V1_FUNC_INFO(fde_type, fre_enc_type) \
> +  (((SFRAME_AARCH64_PAUTH_KEY_A & 0x1) << 5) | \
> +   (((fde_type) & 0x1) << 4) | ((fre_enc_type) & 0xf))
> +
> +#define SFRAME_V1_FUNC_FRE_TYPE(data)	  ((data) & 0xf)
> +#define SFRAME_V1_FUNC_FDE_TYPE(data)	  (((data) >> 4) & 0x1)
> +#define SFRAME_V1_FUNC_PAUTH_KEY(data)	  (((data) >> 5) & 0x1)
> +
> +/*
> + * Size of stack frame offsets in an SFrame Frame Row Entry.  A single
> + * SFrame FRE has all offsets of the same size.  Offset size may vary
> + * across frame row entries.
> + */
> +#define SFRAME_FRE_OFFSET_1B	  0
> +#define SFRAME_FRE_OFFSET_2B	  1
> +#define SFRAME_FRE_OFFSET_4B	  2
> +
> +/* An SFrame Frame Row Entry can be SP or FP based.  */
> +#define SFRAME_BASE_REG_FP	0
> +#define SFRAME_BASE_REG_SP	1
> +
> +/*
> + * The index at which a specific offset is presented in the variable length
> + * bytes of an FRE.
> + */
> +#define SFRAME_FRE_CFA_OFFSET_IDX   0
> +/*
> + * The RA stack offset, if present, will always be at index 1 in the variable
> + * length bytes of the FRE.
> + */
> +#define SFRAME_FRE_RA_OFFSET_IDX    1
> +/*
> + * The FP stack offset may appear at offset 1 or 2, depending on the ABI as RA
> + * may or may not be tracked.
> + */
> +#define SFRAME_FRE_FP_OFFSET_IDX    2
> +
> +struct sframe_fre_info
> +{
> +  /* Information about
> +   *   - 1 bit: base reg for CFA
> +   *   - 4 bits: Number of offsets (N).  A value of upto 3 is allowed to track
> +   *     all three of CFA, FP and RA (fixed implicit order).
> +   *   - 2 bits: information about size of the offsets (S) in bytes.
> +   *     Valid values are SFRAME_FRE_OFFSET_1B, SFRAME_FRE_OFFSET_2B,
> +   *     SFRAME_FRE_OFFSET_4B
> +   *   - 1 bit: Mangled RA state bit (aarch64 only).
> +   * -----------------------------------------------------------------------------------
> +   * | Mangled-RA (aarch64) |  Size of offsets   |   Number of offsets    |   base_reg |
> +   * |  Unused (amd64)      |                    |                        |            |
> +   * -----------------------------------------------------------------------------------
> +   * 8                     7                    5                        1            0
> +   */
> +  uint8_t fre_info;
> +};
> +
> +/* Macros to compose and decompose FRE info. */
> +
> +/* Note: Set mangled_ra_p to zero by default. */
> +#define SFRAME_V1_FRE_INFO(base_reg_id, offset_num, offset_size) \
> +  (((0 & 0x1) << 7) | (((offset_size) & 0x3) << 5) | \
> +   (((offset_num) & 0xf) << 1) | ((base_reg_id) & 0x1))
> +
> +/* Set the mangled_ra_p bit as indicated. */
> +#define SFRAME_V1_FRE_INFO_UPDATE_MANGLED_RA_P(mangled_ra_p, fre_info) \
> +  ((((mangled_ra_p) & 0x1) << 7) | ((fre_info) & 0x7f))
> +
> +#define SFRAME_V1_FRE_CFA_BASE_REG_ID(data)	  ((data) & 0x1)
> +#define SFRAME_V1_FRE_OFFSET_COUNT(data)	  (((data) >> 1) & 0xf)
> +#define SFRAME_V1_FRE_OFFSET_SIZE(data)		  (((data) >> 5) & 0x3)
> +#define SFRAME_V1_FRE_MANGLED_RA_P(data)	  (((data) >> 7) & 0x1)
> +
> +/* SFrame Frame Row Entry definitions. */
> +
> +/*
> + * Used when SFRAME_FRE_TYPE_ADDR1 is specified as FRE type.
> + * Upper limit of start address in sframe_frame_row_entry_addr1 if 0x100 (not
> + * inclusive).
> + */
> +struct sframe_frame_row_entry_addr1
> +{
> +	/*
> +	 * Start address of the frame row entry.  Encoded as an 1-byte unsigned
> +	 * offset, relative to the start address of the function.
> +	 */
> +	uint8_t fre_start_ip_offset;
> +	struct sframe_fre_info fre_info;
> +} __packed;
> +
> +/*
> + * Used when SFRAME_FRE_TYPE_ADDR2 is specified as FRE type.
> + * Upper limit of start address in sframe_frame_row_entry_addr2 is 0x10000 (not
> + * inclusive).
> + */
> +struct sframe_frame_row_entry_addr2
> +{
> +	/*
> +	 * Start address of the frame row entry.  Encoded as an 2-byte unsigned
> +	 * offset, relative to the start address of the function.
> +	 */
> +	uint16_t fre_start_ip_offset;
> +	struct sframe_fre_info fre_info;
> +} __packed;
> +
> +/*
> + * Used when SFRAME_FRE_TYPE_ADDR4 is specified as FRE type.
> + * Upper limit of start address in sframe_frame_row_entry_addr2
> + * is 0x100000000 (not inclusive).
> + */
> +struct sframe_frame_row_entry_addr4
> +{
> +	/*
> +	 * Start address of the frame row entry.  Encoded as a 4-byte unsigned
> +	 * offset, relative to the start address of the function.
> +	 */
> +	uint32_t fre_start_ip_offset;
> +	struct sframe_fre_info fre_info;
> +} __packed;
> +
> +#endif	/* SFRAME_H */
> diff --git a/lib/sframe/sframe_read.c b/lib/sframe/sframe_read.c
> new file mode 100644
> index 000000000000..9d6558d62c54
> --- /dev/null
> +++ b/lib/sframe/sframe_read.c
> @@ -0,0 +1,498 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2023, Oracle and/or its affiliates.
> + */
> +
> +#include <linux/string.h>
> +
> +#include "sframe_read.h"
> +
> +struct sframe_sec {
> +	/* SFrame header. */
> +	struct sframe_header header;
> +	/* SFrame Function Desc Entries. */
> +	void *fdes;
> +	/* SFrame Frame Row Entries. */
> +	void *fres;
> +	/* Number of bytes needed for SFrame FREs. */
> +	uint32_t fre_nbytes;
> +};
> +
> +static int sframe_set_errno(int *errp, int error)
> +{
> +	if (errp != NULL)
> +		*errp = error;
> +	return SFRAME_ERR;
> +}
> +
> +static uint32_t sframe_sec_get_hdr_size(struct sframe_header *sfh)
> +{
> +	return SFRAME_V1_HDR_SIZE(*sfh);
> +}
> +
> +static unsigned int sframe_fre_get_offset_count(unsigned char fre_info)
> +{
> +	return SFRAME_V1_FRE_OFFSET_COUNT(fre_info);
> +}
> +
> +static unsigned int sframe_fre_get_offset_size(unsigned char fre_info)
> +{
> +	return SFRAME_V1_FRE_OFFSET_SIZE(fre_info);
> +}
> +
> +static unsigned int sframe_get_fre_type(struct sframe_func_desc_entry *fdep)
> +{
> +	return (fdep) ? SFRAME_V1_FUNC_FRE_TYPE(fdep->func_info) : 0;
> +}
> +
> +static unsigned int sframe_get_fde_type(struct sframe_func_desc_entry *fdep)
> +{
> +	return (fdep) ? SFRAME_V1_FUNC_FDE_TYPE(fdep->func_info) : 0;
> +}
> +
> +static bool sframe_header_sanity_check_p(struct sframe_header *hp)
> +{
> +	unsigned char all_flags = SFRAME_F_FDE_SORTED | SFRAME_F_FRAME_POINTER;

Add a space here.

> +	/* Check that the preamble is valid. */
> +	if ((hp->preamble.magic != SFRAME_MAGIC)
> +	    || (hp->preamble.version != SFRAME_VERSION)
> +	    || ((hp->preamble.flags | all_flags) != all_flags))
> +		return false;
> +
> +	/* Check that the offsets are valid. */
> +	if (hp->fdeoff > hp->freoff)
> +		return false;
> +
> +	return true;
> +}
> +
> +static bool sframe_fre_sanity_check_p(struct sframe_fre *frep)
> +{
> +	unsigned int offset_size, offset_cnt;
> +
> +	if (frep == NULL)
> +		return false;
> +
> +	offset_size = sframe_fre_get_offset_size(frep->fre_info);
> +
> +	if (offset_size != SFRAME_FRE_OFFSET_1B
> +	    && offset_size != SFRAME_FRE_OFFSET_2B
> +	    && offset_size != SFRAME_FRE_OFFSET_4B)
> +		return false;
> +
> +	offset_cnt = sframe_fre_get_offset_count(frep->fre_info);
> +	if (offset_cnt > MAX_NUM_STACK_OFFSETS)
> +		return false;
> +
> +	return true;
> +}
> +
> +static int32_t sframe_get_fre_offset(struct sframe_fre *frep, uint32_t idx,
> +				     int *errp)
> +{
> +	int offset_cnt, offset_size;
> +
> +	if (frep == NULL || !sframe_fre_sanity_check_p(frep))
> +		return sframe_set_errno(errp, SFRAME_ERR_FRE_INVAL);
> +
> +	offset_cnt = sframe_fre_get_offset_count(frep->fre_info);
> +	offset_size = sframe_fre_get_offset_size(frep->fre_info);
> +
> +	if (offset_cnt < idx + 1)
> +		return sframe_set_errno(errp, SFRAME_ERR_FREOFFSET_NOPRESENT);
> +
> +	if (errp != NULL)
> +		*errp = 0; /* Offset Valid. */
> +
> +	if (offset_size == SFRAME_FRE_OFFSET_1B) {
> +		int8_t *stack_offsets = (int8_t *)frep->fre_offsets;
> +		return stack_offsets[idx];
> +	} else if (offset_size == SFRAME_FRE_OFFSET_2B) {
> +		int16_t *stack_offsets = (int16_t *)frep->fre_offsets;
> +		return stack_offsets[idx];
> +	} else {
> +		int32_t *stack_offsets = (int32_t *)frep->fre_offsets;
> +		return stack_offsets[idx];
> +	}
> +}
> +
> +static struct sframe_header *sframe_sec_get_header(struct sframe_sec *sfsec)
> +{
> +	return sfsec ? &sfsec->header : NULL;
> +}
> +
> +static int sframe_fre_copy(struct sframe_fre *dst,
> +			   struct sframe_fre *src)
> +{
> +	if (dst == NULL || src == NULL)
> +		return SFRAME_ERR;
> +
> +	memcpy(dst, src, sizeof(struct sframe_fre));
> +	return 0;
> +}
> +
> +static int sframe_decode_start_ip_offset(const char *fre_buf,
> +					 uint32_t *start_ip_offset,
> +					 unsigned int fre_type)
> +{
> +	uint32_t saddr = 0;
> +
> +	if (fre_type == SFRAME_FRE_TYPE_ADDR1) {
> +		uint8_t *uc = (uint8_t *)fre_buf;
> +		saddr = (uint32_t)*uc;
> +	} else if (fre_type == SFRAME_FRE_TYPE_ADDR2) {
> +		uint16_t *ust = (uint16_t *)fre_buf;
> +		saddr = (uint32_t)*ust;
> +	} else if (fre_type == SFRAME_FRE_TYPE_ADDR4) {
> +		uint32_t *uit = (uint32_t *)fre_buf;
> +		saddr = (uint32_t)*uit;
> +	} else {
> +		return SFRAME_ERR_INVAL;
> +	}
> +
> +	*start_ip_offset = saddr;
> +	return 0;
> +}
> +
> +/* Get the total size in bytes for the stack offsets. */
> +static size_t sframe_fre_stack_offsets_size(unsigned char fre_info)
> +{
> +	unsigned int offset_size, offset_cnt;
> +
> +	offset_size = sframe_fre_get_offset_size(fre_info);
> +	offset_cnt = sframe_fre_get_offset_count(fre_info);
> +
> +	if (offset_size == SFRAME_FRE_OFFSET_2B
> +	    || offset_size == SFRAME_FRE_OFFSET_4B)	/* 2 or 4 bytes. */
> +		return (offset_cnt * (offset_size * 2));
> +
> +	return offset_cnt;
> +}
> +
> +static size_t sframe_fre_get_start_address_tsize(unsigned int fre_type)
> +{
> +	/* Type size of the start_addr in an FRE. */
> +	size_t saddr_tsize = 0;
> +
> +	switch (fre_type) {
> +	case SFRAME_FRE_TYPE_ADDR1:
> +		saddr_tsize = sizeof(uint8_t);
> +		break;
> +	case SFRAME_FRE_TYPE_ADDR2:
> +		saddr_tsize = sizeof(uint16_t);
> +		break;
> +	case SFRAME_FRE_TYPE_ADDR4:
> +		saddr_tsize = sizeof(uint32_t);
> +		break;
> +	default:
> +		/* No other value is expected. */
> +		break;
> +	}
> +	return saddr_tsize;
> +}
> +
> +static size_t sframe_fre_vlen_size(struct sframe_fre *frep,
> +				   unsigned int fre_type)
> +{
> +	unsigned char fre_info;
> +	size_t ip_offset_tsize;
> +
> +	if (frep == NULL)
> +		return 0;
> +
> +	fre_info = frep->fre_info;
> +	ip_offset_tsize = sframe_fre_get_start_address_tsize(fre_type);
> +
> +	/*
> +	 * An SFrame FRE is a variable length structure.  It includes the start
> +	 * IP offset, FRE info field, and all trailing the stack offsets.
> +	 */
> +	return (ip_offset_tsize + sizeof(fre_info)
> +		+ sframe_fre_stack_offsets_size(fre_info));
> +}
> +
> +/*
> + * Read an SFrame FRE which starts at location FRE_BUF.  The function
> + * updates FRE_SIZE to the size of the FRE as stored in the binary format.
> + *
> + * Returns SFRAME_ERR if failure.
> + */
> +static int sframe_sec_read_fre(const char *fre_buf, struct sframe_fre *frep,
> +			       unsigned int fre_type, size_t *fre_size)
> +{
> +	void *stack_offsets;
> +	size_t stack_offsets_sz;
> +	size_t ip_offset_tsize;
> +	size_t esz;
> +
> +	if (fre_buf == NULL || frep == NULL || fre_size == NULL)
> +		return SFRAME_ERR_INVAL;
> +
> +	/* Copy over the FRE start address. */
> +	sframe_decode_start_ip_offset(fre_buf, &frep->start_ip_offset,
> +				      fre_type);
> +
> +	ip_offset_tsize = sframe_fre_get_start_address_tsize(fre_type);
> +	/* PS: Note how this API works closely with SFrame binary format. */
> +	frep->fre_info = *(unsigned char *)(fre_buf + ip_offset_tsize);
> +
> +	memset(frep->fre_offsets, 0, MAX_STACK_OFFSET_NBYTES);
> +	/* Get stack offsets. */
> +	stack_offsets_sz = sframe_fre_stack_offsets_size(frep->fre_info);
> +	stack_offsets = ((unsigned char *)fre_buf + ip_offset_tsize
> +			 + sizeof(frep->fre_info));
> +	memcpy(frep->fre_offsets, stack_offsets, stack_offsets_sz);
> +
> +	esz = sframe_fre_vlen_size(frep, fre_type);
> +	*fre_size = esz;
> +
> +	return 0;
> +}
> +
> +static struct sframe_func_desc_entry *
> +sframe_sec_find_fde(struct sframe_sec *sfsec, int32_t addr, int *errp)
> +{
> +	struct sframe_header *header;
> +	struct sframe_func_desc_entry *fde;
> +	int low, high, cnt;
> +
> +	if (sfsec == NULL) {
> +		sframe_set_errno(errp, SFRAME_ERR_INVAL);
> +		return NULL;
> +	}
> +
> +	header = sframe_sec_get_header(sfsec);
> +	if (header == NULL || header->num_fdes == 0 || sfsec->fdes == NULL) {
> +		sframe_set_errno(errp, SFRAME_ERR_INIT_INVAL);
> +		return NULL;
> +	}
> +	/*
> +	 * Skip binary search if FDE sub-section is not sorted on PCs.  GNU ld
> +	 * sorts the FDEs on start PC by default though.
> +	 */
> +	if ((header->preamble.flags & SFRAME_F_FDE_SORTED) == 0) {
> +		sframe_set_errno(errp, SFRAME_ERR_FDE_NOTSORTED);
> +		return NULL;
> +	}
> +
> +	/* Find the FDE that may contain the addr. */
> +	fde = (struct sframe_func_desc_entry *)sfsec->fdes;
> +	low = 0;
> +	high = header->num_fdes;
> +	cnt = high;
> +	while (low <= high) {
> +		int mid = low + (high - low) / 2;
> +
> +		if (fde[mid].func_start_address == addr)
> +			return fde + mid;
> +
> +		if (fde[mid].func_start_address < addr) {
> +			if (mid == (cnt - 1))
> +				return fde + (cnt - 1);
> +			else if (fde[mid+1].func_start_address > addr)
> +				return fde + mid;
> +			low = mid + 1;
> +		} else
> +			high = mid - 1;
> +	}
> +
> +	sframe_set_errno(errp, SFRAME_ERR_FDE_NOTFOUND);
> +	return NULL;
> +}
> +
> +static int8_t sframe_sec_get_fixed_fp_offset(struct sframe_sec *sfsec)
> +{
> +	struct sframe_header *header = sframe_sec_get_header(sfsec);
> +	return header->cfa_fixed_fp_offset;
> +}
> +
> +static int8_t sframe_sec_get_fixed_ra_offset(struct sframe_sec *sfsec)
> +{
> +	struct sframe_header *header = sframe_sec_get_header(sfsec);
> +	return header->cfa_fixed_ra_offset;
> +}
> +
> +size_t sframe_sec_sizeof(void)
> +{
> +	return sizeof (struct sframe_sec);
> +}
> +
> +int sframe_sec_init(struct sframe_sec *sfsec, const char *sf_buf,
> +		    size_t sf_size)
> +{
> +	char *frame_buf;
> +	const struct sframe_preamble *preamble;
> +	struct sframe_header *header;
> +
> +	if ((sf_buf == NULL) || (sf_size < sizeof(struct sframe_header)))
> +		return SFRAME_ERR_INVAL;
> +
> +	/* Check for foreign endianness. */
> +	preamble = (const struct sframe_preamble *) sf_buf;
> +	if (preamble->magic != SFRAME_MAGIC)
> +		return SFRAME_ERR_INVAL;
> +
> +	/* Reset the SFrame section object. */
> +	memset(sfsec, 0, sizeof(struct sframe_sec));
> +
> +	frame_buf = (char *)sf_buf;
> +
> +	/* Initialize the reference to the SFrame header. */
> +	sfsec->header = *(struct sframe_header *) frame_buf;
> +	header = &sfsec->header;
> +	if (!sframe_header_sanity_check_p(header))
> +		return SFRAME_ERR_INVAL;
> +
> +	/* Initialize the referece to the SFrame FDE section. */
> +	frame_buf += sframe_sec_get_hdr_size(header);
> +	sfsec->fdes = frame_buf;
> +
> +	/* Initialize the reference to the the SFrame FRE section. */
> +	frame_buf += (header->num_fdes * sizeof(struct sframe_func_desc_entry));
> +	sfsec->fres = frame_buf;
> +
> +	sfsec->fre_nbytes = header->fre_len;
> +
> +	return 0;
> +}
> +
> +/*
> + * Find the SFrame Frame Row Entry which contains the PC.
> + * Returns error code if failure.
> + */
> +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;

Although the sframe_fre structure is relatively small, I always get nervous
when I see structures defined on the kernel stack.

Also, I personally prefer to keep items defined themselves. That is, one
variable per line.

> +	unsigned char *fres;
> +	unsigned int fre_type, fde_type;
> +	size_t esz;
> +	int err = 0;
> +	size_t size = 0;

And to organize it in an "upside-down x-mas" tree fashion:

	struct sframe_func_desc_entry *fdep;
	struct sframe_fre next_fre;
	struct sframe_fre cur_fre
	uint32_t start_address, i;
	unsigned int fre_type;
	unsigned int fde_type;
	unsigned char *fres;
	size_t size = 0;
	size_t esz;
	int err = 0;

Looks much nicer and easier to read that way ;-)

-- Steve


> +	/*
> +	 * 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)) {
> +			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;
> +		}
> +	}
> +	return SFRAME_ERR_FDE_INVAL;
> +}
> +
> +unsigned int sframe_fre_get_base_reg_id(struct sframe_fre *frep,
> +					int *errp)
> +{
> +	if (!frep)
> +		return sframe_set_errno(errp, SFRAME_ERR_FRE_INVAL);
> +
> +	return SFRAME_V1_FRE_CFA_BASE_REG_ID(frep->fre_info);
> +}
> +
> +int32_t sframe_fre_get_cfa_offset(struct sframe_sec *sfsec __always_unused,
> +				  struct sframe_fre *frep, int *errp)
> +{
> +	return sframe_get_fre_offset(frep, SFRAME_FRE_CFA_OFFSET_IDX, errp);
> +}
> +
> +int32_t sframe_fre_get_fp_offset(struct sframe_sec *sfsec,
> +				 struct sframe_fre *frep, int *errp)
> +{
> +	uint32_t fp_offset_idx = 0;
> +	int8_t fp_offset = sframe_sec_get_fixed_fp_offset(sfsec);
> +	/*
> +	 * If the FP offset is not being tracked, return the fixed FP offset
> +	 * from the SFrame header.
> +	 */
> +	if (fp_offset != SFRAME_CFA_FIXED_FP_INVALID) {
> +		*errp = 0;
> +		return fp_offset;
> +	}
> +
> +	/*
> +	 * In some ABIs, the stack offset to recover RA (using the CFA) from is
> +	 * fixed (like AMD64).  In such cases, the stack offset to recover FP
> +	 * will appear at the second index.
> +	 */
> +	fp_offset_idx = ((sframe_sec_get_fixed_ra_offset(sfsec)
> +			  != SFRAME_CFA_FIXED_RA_INVALID)
> +			 ? SFRAME_FRE_RA_OFFSET_IDX
> +			 : SFRAME_FRE_FP_OFFSET_IDX);
> +	return sframe_get_fre_offset(frep, fp_offset_idx, errp);
> +}
> +
> +int32_t sframe_fre_get_ra_offset(struct sframe_sec *sfsec,
> +				 struct sframe_fre *frep, int *errp)
> +{
> +	int8_t ra_offset = sframe_sec_get_fixed_ra_offset(sfsec);
> +	/*
> +	 * If the RA offset was not being tracked, return the fixed RA offset
> +	 * from the SFrame header.
> +	 */
> +	if (ra_offset != SFRAME_CFA_FIXED_RA_INVALID) {
> +		*errp = 0;
> +		return ra_offset;
> +	}
> +
> +	/* Otherwise, get the RA offset from the FRE. */
> +	return sframe_get_fre_offset(frep, SFRAME_FRE_RA_OFFSET_IDX, errp);
> +}
> diff --git a/lib/sframe/sframe_read.h b/lib/sframe/sframe_read.h
> new file mode 100644
> index 000000000000..6632fb76d8b1
> --- /dev/null
> +++ b/lib/sframe/sframe_read.h
> @@ -0,0 +1,75 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2023, Oracle and/or its affiliates.
> + */
> +
> +#ifndef SFRAME_READ_H
> +#define SFRAME_READ_H
> +
> +#include <linux/types.h>
> +
> +#include "sframe.h"
> +
> +struct sframe_sec;
> +
> +#define MAX_NUM_STACK_OFFSETS 3
> +
> +#define MAX_STACK_OFFSET_NBYTES \
> +  ((SFRAME_FRE_OFFSET_4B * 2 * MAX_NUM_STACK_OFFSETS))
> +
> +/*
> + * SFrame Frame Row Entry for the SFrame reader.
> + * Providing such an abstraction helps decouple stack tracer from the
> + * binary format representation of the same.
> + */
> +struct sframe_fre {
> +	uint32_t start_ip_offset;
> +	unsigned char fre_offsets[MAX_STACK_OFFSET_NBYTES];
> +	unsigned char fre_info;
> +};
> +
> +#define SFRAME_ERR ((int) -1)
> +
> +/* SFrame version not supported. */
> +#define SFRAME_ERR_VERSION_INVAL	(-2000)
> +/* Corrupt SFrame. */
> +#define SFRAME_ERR_INVAL		(-2001)
> +/* SFrame Section Initialization Error. */
> +#define SFRAME_ERR_INIT_INVAL		(-2002)
> +/* Corrupt FDE. */
> +#define SFRAME_ERR_FDE_INVAL		(-2003)
> +/* Corrupt FRE. */
> +#define SFRAME_ERR_FRE_INVAL		(-2004)
> +/* FDE not found. */
> +#define SFRAME_ERR_FDE_NOTFOUND		(-2005)
> +/* FDEs not sorted. */
> +#define SFRAME_ERR_FDE_NOTSORTED	(-2006)
> +/* FRE not found. */
> +#define SFRAME_ERR_FRE_NOTFOUND		(-2007)
> +/* FRE offset not present. */
> +#define SFRAME_ERR_FREOFFSET_NOPRESENT	(-2008)
> +
> +extern size_t sframe_sec_sizeof(void);
> +
> +extern int sframe_sec_init(struct sframe_sec *sfsec, const char *cf_buf,
> +			   size_t cf_size);
> +
> +extern int sframe_sec_find_fre(struct sframe_sec *ctx, int32_t pc,
> +			   struct sframe_fre *frep);
> +
> +extern unsigned int sframe_fre_get_base_reg_id(struct sframe_fre *fre,
> +					       int *errp);
> +extern int32_t sframe_fre_get_cfa_offset(struct sframe_sec *dtcx,
> +					 struct sframe_fre *fre,
> +					 int *errp);
> +extern int32_t sframe_fre_get_fp_offset(struct sframe_sec *sfsec,
> +					struct sframe_fre *fre,
> +					int *errp);
> +extern int32_t sframe_fre_get_ra_offset(struct sframe_sec *sfsec,
> +					struct sframe_fre *fre,
> +					int *errp);
> +extern bool sframe_fre_get_ra_mangled_p(struct sframe_sec *sfsec,
> +					struct sframe_fre *fre,
> +					int *errp);
> +
> +#endif /* SFRAME_READ_H */