100% agree. Will work on a test case soon (maybe next week).
On Tue, Jun 6, 2017 at 9:01 PM, Brenden Blanco <bbla...@gmail.com> wrote:
> 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
