[UPDATED: patch actually attached this time]

All --

I am attaching a revised op tracing patch based on the feedback I got.
Its features are:

  * Provides macros in interp_guts.h for setting up arrays with
    op names and op arg counts (done via build_interp_starter.pl).

    These are used in the op tracing

  * The functions in bytecode.[hc] pass around a pointer to the length
    of the bytecode so that later when we get to runops, we know
    how much bytecode we've got and we can detect out-of-bounds
    jumping. The documentation is updated, too.

  * runops in interpreter.c now looks at interpreter->flags to
    decide if the core of runops should be runops_trace_core or
    runops_notrace_core. These new functions contain just the
    while-loop portion of runops. A new function runops_generic
    does any other setup (such as checking the bytecode
    fingerprint) or wrapup (such as complaining if we ended up
    out-of-bounds). NOTE: I didn't know what we should do for
    functions in here that are not part of the api, so I gave
    them docs with 'TODO' marks mentioning they really aren't
    part of the api. Guidance appreciated.

  * test_main.c now checks for '-t' arg and sets the tracing flag
    on its interpreter instance as appropriate.

I'd like to commit this soon to clear out my sandbox so I can go
back and re-commit the constant comparison ops. So, if there
aren't any objections...


Regards,

-- Gregor
 _____________________________________________________________________ 
/     perl -e 'srand(-2091643526); print chr rand 90 for (0..4)'      \

   Gregor N. Purdy                          [EMAIL PROTECTED]
   Focus Research, Inc.                http://www.focusresearch.com/
   8080 Beckett Center Drive #203                   513-860-3570 vox
   West Chester, OH 45069                           513-860-3579 fax
\_____________________________________________________________________/
Index: build_interp_starter.pl
===================================================================
RCS file: /home/perlcvs/parrot/build_interp_starter.pl,v
retrieving revision 1.6
diff -u -r1.6 build_interp_starter.pl
--- build_interp_starter.pl     2001/09/15 16:05:40     1.6
+++ build_interp_starter.pl     2001/09/17 16:25:56
@@ -26,7 +26,39 @@
 }
 print INTERP "} while (0);\n";
 
+
+#
+# BUILD_NAME_TABLE macro:
+#
+
+print INTERP <<CONST;
+#define BUILD_NAME_TABLE(x) do { \\
+CONST
+
+for my $name (sort {$opcodes{$a}{CODE} <=> $opcodes{$b}{CODE}} keys %opcodes) {
+    print INTERP "\tx[$opcodes{$name}{CODE}] = \"$name\"; \\\n";
+}
+print INTERP "} while (0);\n";
+
+
+#
+# BUILD_ARG_TABLE macro:
+#
+
+print INTERP <<CONST;
+#define BUILD_ARG_TABLE(x) do { \\
+CONST
+
+for my $name (sort {$opcodes{$a}{CODE} <=> $opcodes{$b}{CODE}} keys %opcodes) {
+    print INTERP "\tx[$opcodes{$name}{CODE}] = $opcodes{$name}{ARGS}; \\\n";
+}
+print INTERP "} while (0);\n";
+
+
+#
 # Spit out the DO_OP function
+#
+
 print INTERP <<EOI;
 
 #define DO_OP(w,x,y,z) do { \\
Index: bytecode.c
===================================================================
RCS file: /home/perlcvs/parrot/bytecode.c,v
retrieving revision 1.9
diff -u -r1.9 bytecode.c
--- bytecode.c  2001/09/16 01:45:50     1.9
+++ bytecode.c  2001/09/17 16:25:57
@@ -37,19 +37,21 @@
 
 =item C<check_magic>
 
-    Args: void** program_code
+    Args: void** program_code, long* program_size
 
-Check to see if the first C<long> in C<*program_code>
+Check to see if the first C<IV> in C<*program_code>
 matches the Parrot magic number for bytecode; return 1
 if so, and 0 if not. This function is expected to advance
-the C<*program_code> pointer beyond the magic number.
+the C<*program_code> pointer beyond the magic number. It
+also decrements C<*program_size> by the same amount.
 
 =cut
 
 */
 
 static int
