https://codereview.appspot.com/6569068/diff/18001/src/cmd/dist/buildruntime.c
File src/cmd/dist/buildruntime.c (right):
https://codereview.appspot.com/6569068/diff/18001/src/cmd/dist/buildruntime.c#newcode111
src/cmd/dist/buildruntime.c:111: "#define tos_pid 48\n"
Should this be #define tos_pid(r) 48(r) to be like the others?
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c
File src/pkg/runtime/signal_plan9_386.c (right):
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c#newcode33
src/pkg/runtime/signal_plan9_386.c:33: int8 *err[NERRMAX];
This is an array of pointers not an array of char.
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c#newcode39
src/pkg/runtime/signal_plan9_386.c:39: if(s && *s == 0)
I don't understand this. What is going on here?
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c#newcode43
src/pkg/runtime/signal_plan9_386.c:43: if(len <= 4 ||
runtime·mcmp((byte*)s, (byte*)"sys:", 4) != 0)
There are too many exits calls in this function. There shouldn't be any.
In the worst case there should be a noted(NDFLT) to let the kernel kill
the proc (or put it in broken state for debugging).
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c#newcode57
src/pkg/runtime/signal_plan9_386.c:57: runtime·exits(s);
Here's another.
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_386.c#newcode67
src/pkg/runtime/signal_plan9_386.c:67: runtime·memmove((void*)err,
(void*)s, len);
Should check len against ERRMAX. Also, when this function returns the
stack space is gone, so this seems dodgy. Probably you need to save into
a per-m field instead. Or throw it away and use sig->name instead.
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_amd64.c
File src/pkg/runtime/signal_plan9_amd64.c (right):
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signal_plan9_amd64.c#newcode22
src/pkg/runtime/signal_plan9_amd64.c:22: runtime·printf("r8 %X\n",
u->r8);
Shouldn't all these %X be lined up?
Or even better do ax\t%X bx\t%X etc
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signals_plan9.h
File src/pkg/runtime/signals_plan9.h (right):
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/signals_plan9.h#newcode9
src/pkg/runtime/signals_plan9.h:9: T, "sys: trap:",
I think you need some comments here explaining why this is a throw but
the above were panic.
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/stack.h
File src/pkg/runtime/stack.h (right):
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/stack.h#newcode58
src/pkg/runtime/stack.h:58: // purposes like signal handling. Used on
Windows because
Used on Window and on Plan 9
because they do not use a separate stack.
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/stack.h#newcode64
src/pkg/runtime/stack.h:64: StackSystem = 512,
Where does 512 come from? You need space for the Ureg and the note and
the call frame.
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/sys_plan9_386.s
File src/pkg/runtime/sys_plan9_386.s (right):
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/sys_plan9_386.s#newcode135
src/pkg/runtime/sys_plan9_386.s:135: SUBL $90, SP
90 is a strange choice, especially since it is not a multiple of 4.
It looks like 24 would work.
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/sys_plan9_amd64.s
File src/pkg/runtime/sys_plan9_amd64.s (right):
https://codereview.appspot.com/6569068/diff/18001/src/pkg/runtime/sys_plan9_amd64.s#newcode146
src/pkg/runtime/sys_plan9_amd64.s:146: SUBQ $90, SP
Same comment. Looks like you need $40.
https://codereview.appspot.com/6569068/
