diff mbox series

[RISU,v4,26/29] ppc64: Clean up reginfo handling

Message ID 20220708154700.18682-27-richard.henderson@linaro.org
State New
Headers show
Series risu cleanups and improvements | expand

Commit Message

Richard Henderson July 8, 2022, 3:46 p.m. UTC
Several of the gp_reg[] elements are not relevant -- e.g. orig r3,
which is related to system calls.  Omit those from the original
reginfo_init(), so that any differences are automatically hidden.

Do not only compare bit 4 of CCR -- this register is 32 bits wide
with 8 cr subfields.  We should compare all of them.

Tidy reginfo_dump() output.  Especially, do not dump the non-
relevant fields.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 risu_reginfo_ppc64.c | 114 +++++++++++++++++--------------------------
 1 file changed, 44 insertions(+), 70 deletions(-)

Comments

Matheus K. Ferst July 12, 2022, 11:17 a.m. UTC | #1
On 08/07/2022 12:46, Richard Henderson wrote:
> Several of the gp_reg[] elements are not relevant -- e.g. orig r3,
> which is related to system calls.  Omit those from the original
> reginfo_init(), so that any differences are automatically hidden.
> 
> Do not only compare bit 4 of CCR -- this register is 32 bits wide
> with 8 cr subfields.  We should compare all of them.
> 
> Tidy reginfo_dump() output.  Especially, do not dump the non-
> relevant fields.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   risu_reginfo_ppc64.c | 114 +++++++++++++++++--------------------------
>   1 file changed, 44 insertions(+), 70 deletions(-)
> 
> diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
> index 134a152..eeb0460 100644
> --- a/risu_reginfo_ppc64.c
> +++ b/risu_reginfo_ppc64.c
> @@ -21,19 +21,30 @@
>   #include "risu.h"
>   #include "risu_reginfo_ppc64.h"
> 
> -#define XER 37
> -#define CCR 38
> +/* Names for indexes within gregset_t, ignoring those irrelevant here */
> +enum {
> +    NIP = 32,
> +    MSR = 33,
> +    CTR = 35,
> +    LNK = 36,
> +    XER = 37,
> +    CCR = 38,
> +};
> 
>   const struct option * const arch_long_opts;
>   const char * const arch_extra_help;
> 
>   static const char * const greg_names[NGREG] = {
> -    "r0",  "r1",  "r2",  "r3",  "r4",  "r5",  "r6",  "r7",
> -    "r8",  "r9", "r10", "r11", "r12", "r13", "r14", "r15",
> -   "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
> -   "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
> -   [XER] = "xer",
> -   [CCR] = "ccr",
> +     "r0",  "r1",  "r2",  "r3",  "r4",  "r5",  "r6",  "r7",
> +     "r8",  "r9", "r10", "r11", "r12", "r13", "r14", "r15",
> +    "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
> +    "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
> +    [NIP] = "nip",
> +    [MSR] = "msr",
> +    [CTR] = "ctr",
> +    [LNK] = "lnk",
> +    [XER] = "xer",
> +    [CCR] = "ccr",
>   };
> 
NIP value depends on image_start address, and LNK will be the return 
address, so I guess we shouldn't compare them. Some MSR bits are defined 
from start under linux-user (e.g. FE0 and FE1), but I think we can work 
around that.

CTR will likely be image_start since calling a function pointer is 
usually implemented with mtctr/bctr, but risugen doesn't initialize it, 
so the comparison will fail at the first checkpoint. VRSAVE is also not 
initialized, so I guess we need something like

 >    write_mov_ri(0, rand(0xffffffff));
 >    insn32(0x7c0903a6); # mtctr r0
 >
 >    write_mov_ri(0, rand(0xffffffff));
 >    insn32(0x7c0043a6); # mtvrsave r0

in write_random_regdata

