On Wed, Jun 15, 2022 at 4:03 PM Yonghong Song <yhs@xxxxxx> wrote:
BTF_KIND_ENUM64 is supported with latest libbpf, which
supports 64-bit enum values. Latest libbpf also supports
signedness for enum values. Add enum64 support in
dwarf-to-btf conversion.
The following is an example of new encoding which covers
signed/unsigned enum64/enum variations.
$cat t.c
enum { /* signed, enum64 */
A = -1,
B = 0xffffffff,
} g1;
enum { /* unsigned, enum64 */
C = 1,
D = 0xfffffffff,
} g2;
enum { /* signed, enum */
E = -1,
F = 0xfffffff,
} g3;
enum { /* unsigned, enum */
G = 1,
H = 0xfffffff,
} g4;
$ clang -g -c t.c
$ pahole -JV t.o
btf_encoder__new: 't.o' doesn't have '.data..percpu' section
Found 0 per-CPU variables!
File t.o:
[1] ENUM64 (anon) size=8
A val=-1
B val=4294967295
[2] INT long size=8 nr_bits=64 encoding=SIGNED
[3] ENUM64 (anon) size=8
C val=1
D val=68719476735
[4] INT unsigned long size=8 nr_bits=64 encoding=(none)
[5] ENUM (anon) size=4
E val=-1
F val=268435455
[6] INT int size=4 nr_bits=32 encoding=SIGNED
[7] ENUM (anon) size=4
G val=1
H val=268435455
[8] INT unsigned int size=4 nr_bits=32 encoding=(none)
With the flag to skip enum64 encoding,
$ pahole -JV t.o --skip_encoding_btf_enum64
btf_encoder__new: 't.o' doesn't have '.data..percpu' section
Found 0 per-CPU variables!
File t.o:
[1] ENUM (anon) size=8
A val=4294967295
B val=4294967295
[2] INT long size=8 nr_bits=64 encoding=SIGNED
[3] ENUM (anon) size=8
C val=1
D val=4294967295
[4] INT unsigned long size=8 nr_bits=64 encoding=(none)
[5] ENUM (anon) size=4
E val=4294967295
F val=268435455
[6] INT int size=4 nr_bits=32 encoding=SIGNED
[7] ENUM (anon) size=4
G val=1
H val=268435455
[8] INT unsigned int size=4 nr_bits=32 encoding=(none)
In the above btf encoding without enum64, all enum types
with the same type size as the corresponding enum64. All these
enum types have unsigned type (kflag = 0) which is required
before kernel enum64 support.
Signed-off-by: Yonghong Song <yhs@xxxxxx>
---
btf_encoder.c | 65 +++++++++++++++++++++++++++++++++++------------
btf_encoder.h | 2 +-
dwarf_loader.c | 12 +++++++++
dwarves.h | 4 ++-
dwarves_fprintf.c | 6 ++++-
pahole.c | 10 +++++++-
6 files changed, 79 insertions(+), 20 deletions(-)
Sorry for late review, I don't always catch up on emails from older
emails first :(
[...]
size = BITS_ROUNDUP_BYTES(bit_size);
- id = btf__add_enum(btf, name, size);
+ is_enum32 = size <= 4 || no_enum64;
+ if (is_enum32)
+ id = btf__add_enum(btf, name, size);
+ else
+ id = btf__add_enum64(btf, name, size, is_signed);
if (id > 0) {
t = btf__type_by_id(btf, id);
btf_encoder__log_type(encoder, t, false, true, "size=%u", t->size);
} else {
- btf__log_err(btf, BTF_KIND_ENUM, name, true,
+ btf__log_err(btf, is_enum32 ? BTF_KIND_ENUM : BTF_KIND_ENUM64, name, true,
"size=%u Error emitting BTF type", size);
}
return id;
}
-static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int32_t value)
+static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *name, int64_t value,
+ bool is_signed, bool is_enum64, bool no_enum64)
It was quite confusing to see "is_enum64" and "no_enum64" as arguments
to the same function :)
I'll let Arnaldo decide for himself, but I think it would be cleaner
to pass such configuration switches as fields in struct btf_encoder
itself and just check such flags from relevant btf_encoder__add_xxx()
functions. Such flags are global by nature, so it seems fitting.
But other than that looks good to me.
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
{
- int err = btf__add_enum_value(encoder->btf, name, value);
+ const char *fmt_str;
+ int err;
+
+ /* If enum64 is not allowed, generate enum32 with unsigned int value. In enum64-supported
+ * libbpf library, btf__add_enum_value() will set the kflag (sign bit) in common_type
+ * if the value is negative.
+ */
+ if (no_enum64)
+ err = btf__add_enum_value(encoder->btf, name, (uint32_t)value);
+ else if (is_enum64)
+ err = btf__add_enum64_value(encoder->btf, name, value);
+ else
+ err = btf__add_enum_value(encoder->btf, name, value);
[...]
