On Thu, Sep 17, 2026 at 10:39:11AM +1000, Kugan Vivekanandarajah wrote:
> +int
> +popcount64 (uint64_t x)
> +{
> + return __builtin_popcountll (x);
> +}
So this gives up building with other host compilers?
I guess it's ok in this case, but perhaps need explicit documentation
and maybe configure checks. Or have a open coded fallback, since
it could be easily fixed for this case.
> +
> +bool
> +bytes_available (const uint8_t *ptr, const uint8_t *end,
> + uint64_t bytes) noexcept
> +{
> + return ptr <= end && bytes <= static_cast<uint64_t> (end - ptr);
> +}
Something like that should have been used elsewhere too instead of
all the open coded boundary checks.
> +
> +/* Consume PERF_SAMPLE_READ, whose size depends on read format RF. */
> +bool
> +skip_sample_read (const uint8_t *&ptr, const uint8_t *end, uint64_t rf)
> +{
> + if (!bytes_available (ptr, end, sizeof (uint64_t)))
> + return false;
> + const uint64_t first = read_u64 (ptr);
> + ptr += sizeof (uint64_t);
> + if (rf & perf_read_format::TOTAL_TIME_ENABLED)
> + {
> + if (!bytes_available (ptr, end, sizeof (uint64_t)))
> + return false;
Indentation or tab issues again, also elsewhere.
> + ptr += sizeof (uint64_t);
> + }
BTW if you really wanted to do weighting (I'm still not sure why)
you would need to parse these two fields too because they determine
the true wall clock weight when event multiplexing happens.
> + if (rf & perf_read_format::TOTAL_TIME_RUNNING)
> + {
> + if (!bytes_available (ptr, end, sizeof (uint64_t)))
> + return false;
> + ptr += sizeof (uint64_t);
> + }
> + ptr += sizeof (uint64_t);
> + }
> + if (sample_ty & perf_sample_type::CALLCHAIN)
> + {
> + if (!bytes_available (ptr, end, sizeof (uint64_t)))
> + return false;
> + uint64_t nr = read_u64 (ptr);
> + ptr += sizeof (uint64_t);
> + if (nr > SIZE_MAX / sizeof (uint64_t))
> + return false;
> + const size_t sz = static_cast<size_t> (nr) * sizeof (uint64_t);
> + if (!bytes_available (ptr, end, sz))
> + return false;
> + ptr += sz;
> + }
At some future point should probably process that one too, I know LLVM
makes use of call chains.
> + if (sample_ty & perf_sample_type::REGS_USER)
> + {
> + if (!bytes_available (ptr, end, sizeof (uint64_t)))
> + return false;
> + uint64_t abi = read_u64 (ptr);
> + ptr += sizeof (uint64_t);
> + if (abi != 0)
> + {
> + const size_t rsz = static_cast<size_t> (popcount64 (sample_regs_user))
> + * sizeof (uint64_t);
> + if (!bytes_available (ptr, end, rsz))
> + return false;
> + ptr += rsz;
> + }
> + }
Dito here, this could feed some of the instrumented gcov features
that autofdo is missing currently (but would need a whole
different dwarf parsing machinery)
> + if (sample_ty & perf_sample_type::DATA_PAGE_SIZE)
> + {
> + if (!bytes_available (ptr, end, sizeof (uint64_t)))
> + return false;
> + ptr += sizeof (uint64_t);
> + }
I think you reject all the options elsewhere anyways, having
all of this here is somewhat overkill. But I suppose it cannot
hurt. perf has a tendency to add new ones occasionally, so
an exhaustive list will probably not work well anyways.