>   void process_arch_opt(int opt, const char *arg)
> @@ -61,8 +72,13 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
>       ri->nip = uc->uc_mcontext.regs->nip - image_start_address;
> 
>       for (i = 0; i < NGREG; i++) {
> -        ri->gregs[i] = uc->uc_mcontext.gp_regs[i];
> +        /* Do not copy gp_reg entries not relevant to the context. */
> +        if (greg_names[i]) {
> +            ri->gregs[i] = uc->uc_mcontext.gp_regs[i];
> +        }
>       }
> +    ri->gregs[1] = 0xdeadbeef;   /* sp */
> +    ri->gregs[13] = 0xdeadbeef;  /* gp */
> 
>       memcpy(ri->fpregs, uc->uc_mcontext.fp_regs, 32 * sizeof(double));
>       ri->fpscr = uc->uc_mcontext.fp_regs[32];
> @@ -76,79 +92,37 @@ void reginfo_init(struct reginfo *ri, ucontext_t *uc)
>   /* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
>   int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
>   {
> -    int i;
> -    for (i = 0; i < 32; i++) {
> -        if (i == 1 || i == 13) {
> -            continue;
> -        }
> -
> -        if (m->gregs[i] != a->gregs[i]) {
> -            return 0;
> -        }
> -    }
> -
> -    if (m->gregs[XER] != a->gregs[XER]) {
> -        return 0;
> -    }
> -
> -    if ((m->gregs[CCR] & 0x10) != (a->gregs[CCR] & 0x10)) {
> -        return 0;
> -    }
> -
> -    for (i = 0; i < 32; i++) {
> -        if (m->fpregs[i] != a->fpregs[i]) {
> -            return 0;
> -        }
> -    }
> -
> -    for (i = 0; i < 32; i++) {
> -        if (m->vrregs.vrregs[i][0] != a->vrregs.vrregs[i][0] ||
> -            m->vrregs.vrregs[i][1] != a->vrregs.vrregs[i][1] ||
> -            m->vrregs.vrregs[i][2] != a->vrregs.vrregs[i][2] ||
> -            m->vrregs.vrregs[i][3] != a->vrregs.vrregs[i][3]) {
> -            return 0;
> -        }
> -    }
> -    return 1;
> +    return memcmp(m, a, sizeof(*m)) == 0;
>   }
> 
>   /* reginfo_dump: print state to a stream */
>   void reginfo_dump(struct reginfo *ri, FILE * f)
>   {
> -    int i;
> +    const char *sep;
> +    int i, j;
> 
> -    fprintf(f, "  faulting insn 0x%x\n", ri->faulting_insn);
> -    fprintf(f, "  prev insn     0x%x\n", ri->prev_insn);
> -    fprintf(f, "  prev addr    0x%" PRIx64 "\n\n", ri->nip);
> +    fprintf(f, "%6s: %08x\n", "insn", ri->faulting_insn);
> +    fprintf(f, "%6s: %016lx\n", "pc", ri->nip) >
> -    for (i = 0; i < 16; i++) {
> -        fprintf(f, "\tr%2d: %16lx\tr%2d: %16lx\n", i, ri->gregs[i],
> -                i + 16, ri->gregs[i + 16]);
> +    sep = "";
> +    for (i = j = 0; i < NGREG; i++) {
> +        if (greg_names[i] != NULL) {
> +            fprintf(f, "%s%6s: %016lx", sep, greg_names[i], ri->gregs[i]);
> +            sep = (++j & 1 ? "  " : "\n");
> +        }
>       }
> 
> -    fprintf(f, "\n");
> -    fprintf(f, "\tnip    : %16lx\n", ri->gregs[32]);
> -    fprintf(f, "\tmsr    : %16lx\n", ri->gregs[33]);
> -    fprintf(f, "\torig r3: %16lx\n", ri->gregs[34]);
> -    fprintf(f, "\tctr    : %16lx\n", ri->gregs[35]);
> -    fprintf(f, "\tlnk    : %16lx\n", ri->gregs[36]);
> -    fprintf(f, "\txer    : %16lx\n", ri->gregs[37]);
> -    fprintf(f, "\tccr    : %16lx\n", ri->gregs[38]);
> -    fprintf(f, "\tmq     : %16lx\n", ri->gregs[39]);
> -    fprintf(f, "\ttrap   : %16lx\n", ri->gregs[40]);
> -    fprintf(f, "\tdar    : %16lx\n", ri->gregs[41]);
> -    fprintf(f, "\tdsisr  : %16lx\n", ri->gregs[42]);
> -    fprintf(f, "\tresult : %16lx\n", ri->gregs[43]);
> -    fprintf(f, "\tdscr   : %16lx\n\n", ri->gregs[44]);
> -
> -    for (i = 0; i < 16; i++) {
> -        fprintf(f, "\tf%2d: %016lx\tf%2d: %016lx\n", i, ri->fpregs[i],
> -                i + 16, ri->fpregs[i + 16]);
> +    sep = "\n";
> +    for (i = j = 0; i < 32; i++) {
> +        fprintf(f, "%s%*s%d: %016lx",
> +                sep, 6 - (i < 10 ? 1 : 2), "f", i, ri->fpregs[i]);
> +        sep = (++j & 1 ? "  " : "\n");
>       }
> -    fprintf(f, "\tfpscr: %016lx\n\n", ri->fpscr);
> +    fprintf(f, "\n%6s: %016lx\n", "fpscr", ri->fpscr);
> 
>       for (i = 0; i < 32; i++) {
> -        fprintf(f, "vr%02d: %8x, %8x, %8x, %8x\n", i,
> +        fprintf(f, "%*s%d: %08x %08x %08x %08x\n",
> +                6 - (i < 10 ? 1 : 2), "vr", i,
>                   ri->vrregs.vrregs[i][0], ri->vrregs.vrregs[i][1],
>                   ri->vrregs.vrregs[i][2], ri->vrregs.vrregs[i][3]);
>       }
> --
> 2.34.1
> 
>
Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
diff mbox series

Patch

diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index 134a152..eeb0460 100644
--- a/risu_reginfo_ppc64.c
+++ b/risu_reginfo_ppc64.c
@@ -21,19 +21,30 @@ 
 #include "risu.h"
 #include "risu_reginfo_ppc64.h"
 
-#define XER 37
-#define CCR 38
+/* Names for indexes within gregset_t, ignoring those irrelevant here */
+enum {
+    NIP = 32,
+    MSR = 33,
+    CTR = 35,
+    LNK = 36,
+    XER = 37,
+    CCR = 38,
+};
 
 const struct option * const arch_long_opts;
 const char * const arch_extra_help;
 
 static const char * const greg_names[NGREG] = {
-    "r0",  "r1",  "r2",  "r3",  "r4",  "r5",  "r6",  "r7",
-    "r8",  "r9", "r10", "r11", "r12", "r13", "r14", "r15",
-   "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
-   "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
-   [XER] = "xer",
-   [CCR] = "ccr",
+     "r0",  "r1",  "r2",  "r3",  "r4",  "r5",  "r6",  "r7",
+     "r8",  "r9", "r10", "r11", "r12", "r13", "r14", "r15",
+    "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
+    "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
+    [NIP] = "nip",
+    [MSR] = "msr",
+    [CTR] = "ctr",
+    [LNK] = "lnk",
+    [XER] = "xer",
+    [CCR] = "ccr",
 };
 
 void process_arch_opt(int opt, const char *arg)
@@ -61,8 +72,13 @@  void reginfo_init(struct reginfo *ri, ucontext_t *uc)
     ri->nip = uc->uc_mcontext.regs->nip - image_start_address;
 
     for (i = 0; i < NGREG; i++) {
-        ri->gregs[i] = uc->uc_mcontext.gp_regs[i];
+        /* Do not copy gp_reg entries not relevant to the context. */
+        if (greg_names[i]) {
+            ri->gregs[i] = uc->uc_mcontext.gp_regs[i];
+        }
     }
+    ri->gregs[1] = 0xdeadbeef;   /* sp */
+    ri->gregs[13] = 0xdeadbeef;  /* gp */
 
     memcpy(ri->fpregs, uc->uc_mcontext.fp_regs, 32 * sizeof(double));
     ri->fpscr = uc->uc_mcontext.fp_regs[32];
@@ -76,79 +92,37 @@  void reginfo_init(struct reginfo *ri, ucontext_t *uc)
 /* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
 int reginfo_is_eq(struct reginfo *m, struct reginfo *a)
 {
-    int i;
-    for (i = 0; i < 32; i++) {
-        if (i == 1 || i == 13) {
-            continue;
-        }
-
-        if (m->gregs[i] != a->gregs[i]) {
-            return 0;
-        }
-    }
-
-    if (m->gregs[XER] != a->gregs[XER]) {
-        return 0;
-    }
-
-    if ((m->gregs[CCR] & 0x10) != (a->gregs[CCR] & 0x10)) {
-        return 0;
-    }
-
-    for (i = 0; i < 32; i++) {
-        if (m->fpregs[i] != a->fpregs[i]) {
-            return 0;
-        }
-    }
-
-    for (i = 0; i < 32; i++) {
-        if (m->vrregs.vrregs[i][0] != a->vrregs.vrregs[i][0] ||
-            m->vrregs.vrregs[i][1] != a->vrregs.vrregs[i][1] ||
-            m->vrregs.vrregs[i][2] != a->vrregs.vrregs[i][2] ||
-            m->vrregs.vrregs[i][3] != a->vrregs.vrregs[i][3]) {
-            return 0;
-        }
-    }
-    return 1;
+    return memcmp(m, a, sizeof(*m)) == 0;
 }
 
 /* reginfo_dump: print state to a stream */
 void reginfo_dump(struct reginfo *ri, FILE * f)
 {
-    int i;
+    const char *sep;
+    int i, j;
 
-    fprintf(f, "  faulting insn 0x%x\n", ri->faulting_insn);
-    fprintf(f, "  prev insn     0x%x\n", ri->prev_insn);
-    fprintf(f, "  prev addr    0x%" PRIx64 "\n\n", ri->nip);
+    fprintf(f, "%6s: %08x\n", "insn", ri->faulting_insn);
+    fprintf(f, "%6s: %016lx\n", "pc", ri->nip);
 
-    for (i = 0; i < 16; i++) {
-        fprintf(f, "\tr%2d: %16lx\tr%2d: %16lx\n", i, ri->gregs[i],
-                i + 16, ri->gregs[i + 16]);
+    sep = "";
+    for (i = j = 0; i < NGREG; i++) {
+        if (greg_names[i] != NULL) {
+            fprintf(f, "%s%6s: %016lx", sep, greg_names[i], ri->gregs[i]);
+            sep = (++j & 1 ? "  " : "\n");
+        }
     }
 
-    fprintf(f, "\n");
-    fprintf(f, "\tnip    : %16lx\n", ri->gregs[32]);
-    fprintf(f, "\tmsr    : %16lx\n", ri->gregs[33]);
-    fprintf(f, "\torig r3: %16lx\n", ri->gregs[34]);
-    fprintf(f, "\tctr    : %16lx\n", ri->gregs[35]);
-    fprintf(f, "\tlnk    : %16lx\n", ri->gregs[36]);
-    fprintf(f, "\txer    : %16lx\n", ri->gregs[37]);
-    fprintf(f, "\tccr    : %16lx\n", ri->gregs[38]);
-    fprintf(f, "\tmq     : %16lx\n", ri->gregs[39]);
-    fprintf(f, "\ttrap   : %16lx\n", ri->gregs[40]);
-    fprintf(f, "\tdar    : %16lx\n", ri->gregs[41]);
-    fprintf(f, "\tdsisr  : %16lx\n", ri->gregs[42]);
-    fprintf(f, "\tresult : %16lx\n", ri->gregs[43]);
-    fprintf(f, "\tdscr   : %16lx\n\n", ri->gregs[44]);
-
-    for (i = 0; i < 16; i++) {
-        fprintf(f, "\tf%2d: %016lx\tf%2d: %016lx\n", i, ri->fpregs[i],
-                i + 16, ri->fpregs[i + 16]);
+    sep = "\n";
+    for (i = j = 0; i < 32; i++) {
+        fprintf(f, "%s%*s%d: %016lx",
+                sep, 6 - (i < 10 ? 1 : 2), "f", i, ri->fpregs[i]);
+        sep = (++j & 1 ? "  " : "\n");
     }
-    fprintf(f, "\tfpscr: %016lx\n\n", ri->fpscr);
+    fprintf(f, "\n%6s: %016lx\n", "fpscr", ri->fpscr);
 
     for (i = 0; i < 32; i++) {
-        fprintf(f, "vr%02d: %8x, %8x, %8x, %8x\n", i,
+        fprintf(f, "%*s%d: %08x %08x %08x %08x\n",
+                6 - (i < 10 ? 1 : 2), "vr", i,
                 ri->vrregs.vrregs[i][0], ri->vrregs.vrregs[i][1],
                 ri->vrregs.vrregs[i][2], ri->vrregs.vrregs[i][3]);
     }