diff mbox series

[v3,25/25] ppc64: Clean up reginfo handling

Message ID 20200522023440.26261-26-richard.henderson@linaro.org
State Superseded
Headers show
Series risu cleanups and improvements | expand

Commit Message

Richard Henderson May 22, 2020, 2:34 a.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(-)

-- 
2.20.1

Comments

Richard Henderson May 22, 2020, 4:10 a.m. UTC | #1 
On 5/21/20 7:34 PM, Richard Henderson wrote:
> +    return memcmp(m, a, sizeof(*m));


Ho hum.  memcmp(...) == 0.  Fixed locally.


r~
Alex Bennée June 23, 2020, 5:45 p.m. UTC | #2 
Richard Henderson <richard.henderson@linaro.org> writes:

> 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>


I guess this means any traces that exist will need to be redone?

Anyway with your hotfix:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée
Richard Henderson June 23, 2020, 6:06 p.m. UTC | #3 
On 6/23/20 10:45 AM, Alex Bennée wrote:
> 

> Richard Henderson <richard.henderson@linaro.org> writes:

> 

>> 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>

> 

> I guess this means any traces that exist will need to be redone?


Well, that's true anyway because of the format change.

r~
diff mbox series

Patch

diff --git a/risu_reginfo_ppc64.c b/risu_reginfo_ppc64.c
index 134a152..62d7ff2 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));
 }
 
 /* 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]);
     }