Re: [PATCH v2 09/10] tracing/probes: Add fprobe events for tracing function entry and exit.

From: Google
Date: Wed Feb 01 2023 - 07:43:01 EST

On Wed, 1 Feb 2023 15:27:49 +0800
kernel test robot <lkp@xxxxxxxxx> wrote:

> Hi Masami,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on bpf-next/master]
> [also build test WARNING on bpf/master shuah-kselftest/next shuah-kselftest/fixes linus/master v6.2-rc6]
> [cannot apply to rostedt-trace/for-next rostedt-trace/for-next-urgent]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Masami-Hiramatsu-Google/fprobe-Pass-entry_data-to-handlers/20230201-001911
> base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
> patch link: https://lore.kernel.org/r/167518178446.336834.4654027409699647726.stgit%40mhiramat.roam.corp.google.com
> patch subject: [PATCH v2 09/10] tracing/probes: Add fprobe events for tracing function entry and exit.
> config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20230201/202302011530.7vm4O8Ro-lkp@xxxxxxxxx/config)
> compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/90de2b114484f12e2645d2beb964b7d230c9c705
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Masami-Hiramatsu-Google/fprobe-Pass-entry_data-to-handlers/20230201-001911
> git checkout 90de2b114484f12e2645d2beb964b7d230c9c705
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash kernel/trace/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@xxxxxxxxx>
>
> All warnings (new ones prefixed by >>):
>
> >> kernel/trace/trace_probe.c:394:32: warning: use of logical '||' with constant operand [-Wconstant-logical-operand]
> if (flags & (TPARG_FL_TPOINT || TPARG_FL_FPROBE)) {
> ^ ~~~~~~~~~~~~~~~
> kernel/trace/trace_probe.c:394:32: note: use '|' for a bitwise operation
> if (flags & (TPARG_FL_TPOINT || TPARG_FL_FPROBE)) {
> ^~
> |

Oops, thanks! this is an actual bug.

Let me fix it and update.

Thanks!

> 1 warning generated.
>
>
> vim +394 kernel/trace/trace_probe.c
>
> 374
> 375 /* Recursive argument parser */
> 376 static int
> 377 parse_probe_arg(char *arg, const struct fetch_type *type,
> 378 struct fetch_insn **pcode, struct fetch_insn *end,
> 379 unsigned int flags, int offs)
> 380 {
> 381 struct fetch_insn *code = *pcode;
> 382 unsigned long param;
> 383 int deref = FETCH_OP_DEREF;
> 384 long offset = 0;
> 385 char *tmp;
> 386 int ret = 0;
> 387
> 388 switch (arg[0]) {
> 389 case '$':
> 390 ret = parse_probe_vars(arg + 1, type, code, flags, offs);
> 391 break;
> 392
> 393 case '%': /* named register */
> > 394 if (flags & (TPARG_FL_TPOINT || TPARG_FL_FPROBE)) {
> 395 /* eprobe and fprobe do not handle registers */
> 396 trace_probe_log_err(offs, BAD_VAR);
> 397 break;
> 398 }
> 399 ret = regs_query_register_offset(arg + 1);
> 400 if (ret >= 0) {
> 401 code->op = FETCH_OP_REG;
> 402 code->param = (unsigned int)ret;
> 403 ret = 0;
> 404 } else
> 405 trace_probe_log_err(offs, BAD_REG_NAME);
> 406 break;
> 407
> 408 case '@': /* memory, file-offset or symbol */
> 409 if (isdigit(arg[1])) {
> 410 ret = kstrtoul(arg + 1, 0, &param);
> 411 if (ret) {
> 412 trace_probe_log_err(offs, BAD_MEM_ADDR);
> 413 break;
> 414 }
> 415 /* load address */
> 416 code->op = FETCH_OP_IMM;
> 417 code->immediate = param;
> 418 } else if (arg[1] == '+') {
> 419 /* kprobes don't support file offsets */
> 420 if (flags & TPARG_FL_KERNEL) {
> 421 trace_probe_log_err(offs, FILE_ON_KPROBE);
> 422 return -EINVAL;
> 423 }
> 424 ret = kstrtol(arg + 2, 0, &offset);
> 425 if (ret) {
> 426 trace_probe_log_err(offs, BAD_FILE_OFFS);
> 427 break;
> 428 }
> 429
> 430 code->op = FETCH_OP_FOFFS;
> 431 code->immediate = (unsigned long)offset; // imm64?
> 432 } else {
> 433 /* uprobes don't support symbols */
> 434 if (!(flags & TPARG_FL_KERNEL)) {
> 435 trace_probe_log_err(offs, SYM_ON_UPROBE);
> 436 return -EINVAL;
> 437 }
> 438 /* Preserve symbol for updating */
> 439 code->op = FETCH_NOP_SYMBOL;
> 440 code->data = kstrdup(arg + 1, GFP_KERNEL);
> 441 if (!code->data)
> 442 return -ENOMEM;
> 443 if (++code == end) {
> 444 trace_probe_log_err(offs, TOO_MANY_OPS);
> 445 return -EINVAL;
> 446 }
> 447 code->op = FETCH_OP_IMM;
> 448 code->immediate = 0;
> 449 }
> 450 /* These are fetching from memory */
> 451 if (++code == end) {
> 452 trace_probe_log_err(offs, TOO_MANY_OPS);
> 453 return -EINVAL;
> 454 }
> 455 *pcode = code;
> 456 code->op = FETCH_OP_DEREF;
> 457 code->offset = offset;
> 458 break;
> 459
> 460 case '+': /* deref memory */
> 461 case '-':
> 462 if (arg[1] == 'u') {
> 463 deref = FETCH_OP_UDEREF;
> 464 arg[1] = arg[0];
> 465 arg++;
> 466 }
> 467 if (arg[0] == '+')
> 468 arg++; /* Skip '+', because kstrtol() rejects it. */
> 469 tmp = strchr(arg, '(');
> 470 if (!tmp) {
> 471 trace_probe_log_err(offs, DEREF_NEED_BRACE);
> 472 return -EINVAL;
> 473 }
> 474 *tmp = '\0';
> 475 ret = kstrtol(arg, 0, &offset);
> 476 if (ret) {
> 477 trace_probe_log_err(offs, BAD_DEREF_OFFS);
> 478 break;
> 479 }
> 480 offs += (tmp + 1 - arg) + (arg[0] != '-' ? 1 : 0);
> 481 arg = tmp + 1;
> 482 tmp = strrchr(arg, ')');
> 483 if (!tmp) {
> 484 trace_probe_log_err(offs + strlen(arg),
> 485 DEREF_OPEN_BRACE);
> 486 return -EINVAL;
> 487 } else {
> 488 const struct fetch_type *t2 = find_fetch_type(NULL, flags);
> 489
> 490 *tmp = '\0';
> 491 ret = parse_probe_arg(arg, t2, &code, end, flags, offs);
> 492 if (ret)
> 493 break;
> 494 if (code->op == FETCH_OP_COMM ||
> 495 code->op == FETCH_OP_DATA) {
> 496 trace_probe_log_err(offs, COMM_CANT_DEREF);
> 497 return -EINVAL;
> 498 }
> 499 if (++code == end) {
> 500 trace_probe_log_err(offs, TOO_MANY_OPS);
> 501 return -EINVAL;
> 502 }
> 503 *pcode = code;
> 504
> 505 code->op = deref;
> 506 code->offset = offset;
> 507 }
> 508 break;
> 509 case '\\': /* Immediate value */
> 510 if (arg[1] == '"') { /* Immediate string */
> 511 ret = __parse_imm_string(arg + 2, &tmp, offs + 2);
> 512 if (ret)
> 513 break;
> 514 code->op = FETCH_OP_DATA;
> 515 code->data = tmp;
> 516 } else {
> 517 ret = str_to_immediate(arg + 1, &code->immediate);
> 518 if (ret)
> 519 trace_probe_log_err(offs + 1, BAD_IMM);
> 520 else
> 521 code->op = FETCH_OP_IMM;
> 522 }
> 523 break;
> 524 }
> 525 if (!ret && code->op == FETCH_OP_NOP) {
> 526 /* Parsed, but do not find fetch method */
> 527 trace_probe_log_err(offs, BAD_FETCH_ARG);
> 528 ret = -EINVAL;
> 529 }
> 530 return ret;
> 531 }
> 532
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests


--
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>