Also, I would suggest that after that fix, we also update the test to
include actually testing for proper values. Probably you can send the
values over the perf ring buffer and assert at the end that all of the
values are collected properly.
On Tue, Jun 6, 2017 at 8:59 PM, Brenden Blanco <bbla...@gmail.com> wrote:
> Yonghong, can you send a PR for your branch?
>
> On Tue, Jun 6, 2017 at 12:20 AM, Y Song via iovisor-dev <
> iovisor-dev@lists.iovisor.org> wrote:
>
>> Hi, Tetsuo,
>>
>> You are right. The bug is actually introduced by my last patch. I just
>> focused one aspect of issue and inadvertently introduced another
>> *blocker* bug.
>>
>> I just pushed a patch. Thanks a lot for reporting the issue and
>> testing out the fix.
>>
>> Yonghong
>>
>> On Mon, Jun 5, 2017 at 5:25 PM, Tetsuo Handa
>> <penguin-ker...@i-love.sakura.ne.jp> wrote:
>> > Hello.
>> >
>> > I changed the hook as below and confirmed that cp == NULL at
>> bpf_probe_read().
>> > That is, it is bpf_usdt_readarg() which got broken.
>> >
>> > ----------
>> > int do_start(struct pt_regs *ctx) {
>> > char *cp = NULL;
>> > bpf_usdt_readarg(1, ctx, &cp);
>> > struct { char query[QUERY_MAX]; } data = { };
>> > if (cp != NULL)
>> > bpf_probe_read(&data.query, sizeof(data.query), (void *) cp);
>> > else
>> > data.query[0] = '!';
>> > events.perf_submit(ctx, &data, sizeof(data));
>> > return 0;
>> > };
>> > ----------
>> >
>> > If I do
>> >
>> > - std::string cptr = tfm::format("*((volatile %s *)dest)", ctype);
>> > + std::string cptr = tfm::format("*((%s *)dest)", ctype);
>> >
>> > (i.e. drop only "volatile " part) in that commit like below
>> >
>> > ----------
>> > diff --git a/src/cc/usdt.cc b/src/cc/usdt.cc
>> > --- a/src/cc/usdt.cc
>> > +++ b/src/cc/usdt.cc
>> > @@ -152,7 +152,7 @@ bool Probe::usdt_getarg(std::ostream &stream) {
>> >
>> > for (size_t arg_n = 0; arg_n < arg_count; ++arg_n) {
>> > std::string ctype = largest_arg_type(arg_n);
>> > - std::string cptr("dest");
>> > + std::string cptr = tfm::format("*((%s *)dest)", ctype);
>> >
>> > tfm::format(stream,
>> > "static __always_inline int _bpf_readarg_%s_%d("
>> > ----------
>> >
>> > USDT probe works again as well as build warning shown below goes away.
>> >
>> > ----------
>> > /virtual/main.c:4:8: warning: incompatible integer to pointer
>> conversion assigning to 'void *' from 'volatile uint64_t' (aka 'volatile
>> unsigned long long') [-Wint-conversion]
>> > dest = *(volatile uint64_t *)&ctx->bx;
>> > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> > ----------
>> _______________________________________________
>> iovisor-dev mailing list
>> iovisor-dev@lists.iovisor.org
>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>>
>
>
_______________________________________________
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev
