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 <bblanco@...> 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@...> 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-kernel@...> 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@...
https://lists.iovisor.org/mailman/listinfo/iovisor-dev