All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Maguire <alan.maguire@oracle.com>
To: acme@kernel.org, olsajiri@gmail.com, ast@kernel.org
Cc: daniel@iogearbox.net, andrii@kernel.org, yhs@fb.com,
	eddyz87@gmail.com, sinquersw@gmail.com, timo@incline.eu,
	songliubraving@fb.com, john.fastabend@gmail.com,
	kpsingh@chromium.org, sdf@google.com, haoluo@google.com,
	martin.lau@kernel.org, bpf@vger.kernel.org,
	Alan Maguire <alan.maguire@oracle.com>
Subject: [RFC dwarves 1/4] dwarf_loader: mark functions that do not use expected registers for params
Date: Fri, 17 Feb 2023 23:10:30 +0000	[thread overview]
Message-ID: <1676675433-10583-2-git-send-email-alan.maguire@oracle.com> (raw)
In-Reply-To: <1676675433-10583-1-git-send-email-alan.maguire@oracle.com>

Calling conventions dictate which registers are used for
function parameters.

When a function is optimized however, we need to ensure that
the non-optimized parameters do not violate expectations about
register use as this would violate expectations for tracing.
At CU initialization, create a mapping from parameter index
to expected DW_OP_reg, and use it to validate parameters
match with expectations.  A parameter which is passed via
the stack, as a constant, or uses an unexpected register,
violates these expectations and it (and the associated
function) are marked as having unexpected register mapping.

Note though that there is as exception here that needs to
be handled; when a (typedef) struct is passed as a parameter,
it can use multiple registers so will throw off later register
expectations.  Exempt functions that have unexpected
register usage _and_ struct parameters (examples are found
in the "tracing_struct" test).

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 dwarf_loader.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++---
 dwarves.h      |   5 +++
 2 files changed, 109 insertions(+), 5 deletions(-)

diff --git a/dwarf_loader.c b/dwarf_loader.c
index acdb68d..014e130 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -1022,6 +1022,51 @@ static int arch__nr_register_params(const GElf_Ehdr *ehdr)
 	return 0;
 }
 
+/* map from parameter index (0 for first, ...) to expected DW_OP_reg.
+ * This will allow us to identify cases where optimized-out parameters
+ * interfere with expectations about register contents on function
+ * entry.
+ */
+static void arch__set_register_params(const GElf_Ehdr *ehdr, struct cu *cu)
+{
+	memset(cu->register_params, -1, sizeof(cu->register_params));
+
+	switch (ehdr->e_machine) {
+	case EM_S390:
+		/* https://github.com/IBM/s390x-abi/releases/download/v1.6/lzsabi_s390x.pdf */
+		cu->register_params[0] = DW_OP_reg2;	// %r2
+		cu->register_params[1] = DW_OP_reg3;	// %r3
+		cu->register_params[2] = DW_OP_reg4;	// %r4
+		cu->register_params[3] = DW_OP_reg5;	// %r5
+		cu->register_params[4] = DW_OP_reg6;	// %r6
+		return;
+	case EM_X86_64:
+		/* //en.wikipedia.org/wiki/X86_calling_conventions#System_V_AMD64_ABI */
+		cu->register_params[0] = DW_OP_reg5;	// %rdi
+		cu->register_params[1] = DW_OP_reg4;	// %rsi
+		cu->register_params[2] = DW_OP_reg1;	// %rdx
+		cu->register_params[3] = DW_OP_reg2;	// %rcx
+		cu->register_params[4] = DW_OP_reg8;	// %r8
+		cu->register_params[5] = DW_OP_reg9;	// %r9
+		return;
+	case EM_ARM:
+		/* https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst#machine-registers */
+	case EM_AARCH64:
+		/* https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#machine-registers */
+		cu->register_params[0] = DW_OP_reg0;
+		cu->register_params[1] = DW_OP_reg1;
+		cu->register_params[2] = DW_OP_reg2;
+		cu->register_params[3] = DW_OP_reg3;
+		cu->register_params[4] = DW_OP_reg4;
+		cu->register_params[5] = DW_OP_reg5;
+		cu->register_params[6] = DW_OP_reg6;
+		cu->register_params[7] = DW_OP_reg7;
+		return;
+	default:
+		return;
+	}
+}
+
 static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
 					struct conf_load *conf, int param_idx)
 {
@@ -1075,18 +1120,28 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
 		if (parm->has_loc &&
 		    attr_location(die, &loc.expr, &loc.exprlen) == 0 &&
 			loc.exprlen != 0) {
+			int expected_reg = cu->register_params[param_idx];
 			Dwarf_Op *expr = loc.expr;
 
 			switch (expr->atom) {
 			case DW_OP_reg0 ... DW_OP_reg31:
+				/* mark parameters that use an unexpected
+				 * register to hold a parameter; these will
+				 * be problematic for users of BTF as they
+				 * violate expectations about register
+				 * contents.
+				 */
+				if (expected_reg >= 0 && expected_reg != expr->atom)
+					parm->unexpected_reg = 1;
+				break;
 			case DW_OP_breg0 ... DW_OP_breg31:
 				break;
 			default:
-				parm->optimized = 1;
+				parm->unexpected_reg = 1;
 				break;
 			}
 		} else if (has_const_value) {
-			parm->optimized = 1;
+			parm->unexpected_reg = 1;
 		}
 	}
 
