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@xxxxxxxxxx>
> Suggested-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
> Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> ---
> 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);
}
