On Mon, 19 Jul 2021, Andrii Nakryiko wrote:
On Mon, Jul 19, 2021 at 2:41 PM Alan Maguire alan.maguire@oracle.com wrote:
__int128 is not supported for some 32-bit platforms (arm and i386). __int128 was used in carrying out computations on bitfields which aid display, but the same calculations could be done with __u64 with the small effect of not supporting 128-bit bitfields.
With these changes, a big-endian issue with casting 128-bit integers to 64-bit for enum bitfields is solved also, as we now use 64-bit integers for bitfield calculations.
Reported-by: Naresh Kamboju naresh.kamboju@linaro.org Reported-by: Linux Kernel Functional Testing lkft@linaro.org Signed-off-by: Alan Maguire alan.maguire@oracle.com
Changes look good to me, thanks. But they didn't appear in patchworks yet so I can't easily test and apply them. It might be because of patchworks delay or due to a very long CC list. Try trimming the cc list down and re-submit?
Done, looks like the v2 with the trimmed cc list made it into patchwork this time.
Also, while I agree that supporting 128-bit bitfields isn't important, I wonder if we should warn/error on that (instead of shifting by negative amount and reporting some garbage value), what do you think? Is there one place in the code where we can error out early if the type actually has bitfield with > 64 bits? I'd prefer to keep btf_dump_bitfield_get_data() itself non-failing though.
Sorry, I missed the last part and made that function fail since it's probably the easiest place to capture too-large bitfields. I renamed it to btf_dump_get_bitfield_value() to match btf_dump_get_enum_value() which as a similar function signature (return int, pass in a pointer to the value we want to retrieve).
We can't localize bitfield size checking to btf_dump_type_data_check_zero() because - depending on flags - the associated checks might not be carried out. So duplication of bitfield size checks between the zero checking and bitfield/enum bitfield display seems inevitable, and that being the case, the extra error checking required around btf_dump_get_bitfield_value() seems to be required.
I might be missing a better approach here of course; let me know what you think. Thanks again!
Alan