@@ -2302,13 +2357,17 @@ static void ftype__recode_dwarf_types(struct tag *tag, struct cu *cu)
 			pos->tag.type = dtype->tag->type;
 			/* share location information between parameter and
 			 * abstract origin; if neither have location, we will
-			 * mark the parameter as optimized out.
+			 * mark the parameter as optimized out.  Also share
+			 * info regarding unexpected register use for
+			 * parameters.
 			 */
 			if (pos->has_loc)
 				opos->has_loc = pos->has_loc;
 
 			if (pos->optimized)
 				opos->optimized = pos->optimized;
+			if (pos->unexpected_reg)
+				opos->unexpected_reg = pos->unexpected_reg;
 			continue;
 		}
 
@@ -2584,6 +2643,27 @@ out:
 	return 0;
 }
 
+static bool param__is_struct(struct cu *cu, struct tag *tag)
+{
+	const struct dwarf_tag *dtag = tag->priv;
+	struct dwarf_tag *dtype = dwarf_cu__find_type_by_ref(cu->priv, &dtag->type);
+	struct tag *type;
+
+	if (!dtype)
+		return false;
+	type = dtype->tag;
+
+	switch (type->tag) {
+	case DW_TAG_structure_type:
+		return true;
+	case DW_TAG_typedef:
+		/* handle "typedef struct" */
+		return param__is_struct(cu, type);
+	default:
+		return false;
+	}
+}
+
 static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 {
 	struct ptr_table *pt = &cu->functions_table;
@@ -2593,6 +2673,7 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 		struct tag *tag = pt->entries[i];
 		struct parameter *pos;
 		struct function *fn = tag__function(tag);
+		bool has_unexpected_reg = false, has_struct_param = false;
 
 		/* mark function as optimized if parameter is, or
 		 * if parameter does not have a location; at this
@@ -2600,12 +2681,29 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu)
 		 * abstract origins for cases where a parameter
 		 * location is not stored in the original function
 		 * parameter tag.
+		 *
+		 * Also mark functions which, due to optimization,
+		 * use an unexpected register for a parameter.
+		 * Exception is functions which have a struct
+		 * as a parameter, as multiple registers may
+		 * be used to represent it, throwing off register
+		 * to parameter mapping.
 		 */
 		ftype__for_each_parameter(&fn->proto, pos) {
-			if (pos->optimized || !pos->has_loc) {
+			if (pos->optimized || !pos->has_loc)
 				fn->proto.optimized_parms = 1;
-				break;
+
+			if (pos->unexpected_reg)
+				has_unexpected_reg = true;
+		}
+		if (has_unexpected_reg) {
+			ftype__for_each_parameter(&fn->proto, pos) {
+				has_struct_param = param__is_struct(cu, &pos->tag);
+				if (has_struct_param)
+					break;
 			}
+			if (!has_struct_param)
+				fn->proto.unexpected_reg = 1;
 		}
 
 		if (tag == NULL || tag->type != 0)
@@ -2917,6 +3015,7 @@ static int cu__set_common(struct cu *cu, struct conf_load *conf,
 
 	cu->little_endian = ehdr.e_ident[EI_DATA] == ELFDATA2LSB;
 	cu->nr_register_params = arch__nr_register_params(&ehdr);
+	arch__set_register_params(&ehdr, cu);
 	return 0;
 }
 
diff --git a/dwarves.h b/dwarves.h
index 24a1909..5074cf8 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -234,6 +234,8 @@ struct debug_fmt_ops {
 	bool		   has_alignment_info;
 };
 
+#define ARCH_MAX_REGISTER_PARAMS	8
+
 /*
  * unspecified_type: If this CU has a DW_TAG_unspecified_type, as BTF doesn't have a representation for this
  * 		     and thus we need to check functions returning this to convert it to void.
@@ -265,6 +267,7 @@ struct cu {
 	uint8_t		 uses_global_strings:1;
 	uint8_t		 little_endian:1;
 	uint8_t		 nr_register_params;
+	int		 register_params[ARCH_MAX_REGISTER_PARAMS];
 	uint16_t	 language;
 	unsigned long	 nr_inline_expansions;
 	size_t		 size_inline_expansions;
@@ -812,6 +815,7 @@ struct parameter {
 	struct tag tag;
 	const char *name;
 	uint8_t optimized:1;
+	uint8_t unexpected_reg:1;
 	uint8_t has_loc:1;
 };
 
@@ -834,6 +838,7 @@ struct ftype {
 	uint16_t	 nr_parms;
 	uint8_t		 unspec_parms:1; /* just one bit is needed */
 	uint8_t		 optimized_parms:1;
+	uint8_t		 unexpected_reg:1;
 	uint8_t		 processed:1;
 	uint8_t		 inconsistent_proto:1;
 };
-- 
2.31.1


  reply	other threads:[~2023-02-18  2:21 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-17 23:10 [RFC dwarves 0/4] dwarves: change BTF encoding skip logic for functions Alan Maguire
2023-02-17 23:10 ` Alan Maguire [this message]
2023-02-20 19:03   ` [RFC dwarves 1/4] dwarf_loader: mark functions that do not use expected registers for params Alexei Starovoitov
2023-02-20 19:42     ` Alan Maguire
2023-02-20 22:30       ` Alexei Starovoitov
2023-02-21 15:52         ` Alan Maguire
2023-02-17 23:10 ` [RFC dwarves 2/4] btf_encoder: exclude functions with unexpected param register use not optimizations Alan Maguire
2023-02-17 23:10 ` [RFC dwarves 3/4] pahole: update descriptions for btf_gen_optimized, skip_encoding_btf_inconsistent_proto Alan Maguire
2023-02-17 23:10 ` [RFC dwarves 4/4] pahole: update man page for options also Alan Maguire
2023-02-18 14:12 ` [RFC dwarves 0/4] dwarves: change BTF encoding skip logic for functions Arnaldo Carvalho de Melo
2023-02-18 14:37   ` Arnaldo Carvalho de Melo
2023-02-19 22:23 ` Jiri Olsa

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=1676675433-10583-2-git-send-email-alan.maguire@oracle.com \
    --to=alan.maguire@oracle.com \
    --cc=acme@kernel.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kpsingh@chromium.org \
    --cc=martin.lau@kernel.org \
    --cc=olsajiri@gmail.com \
    --cc=sdf@google.com \
    --cc=sinquersw@gmail.com \
    --cc=songliubraving@fb.com \
    --cc=timo@incline.eu \
    --cc=yhs@fb.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.