diff mbox series

[v3,dwarves,3/5] pahole: add --btf_features support

Message ID 20231018122926.735416-4-alan.maguire@oracle.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series pahole, btf_encoder: support --btf_features | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Alan Maguire Oct. 18, 2023, 12:29 p.m. UTC
This allows consumers to specify an opt-in set of features
they want to use in BTF encoding.

Supported features are a comma-separated combination of

	encode_force    Ignore invalid symbols when encoding BTF.
	var             Encode variables using BTF_KIND_VAR in BTF.
	float           Encode floating-point types in BTF.
	decl_tag        Encode declaration tags using BTF_KIND_DECL_TAG.
	type_tag        Encode type tags using BTF_KIND_TYPE_TAG.
	enum64          Encode enum64 values with BTF_KIND_ENUM64.
	optimized_func  Encode representations of optimized functions
	                with suffixes like ".isra.0" etc
	consistent_func Avoid encoding inconsistent static functions.
	                These occur when a parameter is optimized out
	                in some CUs and not others, or when the same
	                function name has inconsistent BTF descriptions
	                in different CUs.

Specifying "--btf_features=all" is the equivalent to setting
all of the above.  If pahole does not know about a feature
specified in --btf_features it silently ignores it.

The --btf_features can either be specified via a single comma-separated
list
	--btf_features=enum64,float

...or via multiple --btf_features values

	--btf_features=enum64 --btf_features=float

These properties allow us to use the --btf_features option in
the kernel scripts/pahole_flags.sh script to specify the desired
set of BTF features.

If a feature named in --btf_features is not present in the version
of pahole used, BTF encoding will not complain.  This is desired
because it means we no longer have to tie new features to a specific
pahole version.

Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
 man-pages/pahole.1 |  24 ++++++++
 pahole.c           | 137 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 160 insertions(+), 1 deletion(-)

Comments

