diff mbox series

[dwarves,5/5] btf_encoder: skip BTF encoding of static functions with inconsistent prototypes

Message ID 1674567931-26458-6-git-send-email-alan.maguire@oracle.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series dwarves: support encoding of optimized-out parameters, removal of inconsistent static functions | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch
bpf/vmtest-bpf-PR fail merge-conflict
bpf/vmtest-bpf-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-VM_Test-3 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-4 success Logs for build for s390x with gcc
bpf/vmtest-bpf-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-6 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-7 success Logs for llvm-toolchain
bpf/vmtest-bpf-VM_Test-8 success Logs for set-matrix
bpf/vmtest-bpf-VM_Test-9 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-10 success Logs for test_maps on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-11 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-VM_Test-12 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-13 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-14 fail Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-15 success Logs for test_progs on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-16 success Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-VM_Test-17 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-18 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-19 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-20 success Logs for test_progs_no_alu32 on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-21 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-VM_Test-22 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-23 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-24 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-25 success Logs for test_progs_no_alu32_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-26 success Logs for test_progs_no_alu32_parallel on s390x with gcc
bpf/vmtest-bpf-VM_Test-27 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-28 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-29 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-30 success Logs for test_progs_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-31 success Logs for test_progs_parallel on s390x with gcc
bpf/vmtest-bpf-VM_Test-32 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-33 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-34 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-35 success Logs for test_verifier on aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-36 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-VM_Test-37 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-38 success Logs for test_verifier on x86_64 with llvm-16

Commit Message

Alan Maguire Jan. 24, 2023, 1:45 p.m. UTC 
Two static functions in different CUs can share the same name but
have different function prototypes.  In such a case BTF has no
way currently to map which function description matches which
function site, so it is safer to eliminate such duplicates by
using the same global matching procedure used to identify
functions with optimized-out parameters.  Protoype checking
is done by comparing parameter numbers and then names.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 btf_encoder.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++----------
 dwarves.h     |  1 +
 2 files changed, 78 insertions(+), 16 deletions(-)

Comments

Jiri Olsa Jan. 25, 2023, 1:39 p.m. UTC | #1 
On Tue, Jan 24, 2023 at 01:45:31PM +0000, Alan Maguire wrote:

SNIP

