On Tue, Feb 28, 2023 at 12:24 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > Recent changes to handle unspecified types (see [1]) cause BTF ID drift. > > Specifically, the intent of commits [2], [3] and [4] is to render > references to unspecified types as void type. > However, as a consequence: > - in `die__process_unit()` call to `cu__add_tag()` allocates `small_id` > for unspecified type tags and adds these tags to `cu->types_table`; > - `btf_encoder__encode_tag()` skips generation of BTF entries for > `DW_TAG_unspecified_type` tags. > > Such logic causes ID drift if unspecified type is not the last type > processed for compilation unit. `small_id` of each type following > unspecified type in the `cu->types_table` would have its BTF id off by -1. > Thus, rendering references established on recode phase invalid. > > This commit reverts `unspecified_type` id/tag tracking. > Instead, the following is done: > - `small_id` for unspecified type tags is set to 0, thus reference to > unspecified type tag would render BTF id of a `void` on recode phase; > - unspecified type tags are not added to `cu->types_table`. > > This change also happens to fix issue reported in [5], the gist of > that issue is that the field `encoder->unspecified_type` is set but > not reset by function `btf_encoder__encode_cu()`. Thus, the following > sequence of events might occur when BTF encoding is requested: > - CU with unspecified type is processed: > - unspecified type id is 42 > - encoder->unspecified_type is set to 42 > - CU without unspecified type is processed next using the same > `encoder` object: > - some `struct foo` has id 42 in this CU > - the references to `struct foo` are set 0 by function > `btf_encoder__tag_type()`. > > [1] https://lore.kernel.org/all/Y0R7uu3s%2FimnvPzM@xxxxxxxxxx/ > [2] bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void") > [3] cffe5e1f75e1 ("core: Record if a CU has a DW_TAG_unspecified_type") > [4] 75e0fe28bb02 ("core: Add DW_TAG_unspecified_type to tag__is_tag_type() set") > [5] https://lore.kernel.org/bpf/Y%2FP1yxAuV6Wj3A0K@xxxxxxxxxx/ > > Fixes: bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void") > Fixes: 52b25808e44a ("btf_encoder: Store type_id_off, unspecified type in encoder") > Tested-by: KP Singh <kpsingh@xxxxxxxxxx> > Reported-by: Matt Bobrowski <mattbobrowski@xxxxxxxxxx> > Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx> > --- Looks like a sensible approach to me. Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > btf_encoder.c | 8 -------- > dwarf_loader.c | 25 +++++++++++++++++++------ > dwarves.h | 8 -------- > 3 files changed, 19 insertions(+), 22 deletions(-) > [...]
