From 0f6557beeaa96768c544bf2393b51bad657e073a Mon Sep 17 00:00:00 2001 From: wfjm Date: Fri, 5 Aug 2022 08:09:45 +0200 Subject: [PATCH] stktst: minor updates and consolidations [skip ci] - handle comments from Johnny Billquist - use page instead of segment language (-s -> -p) - call printf before 'r' tests done to have stack action --- tools/tests/stktst/README.md | 45 +++++++----- .../data/2022-08-03_simh_3.11_bsd_447.md | 71 +++++++------------ tools/tests/stktst/dotst.s | 14 ++-- tools/tests/stktst/stktst.c | 59 +++++++++------ 4 files changed, 96 insertions(+), 93 deletions(-) diff --git a/tools/tests/stktst/README.md b/tools/tests/stktst/README.md index 1197dbbe..3edb1197 100644 --- a/tools/tests/stktst/README.md +++ b/tools/tests/stktst/README.md @@ -2,46 +2,47 @@ The `stktst` program exercises the 2.11BSD stack extension logic. In a first step, the `sp` can be aligned to a click (64 byte) or a -segment (8129 byte) boundary. +page (8129 byte) boundary. An offset can also be applied after this alignment. In a second step, a sequence of integer and floating point instructions with a `-(sp)` destination is executed. This allows to set up almost every possible stack extension situation. Motivation for `stktst` were differences in the `MMR1` register implementation -in different PDP-11 CPUs, and differences in the modeling of `MMR1` in -different PDP-11 simulators. +in different PDP-11 CPUs and differences in the modeling of `MMR1` in +PDP-11 simulators (see +[simh@groups.io post](https://groups.io/g/simh/topic/the_mysteries_of_mmr1/92661872)). That combined with the 2.11BSD stack extension handling prior to #473 can lead -to unexpected "Memory fault" aborts in 2.11BSD. +to unexpected "segmentation fault" aborts in 2.11BSD. The results are collected in the [data](data) folder. `stktst` has an assembler core [dotst.s](dotst.s) which is called from a C main program [stktst.c](stktst.c). It is called as ``` - ./stktst [-s ns] [-c nc] [-o no] + ./stktst [-p np] [-c nc] [-o no] ``` The options control the initial stack alignment: -- **`-s ns`**: aligns to 8192 byte segment boundaries. ns=1 to the next one, - nc=2 to the second next one, etc. - Obviously, `ns` should be smaller than 8. - The option is ignored if ns<=0. +- **`-p np`**: aligns to 8192 byte page boundaries. np=1 to the next one, + np=2 to the second next one, etc. + Obviously, `np` should be smaller than 8. + The option is ignored if np<=0. - **`-c nc`**: aligns to 64 byte click boundaries. nc=1 to the next one, nc=2 to the second next one, etc. The first alignment step will not change the stack if it was alreay on a - click bounday, it will therefore add 0 to 64 bytes to the stack. + click bounday, it will therefore add 0 to 62 bytes to the stack. The option is ignored if nc<=0. - Click alignment is done after segment alignment. + Click alignment is done after page alignment. - **`-o no`**: adds `no` to `sp`. `no` must be even and can be positive or negative. **Notes**: -- no range checks is done for `no`. - After a **-s** is is save to use small positive `no` values to position `sp` - a bit before a segment boundary. - After a **-c** is is better to use negative `no` values to position `sp` +- no range check is done for `no`. + After a **-p** it is safe to use small positive `no` values to position `sp` + a bit before a page boundary. + After a **-c** it is better to use negative `no` values to position `sp` before the next click boundary. -- the stack allocated below the argument and environment values. +- the stack is allocated below the argument and environment values. The initial `sp` value will therefore decrease when the number of characters in the argument list increases because the stack base moves down. In some cases it is therefore prudent to specify the numbers as quoted @@ -49,8 +50,16 @@ The options control the initial stack alignment: ``` ./stktst d ' 3' -c ' 2' -o ' 4' ``` - That allows to change the counts without changing the length of the + That allows changing the counts without changing the length of the argument list. +- the code was called _horrible_ and is indeed _awkward_ to use. That's + mostly because the stack is moving target. A change from `sh` to `tcsh`, + which gives a different environment, changes already the stack base and + alignment. Library calls, like `printf`, may temporarily use significant + stack space and trigger a stack extension, and change the environment. + So chasing issues in the stack extension logic, especially when it is + FPP specific, is subtle. `stktst` tries the best and should be forgotten + when all issues have been resolved. The `cmd` argument selects the instruction that does the stack push and `count` determines how often it is executed. @@ -76,7 +85,7 @@ and gives the `sp` value - after alignments and offsets were applied - after stack pushes were executed -and prints it in octal and broken down in segment, click and byte offset. +and prints it in octal and broken down in page, click and byte offset. Because the stack is a downward growing segment, all offsets measure the distance to the top of memory and increase when the `sp` decreases. diff --git a/tools/tests/stktst/data/2022-08-03_simh_3.11_bsd_447.md b/tools/tests/stktst/data/2022-08-03_simh_3.11_bsd_447.md index 3003ab08..b268405d 100644 --- a/tools/tests/stktst/data/2022-08-03_simh_3.11_bsd_447.md +++ b/tools/tests/stktst/data/2022-08-03_simh_3.11_bsd_447.md @@ -2,10 +2,10 @@ ### Background The `MMR1` response after an MMU abort in an FPP instruction depends on the CPU. -In an 11/70, the registers reflect the state at abort and `MMR1` shows the -change. In a J11, the registers are unchanged, and `MMR1` shows zero. +On an 11/70, the registers reflect the state at abort and `MMR1` shows the +change. On a J11, the registers are unchanged, and `MMR1` shows zero. -SimH V3.11-0 used the FPP MMU abort handling for _all_ CPU models. +SimH V3.11-0 used the J11 FPP MMU abort handling for _all_ CPU models. So even when an 11/70 is modeled, the behavior is like a J11. The 2.11BSD stack extension logic checks whether the `sp` is below the @@ -54,56 +54,37 @@ has grown to 020000 and memory is really exhausted: ./stktst f '14264' # stktst-I: before sp 177334 (0, 4,36); 177334 (0, 4,36); # Segmentation fault (core dumped) - ``` -Extending with `double` pushes works when the `double` is aligned on an -8 byte border. In this case the 4 byte correction done in 2.11BSD #473 -ensures that the `sp` is interpreted as below current allocation. -The alignment is set up with `-c 2 -o 0`: +Extending with `double` pushes can fail depending on the alignment of the +`double`. If they are click aligned all is fine, the 4 byte offset in +2.11BSB ensures that the `sp` is below the allocation. In case of a +misalignment of 2 or 4 bytes, this is not the case, one gets a `SIGSEGV`. +In case of a misalignment of 6 bytes, it works again. ``` -./stktst d ' 512' -c ' 2' -o ' 0' - # stktst-I: before sp 177304 (0, 4,60); 177200 (0, 5,64); - # stktst-I: after sp 177304 (0, 4,60); 177200 (0, 5,64); 167200 (0, 69,64); -./stktst d '7120' -c ' 2' -o ' 0' - # stktst-I: before sp 177304 (0, 4,60); 177200 (0, 5,64); - # stktst-I: after sp 177304 (0, 4,60); 177200 (0, 5,64); 020000 (6,127,64); -./stktst d '7121' -c ' 2' -o ' 0' +./stktst d '1024' -c ' 2' -o ' 0' # stktst-I: before sp 177304 (0, 4,60); 177200 (0, 5,64); + # stktst-I: after sp 177304 (0, 4,60); 177200 (0, 5,64); 157200 (1, 5,64); +./stktst d '1024' -c ' 2' -o ' -2' + # stktst-I: before sp 177304 (0, 4,60); 177176 (0, 6, 2); # Segmentation fault (core dumped) -``` - -Misaligned `double` pushes fail as expected. -A 4 byte misalignment is set up with `-c 2 -o -4`. -Even with the 4 byte correction the `sp` appears to be in the allocated area, -and a `SIGSEGV` is taken: -``` -./stktst d ' 1' -c ' 2' -o ' -4' - # stktst-I: before sp 177304 (0, 4,60); 177174 (0, 6, 4); - # stktst-I: after sp 177304 (0, 4,60); 177174 (0, 6, 4); 177164 (0, 6,12); -./stktst d ' 319' -c ' 2' -o ' -4' - # stktst-I: before sp 177304 (0, 4,60); 177174 (0, 6, 4); - # stktst-I: after sp 177304 (0, 4,60); 177174 (0, 6, 4); 172204 (0, 45,60); -./stktst d ' 320' -c ' 2' -o ' -4' +./stktst d '1024' -c ' 2' -o ' -4' # stktst-I: before sp 177304 (0, 4,60); 177174 (0, 6, 4); # Segmentation fault (core dumped) +./stktst d '1024' -c ' 2' -o ' -6' + # stktst-I: before sp 177304 (0, 4,60); 177172 (0, 6, 6); + # stktst-I: after sp 177304 (0, 4,60); 177172 (0, 6, 6); 157172 (1, 6, 6); ``` - -The initial stack size is 20 clicks -(see [SSIZE](https://www.retro11.de/ouxr/211bsd/usr/src/sys/pdp/machparam.h.html#m:SSIZE)), -the stack increment is also 20 clicks -(see [SINCR](https://www.retro11.de/ouxr/211bsd/usr/src/sys/pdp/machparam.h.html#m:SINCR)). -The initial stack segment size is a bit larger due to argument and environment -allocations. -In the tests it was 24 clicks as tests with the **r** command show: +With some trial and error one can determine the situation where it fails: ``` -./stktst r 0175004 - # OK -./stktst r 0175000 - # OK -./stktst r 0174776 +./stktst d ' 151' -c ' 2' -o ' -2' + # stktst-I: before sp 177304 (0, 4,60); 177176 (0, 6, 2); + # stktst-I: after sp 177304 (0, 4,60); 177176 (0, 6, 2); 174706 (0, 24,58); +./stktst d ' 152' -c ' 2' -o ' -2' + # stktst-I: before sp 177304 (0, 4,60); 177176 (0, 6, 2); # Segmentation fault (core dumped) ``` - -It is therefore a bit surprising that the failure happens at the border -expected for the second stack extend and not around 0175000. +That's after 25 clicks. The initial stack size is 20 clicks +(see [SSIZE](https://www.retro11.de/ouxr/211bsd/usr/src/sys/pdp/machparam.h.html#m:SSIZE)) and the stack segment is a bit larger to accommodate argument and +environment structures. +So the `double` pushes fail when the initial allocation is exhausted. diff --git a/tools/tests/stktst/dotst.s b/tools/tests/stktst/dotst.s index 203272a8..a635fc20 100644 --- a/tools/tests/stktst/dotst.s +++ b/tools/tests/stktst/dotst.s @@ -1,4 +1,4 @@ -/ $Id: dotst.s 1268 2022-08-04 07:03:08Z mueller $ +/ $Id: dotst.s 1269 2022-08-05 06:00:38Z mueller $ / SPDX-License-Identifier: GPL-3.0-or-later / Copyright 2022- by Walter F.J. Mueller / @@ -8,7 +8,7 @@ / idat[0] command ('I','i','l','f','d','r','w', or 'h') / idat[1] command repeat count / idat[2] -c repeat count -/ idat[3] -s repeat count +/ idat[3] -p repeat count / idat[4] -o byte offset / and returns in odat / odat[0] sp before align and offset @@ -70,20 +70,20 @@ testx: / apply sp aligns and offset. In python pseudo code do / # segmemt align / if idat[3] > 0: -/ sp &= 017777 +/ sp &= ~017777 / sp -= 8192 * (idat[3]-1) / # click align / if idat[2] > 0: -/ sp &= 077 +/ sp &= ~077 / sp -= 64 * (idat[2]-1) / # byte offset / sp += idat[4] / mov sp,r4 -/ handle -s +/ handle -p tst 6(r2) / idat[3] -s count > 0 ? ble optc - bic $017777,r4 / sp &= 017777 + bic $017777,r4 / sp &= ~017777 mov 6(r2),r1 dec r1 / idat[3]-1 mul $020000,r1 @@ -91,7 +91,7 @@ testx: / handle -c optc: tst 4(r2) / idat[2] -c count > 0 ? ble opto - bic $000077,r4 / sp &= 077 + bic $000077,r4 / sp &= ~077 mov 4(r2),r1 dec r1 / idat[2]-1 mul $000100,r1 diff --git a/tools/tests/stktst/stktst.c b/tools/tests/stktst/stktst.c index 58cc88c7..067c052a 100644 --- a/tools/tests/stktst/stktst.c +++ b/tools/tests/stktst/stktst.c @@ -1,9 +1,10 @@ -/* $Id: stktst.c 1268 2022-08-04 07:03:08Z mueller $ */ +/* $Id: stktst.c 1269 2022-08-05 06:00:38Z mueller $ */ /* SPDX-License-Identifier: GPL-3.0-or-later */ /* Copyright 2022- by Walter F.J. Mueller */ /* Revision History: */ /* Date Rev Version Comment */ +/* 2022-08-04 1269 1.1 rename -s to -p; warmup before r,w,h */ /* 2022-08-03 1268 1.0 Initial version */ #include @@ -37,16 +38,24 @@ int doconv(arg) return ires; } -print_stack(val) - unsigned int val; +print_stack(str, odat, nodat) + char *str; + unsigned int *odat; + int nodat; { - unsigned int ns = 7-(val>>13); - unsigned int nc = 127-((val&017777)>>6); - unsigned int no = 64-(val&077); - fprintf(stdout," %06o (%1d,%3d,%2d);", val, ns, nc, no); + unsigned int val, np, nc, no; + int i; + printf("%s", str); + for (i=0; i>13); + nc = 127-((val&017777)>>6); + no = 64-(val&077); + printf(" %06o (%1d,%3d,%2d);", val, np, nc, no); + } + printf("\n"); } - int main(argc, argv) int argc; char *argv[]; @@ -67,10 +76,10 @@ int main(argc, argv) cmd = argv[1][0]; /* get command */ rcnt = doconv(argv[2]); - idat[0] = cmd; /* command code */ + idat[0] = 0; /* command code */ idat[1] = 0; /* command repeat */ idat[2] = 0; /* -c repeat */ - idat[3] = 0; /* -s repeat */ + idat[3] = 0; /* -p repeat */ idat[4] = 0; /* -o offset */ /* check that command is valid */ @@ -91,7 +100,19 @@ int main(argc, argv) exit(1); } - /* process options */ + if (optrwh) { /* handle r,w,h tests */ + /* warmup: noop test to get ps and call printf to get stack action */ + idat[0] = 'I'; + dotst(idat, odat); + print_stack("stktst-I: before sp", odat, 1); + /* now do r,w, or h test, nothing to print afterwards */ + idat[0] = cmd; + idat[1] = rcnt; + dotst(idat, odat); + exit(0); + } + + /* process options only for normal tests */ for (argi=3; argi= argc) { fprintf(stderr, "stktst-E: no value after %s \n", argv[argi]); @@ -100,7 +121,7 @@ int main(argc, argv) cnt = doconv(argv[argi+1]); if (strcmp(argv[argi], "-c") == 0) { idat[2] = cnt; - } else if (strcmp(argv[argi], "-s") == 0) { + } else if (strcmp(argv[argi], "-p") == 0) { idat[3] = cnt; } else if (strcmp(argv[argi], "-o") == 0) { idat[4] = cnt; @@ -110,23 +131,15 @@ int main(argc, argv) } } - if (optrwh) idat[1] = rcnt; /* call test: 1st round */ + idat[0] = cmd; dotst(idat, odat); - if (optrwh) exit(0); - printf("stktst-I: before sp"); - print_stack(odat[0]); - print_stack(odat[1]); - printf("\n"); + print_stack("stktst-I: before sp", odat, 2); /* call test: 2nd round */ idat[1] = rcnt; dotst(idat, odat); - printf("stktst-I: after sp"); - print_stack(odat[0]); - print_stack(odat[1]); - print_stack(odat[2]); - printf("\n"); + print_stack("stktst-I: after sp", odat, 3); exit(0); }