On Fri, Oct 04, 2024 at 05:01:46PM -0700, Stephen Brennan wrote:
> We only want to consider PROGBITS and NOBITS. However, when refactoring
> this function for clarity, I managed to miss flip this condition. The
> result is fewer variables output, and bad section names used for the
> ones that are emitted.
>
> Fixes: bf2eedb ("btf_encoder: Stop indexing symbols for VARs")
>
> Signed-off-by: Stephen Brennan <stephen.s.brennan@xxxxxxxxxx>
> ---
>
> Hi Arnaldo,
>
> This clearly slipped by me in my last small edit based on Alan's feedback, and I
> didn't run a full enough validation test after the last tweak since it was "just
> some small nits".
>
> (His code review suggestion was not buggy... I introduced it as I shoddily
> redid his suggestion).
>
> Sorry for the bug introduced at the last second - feel free to fold this into
> the commit or keep the commit as a monument to the bug :)
Nope, as it was just in the next branch, I folded it into the fixed
commit and kept just a lore link to this fixup, all is back in next now
and I'm redoing tests here.
Thanks for realizing the mistake and providing a fix, and thanks to Jiri
and Alan for reviewing the fix.
- Arnaldo
> Thanks,
> Stephen
>
> btf_encoder.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 201a48c..5954238 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -2137,8 +2137,8 @@ static size_t get_elf_section(struct btf_encoder *encoder, uint64_t addr)
> /* Start at index 1 to ignore initial SHT_NULL section */
> for (size_t i = 1; i < encoder->seccnt; i++) {
> /* Variables are only present in PROGBITS or NOBITS (.bss) */
> - if (encoder->secinfo[i].type == SHT_PROGBITS ||
> - encoder->secinfo[i].type == SHT_NOBITS)
> + if (!(encoder->secinfo[i].type == SHT_PROGBITS ||
> + encoder->secinfo[i].type == SHT_NOBITS))
> continue;
>
> if (encoder->secinfo[i].addr <= addr &&
> --
> 2.43.5