>  static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn)
>  {
> @@ -819,13 +837,51 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
>  	}
>  	/* If we find an existing entry, we want to merge observations
>  	 * across both functions, checking that the "seen optimized-out
> -	 * parameters" status is reflected in our tree entry.
> +	 * parameters"/inconsistent proto status is reflected in tree entry.
>  	 * If the entry is new, record encoder state required
>  	 * to add the local function later (encoder + type_id_off)
> -	 * such that we can add the function later.
> +	 * such that we can add the function later.  Parameter names are
> +	 * also stored in state to speed up multiple static function
> +	 * comparisons.
>  	 */
>  	if (*nodep != fn) {
> -		(*nodep)->proto.optimized_parms |= fn->proto.optimized_parms;
> +		struct function *ofn = *nodep;
> +
> +		ofn->proto.optimized_parms |= fn->proto.optimized_parms;
> +		/* compare parameters to see if signatures match */
> +
> +		if (ofn->proto.inconsistent_proto)
> +			goto out;
> +
> +		if (ofn->proto.nr_parms != fn->proto.nr_parms) {
> +			ofn->proto.inconsistent_proto = 1;
> +			goto out;
> +		}
> +		if (ofn->proto.nr_parms > 0) {
> +			struct btf_encoder_state *state = ofn->priv;
> +			const char *parameter_names[BTF_ENCODER_MAX_PARAMETERS];
> +			int i;
> +
> +			if (!state->got_parameter_names) {
> +				parameter_names__get(&ofn->proto, BTF_ENCODER_MAX_PARAMETERS,
> +						     state->parameter_names);
> +				state->got_parameter_names = true;
> +			}
> +			parameter_names__get(&fn->proto, BTF_ENCODER_MAX_PARAMETERS,
> +					     parameter_names);
> +			for (i = 0; i < ofn->proto.nr_parms; i++) {
> +				if (!state->parameter_names[i]) {
> +					if (!parameter_names[i])
> +						continue;
> +				} else if (parameter_names[i]) {
> +					if (strcmp(state->parameter_names[i],
> +						   parameter_names[i]) == 0)
> +						continue;

I guess we can't check type easily? tag has type field,
but I'm not sure if we can get reasonable type info from that

jirka

> +				}
> +				ofn->proto.inconsistent_proto = 1;
> +				goto out;
> +			}
> +		}
>  	} else {
>  		struct btf_encoder_state *state = zalloc(sizeof(*state));
>  
> @@ -898,10 +954,12 @@ static void btf_encoder__add_saved_func(const void *nodep, const VISIT which,
>  	/* we can safely free encoder state since we visit each node once */
>  	free(fn->priv);
>  	fn->priv = NULL;
> -	if (fn->proto.optimized_parms) {
> +	if (fn->proto.optimized_parms || fn->proto.inconsistent_proto) {
>  		if (encoder->verbose)
> -			printf("skipping addition of '%s' due to optimized-out parameters\n",
> -			       function__name(fn));
> +			printf("skipping addition of '%s' due to %s\n",
> +			       function__name(fn),
> +			       fn->proto.optimized_parms ? "optimized-out parameters" :
> +							   "multiple inconsistent function prototypes");
>  	} else {
>  		btf_encoder__add_func(encoder, fn);
>  	}
> @@ -1775,6 +1833,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>  		 */
>  		if (fn->declaration)
>  			continue;
> +		if (!fn->external)
> +			save = true;
>  		if (!ftype__has_arg_names(&fn->proto))
>  			continue;
>  		if (encoder->functions.cnt) {
> @@ -1790,7 +1850,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>  			if (func) {
>  				if (func->generated)
>  					continue;
> -				func->generated = true;
> +				if (!save)
> +					func->generated = true;
>  			} else if (encoder->functions.suffix_cnt) {
>  				/* falling back to name.isra.0 match if no exact
>  				 * match is found; only bother if we found any
> diff --git a/dwarves.h b/dwarves.h
> index 1ad1b3b..9b80262 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -830,6 +830,7 @@ struct ftype {
>  	uint16_t	 nr_parms;
>  	uint8_t		 unspec_parms:1; /* just one bit is needed */
>  	uint8_t		 optimized_parms:1;
> +	uint8_t		 inconsistent_proto:1;
>  };
>  
>  static inline struct ftype *tag__ftype(const struct tag *tag)
> -- 
> 1.8.3.1
>
Alan Maguire Jan. 25, 2023, 2:18 p.m. UTC | #2 
On 25/01/2023 13:39, Jiri Olsa wrote:
> On Tue, Jan 24, 2023 at 01:45:31PM +0000, Alan Maguire wrote:
> 
> SNIP
> 
>>  static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn)
>>  {
>> @@ -819,13 +837,51 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
>>  	}
>>  	/* If we find an existing entry, we want to merge observations
>>  	 * across both functions, checking that the "seen optimized-out
>> -	 * parameters" status is reflected in our tree entry.
>> +	 * parameters"/inconsistent proto status is reflected in tree entry.
>>  	 * If the entry is new, record encoder state required
>>  	 * to add the local function later (encoder + type_id_off)
>> -	 * such that we can add the function later.
>> +	 * such that we can add the function later.  Parameter names are
>> +	 * also stored in state to speed up multiple static function
>> +	 * comparisons.
>>  	 */
>>  	if (*nodep != fn) {
>> -		(*nodep)->proto.optimized_parms |= fn->proto.optimized_parms;
>> +		struct function *ofn = *nodep;
>> +
>> +		ofn->proto.optimized_parms |= fn->proto.optimized_parms;
>> +		/* compare parameters to see if signatures match */
>> +
>> +		if (ofn->proto.inconsistent_proto)
>> +			goto out;
>> +
>> +		if (ofn->proto.nr_parms != fn->proto.nr_parms) {
>> +			ofn->proto.inconsistent_proto = 1;
>> +			goto out;
>> +		}
>> +		if (ofn->proto.nr_parms > 0) {
>> +			struct btf_encoder_state *state = ofn->priv;
>> +			const char *parameter_names[BTF_ENCODER_MAX_PARAMETERS];
>> +			int i;
>> +
>> +			if (!state->got_parameter_names) {
>> +				parameter_names__get(&ofn->proto, BTF_ENCODER_MAX_PARAMETERS,
>> +						     state->parameter_names);
>> +				state->got_parameter_names = true;
>> +			}
>> +			parameter_names__get(&fn->proto, BTF_ENCODER_MAX_PARAMETERS,
>> +					     parameter_names);
>> +			for (i = 0; i < ofn->proto.nr_parms; i++) {
>> +				if (!state->parameter_names[i]) {
>> +					if (!parameter_names[i])
>> +						continue;
>> +				} else if (parameter_names[i]) {
>> +					if (strcmp(state->parameter_names[i],
>> +						   parameter_names[i]) == 0)
>> +						continue;
> 
> I guess we can't check type easily? tag has type field,
> but I'm not sure if we can get reasonable type info from that
>

Ideally we'd do that definitely; my worry is that we'd have to
provide a buffer for each parameter type, and representing some parameter
types can be quite complex (like function pointer parameters). 
The memory and computation overheads would likely be significant to compute
the exact parameter types I suspect, and this would need to be
done for > 3000 vmlinux functions which have multiple instances.

In practice, the simplistic approach does seem to work;
I'd suggest we stick with the simple approach for now and
see if we can improve on it over time.

Thanks!

Alan 
> jirka
> 
>> +				}
>> +				ofn->proto.inconsistent_proto = 1;
>> +				goto out;
>> +			}
>> +		}
>>  	} else {
>>  		struct btf_encoder_state *state = zalloc(sizeof(*state));
>>  
>> @@ -898,10 +954,12 @@ static void btf_encoder__add_saved_func(const void *nodep, const VISIT which,
>>  	/* we can safely free encoder state since we visit each node once */
>>  	free(fn->priv);
>>  	fn->priv = NULL;
>> -	if (fn->proto.optimized_parms) {
>> +	if (fn->proto.optimized_parms || fn->proto.inconsistent_proto) {
>>  		if (encoder->verbose)
>> -			printf("skipping addition of '%s' due to optimized-out parameters\n",
>> -			       function__name(fn));
>> +			printf("skipping addition of '%s' due to %s\n",
>> +			       function__name(fn),
>> +			       fn->proto.optimized_parms ? "optimized-out parameters" :
>> +							   "multiple inconsistent function prototypes");
>>  	} else {
>>  		btf_encoder__add_func(encoder, fn);
>>  	}
>> @@ -1775,6 +1833,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>>  		 */
>>  		if (fn->declaration)
>>  			continue;
>> +		if (!fn->external)
>> +			save = true;
>>  		if (!ftype__has_arg_names(&fn->proto))
>>  			continue;
>>  		if (encoder->functions.cnt) {
>> @@ -1790,7 +1850,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>>  			if (func) {
>>  				if (func->generated)
>>  					continue;
>> -				func->generated = true;
>> +				if (!save)
>> +					func->generated = true;
>>  			} else if (encoder->functions.suffix_cnt) {
>>  				/* falling back to name.isra.0 match if no exact
>>  				 * match is found; only bother if we found any
>> diff --git a/dwarves.h b/dwarves.h
>> index 1ad1b3b..9b80262 100644
>> --- a/dwarves.h
>> +++ b/dwarves.h
>> @@ -830,6 +830,7 @@ struct ftype {
>>  	uint16_t	 nr_parms;
>>  	uint8_t		 unspec_parms:1; /* just one bit is needed */
>>  	uint8_t		 optimized_parms:1;
>> +	uint8_t		 inconsistent_proto:1;
>>  };
>>  
>>  static inline struct ftype *tag__ftype(const struct tag *tag)
>> -- 
>> 1.8.3.1
>>
Jiri Olsa Jan. 25, 2023, 4:53 p.m. UTC | #3 
On Tue, Jan 24, 2023 at 01:45:31PM +0000, Alan Maguire wrote:

SNIP

>  	} else {
>  		struct btf_encoder_state *state = zalloc(sizeof(*state));
>  
> @@ -898,10 +954,12 @@ static void btf_encoder__add_saved_func(const void *nodep, const VISIT which,
>  	/* we can safely free encoder state since we visit each node once */
>  	free(fn->priv);
>  	fn->priv = NULL;
> -	if (fn->proto.optimized_parms) {
> +	if (fn->proto.optimized_parms || fn->proto.inconsistent_proto) {
>  		if (encoder->verbose)
> -			printf("skipping addition of '%s' due to optimized-out parameters\n",
> -			       function__name(fn));
> +			printf("skipping addition of '%s' due to %s\n",
> +			       function__name(fn),
> +			       fn->proto.optimized_parms ? "optimized-out parameters" :
> +							   "multiple inconsistent function prototypes");
>  	} else {
>  		btf_encoder__add_func(encoder, fn);
>  	}
> @@ -1775,6 +1833,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>  		 */
>  		if (fn->declaration)
>  			continue;
> +		if (!fn->external)
> +			save = true;

how about conflicts between static and global functions,
I can see still few cases like:

  void __init msg_init(void)
  static void msg_init(struct spi_message *m, struct spi_transfer *x,
                       u8 *addr, size_t count, char *tx, char *rx)

  static inline void free_pgtable_page(u64 *pt)
  void free_pgtable_page(void *vaddr)

  STATIC inline long INIT parse_header(u8 *input, long *skip, long in_len)
  static struct tb_cfg_result parse_header(const struct ctl_pkg *pkg, u32 len,
                                         enum tb_cfg_pkg_type type, u64 route)
  static void __init parse_header(char *s)


could we enable the check/save globaly?

jirka

>  		if (!ftype__has_arg_names(&fn->proto))
>  			continue;
>  		if (encoder->functions.cnt) {
> @@ -1790,7 +1850,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>  			if (func) {
>  				if (func->generated)
>  					continue;
> -				func->generated = true;
> +				if (!save)
> +					func->generated = true;
>  			} else if (encoder->functions.suffix_cnt) {
>  				/* falling back to name.isra.0 match if no exact
>  				 * match is found; only bother if we found any
> diff --git a/dwarves.h b/dwarves.h
> index 1ad1b3b..9b80262 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -830,6 +830,7 @@ struct ftype {
>  	uint16_t	 nr_parms;
>  	uint8_t		 unspec_parms:1; /* just one bit is needed */
>  	uint8_t		 optimized_parms:1;
> +	uint8_t		 inconsistent_proto:1;
>  };
>  
>  static inline struct ftype *tag__ftype(const struct tag *tag)
> -- 
> 1.8.3.1
>
Alan Maguire Jan. 26, 2023, 2:12 p.m. UTC | #4 
On 25/01/2023 16:53, Jiri Olsa wrote:
> On Tue, Jan 24, 2023 at 01:45:31PM +0000, Alan Maguire wrote:
> 
> SNIP
> 
>>  	} else {
>>  		struct btf_encoder_state *state = zalloc(sizeof(*state));
>>  
>> @@ -898,10 +954,12 @@ static void btf_encoder__add_saved_func(const void *nodep, const VISIT which,
>>  	/* we can safely free encoder state since we visit each node once */
>>  	free(fn->priv);
>>  	fn->priv = NULL;
>> -	if (fn->proto.optimized_parms) {
>> +	if (fn->proto.optimized_parms || fn->proto.inconsistent_proto) {
>>  		if (encoder->verbose)
>> -			printf("skipping addition of '%s' due to optimized-out parameters\n",
>> -			       function__name(fn));
>> +			printf("skipping addition of '%s' due to %s\n",
>> +			       function__name(fn),
>> +			       fn->proto.optimized_parms ? "optimized-out parameters" :
>> +							   "multiple inconsistent function prototypes");
>>  	} else {
>>  		btf_encoder__add_func(encoder, fn);
>>  	}
>> @@ -1775,6 +1833,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>>  		 */
>>  		if (fn->declaration)
>>  			continue;
>> +		if (!fn->external)
>> +			save = true;
> 
> how about conflicts between static and global functions,
> I can see still few cases like:
> 
>   void __init msg_init(void)
>   static void msg_init(struct spi_message *m, struct spi_transfer *x,
>                        u8 *addr, size_t count, char *tx, char *rx)
> 
>   static inline void free_pgtable_page(u64 *pt)
>   void free_pgtable_page(void *vaddr)
> 
>   STATIC inline long INIT parse_header(u8 *input, long *skip, long in_len)
>   static struct tb_cfg_result parse_header(const struct ctl_pkg *pkg, u32 len,
>                                          enum tb_cfg_pkg_type type, u64 route)
>   static void __init parse_header(char *s)
> 
>

great catch; I hadn't thought of this at all. Looks like it is often
the case that the first function found will actually end up in BTF
currently, so sometimes we get the static, sometimes the non-static.
I'm seeing the same list as above.

> could we enable the check/save globaly?
>

I think we could, though at the additional cost of a larger tree
of functions. I can't see another way to handle it though right
now.

Alan

 
> jirka
> 
>>  		if (!ftype__has_arg_names(&fn->proto))
>>  			continue;
>>  		if (encoder->functions.cnt) {
>> @@ -1790,7 +1850,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>>  			if (func) {
>>  				if (func->generated)
>>  					continue;
>> -				func->generated = true;
>> +				if (!save)
>> +					func->generated = true;
>>  			} else if (encoder->functions.suffix_cnt) {
>>  				/* falling back to name.isra.0 match if no exact
>>  				 * match is found; only bother if we found any
>> diff --git a/dwarves.h b/dwarves.h
>> index 1ad1b3b..9b80262 100644
>> --- a/dwarves.h
>> +++ b/dwarves.h
>> @@ -830,6 +830,7 @@ struct ftype {
>>  	uint16_t	 nr_parms;
>>  	uint8_t		 unspec_parms:1; /* just one bit is needed */
>>  	uint8_t		 optimized_parms:1;
>> +	uint8_t		 inconsistent_proto:1;
>>  };
>>  
>>  static inline struct ftype *tag__ftype(const struct tag *tag)
>> -- 
>> 1.8.3.1
>>
diff mbox series

Patch

diff --git a/btf_encoder.c b/btf_encoder.c
index a86b099..e0acd29 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -786,21 +786,39 @@  static int function__compare(const void *a, const void *b)
 	return strcmp(function__name(fa), function__name(fb));
 }
 
+#define BTF_ENCODER_MAX_PARAMETERS	10
+
 struct btf_encoder_state {
 	struct btf_encoder *encoder;
 	uint32_t type_id_off;
+	bool got_parameter_names;
+	const char *parameter_names[BTF_ENCODER_MAX_PARAMETERS];
 };
 
+void parameter_names__get(struct ftype *ftype, size_t nr_parameters, const char **parameter_names)
+{
+	struct parameter *parameter;
+	int i = 0;
+
+	ftype__for_each_parameter(ftype, parameter) {
+		if (i >= nr_parameters)
+			break;
+		parameter_names[i++] = parameter__name(parameter);
+	}
+}
+
 /*
- * static functions with suffixes are not added yet - we need to
- * observe across all CUs to see if the static function has
- * optimized parameters in any CU, since in such a case it should
- * not be included in the final BTF.  NF_HOOK.constprop.0() is
- * a case in point - it has optimized-out parameters in some CUs
- * but not others.  In order to have consistency (since we do not
- * know which instance the BTF-specified function signature will
- * apply to), we simply skip adding functions which have optimized
- * out parameters anywhere.
+ * static functions are not added yet - we need to observe across
+ * all CUs to see if the static function has optimized-out parameters
+ * in any CU, or inconsistent function prototypes; in either case,
+ * it should not be included in the final BTF.  For the optimized-out
+ * case, NF_HOOK.constprop.0() is a case in point - it has optimized-out
+ * parameters in some CUs but not others.  Similarly, a static function
+ * like enable_store() has inconsistent function prototypes in different
+ * CUs so should not be present.  In order to have consistency (since
+ * we do not know which instance the BTF-specified function signature
+ * will apply to), we simply skip adding functions which have
+ * optimized-out parameters/inconsistent function prototypes anywhere.
  */
 static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn)
 {
@@ -819,13 +837,51 @@  static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
 	}
 	/* If we find an existing entry, we want to merge observations
 	 * across both functions, checking that the "seen optimized-out
-	 * parameters" status is reflected in our tree entry.
+	 * parameters"/inconsistent proto status is reflected in tree entry.
 	 * If the entry is new, record encoder state required
 	 * to add the local function later (encoder + type_id_off)
-	 * such that we can add the function later.
+	 * such that we can add the function later.  Parameter names are
+	 * also stored in state to speed up multiple static function
+	 * comparisons.
 	 */
 	if (*nodep != fn) {
-		(*nodep)->proto.optimized_parms |= fn->proto.optimized_parms;
+		struct function *ofn = *nodep;
+
+		ofn->proto.optimized_parms |= fn->proto.optimized_parms;
+		/* compare parameters to see if signatures match */
+
+		if (ofn->proto.inconsistent_proto)
+			goto out;
+
+		if (ofn->proto.nr_parms != fn->proto.nr_parms) {
+			ofn->proto.inconsistent_proto = 1;
+			goto out;
+		}
+		if (ofn->proto.nr_parms > 0) {
+			struct btf_encoder_state *state = ofn->priv;
+			const char *parameter_names[BTF_ENCODER_MAX_PARAMETERS];
+			int i;
+
+			if (!state->got_parameter_names) {
+				parameter_names__get(&ofn->proto, BTF_ENCODER_MAX_PARAMETERS,
+						     state->parameter_names);
+				state->got_parameter_names = true;
+			}
+			parameter_names__get(&fn->proto, BTF_ENCODER_MAX_PARAMETERS,
+					     parameter_names);
+			for (i = 0; i < ofn->proto.nr_parms; i++) {
+				if (!state->parameter_names[i]) {
+					if (!parameter_names[i])
+						continue;
+				} else if (parameter_names[i]) {
+					if (strcmp(state->parameter_names[i],
+						   parameter_names[i]) == 0)
+						continue;
+				}
+				ofn->proto.inconsistent_proto = 1;
+				goto out;
+			}
+		}
 	} else {
 		struct btf_encoder_state *state = zalloc(sizeof(*state));
 
@@ -898,10 +954,12 @@  static void btf_encoder__add_saved_func(const void *nodep, const VISIT which,
 	/* we can safely free encoder state since we visit each node once */
 	free(fn->priv);
 	fn->priv = NULL;
-	if (fn->proto.optimized_parms) {
+	if (fn->proto.optimized_parms || fn->proto.inconsistent_proto) {
 		if (encoder->verbose)
-			printf("skipping addition of '%s' due to optimized-out parameters\n",
-			       function__name(fn));
+			printf("skipping addition of '%s' due to %s\n",
+			       function__name(fn),
+			       fn->proto.optimized_parms ? "optimized-out parameters" :
+							   "multiple inconsistent function prototypes");
 	} else {
 		btf_encoder__add_func(encoder, fn);
 	}
@@ -1775,6 +1833,8 @@  int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
 		 */
 		if (fn->declaration)
 			continue;
+		if (!fn->external)
+			save = true;
 		if (!ftype__has_arg_names(&fn->proto))
 			continue;
 		if (encoder->functions.cnt) {
@@ -1790,7 +1850,8 @@  int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
 			if (func) {
 				if (func->generated)
 					continue;
-				func->generated = true;
+				if (!save)
+					func->generated = true;
 			} else if (encoder->functions.suffix_cnt) {
 				/* falling back to name.isra.0 match if no exact
 				 * match is found; only bother if we found any
diff --git a/dwarves.h b/dwarves.h
index 1ad1b3b..9b80262 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -830,6 +830,7 @@  struct ftype {
 	uint16_t	 nr_parms;
 	uint8_t		 unspec_parms:1; /* just one bit is needed */
 	uint8_t		 optimized_parms:1;
+	uint8_t		 inconsistent_proto:1;
 };
 
 static inline struct ftype *tag__ftype(const struct tag *tag)