On Wed, Jan 16, 2019 at 09:17:33PM +0100, Anton Lindqvist wrote:
> + idx = kd->kd_buf[0];
> + if (idx * 4 + 4 + 1 >= kd->kd_nmemb)
> + return;
> +
> + kd->kd_buf[idx * 4 + 1] = type;
> + kd->kd_buf[idx * 4 + 2] = arg1;
> + kd->kd_buf[idx * 4 + 3] = arg2;
> + kd->kd_buf[idx * 4 + 4] = pc;
> + kd->kd_buf[0] = idx + 1;
I think this boundary calculation is not correct. My reasoning is:
- You allocate nmemb * sizeof(uintptr_t) bytes.
- kd_buf[nmemb - 1] is the largest valid index.
- kd_nmemb is nmemb - 1.
- kd_buf[idx * 4 + 4] is the access at the largest position.
- So return if (idx * 4 + 4 > kd_nmemb).
In __sanitizer_cov_trace_pc() you have the logic the other way around.
idx = kd->kd_buf[0];
if (idx < kd->kd_nmemb) {
kd->kd_buf[idx + 1] = (uintptr_t)__builtin_return_address(0);
kd->kd_buf[0] = idx + 1;
}
And it is not equivalent to the calculation in trace_cmp().
Could you use the same style in both functions? I would prefer
this if you think my suggestion is correct.
trace_cmp():
if (idx * 4 + 4 > kd->kd_nmemb)
return;
__sanitizer_cov_trace_pc():
if (idx + 1 > kd->kd_nmemb)
return;
bluhm