Eduard Zingerman Oct. 19, 2023, 11:07 a.m. UTC | #1
On Wed, 2023-10-18 at 13:29 +0100, Alan Maguire wrote:
> This allows consumers to specify an opt-in set of features
> they want to use in BTF encoding.
> 
> Supported features are a comma-separated combination of
> 
> 	encode_force    Ignore invalid symbols when encoding BTF.
> 	var             Encode variables using BTF_KIND_VAR in BTF.
> 	float           Encode floating-point types in BTF.
> 	decl_tag        Encode declaration tags using BTF_KIND_DECL_TAG.
> 	type_tag        Encode type tags using BTF_KIND_TYPE_TAG.
> 	enum64          Encode enum64 values with BTF_KIND_ENUM64.
> 	optimized_func  Encode representations of optimized functions
> 	                with suffixes like ".isra.0" etc
> 	consistent_func Avoid encoding inconsistent static functions.
> 	                These occur when a parameter is optimized out
> 	                in some CUs and not others, or when the same
> 	                function name has inconsistent BTF descriptions
> 	                in different CUs.
> 
> Specifying "--btf_features=all" is the equivalent to setting
> all of the above.  If pahole does not know about a feature
> specified in --btf_features it silently ignores it.
> 
> The --btf_features can either be specified via a single comma-separated
> list
> 	--btf_features=enum64,float
> 
> ...or via multiple --btf_features values
> 
> 	--btf_features=enum64 --btf_features=float
> 
> These properties allow us to use the --btf_features option in
> the kernel scripts/pahole_flags.sh script to specify the desired
> set of BTF features.
> 
> If a feature named in --btf_features is not present in the version
> of pahole used, BTF encoding will not complain.  This is desired
> because it means we no longer have to tie new features to a specific
> pahole version.
> 
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  man-pages/pahole.1 |  24 ++++++++
>  pahole.c           | 137 ++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 160 insertions(+), 1 deletion(-)
> 
> diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
> index c1b48de..a09885f 100644
> --- a/man-pages/pahole.1
> +++ b/man-pages/pahole.1
> @@ -273,6 +273,30 @@ Generate BTF for functions with optimization-related suffixes (.isra, .constprop
>  .B \-\-btf_gen_all
>  Allow using all the BTF features supported by pahole.
>  
> +.TP
> +.B \-\-btf_features=FEATURE_LIST
> +Encode BTF using the specified feature list, or specify 'all' for all features supported.  This option can be used as an alternative to unsing multiple BTF-related options. Supported features are
> +
> +.nf
> +	encode_force       Ignore invalid symbols when encoding BTF; for example
> +	                   if a symbol has an invalid name, it will be ignored
> +	                   and BTF encoding will continue.
> +	var                Encode variables using BTF_KIND_VAR in BTF.
> +	float              Encode floating-point types in BTF.
> +	decl_tag           Encode declaration tags using BTF_KIND_DECL_TAG.
> +	type_tag           Encode type tags using BTF_KIND_TYPE_TAG.
> +	enum64             Encode enum64 values with BTF_KIND_ENUM64.
> +	optimized_func     Encode representations of optimized functions
> +	                   with suffixes like ".isra.0".
> +	consistent_func    Avoid encoding inconsistent static functions.
> +	                   These occur when a parameter is optimized out
> +	                   in some CUs and not others, or when the same
> +	                   function name has inconsistent BTF descriptions
> +	                   in different CUs.
> +.fi
> +
> +So for example, specifying \-\-btf_encode=var,enum64 will result in a BTF encoding that (as well as encoding basic BTF information) will contain variables and enum64 values.
> +
>  .TP
>  .B \-l, \-\-show_first_biggest_size_base_type_member
>  Show first biggest size base_type member.
> diff --git a/pahole.c b/pahole.c
> index 7a41dc3..0e889cf 100644
> --- a/pahole.c
> +++ b/pahole.c
> @@ -1229,6 +1229,133 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
>  #define ARGP_skip_emitting_atomic_typedefs 338
>  #define ARGP_btf_gen_optimized  339
>  #define ARGP_skip_encoding_btf_inconsistent_proto 340
> +#define ARGP_btf_features	341
> +
> +/* --btf_features=feature1[,feature2,..] allows us to specify
> + * a list of requested BTF features or "all" to enable all features.
> + * These are translated into the appropriate conf_load values via a
> + * struct btf_feature which specifies the associated conf_load
> + * boolean field and whether its default (representing the feature being
> + * off) is false or true.
> + *
> + * btf_features is for opting _into_ features so for a case like
> + * conf_load->btf_gen_floats, the translation is simple; the presence
> + * of the "float" feature in --btf_features sets conf_load->btf_gen_floats
> + * to true.
> + *
> + * The more confusing case is for features that are enabled unless
> + * skipping them is specified; for example
> + * conf_load->skip_encoding_btf_type_tag.  By default - to support
> + * the opt-in model of only enabling features the user asks for -
> + * conf_load->skip_encoding_btf_type_tag is set to true (meaning no
> + * type_tags) and it is only set to false if --btf_features contains
> + * the "type_tag" keyword.
> + *
> + * So from the user perspective, all features specified via
> + * --btf_features are enabled, and if a feature is not specified,
> + * it is disabled.
> + *
> + * If --btf_features is not used, the usual pahole defaults for
> + * BTF encoding apply; we encode type/decl tags, do not encode
> + * floats, etc.  This ensures backwards compatibility.
> + */
> +#define BTF_FEATURE(name, alias, default_value)			\
> +	{ #name, #alias, &conf_load.alias, default_value }
> +
> +struct btf_feature {
> +	const char      *name;
> +	const char      *option_alias;
> +	bool		*conf_value;
> +	bool		default_value;
> +} btf_features[] = {
> +	BTF_FEATURE(encode_force, btf_encode_force, false),
> +	BTF_FEATURE(var, skip_encoding_btf_vars, true),
> +	BTF_FEATURE(float, btf_gen_floats, false),
> +	BTF_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true),
> +	BTF_FEATURE(type_tag, skip_encoding_btf_type_tag, true),
> +	BTF_FEATURE(enum64, skip_encoding_btf_enum64, true),
> +	BTF_FEATURE(optimized_func, btf_gen_optimized, false),
> +	BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false),
> +};
> +
> +#define BTF_MAX_FEATURES	32
> +#define BTF_MAX_FEATURE_STR	1024
> +
> +bool set_btf_features_defaults;
> +
> +static void init_btf_features(void)
> +{
> +	int i;
> +
> +	/* Only set default values once, as multiple --btf_features=
> +	 * may be specified on command-line, and setting defaults
> +	 * again could clobber values.   The aim is to enable
> +	 * all features set across all --btf_features options.
> +	 */
> +	if (set_btf_features_defaults)
> +		return;
> +	for (i = 0; i < ARRAY_SIZE(btf_features); i++)
> +		*btf_features[i].conf_value = btf_features[i].default_value;
> +	set_btf_features_defaults = true;
> +}
> +
> +static struct btf_feature *find_btf_feature(char *name)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(btf_features); i++) {
> +		if (strcmp(name, btf_features[i].name) == 0)
> +			return &btf_features[i];
> +	}
> +	return NULL;
> +}
> +
> +static void enable_btf_feature(struct btf_feature *feature)
> +{
> +	/* switch "default-off" features on, and "default-on" features
> +	 * off; i.e. negate the default value.
> +	 */
> +	*feature->conf_value = !feature->default_value;
> +}
> +
> +/* Translate --btf_features=feature1[,feature2] into conf_load values.
> + * Explicitly ignores unrecognized features to allow future specification
> + * of new opt-in features.
> + */
> +static void parse_btf_features(const char *features)
> +{
> +	char *feature_list[BTF_MAX_FEATURES] = {};
> +	char *saveptr = NULL, *s, *t;
> +	char f[BTF_MAX_FEATURE_STR];
> +	int i, n = 0;
> +
> +	init_btf_features();
> +
> +	if (strcmp(features, "all") == 0) {
> +		for (i = 0; i < ARRAY_SIZE(btf_features); i++)
> +			enable_btf_feature(&btf_features[i]);
> +		return;
> +	}
> +
> +	strncpy(f, features, sizeof(f));
> +	s = f;
> +	while ((t = strtok_r(s, ",", &saveptr)) != NULL && n < BTF_MAX_FEATURES) {
> +		s = NULL;
> +		feature_list[n++] = t;
> +	}
> +

Sorry, I should have realized it when I sent suggestion for v2.
It should be possible to merge the "while" and "for" loops and avoid
hypothetical edge case when old version of pahole is fed with 33 items
long feature list. As in the diff attached to the end of the email.
Feel free to ignore this if you think code is fine as it is.

> +	for (i = 0; i < n; i++) {
> +		struct btf_feature *feature = find_btf_feature(feature_list[i]);
> +
> +		if (!feature) {
> +			if (global_verbose)
> +				fprintf(stderr, "Ignoring unsupported feature '%s'\n",
> +					feature_list[i]);
> +		} else {
> +			enable_btf_feature(feature);
> +		}
> +	}
> +}
>  
>  static const struct argp_option pahole__options[] = {
>  	{
> @@ -1651,6 +1778,12 @@ static const struct argp_option pahole__options[] = {
>  		.key = ARGP_skip_encoding_btf_inconsistent_proto,
>  		.doc = "Skip functions that have multiple inconsistent function prototypes sharing the same name, or that use unexpected registers for parameter values."
>  	},
> +	{
> +		.name = "btf_features",
> +		.key = ARGP_btf_features,
> +		.arg = "FEATURE_LIST",
> +		.doc = "Specify supported BTF features in FEATURE_LIST or 'all' for all supported features. See the pahole manual page for the list of supported features."
> +	},
>  	{
>  		.name = NULL,
>  	}
> @@ -1796,7 +1929,7 @@ static error_t pahole__options_parser(int key, char *arg,
>  	case ARGP_btf_gen_floats:
>  		conf_load.btf_gen_floats = true;	break;
>  	case ARGP_btf_gen_all:
> -		conf_load.btf_gen_floats = true;	break;
> +		parse_btf_features("all");		break;
>  	case ARGP_with_flexible_array:
>  		show_with_flexible_array = true;	break;
>  	case ARGP_prettify_input_filename:
> @@ -1826,6 +1959,8 @@ static error_t pahole__options_parser(int key, char *arg,
>  		conf_load.btf_gen_optimized = true;		break;
>  	case ARGP_skip_encoding_btf_inconsistent_proto:
>  		conf_load.skip_encoding_btf_inconsistent_proto = true; break;
> +	case ARGP_btf_features:
> +		parse_btf_features(arg);		break;
>  	default:
>  		return ARGP_ERR_UNKNOWN;
>  	}

---

diff --git a/pahole.c b/pahole.c
index e308dd1..b9bf395 100644
--- a/pahole.c
+++ b/pahole.c
@@ -1280,7 +1280,6 @@ struct btf_feature {
 	BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false),
 };
 
-#define BTF_MAX_FEATURES	32
 #define BTF_MAX_FEATURE_STR	1024
 
 bool set_btf_features_defaults;
@@ -1338,10 +1337,10 @@ static void show_supported_btf_features(FILE *output)
  */
 static void parse_btf_features(const char *features, bool strict)
 {
-	char *feature_list[BTF_MAX_FEATURES] = {};
-	char *saveptr = NULL, *s, *t;
+	char *saveptr = NULL, *s, *requested;
 	char f[BTF_MAX_FEATURE_STR];
-	int i, n = 0;
+	struct btf_feature *feature;
+	int i;
 
 	init_btf_features();
 
@@ -1353,24 +1352,19 @@ static void parse_btf_features(const char *features, bool strict)
 
 	strncpy(f, features, sizeof(f));
 	s = f;
-	while ((t = strtok_r(s, ",", &saveptr)) != NULL && n < BTF_MAX_FEATURES) {
+	while ((requested = strtok_r(s, ",", &saveptr)) != NULL) {
 		s = NULL;
-		feature_list[n++] = t;
-	}
-
-	for (i = 0; i < n; i++) {
-		struct btf_feature *feature = find_btf_feature(feature_list[i]);
-
+		feature = find_btf_feature(requested);
 		if (!feature) {
 			if (strict) {
 				fprintf(stderr, "Feature '%s' in '%s' is not supported.  Supported BTF features are:\n",
-					feature_list[i], features);
+					requested, features);
 				show_supported_btf_features(stderr);
 				exit(EXIT_FAILURE);
 			}
 			if (global_verbose)
 				fprintf(stderr, "Ignoring unsupported feature '%s'\n",
-					feature_list[i]);
+					requested);
 		} else {
 			enable_btf_feature(feature);
 		}
Alan Maguire Oct. 19, 2023, 9:24 p.m. UTC | #2
On 19/10/2023 12:07, Eduard Zingerman wrote:
> On Wed, 2023-10-18 at 13:29 +0100, Alan Maguire wrote:
>> This allows consumers to specify an opt-in set of features
>> they want to use in BTF encoding.
>>
>> Supported features are a comma-separated combination of
>>
>> 	encode_force    Ignore invalid symbols when encoding BTF.
>> 	var             Encode variables using BTF_KIND_VAR in BTF.
>> 	float           Encode floating-point types in BTF.
>> 	decl_tag        Encode declaration tags using BTF_KIND_DECL_TAG.
>> 	type_tag        Encode type tags using BTF_KIND_TYPE_TAG.
>> 	enum64          Encode enum64 values with BTF_KIND_ENUM64.
>> 	optimized_func  Encode representations of optimized functions
>> 	                with suffixes like ".isra.0" etc
>> 	consistent_func Avoid encoding inconsistent static functions.
>> 	                These occur when a parameter is optimized out
>> 	                in some CUs and not others, or when the same
>> 	                function name has inconsistent BTF descriptions
>> 	                in different CUs.
>>
>> Specifying "--btf_features=all" is the equivalent to setting
>> all of the above.  If pahole does not know about a feature
>> specified in --btf_features it silently ignores it.
>>
>> The --btf_features can either be specified via a single comma-separated
>> list
>> 	--btf_features=enum64,float
>>
>> ...or via multiple --btf_features values
>>
>> 	--btf_features=enum64 --btf_features=float
>>
>> These properties allow us to use the --btf_features option in
>> the kernel scripts/pahole_flags.sh script to specify the desired
>> set of BTF features.
>>
>> If a feature named in --btf_features is not present in the version
>> of pahole used, BTF encoding will not complain.  This is desired
>> because it means we no longer have to tie new features to a specific
>> pahole version.
>>
>> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
>> Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>> Acked-by: Jiri Olsa <jolsa@kernel.org>
>> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>> ---
>>  man-pages/pahole.1 |  24 ++++++++
>>  pahole.c           | 137 ++++++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 160 insertions(+), 1 deletion(-)
>>
>> diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
>> index c1b48de..a09885f 100644
>> --- a/man-pages/pahole.1
>> +++ b/man-pages/pahole.1
>> @@ -273,6 +273,30 @@ Generate BTF for functions with optimization-related suffixes (.isra, .constprop
>>  .B \-\-btf_gen_all
>>  Allow using all the BTF features supported by pahole.
>>  
>> +.TP
>> +.B \-\-btf_features=FEATURE_LIST
>> +Encode BTF using the specified feature list, or specify 'all' for all features supported.  This option can be used as an alternative to unsing multiple BTF-related options. Supported features are
>> +
>> +.nf
>> +	encode_force       Ignore invalid symbols when encoding BTF; for example
>> +	                   if a symbol has an invalid name, it will be ignored
>> +	                   and BTF encoding will continue.
>> +	var                Encode variables using BTF_KIND_VAR in BTF.
>> +	float              Encode floating-point types in BTF.
>> +	decl_tag           Encode declaration tags using BTF_KIND_DECL_TAG.
>> +	type_tag           Encode type tags using BTF_KIND_TYPE_TAG.
>> +	enum64             Encode enum64 values with BTF_KIND_ENUM64.
>> +	optimized_func     Encode representations of optimized functions
>> +	                   with suffixes like ".isra.0".
>> +	consistent_func    Avoid encoding inconsistent static functions.
>> +	                   These occur when a parameter is optimized out
>> +	                   in some CUs and not others, or when the same
>> +	                   function name has inconsistent BTF descriptions
>> +	                   in different CUs.
>> +.fi
>> +
>> +So for example, specifying \-\-btf_encode=var,enum64 will result in a BTF encoding that (as well as encoding basic BTF information) will contain variables and enum64 values.
>> +
>>  .TP
>>  .B \-l, \-\-show_first_biggest_size_base_type_member
>>  Show first biggest size base_type member.
>> diff --git a/pahole.c b/pahole.c
>> index 7a41dc3..0e889cf 100644
>> --- a/pahole.c
>> +++ b/pahole.c
>> @@ -1229,6 +1229,133 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
>>  #define ARGP_skip_emitting_atomic_typedefs 338
>>  #define ARGP_btf_gen_optimized  339
>>  #define ARGP_skip_encoding_btf_inconsistent_proto 340
>> +#define ARGP_btf_features	341
>> +
>> +/* --btf_features=feature1[,feature2,..] allows us to specify
>> + * a list of requested BTF features or "all" to enable all features.
>> + * These are translated into the appropriate conf_load values via a
>> + * struct btf_feature which specifies the associated conf_load
>> + * boolean field and whether its default (representing the feature being
>> + * off) is false or true.
>> + *
>> + * btf_features is for opting _into_ features so for a case like
>> + * conf_load->btf_gen_floats, the translation is simple; the presence
>> + * of the "float" feature in --btf_features sets conf_load->btf_gen_floats
>> + * to true.
>> + *
>> + * The more confusing case is for features that are enabled unless
>> + * skipping them is specified; for example
>> + * conf_load->skip_encoding_btf_type_tag.  By default - to support
>> + * the opt-in model of only enabling features the user asks for -
>> + * conf_load->skip_encoding_btf_type_tag is set to true (meaning no
>> + * type_tags) and it is only set to false if --btf_features contains
>> + * the "type_tag" keyword.
>> + *
>> + * So from the user perspective, all features specified via
>> + * --btf_features are enabled, and if a feature is not specified,
>> + * it is disabled.
>> + *
>> + * If --btf_features is not used, the usual pahole defaults for
>> + * BTF encoding apply; we encode type/decl tags, do not encode
>> + * floats, etc.  This ensures backwards compatibility.
>> + */
>> +#define BTF_FEATURE(name, alias, default_value)			\
>> +	{ #name, #alias, &conf_load.alias, default_value }
>> +
>> +struct btf_feature {
>> +	const char      *name;
>> +	const char      *option_alias;
>> +	bool		*conf_value;
>> +	bool		default_value;
>> +} btf_features[] = {
>> +	BTF_FEATURE(encode_force, btf_encode_force, false),
>> +	BTF_FEATURE(var, skip_encoding_btf_vars, true),
>> +	BTF_FEATURE(float, btf_gen_floats, false),
>> +	BTF_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true),
>> +	BTF_FEATURE(type_tag, skip_encoding_btf_type_tag, true),
>> +	BTF_FEATURE(enum64, skip_encoding_btf_enum64, true),
>> +	BTF_FEATURE(optimized_func, btf_gen_optimized, false),
>> +	BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false),
>> +};
>> +
>> +#define BTF_MAX_FEATURES	32
>> +#define BTF_MAX_FEATURE_STR	1024
>> +
>> +bool set_btf_features_defaults;
>> +
>> +static void init_btf_features(void)
>> +{
>> +	int i;
>> +
>> +	/* Only set default values once, as multiple --btf_features=
>> +	 * may be specified on command-line, and setting defaults
>> +	 * again could clobber values.   The aim is to enable
>> +	 * all features set across all --btf_features options.
>> +	 */
>> +	if (set_btf_features_defaults)
>> +		return;
>> +	for (i = 0; i < ARRAY_SIZE(btf_features); i++)
>> +		*btf_features[i].conf_value = btf_features[i].default_value;
>> +	set_btf_features_defaults = true;
>> +}
>> +
>> +static struct btf_feature *find_btf_feature(char *name)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(btf_features); i++) {
>> +		if (strcmp(name, btf_features[i].name) == 0)
>> +			return &btf_features[i];
>> +	}
>> +	return NULL;
>> +}
>> +
>> +static void enable_btf_feature(struct btf_feature *feature)
>> +{
>> +	/* switch "default-off" features on, and "default-on" features
>> +	 * off; i.e. negate the default value.
>> +	 */
>> +	*feature->conf_value = !feature->default_value;
>> +}
>> +
>> +/* Translate --btf_features=feature1[,feature2] into conf_load values.
>> + * Explicitly ignores unrecognized features to allow future specification
>> + * of new opt-in features.
>> + */
>> +static void parse_btf_features(const char *features)
>> +{
>> +	char *feature_list[BTF_MAX_FEATURES] = {};
>> +	char *saveptr = NULL, *s, *t;
>> +	char f[BTF_MAX_FEATURE_STR];
>> +	int i, n = 0;
>> +
>> +	init_btf_features();
>> +
>> +	if (strcmp(features, "all") == 0) {
>> +		for (i = 0; i < ARRAY_SIZE(btf_features); i++)
>> +			enable_btf_feature(&btf_features[i]);
>> +		return;
>> +	}
>> +
>> +	strncpy(f, features, sizeof(f));
>> +	s = f;
>> +	while ((t = strtok_r(s, ",", &saveptr)) != NULL && n < BTF_MAX_FEATURES) {
>> +		s = NULL;
>> +		feature_list[n++] = t;
>> +	}
>> +
> 
> Sorry, I should have realized it when I sent suggestion for v2.
> It should be possible to merge the "while" and "for" loops and avoid
> hypothetical edge case when old version of pahole is fed with 33 items
> long feature list. As in the diff attached to the end of the email.
> Feel free to ignore this if you think code is fine as it is.

Yeah, it's definitely neater, thanks! I suggest we wait to see if anyone
else has additional suggestions, and I can roll the below into a v4
unless anyone objects.

Alan

> 
>> +	for (i = 0; i < n; i++) {
>> +		struct btf_feature *feature = find_btf_feature(feature_list[i]);
>> +
>> +		if (!feature) {
>> +			if (global_verbose)
>> +				fprintf(stderr, "Ignoring unsupported feature '%s'\n",
>> +					feature_list[i]);
>> +		} else {
>> +			enable_btf_feature(feature);
>> +		}
>> +	}
>> +}
>>  
>>  static const struct argp_option pahole__options[] = {
>>  	{
>> @@ -1651,6 +1778,12 @@ static const struct argp_option pahole__options[] = {
>>  		.key = ARGP_skip_encoding_btf_inconsistent_proto,
>>  		.doc = "Skip functions that have multiple inconsistent function prototypes sharing the same name, or that use unexpected registers for parameter values."
>>  	},
>> +	{
>> +		.name = "btf_features",
>> +		.key = ARGP_btf_features,
>> +		.arg = "FEATURE_LIST",
>> +		.doc = "Specify supported BTF features in FEATURE_LIST or 'all' for all supported features. See the pahole manual page for the list of supported features."
>> +	},
>>  	{
>>  		.name = NULL,
>>  	}
>> @@ -1796,7 +1929,7 @@ static error_t pahole__options_parser(int key, char *arg,
>>  	case ARGP_btf_gen_floats:
>>  		conf_load.btf_gen_floats = true;	break;
>>  	case ARGP_btf_gen_all:
>> -		conf_load.btf_gen_floats = true;	break;
>> +		parse_btf_features("all");		break;
>>  	case ARGP_with_flexible_array:
>>  		show_with_flexible_array = true;	break;
>>  	case ARGP_prettify_input_filename:
>> @@ -1826,6 +1959,8 @@ static error_t pahole__options_parser(int key, char *arg,
>>  		conf_load.btf_gen_optimized = true;		break;
>>  	case ARGP_skip_encoding_btf_inconsistent_proto:
>>  		conf_load.skip_encoding_btf_inconsistent_proto = true; break;
>> +	case ARGP_btf_features:
>> +		parse_btf_features(arg);		break;
>>  	default:
>>  		return ARGP_ERR_UNKNOWN;
>>  	}
> 
> ---
> 
> diff --git a/pahole.c b/pahole.c
> index e308dd1..b9bf395 100644
> --- a/pahole.c
> +++ b/pahole.c
> @@ -1280,7 +1280,6 @@ struct btf_feature {
>  	BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false),
>  };
>  
> -#define BTF_MAX_FEATURES	32
>  #define BTF_MAX_FEATURE_STR	1024
>  
>  bool set_btf_features_defaults;
> @@ -1338,10 +1337,10 @@ static void show_supported_btf_features(FILE *output)
>   */
>  static void parse_btf_features(const char *features, bool strict)
>  {
> -	char *feature_list[BTF_MAX_FEATURES] = {};
> -	char *saveptr = NULL, *s, *t;
> +	char *saveptr = NULL, *s, *requested;
>  	char f[BTF_MAX_FEATURE_STR];
> -	int i, n = 0;
> +	struct btf_feature *feature;
> +	int i;
>  
>  	init_btf_features();
>  
> @@ -1353,24 +1352,19 @@ static void parse_btf_features(const char *features, bool strict)
>  
>  	strncpy(f, features, sizeof(f));
>  	s = f;
> -	while ((t = strtok_r(s, ",", &saveptr)) != NULL && n < BTF_MAX_FEATURES) {
> +	while ((requested = strtok_r(s, ",", &saveptr)) != NULL) {
>  		s = NULL;
> -		feature_list[n++] = t;
> -	}
> -
> -	for (i = 0; i < n; i++) {
> -		struct btf_feature *feature = find_btf_feature(feature_list[i]);
> -
> +		feature = find_btf_feature(requested);
>  		if (!feature) {
>  			if (strict) {
>  				fprintf(stderr, "Feature '%s' in '%s' is not supported.  Supported BTF features are:\n",
> -					feature_list[i], features);
> +					requested, features);
>  				show_supported_btf_features(stderr);
>  				exit(EXIT_FAILURE);
>  			}
>  			if (global_verbose)
>  				fprintf(stderr, "Ignoring unsupported feature '%s'\n",
> -					feature_list[i]);
> +					requested);
>  		} else {
>  			enable_btf_feature(feature);
>  		}
> 
>
diff mbox series

Patch

diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
index c1b48de..a09885f 100644
--- a/man-pages/pahole.1
+++ b/man-pages/pahole.1
@@ -273,6 +273,30 @@  Generate BTF for functions with optimization-related suffixes (.isra, .constprop
 .B \-\-btf_gen_all
 Allow using all the BTF features supported by pahole.
 
+.TP
+.B \-\-btf_features=FEATURE_LIST
+Encode BTF using the specified feature list, or specify 'all' for all features supported.  This option can be used as an alternative to unsing multiple BTF-related options. Supported features are
+
+.nf
+	encode_force       Ignore invalid symbols when encoding BTF; for example
+	                   if a symbol has an invalid name, it will be ignored
+	                   and BTF encoding will continue.
+	var                Encode variables using BTF_KIND_VAR in BTF.
+	float              Encode floating-point types in BTF.
+	decl_tag           Encode declaration tags using BTF_KIND_DECL_TAG.
+	type_tag           Encode type tags using BTF_KIND_TYPE_TAG.
+	enum64             Encode enum64 values with BTF_KIND_ENUM64.
+	optimized_func     Encode representations of optimized functions
+	                   with suffixes like ".isra.0".
+	consistent_func    Avoid encoding inconsistent static functions.
+	                   These occur when a parameter is optimized out
+	                   in some CUs and not others, or when the same
+	                   function name has inconsistent BTF descriptions
+	                   in different CUs.
+.fi
+
+So for example, specifying \-\-btf_encode=var,enum64 will result in a BTF encoding that (as well as encoding basic BTF information) will contain variables and enum64 values.
+
 .TP
 .B \-l, \-\-show_first_biggest_size_base_type_member
 Show first biggest size base_type member.
diff --git a/pahole.c b/pahole.c
index 7a41dc3..0e889cf 100644
--- a/pahole.c
+++ b/pahole.c
@@ -1229,6 +1229,133 @@  ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
 #define ARGP_skip_emitting_atomic_typedefs 338
 #define ARGP_btf_gen_optimized  339
 #define ARGP_skip_encoding_btf_inconsistent_proto 340
+#define ARGP_btf_features	341
+
+/* --btf_features=feature1[,feature2,..] allows us to specify
+ * a list of requested BTF features or "all" to enable all features.
+ * These are translated into the appropriate conf_load values via a
+ * struct btf_feature which specifies the associated conf_load
+ * boolean field and whether its default (representing the feature being
+ * off) is false or true.
+ *
+ * btf_features is for opting _into_ features so for a case like
+ * conf_load->btf_gen_floats, the translation is simple; the presence
+ * of the "float" feature in --btf_features sets conf_load->btf_gen_floats
+ * to true.
+ *
+ * The more confusing case is for features that are enabled unless
+ * skipping them is specified; for example
+ * conf_load->skip_encoding_btf_type_tag.  By default - to support
+ * the opt-in model of only enabling features the user asks for -
+ * conf_load->skip_encoding_btf_type_tag is set to true (meaning no
+ * type_tags) and it is only set to false if --btf_features contains
+ * the "type_tag" keyword.
+ *
+ * So from the user perspective, all features specified via
+ * --btf_features are enabled, and if a feature is not specified,
+ * it is disabled.
+ *
+ * If --btf_features is not used, the usual pahole defaults for
+ * BTF encoding apply; we encode type/decl tags, do not encode
+ * floats, etc.  This ensures backwards compatibility.
+ */
+#define BTF_FEATURE(name, alias, default_value)			\
+	{ #name, #alias, &conf_load.alias, default_value }
+
+struct btf_feature {
+	const char      *name;
+	const char      *option_alias;
+	bool		*conf_value;
+	bool		default_value;
+} btf_features[] = {
+	BTF_FEATURE(encode_force, btf_encode_force, false),
+	BTF_FEATURE(var, skip_encoding_btf_vars, true),
+	BTF_FEATURE(float, btf_gen_floats, false),
+	BTF_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true),
+	BTF_FEATURE(type_tag, skip_encoding_btf_type_tag, true),
+	BTF_FEATURE(enum64, skip_encoding_btf_enum64, true),
+	BTF_FEATURE(optimized_func, btf_gen_optimized, false),
+	BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false),
+};
+
+#define BTF_MAX_FEATURES	32
+#define BTF_MAX_FEATURE_STR	1024
+
+bool set_btf_features_defaults;
+
+static void init_btf_features(void)
+{
+	int i;
+
+	/* Only set default values once, as multiple --btf_features=
+	 * may be specified on command-line, and setting defaults
+	 * again could clobber values.   The aim is to enable
+	 * all features set across all --btf_features options.
+	 */
+	if (set_btf_features_defaults)
+		return;
+	for (i = 0; i < ARRAY_SIZE(btf_features); i++)
+		*btf_features[i].conf_value = btf_features[i].default_value;
+	set_btf_features_defaults = true;
+}
+
+static struct btf_feature *find_btf_feature(char *name)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(btf_features); i++) {
+		if (strcmp(name, btf_features[i].name) == 0)
+			return &btf_features[i];
+	}
+	return NULL;
+}
+
+static void enable_btf_feature(struct btf_feature *feature)
+{
+	/* switch "default-off" features on, and "default-on" features
+	 * off; i.e. negate the default value.
+	 */
+	*feature->conf_value = !feature->default_value;
+}
+
+/* Translate --btf_features=feature1[,feature2] into conf_load values.
+ * Explicitly ignores unrecognized features to allow future specification
+ * of new opt-in features.
+ */
+static void parse_btf_features(const char *features)
+{
+	char *feature_list[BTF_MAX_FEATURES] = {};
+	char *saveptr = NULL, *s, *t;
+	char f[BTF_MAX_FEATURE_STR];
+	int i, n = 0;
+
+	init_btf_features();
+
+	if (strcmp(features, "all") == 0) {
+		for (i = 0; i < ARRAY_SIZE(btf_features); i++)
+			enable_btf_feature(&btf_features[i]);
+		return;
+	}
+
+	strncpy(f, features, sizeof(f));
+	s = f;
+	while ((t = strtok_r(s, ",", &saveptr)) != NULL && n < BTF_MAX_FEATURES) {
+		s = NULL;
+		feature_list[n++] = t;
+	}
+
+	for (i = 0; i < n; i++) {
+		struct btf_feature *feature = find_btf_feature(feature_list[i]);
+
+		if (!feature) {
+			if (global_verbose)
+				fprintf(stderr, "Ignoring unsupported feature '%s'\n",
+					feature_list[i]);
+		} else {
+			enable_btf_feature(feature);
+		}
+	}
+}
 
 static const struct argp_option pahole__options[] = {
 	{
@@ -1651,6 +1778,12 @@  static const struct argp_option pahole__options[] = {
 		.key = ARGP_skip_encoding_btf_inconsistent_proto,
 		.doc = "Skip functions that have multiple inconsistent function prototypes sharing the same name, or that use unexpected registers for parameter values."
 	},
+	{
+		.name = "btf_features",
+		.key = ARGP_btf_features,
+		.arg = "FEATURE_LIST",
+		.doc = "Specify supported BTF features in FEATURE_LIST or 'all' for all supported features. See the pahole manual page for the list of supported features."
+	},
 	{
 		.name = NULL,
 	}
@@ -1796,7 +1929,7 @@  static error_t pahole__options_parser(int key, char *arg,
 	case ARGP_btf_gen_floats:
 		conf_load.btf_gen_floats = true;	break;
 	case ARGP_btf_gen_all:
-		conf_load.btf_gen_floats = true;	break;
+		parse_btf_features("all");		break;
 	case ARGP_with_flexible_array:
 		show_with_flexible_array = true;	break;
 	case ARGP_prettify_input_filename:
@@ -1826,6 +1959,8 @@  static error_t pahole__options_parser(int key, char *arg,
 		conf_load.btf_gen_optimized = true;		break;
 	case ARGP_skip_encoding_btf_inconsistent_proto:
 		conf_load.skip_encoding_btf_inconsistent_proto = true; break;
+	case ARGP_btf_features:
+		parse_btf_features(arg);		break;
 	default:
 		return ARGP_ERR_UNKNOWN;
 	}