-check_magic(void** program_code) {
+check_magic(void** program_code, long* program_size) {
+    program_size -= sizeof(IV);
     return (GRAB_IV(program_code) == PARROT_MAGIC);
 }
 
@@ -57,24 +59,27 @@
 
 =item C<read_constants_table>
 
-    Args: void** program_code
+    Args: void** program_code, long* program_size
 
 Reads the constants segment from C<*program_code>, and
 creates the referenced constants. See L<parrotbyte/Constants Segment>
 for the structure of the constants segment. Advances
-C<*program_code> beyond the constants segment.
+C<*program_code> beyond the constants segment and decrements
+C<*program_size> byt the amount consumed.
 
 =cut
 
 */
 
 static void
-read_constants_table(void** program_code)
+read_constants_table(void** program_code, long* program_size)
 {
     IV len = GRAB_IV(program_code);
     IV num;
     IV i = 0;
 
+    *program_size -= sizeof(IV);
+
     Parrot_num_string_constants = len;
     if (len == 0) {
        return;
@@ -82,6 +87,7 @@
 
     num = GRAB_IV(program_code);
     len -= sizeof(IV);
+    *program_size -= sizeof(IV);
     
     Parrot_string_constants = mem_allocate_aligned(num * sizeof(STRING*));
 
@@ -93,10 +99,12 @@
        int pad;
 
         len -= 4 * sizeof(IV);
+        *program_size -= 4 * sizeof(IV);
 
         Parrot_string_constants[i++] = string_make(*program_code /* ouch */, buflen, 
encoding, flags, type);
         (char*)*program_code += buflen;
         len -= buflen;
+        *program_size -= buflen;
 
         /* Padding */
        pad=buflen % sizeof(IV);
@@ -104,6 +112,7 @@
          pad=sizeof(IV)-pad;
          len -= pad;
          (char*)*program_code += pad;       
+          *program_size -= pad;
        }
         num--;
         if (len < 0 || (len > 0 && num == 0)) {
@@ -129,18 +138,20 @@
 */
 
 static void
-read_fixup_table(void** program_code)
+read_fixup_table(void** program_code, long* program_size)
 {
     IV len = GRAB_IV(program_code);
+    *program_size -= sizeof(IV);
     /* For now, just skip over it */
     ((IV*)*program_code) += len;
+    *program_size -= len;
 }
 
 /*
 
 =item C<init_bytecode>
 
-    Args: void* program_code
+    Args: void* program_code, long* program_size
 
 This function is responsible for calling the above three
 functions, exiting if the Parrot magic is not found, and
@@ -152,15 +163,15 @@
 */
 
 void *
-init_bytecode(void* program_code) 
+init_bytecode(void* program_code, long* program_size) 
 {
-    if (!check_magic(&program_code)) {
+    if (!check_magic(&program_code, program_size)) {
         printf("This isn't Parrot bytecode!\n");
         exit(1);
     }
 
-    read_fixup_table(&program_code);
-    read_constants_table(&program_code);
+    read_fixup_table(&program_code, program_size);
+    read_constants_table(&program_code, program_size);
     return program_code;
 }
 
Index: bytecode.h
===================================================================
RCS file: /home/perlcvs/parrot/bytecode.h,v
retrieving revision 1.5
diff -u -r1.5 bytecode.h
--- bytecode.h  2001/09/17 14:51:33     1.5
+++ bytecode.h  2001/09/17 16:25:57
@@ -14,7 +14,7 @@
 #define PARROT_BYTECODE_H_GUARD
 
 void*
-init_bytecode(void* program_code);
+init_bytecode(void* program_code, long* program_size);
 
 VAR_SCOPE IV Parrot_num_string_constants;
 VAR_SCOPE STRING** Parrot_string_constants;
Index: interpreter.c
===================================================================
RCS file: /home/perlcvs/parrot/interpreter.c,v
retrieving revision 1.12
diff -u -r1.12 interpreter.c
--- interpreter.c       2001/09/17 12:56:55     1.12
+++ interpreter.c       2001/09/17 16:25:57
@@ -13,18 +13,17 @@
 #include "parrot/parrot.h"
 #include "parrot/interp_guts.h"
 
-/*=for api interpreter runops
- * run parrot operations until the program is complete
+char *op_names[2048];
+int   op_args[2048];
+
+/*=for api interpreter check_fingerprint
+ * TODO: Not really part of the API, but here's the docs.
+ * Check the bytecode's opcode table fingerprint.
  */
 void
-runops (struct Parrot_Interp *interpreter, IV *code) {
-    /* Move these out of the inner loop. No need to redeclare 'em each
-       time through */
-    IV *(*func)();
-    void **temp; 
-    
+check_fingerprint(void) {
     if (Parrot_num_string_constants == 0) {
-        printf("Warning: Bytecode does not include opcode table fingerprint!\n");
+        fprintf(stderr, "Warning: Bytecode does not include opcode table 
+fingerprint!\n");
     } else {
         const char * fp_data;
         IV           fp_len;
@@ -33,18 +32,120 @@
         fp_len  = Parrot_string_constants[0]->buflen;
         
         if (strncmp(OPCODE_FINGERPRINT, fp_data, fp_len)) {
-            printf("Error: Opcode table fingerprint in bytecode does not match 
interpreter!\n");
-            printf("       Bytecode:    %*s\n", -fp_len, fp_data);
-            printf("       Interpreter: %s\n", OPCODE_FINGERPRINT);
+            fprintf(stderr, "Error: Opcode table fingerprint in bytecode does not 
+match interpreter!\n");
+            fprintf(stderr, "       Bytecode:    %*s\n", -fp_len, fp_data);
+            fprintf(stderr, "       Interpreter: %s\n", OPCODE_FINGERPRINT);
             exit(1);
         }
     }
+}
 
-    while (*code) {
+/*=for api interpreter runops
+ * run parrot operations until the program is complete
+ */
+IV *
+runops_notrace_core (struct Parrot_Interp *interpreter, IV *code, IV code_size) {
+    /* Move these out of the inner loop. No need to redeclare 'em each
+       time through */
+    IV *(*func)();
+    void **temp; 
+    IV *code_start;
+
+    code_start = code;
+
+    while (code >= code_start && code < (code_start + code_size) && *code) {
         DO_OP(code, temp, func, interpreter);
     }
+
+    return code;
 }
 
+/*
+ *=for api interpreter trace_op
+ * TODO: This isn't really part of the API, but here's its documentation. Prints the 
+PC, OP
+ * and ARGS. Used by runops_trace.
+ */
+void
+trace_op(IV * code_start, long code_size, IV *code) {
+    int i;
+
+    if (code >= code_start && code < (code_start + code_size)) {
+        fprintf(stderr, "PC=%ld; OP=%ld (%s)", code - code_start, *code, 
+op_names[*code]);
+        if (op_args[*code]) {
+            fprintf(stderr, "; ARGS=(");
+            for(i = 0; i < op_args[*code]; i++) {
+                if (i) { fprintf(stderr, ", "); }
+                fprintf(stderr, "%ld", *(code + i + 1));
+            }
+            fprintf(stderr, ")");
+        }
+        fprintf(stderr, "\n");
+    } else {
+        fprintf(stderr, "PC=%ld; OP=<err>\n", code - code_start);
+    }
+}
+
+/*=for api interpreter runops_trace_core
+ * TODO: Not really part of the API, but here's the docs.
+ * Passed to runops_generic() by runops_trace().
+ */
+IV *
+runops_trace_core (struct Parrot_Interp *interpreter, IV *code, IV code_size) {
+    /* Move these out of the inner loop. No need to redeclare 'em each
+       time through */
+    IV *(*func)();
+    void **temp; 
+    IV *code_start;
+
+    code_start = code;
+
+    trace_op(code_start, code_size, code);
+
+    while (code >= code_start && code < (code_start + code_size) && *code) {
+        DO_OP(code, temp, func, interpreter);
+
+        trace_op(code_start, code_size, code);
+    }
+
+    return code;
+}
+
+/*=for api interpreter runops_generic
+ * TODO: Not really part of the API, but here's the docs.
+ * Generic runops, which takes a function pointer for the core.
+ */
+void
+runops_generic (IV * (*core)(struct Parrot_Interp *, IV *, IV), struct Parrot_Interp 
+*interpreter, IV *code, IV code_size) {
+    IV * code_start;
+
+    check_fingerprint();
+
+    code_start = code;
+    code = core(interpreter, code, code_size);
+
+    if (code < code_start || code >= (code_start + code_size)) {
+        fprintf(stderr, "Error: Control left bounds of byte-code block (now at 
+location %d)!\n", code - code_start);
+        exit(1);
+    }
+}
+
+
+/*=for api interpreter runops
+ * run parrot operations until the program is complete
+ */
+void
+runops (struct Parrot_Interp *interpreter, IV *code, IV code_size) {
+    IV * (*core)(struct Parrot_Interp *, IV *, IV);
+
+    if (interpreter->flags & PARROT_TRACE_FLAG) {
+        core = runops_trace_core;
+    } else {
+        core = runops_notrace_core;
+    }
+
+    runops_generic(core, interpreter, code, code_size);
+}
+
 /*=for api interpreter make_interpreter
  *  Create the Parrot interpreter.  Allocate memory and clear the registers.
  */
@@ -111,6 +212,9 @@
         BUILD_TABLE(foo);
         
         interpreter->opcode_funcs = (void*)foo;
+
+        BUILD_NAME_TABLE(op_names);
+        BUILD_ARG_TABLE(op_args);
     }
     
     /* In case the I/O system needs something */
Index: interpreter.h
===================================================================
RCS file: /home/perlcvs/parrot/interpreter.h,v
retrieving revision 1.7
diff -u -r1.7 interpreter.h
--- interpreter.h       2001/09/17 15:15:31     1.7
+++ interpreter.h       2001/09/17 16:25:57
@@ -46,7 +46,7 @@
 make_interpreter();
 
 void
-runops(struct Parrot_Interp *, IV *);
+runops(struct Parrot_Interp *, IV *, IV);
 
 #endif
 
Index: test_main.c
===================================================================
RCS file: /home/perlcvs/parrot/test_main.c,v
retrieving revision 1.8
diff -u -r1.8 test_main.c
--- test_main.c 2001/09/16 22:05:21     1.8
+++ test_main.c 2001/09/17 16:25:57
@@ -27,14 +27,29 @@
 
 int
 main(int argc, char **argv) {
+    int i;
+    int tracing;
+
     struct Parrot_Interp *interpreter;
     init_world();
   
     interpreter = make_interpreter();
     
+    /* Look for the '-t' tracing switch. We really should use getopt, but are we 
+allowed? */
+
+    if (argc > 1 && strcmp(argv[1], "-t") == 0) {
+        tracing = 1;
+        for(i = 2; i < argc; i++) {
+            argv[i-1] = argv[i];
+        }
+        argc--;
+    } else {
+        tracing = 0;
+    }
+
     /* If we got only the program name, run the test program */
     if (argc == 1) {
-        runops(interpreter, opcodes);
+        runops(interpreter, opcodes, sizeof(opcodes));
     }
     else if (argc == 2 && !strcmp(argv[1], "-s")) { /* String tests */
         STRING *s = string_make("foo", 3, enc_native, 0, 0);
@@ -60,8 +75,10 @@
     /* Otherwise load in the program they gave and try that */
     else {
         void *program_code;
+        long program_size;
         struct stat file_stat;
         int fd;
+
         if (stat(argv[1], &file_stat)) {
             printf("can't stat %s, code %i\n", argv[1], errno);
             return 1;
@@ -72,20 +89,27 @@
             return 1;
         }
         
+        program_size = file_stat.st_size;
+
 #ifndef HAS_HEADER_SYSMMAN
-               program_code = mem_sys_allocate(file_stat.st_size);
-               _read(fd, program_code, file_stat.st_size);
+        program_code = mem_sys_allocate(program_size);
+        _read(fd, program_code, program_size);
 #else
-               program_code = mmap(0, file_stat.st_size, PROT_READ, MAP_SHARED, fd, 
0);
+        program_code = mmap(0, program_size, PROT_READ, MAP_SHARED, fd, 0);
 #endif
+
         if (!program_code) {
             printf("Can't mmap, code %i\n", errno);
             return 1;
         }
         
-        program_code = init_bytecode(program_code);
+        program_code = init_bytecode(program_code, &program_size);
         
-        runops(interpreter, program_code);
+        if (tracing) {
+            interpreter->flags |= PARROT_TRACE_FLAG;
+        }
+
+        runops(interpreter, program_code, program_size);
         
     }
     return 0;

Reply via email to