Re: sparc64: clean up db_trace.c

[Mark Kettenis]

> Date: Fri, 9 Sep 2016 20:44:00 -0700
> From: Philip Guenther 
> 
> On Fri, 9 Sep 2016, Philip Guenther wrote:
> > Noticed while looking at Jasper's diff.
> >  - convert declarations from k to standard C
> >  - delete support for 32bit frame backtracing.  I doubt this code has ever 
> >been executed on OpenBSD.  If a 32bit frame is encountered ((sp&1)==0)
> >then print a warning and stop processing the frames
> >  - delete a pile of casts that are unnecessary
> >  - minor whitespace tweaks
> > 
> > build tested
> 
> Testing "tr", "mach stack", "mach window", and "mach tf" showed one 
> reversed test.  Updated diff below.
> 
> ok?

ok kettenis@

> Index: db_trace.c
> ===
> RCS file: /data/src/openbsd/src/sys/arch/sparc64/sparc64/db_trace.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 db_trace.c
> --- db_trace.c9 Feb 2015 09:21:30 -   1.10
> +++ db_trace.c10 Sep 2016 03:36:20 -
> @@ -56,12 +56,8 @@ void db_print_window(u_int64_t);
>  #define ULOAD(x) probeget((paddr_t)(u_long)&(x), ASI_AIUS, sizeof(x))
>  
>  void
> -db_stack_trace_print(addr, have_addr, count, modif, pr)
> - db_expr_t   addr;
> - int have_addr;
> - db_expr_t   count;
> - char*modif;
> - int (*pr)(const char *, ...);
> +db_stack_trace_print(db_expr_t addr, int have_addr, db_expr_t count,
> +char *modif, int (*pr)(const char *, ...))
>  {
>   vaddr_t frame;
>   boolean_t   kernel_only = TRUE;
> @@ -95,28 +91,26 @@ db_stack_trace_print(addr, have_addr, co
>   }
>   }
>  
> + if ((frame & 1) == 0) {
> + db_printf("WARNING: corrupt frame at %lx\n", frame);
> + return;
> + }
> +
>   while (count--) {
>   int i;
>   db_expr_t   offset;
>   char*name;
>   db_addr_t   pc;
>   struct frame64  *f64;
> - struct frame32  *f32;
>  
>   /*
>* Switch to frame that contains arguments
>*/
> - if (frame & 1) {
> - f64 = (struct frame64 *)(frame + BIAS);
> - pc = (db_addr_t)KLOAD(f64->fr_pc);
> - 
> - frame = KLOAD(f64->fr_fp);
> - } else {
> - f32 = (struct frame32 *)(frame);
> - pc = (db_addr_t)KLOAD(f32->fr_pc);
> - 
> - frame = (long)KLOAD(f32->fr_fp);
> - }
> +
> + f64 = (struct frame64 *)(frame + BIAS);
> + pc = (db_addr_t)KLOAD(f64->fr_pc);
> + 
> + frame = KLOAD(f64->fr_fp);
>  
>   if (kernel_only) {
>   if (pc < KERNBASE || pc >= KERNEND)
> @@ -137,22 +131,20 @@ db_stack_trace_print(addr, have_addr, co
>   name = "?";
>   
>   (*pr)("%s(", name);
> +
> + if ((frame & 1) == 0) {
> + db_printf(")\nWARNING: corrupt frame at %lx\n", frame);
> + break;
> + }
>   
>   /*
>* Print %i0..%i5; hope these still reflect the
>* actual arguments somewhat...
>*/
> - if (frame & 1) {
> - f64 = (struct frame64 *)(frame + BIAS);
> - for (i = 0; i < 5; i++)
> - (*pr)("%lx, ", (long)KLOAD(f64->fr_arg[i]));
> - (*pr)("%lx) at ", (long)KLOAD(f64->fr_arg[i]));
> - } else {
> - f32 = (struct frame32 *)(frame);
> - for (i = 0; i < 5; i++)
> - (*pr)("%x, ", (u_int)KLOAD(f32->fr_arg[i]));
> - (*pr)("%x) at ", (u_int)KLOAD(f32->fr_arg[i]));
> - }
> + f64 = (struct frame64 *)(frame + BIAS);
> + for (i = 0; i < 5; i++)
> + (*pr)("%lx, ", (long)KLOAD(f64->fr_arg[i]));
> + (*pr)("%lx) at ", (long)KLOAD(f64->fr_arg[i]));
>   db_printsym(pc, DB_STGY_PROC, pr);
>   (*pr)("\n");
>   }
> @@ -160,11 +152,7 @@ db_stack_trace_print(addr, have_addr, co
>  
>  
>  void
> -db_dump_window(addr, have_addr, count, modif)
> - db_expr_t addr;
> - int have_addr;
> - db_expr_t count;
> - char *modif;
> +db_dump_window(db_expr_t addr, int have_addr, db_expr_t count, char *modif)
>  {
>   int i;
>   u_int64_t frame = DDB_TF->tf_out[6];
> @@ -174,10 +162,15 @@ db_dump_window(addr, have_addr, count, m
>   addr = 0;
>  
>   /* Traverse window stack */
> - for (i=0; i - if (frame & 1) 
> - frame = (u_int64_t)((struct frame64 *)(u_long)(frame + 
> BIAS))->fr_fp;
> - else frame = 

Re: sparc64: clean up db_trace.c

[Philip Guenther]

On Fri, 9 Sep 2016, Philip Guenther wrote:
> Noticed while looking at Jasper's diff.
>  - convert declarations from k to standard C
>  - delete support for 32bit frame backtracing.  I doubt this code has ever 
>been executed on OpenBSD.  If a 32bit frame is encountered ((sp&1)==0)
>then print a warning and stop processing the frames
>  - delete a pile of casts that are unnecessary
>  - minor whitespace tweaks
> 
> build tested

Testing "tr", "mach stack", "mach window", and "mach tf" showed one 
reversed test.  Updated diff below.

ok?

Index: db_trace.c
===
RCS file: /data/src/openbsd/src/sys/arch/sparc64/sparc64/db_trace.c,v
retrieving revision 1.10
diff -u -p -r1.10 db_trace.c
--- db_trace.c  9 Feb 2015 09:21:30 -   1.10
+++ db_trace.c  10 Sep 2016 03:36:20 -
@@ -56,12 +56,8 @@ void db_print_window(u_int64_t);
 #define ULOAD(x)   probeget((paddr_t)(u_long)&(x), ASI_AIUS, sizeof(x))
 
 void
-db_stack_trace_print(addr, have_addr, count, modif, pr)
-   db_expr_t   addr;
-   int have_addr;
-   db_expr_t   count;
-   char*modif;
-   int (*pr)(const char *, ...);
+db_stack_trace_print(db_expr_t addr, int have_addr, db_expr_t count,
+char *modif, int (*pr)(const char *, ...))
 {
vaddr_t frame;
boolean_t   kernel_only = TRUE;
@@ -95,28 +91,26 @@ db_stack_trace_print(addr, have_addr, co
}
}
 
+   if ((frame & 1) == 0) {
+   db_printf("WARNING: corrupt frame at %lx\n", frame);
+   return;
+   }
+
while (count--) {
int i;
db_expr_t   offset;
char*name;
db_addr_t   pc;
struct frame64  *f64;
-   struct frame32  *f32;
 
/*
 * Switch to frame that contains arguments
 */
-   if (frame & 1) {
-   f64 = (struct frame64 *)(frame + BIAS);
-   pc = (db_addr_t)KLOAD(f64->fr_pc);
-   
-   frame = KLOAD(f64->fr_fp);
-   } else {
-   f32 = (struct frame32 *)(frame);
-   pc = (db_addr_t)KLOAD(f32->fr_pc);
-   
-   frame = (long)KLOAD(f32->fr_fp);
-   }
+
+   f64 = (struct frame64 *)(frame + BIAS);
+   pc = (db_addr_t)KLOAD(f64->fr_pc);
+   
+   frame = KLOAD(f64->fr_fp);
 
if (kernel_only) {
if (pc < KERNBASE || pc >= KERNEND)
@@ -137,22 +131,20 @@ db_stack_trace_print(addr, have_addr, co
name = "?";

(*pr)("%s(", name);
+
+   if ((frame & 1) == 0) {
+   db_printf(")\nWARNING: corrupt frame at %lx\n", frame);
+   break;
+   }

/*
 * Print %i0..%i5; hope these still reflect the
 * actual arguments somewhat...
 */
-   if (frame & 1) {
-   f64 = (struct frame64 *)(frame + BIAS);
-   for (i = 0; i < 5; i++)
-   (*pr)("%lx, ", (long)KLOAD(f64->fr_arg[i]));
-   (*pr)("%lx) at ", (long)KLOAD(f64->fr_arg[i]));
-   } else {
-   f32 = (struct frame32 *)(frame);
-   for (i = 0; i < 5; i++)
-   (*pr)("%x, ", (u_int)KLOAD(f32->fr_arg[i]));
-   (*pr)("%x) at ", (u_int)KLOAD(f32->fr_arg[i]));
-   }
+   f64 = (struct frame64 *)(frame + BIAS);
+   for (i = 0; i < 5; i++)
+   (*pr)("%lx, ", (long)KLOAD(f64->fr_arg[i]));
+   (*pr)("%lx) at ", (long)KLOAD(f64->fr_arg[i]));
db_printsym(pc, DB_STGY_PROC, pr);
(*pr)("\n");
}
@@ -160,11 +152,7 @@ db_stack_trace_print(addr, have_addr, co
 
 
 void
-db_dump_window(addr, have_addr, count, modif)
-   db_expr_t addr;
-   int have_addr;
-   db_expr_t count;
-   char *modif;
+db_dump_window(db_expr_t addr, int have_addr, db_expr_t count, char *modif)
 {
int i;
u_int64_t frame = DDB_TF->tf_out[6];
@@ -174,10 +162,15 @@ db_dump_window(addr, have_addr, count, m
addr = 0;
 
/* Traverse window stack */
-   for (i=0; ifr_fp;
-   else frame = (u_int64_t)((struct frame32 
*)(u_long)frame)->fr_fp;
+   for (i = 0; i < addr && frame; i++) {
+   if ((frame & 1) == 0)
+   break;
+   frame = ((struct frame64 *)(frame + BIAS))->fr_fp;
+   }
+
+   if ((frame & 1) == 0) {
+   db_printf("WARNING: corrupt frame at 

sparc64: clean up db_trace.c

[Philip Guenther]

Noticed while looking at Jasper's diff.
 - convert declarations from k to standard C
 - delete support for 32bit frame backtracing.  I doubt this code has ever 
   been executed on OpenBSD.  If a 32bit frame is encountered ((sp&1)==0)
   then print a warning and stop processing the frames
 - delete a pile of casts that are unnecessary
 - minor whitespace tweaks

build tested

ok?

Philip

Index: db_trace.c
===
RCS file: /data/src/openbsd/src/sys/arch/sparc64/sparc64/db_trace.c,v
retrieving revision 1.10
diff -u -p -r1.10 db_trace.c
--- db_trace.c  9 Feb 2015 09:21:30 -   1.10
+++ db_trace.c  10 Sep 2016 03:07:37 -
@@ -56,12 +56,8 @@ void db_print_window(u_int64_t);
 #define ULOAD(x)   probeget((paddr_t)(u_long)&(x), ASI_AIUS, sizeof(x))
 
 void
-db_stack_trace_print(addr, have_addr, count, modif, pr)
-   db_expr_t   addr;
-   int have_addr;
-   db_expr_t   count;
-   char*modif;
-   int (*pr)(const char *, ...);
+db_stack_trace_print(db_expr_t addr, int have_addr, db_expr_t count,
+char *modif, int (*pr)(const char *, ...))
 {
vaddr_t frame;
boolean_t   kernel_only = TRUE;
@@ -95,28 +91,26 @@ db_stack_trace_print(addr, have_addr, co
}
}
 
+   if ((frame & 1) == 0) {
+   db_printf("WARNING: corrupt frame at %lx\n", frame);
+   return;
+   }
+
while (count--) {
int i;
db_expr_t   offset;
char*name;
db_addr_t   pc;
struct frame64  *f64;
-   struct frame32  *f32;
 
/*
 * Switch to frame that contains arguments
 */
-   if (frame & 1) {
-   f64 = (struct frame64 *)(frame + BIAS);
-   pc = (db_addr_t)KLOAD(f64->fr_pc);
-   
-   frame = KLOAD(f64->fr_fp);
-   } else {
-   f32 = (struct frame32 *)(frame);
-   pc = (db_addr_t)KLOAD(f32->fr_pc);
-   
-   frame = (long)KLOAD(f32->fr_fp);
-   }
+
+   f64 = (struct frame64 *)(frame + BIAS);
+   pc = (db_addr_t)KLOAD(f64->fr_pc);
+   
+   frame = KLOAD(f64->fr_fp);
 
if (kernel_only) {
if (pc < KERNBASE || pc >= KERNEND)
@@ -137,22 +131,20 @@ db_stack_trace_print(addr, have_addr, co
name = "?";

(*pr)("%s(", name);
+
+   if ((frame & 1) == 0) {
+   db_printf(")\nWARNING: corrupt frame at %lx\n", frame);
+   break;
+   }

/*
 * Print %i0..%i5; hope these still reflect the
 * actual arguments somewhat...
 */
-   if (frame & 1) {
-   f64 = (struct frame64 *)(frame + BIAS);
-   for (i = 0; i < 5; i++)
-   (*pr)("%lx, ", (long)KLOAD(f64->fr_arg[i]));
-   (*pr)("%lx) at ", (long)KLOAD(f64->fr_arg[i]));
-   } else {
-   f32 = (struct frame32 *)(frame);
-   for (i = 0; i < 5; i++)
-   (*pr)("%x, ", (u_int)KLOAD(f32->fr_arg[i]));
-   (*pr)("%x) at ", (u_int)KLOAD(f32->fr_arg[i]));
-   }
+   f64 = (struct frame64 *)(frame + BIAS);
+   for (i = 0; i < 5; i++)
+   (*pr)("%lx, ", (long)KLOAD(f64->fr_arg[i]));
+   (*pr)("%lx) at ", (long)KLOAD(f64->fr_arg[i]));
db_printsym(pc, DB_STGY_PROC, pr);
(*pr)("\n");
}
@@ -160,11 +152,7 @@ db_stack_trace_print(addr, have_addr, co
 
 
 void
-db_dump_window(addr, have_addr, count, modif)
-   db_expr_t addr;
-   int have_addr;
-   db_expr_t count;
-   char *modif;
+db_dump_window(db_expr_t addr, int have_addr, db_expr_t count, char *modif)
 {
int i;
u_int64_t frame = DDB_TF->tf_out[6];
@@ -174,10 +162,15 @@ db_dump_window(addr, have_addr, count, m
addr = 0;
 
/* Traverse window stack */
-   for (i=0; ifr_fp;
-   else frame = (u_int64_t)((struct frame32 
*)(u_long)frame)->fr_fp;
+   for (i = 0; i < addr && frame; i++) {
+   if ((frame & 1) == 0)
+   break;
+   frame = ((struct frame64 *)(frame + BIAS))->fr_fp;
+   }
+
+   if (frame & 1) {
+   db_printf("WARNING: corrupt frame at %llx\n", frame);
+   return;
}
 
db_printf("Window %lx ", addr);
@@ -185,96 +178,67 @@ db_dump_window(addr, have_addr, count, m
 }
 
 void