Re: ddb trace: fix output for too many arguments
On Mon, Jul 12, 2021 at 08:18:20PM +0200, Mark Kettenis wrote: > > Date: Mon, 12 Jul 2021 20:11:30 +0200 > > From: Jasper Lievisse Adriaanse > > > > On Sun, Jul 11, 2021 at 03:58:05PM +0200, Jasper Lievisse Adriaanse wrote: > > > Hi, > > > > > > When printing a trace from ddb, some architectures are limited by the > > > number of > > > registers which are used to pass arguments. If the number of arguments to > > > a function > > > exceeded this number, the code in db_stack_trace_print() would print that > > > many arguments > > > without any indication that one or more arguments aren't printed. > > > > > > Here's a diff that tweaks the output to make it clear there were more > > > arguments. > > > Do we want to print ',...' for each ommited argument (like this diff does) > > > or perhaps just a single ',...'? > > > > I think just printing a single instance of ',...' gets the point across. > > OK? > > Actually, since we use -msave-args on amd64 the arguments are saved on > the stack. I think this means there is no limit on the number of > arguments we can print... Good point, there's no reason to cap narg at 6 any longer unless db_ctf_func_numargs() failed. Here's the diff for amd64. Other platforms could benefit from the ",..." approach but those'll be separate diffs. OK? Index: db_trace.c === RCS file: /cvs/src/sys/arch/amd64/amd64/db_trace.c,v retrieving revision 1.53 diff -u -p -r1.53 db_trace.c --- db_trace.c 14 May 2020 06:58:54 - 1.53 +++ db_trace.c 30 Aug 2021 08:44:36 - @@ -164,8 +164,7 @@ db_stack_trace_print(db_expr_t addr, int } } - narg = db_ctf_func_numargs(sym); - if (narg < 0 || narg > 6) + if ((narg = db_ctf_func_numargs(sym)) < 0) narg = 6; if (name == NULL)
Re: ddb trace: fix output for too many arguments
> Date: Mon, 12 Jul 2021 20:11:30 +0200
> From: Jasper Lievisse Adriaanse
>
> On Sun, Jul 11, 2021 at 03:58:05PM +0200, Jasper Lievisse Adriaanse wrote:
> > Hi,
> >
> > When printing a trace from ddb, some architectures are limited by the
> > number of
> > registers which are used to pass arguments. If the number of arguments to a
> > function
> > exceeded this number, the code in db_stack_trace_print() would print that
> > many arguments
> > without any indication that one or more arguments aren't printed.
> >
> > Here's a diff that tweaks the output to make it clear there were more
> > arguments.
> > Do we want to print ',...' for each ommited argument (like this diff does)
> > or perhaps just a single ',...'?
>
> I think just printing a single instance of ',...' gets the point across.
> OK?
Actually, since we use -msave-args on amd64 the arguments are saved on
the stack. I think this means there is no limit on the number of
arguments we can print...
> Index: arch/amd64/amd64/db_trace.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/db_trace.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 db_trace.c
> --- arch/amd64/amd64/db_trace.c 14 May 2020 06:58:54 - 1.53
> +++ arch/amd64/amd64/db_trace.c 12 Jul 2021 18:08:21 -
> @@ -137,7 +137,7 @@ db_stack_trace_print(db_expr_t addr, int
>
> lastframe = 0;
> while (count && frame != 0) {
> - int narg;
> + int narg, extra_args = 0;
> unsigned inti;
> char * name;
> db_expr_t offset;
> @@ -165,8 +165,12 @@ db_stack_trace_print(db_expr_t addr, int
> }
>
> narg = db_ctf_func_numargs(sym);
> - if (narg < 0 || narg > 6)
> + if (narg < 0)
> narg = 6;
> + else if (narg > 6) {
> + narg = 6;
> + extra_args = 1;
> + }
>
> if (name == NULL)
> (*pr)("%lx(", callpc);
> @@ -204,6 +208,10 @@ db_stack_trace_print(db_expr_t addr, int
> if (--narg != 0)
> (*pr)(",");
> }
> +
> + if (extra_args)
> + (*pr)(",...");
> +
> (*pr)(") at ");
> db_printsym(callpc, DB_STGY_PROC, pr);
> (*pr)("\n");
> Index: arch/powerpc/ddb/db_trace.c
> ===
> RCS file: /cvs/src/sys/arch/powerpc/ddb/db_trace.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 db_trace.c
> --- arch/powerpc/ddb/db_trace.c 14 May 2020 06:58:54 - 1.17
> +++ arch/powerpc/ddb/db_trace.c 12 Jul 2021 18:08:21 -
> @@ -123,7 +123,7 @@ db_stack_trace_print(db_expr_t addr, int
> Elf_Sym *sym;
> char*name;
> char c, *cp = modif;
> - int i, narg, trace_proc = 0;
> + int i, narg, trace_proc = 0, extra_args = 0;
>
> while ((c = *cp++) != 0) {
> if (c == 't')
> @@ -158,8 +158,12 @@ db_stack_trace_print(db_expr_t addr, int
> (*pr)("at 0x%lx", lr - 4);
> } else {
> narg = db_ctf_func_numargs(sym);
> - if (narg < 0 || narg > 8)
> + if (narg < 0)
> narg = 8;
> + else if (narg > 8) {
> + narg = 8;
> + extra_args = 1;
> + }
>
> (*pr)("%s(", name);
>
> @@ -172,6 +176,9 @@ db_stack_trace_print(db_expr_t addr, int
> (*pr)(",");
> }
> }
> +
> + if (extra_args)
> + (*pr)(",...");
>
> (*pr)(") at ");
> db_printsym(lr - 4, DB_STGY_PROC, pr);
>
> --
> jasper
>
>
Re: ddb trace: fix output for too many arguments
On Sun, Jul 11, 2021 at 03:58:05PM +0200, Jasper Lievisse Adriaanse wrote:
> Hi,
>
> When printing a trace from ddb, some architectures are limited by the number
> of
> registers which are used to pass arguments. If the number of arguments to a
> function
> exceeded this number, the code in db_stack_trace_print() would print that
> many arguments
> without any indication that one or more arguments aren't printed.
>
> Here's a diff that tweaks the output to make it clear there were more
> arguments.
> Do we want to print ',...' for each ommited argument (like this diff does)
> or perhaps just a single ',...'?
I think just printing a single instance of ',...' gets the point across.
OK?
Index: arch/amd64/amd64/db_trace.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/db_trace.c,v
retrieving revision 1.53
diff -u -p -r1.53 db_trace.c
--- arch/amd64/amd64/db_trace.c 14 May 2020 06:58:54 - 1.53
+++ arch/amd64/amd64/db_trace.c 12 Jul 2021 18:08:21 -
@@ -137,7 +137,7 @@ db_stack_trace_print(db_expr_t addr, int
lastframe = 0;
while (count && frame != 0) {
- int narg;
+ int narg, extra_args = 0;
unsigned inti;
char * name;
db_expr_t offset;
@@ -165,8 +165,12 @@ db_stack_trace_print(db_expr_t addr, int
}
narg = db_ctf_func_numargs(sym);
- if (narg < 0 || narg > 6)
+ if (narg < 0)
narg = 6;
+ else if (narg > 6) {
+ narg = 6;
+ extra_args = 1;
+ }
if (name == NULL)
(*pr)("%lx(", callpc);
@@ -204,6 +208,10 @@ db_stack_trace_print(db_expr_t addr, int
if (--narg != 0)
(*pr)(",");
}
+
+ if (extra_args)
+ (*pr)(",...");
+
(*pr)(") at ");
db_printsym(callpc, DB_STGY_PROC, pr);
(*pr)("\n");
Index: arch/powerpc/ddb/db_trace.c
===
RCS file: /cvs/src/sys/arch/powerpc/ddb/db_trace.c,v
retrieving revision 1.17
diff -u -p -r1.17 db_trace.c
--- arch/powerpc/ddb/db_trace.c 14 May 2020 06:58:54 - 1.17
+++ arch/powerpc/ddb/db_trace.c 12 Jul 2021 18:08:21 -
@@ -123,7 +123,7 @@ db_stack_trace_print(db_expr_t addr, int
Elf_Sym *sym;
char*name;
char c, *cp = modif;
- int i, narg, trace_proc = 0;
+ int i, narg, trace_proc = 0, extra_args = 0;
while ((c = *cp++) != 0) {
if (c == 't')
@@ -158,8 +158,12 @@ db_stack_trace_print(db_expr_t addr, int
(*pr)("at 0x%lx", lr - 4);
} else {
narg = db_ctf_func_numargs(sym);
- if (narg < 0 || narg > 8)
+ if (narg < 0)
narg = 8;
+ else if (narg > 8) {
+ narg = 8;
+ extra_args = 1;
+ }
(*pr)("%s(", name);
@@ -172,6 +176,9 @@ db_stack_trace_print(db_expr_t addr, int
(*pr)(",");
}
}
+
+ if (extra_args)
+ (*pr)(",...");
(*pr)(") at ");
db_printsym(lr - 4, DB_STGY_PROC, pr);
--
jasper
ddb trace: fix output for too many arguments
Hi,
When printing a trace from ddb, some architectures are limited by the number of
registers which are used to pass arguments. If the number of arguments to a
function
exceeded this number, the code in db_stack_trace_print() would print that many
arguments
without any indication that one or more arguments aren't printed.
Here's a diff that tweaks the output to make it clear there were more arguments.
Do we want to print ',...' for each ommited argument (like this diff does)
or perhaps just a single ',...'?
Index: amd64/amd64/db_trace.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/db_trace.c,v
retrieving revision 1.53
diff -u -p -r1.53 db_trace.c
--- amd64/amd64/db_trace.c 14 May 2020 06:58:54 - 1.53
+++ amd64/amd64/db_trace.c 11 Jul 2021 13:23:04 -
@@ -137,7 +136,7 @@ db_stack_trace_print(db_expr_t addr, int
lastframe = 0;
while (count && frame != 0) {
- int narg;
+ int narg, extra_args = 0;
unsigned inti;
char * name;
db_expr_t offset;
@@ -165,8 +164,12 @@ db_stack_trace_print(db_expr_t addr, int
}
narg = db_ctf_func_numargs(sym);
- if (narg < 0 || narg > 6)
+ if (narg < 0)
+ narg = 6;
+ else if (narg > 6) {
+ extra_args = narg % 6;
narg = 6;
+ }
if (name == NULL)
(*pr)("%lx(", callpc);
@@ -204,6 +207,9 @@ db_stack_trace_print(db_expr_t addr, int
if (--narg != 0)
(*pr)(",");
}
+ for (i = extra_args; i > 0; i--)
+ (*pr)(",...");
+
(*pr)(") at ");
db_printsym(callpc, DB_STGY_PROC, pr);
(*pr)("\n");
Index: powerpc/ddb/db_trace.c
===
RCS file: /cvs/src/sys/arch/powerpc/ddb/db_trace.c,v
retrieving revision 1.17
diff -u -p -r1.17 db_trace.c
--- powerpc/ddb/db_trace.c 14 May 2020 06:58:54 - 1.17
+++ powerpc/ddb/db_trace.c 11 Jul 2021 13:23:04 -
@@ -123,7 +123,7 @@ db_stack_trace_print(db_expr_t addr, int
Elf_Sym *sym;
char*name;
char c, *cp = modif;
- int i, narg, trace_proc = 0;
+ int i, narg, trace_proc = 0, extra_args = 0;
while ((c = *cp++) != 0) {
if (c == 't')
@@ -158,8 +158,12 @@ db_stack_trace_print(db_expr_t addr, int
(*pr)("at 0x%lx", lr - 4);
} else {
narg = db_ctf_func_numargs(sym);
- if (narg < 0 || narg > 8)
+ if (narg < 0)
narg = 8;
+ else if (narg > 8) {
+ extra_args = narg % 8;
+ narg = 8;
+ }
(*pr)("%s(", name);
@@ -172,6 +176,9 @@ db_stack_trace_print(db_expr_t addr, int
(*pr)(",");
}
}
+
+ for (i = extra_args; i > 0; i--)
+ (*pr)(",...");
(*pr)(") at ");
db_printsym(lr - 4, DB_STGY_PROC, pr);
--
jasper
