diff --git a/CHANGES b/CHANGES index 804f1cb..5dde7e1 100644 --- a/CHANGES +++ b/CHANGES @@ -12,6 +12,96 @@ various other people, all of whom are (hopefully) listed below. ------------------------------------------------------------ +OS/161 2.0.3 released 20160124 +------------------------------ + +20170124 dholland in base + - Remove obsolete, redundant, or not useful test programs: + guzzle (same as hog) + kitchen (equivalent to multiexec -n 4 sink) + sink (same as conman) + sty (equivalent to multiexec -n 6 hog) + quinthuge (offers little over triplehuge, can be done with multiexec) + quintmat, quintsort (ditto) + +20170118 dholland in base + - Make -g -Og the flags when "debug" is enabled in a kernel config + (which it is in the non-OPT ones) and add an additional kernel + config verb "debugonly" to get just -g in case that becomes + necessary. This should significantly improve the output code + quality from gcc without compromising debugging. (However, gcc + being gcc, it also sometimes leads to additional spurious + warnings that don't occur with either -g or -O2.) + +20170118 dholland in base + - Add a MI mainbus_debugger() function that goes through the right + MD paths to trigger the debugger hook in the ltrace device. Also + add a menu function "debug" to trigger it. + +20170118 dholland in base + - Add some bits to forktest to try to catch the case where the fork + child returns from the next system call instead of from fork. + (Which is a moderately common bug, caused by races copying the + trapframe information in the kernel.) + +20170117 dholland in base + - Add a menu command "deadlock" to intentionally deadlock. + +20170117 dholland in base, from Sam Fishman + - Fix parallelvm -w so that if one of the forks fails the whole + thing doesn't wedge. + +20170117 dholland in base, reported by Sam Fishman + - Don't do semfs I/O from NULL, or from/to insufficiently sized + buffers. Like the 20150615 change, except covering the rest of + the tests that use the semaphores that were doing it wrong: + multiexec, parallelvm, and schedpong. + +20170117 dholland in base, from Sam Fishman + - Add assembler directives to exception-mips1.S that tell gdb how + to read trap frames correctly. Garbage-collect old stuff left + over from making it work with a (much) older version of gdb a + long time back. This also usually makes it possible to trace back + through a syscall into a userlevel process; include a gdb script + with tools for making this useful. + +20170117 dholland in base + - Merge the deadlock detector into base. It was a success last year. + - Mention in the comments that the hangman hooks in locks need to + be called atomically. + +20170117 dholland in base, reported by Jeffrey Cai, patch from Sam Fishman + - Fix off-by-one in tac that makes it skip the first line of files. + +20170117 dholland in base, from Sam Fishman + - Make badcall's "pipe with unaligned pointer" test clean up after + itself if the operation succeeds. Otherwise it leaks fds and that + can intefere with other tests. + +20170116 dholland in base, reported by Sam Fishman + - Don't allow opening an entirely empty pathname to succeed, and + don't allow success for this case in badcall either. + +20170116 dholland in base, from Sasha Fedorova + - Fix write buffer size in filetest. + +20160325 dholland in base + - Fix macro parenthesis bug in ROUNDUP(). + +20160304 dholland in base, from Sam Fishman + - Make runtest.py handle spacing in the command strings it's given. + +20160216 dholland in base + - Fix spacing problems in ls -l output for large files. + +20160203 dholland in base + - Expand comments attached to cpustacks[]/cputhreads[], prompted by + James Mickens. + +20160125 dholland in base, from Nikhil Benesch. + - Fix stupid argument handling bug in test.py. + + OS/161 2.0.2 released 20160112 ------------------------------ @@ -53,6 +143,14 @@ OS/161 2.0.2 released 20160112 anyway. If they aren't actually constant because of bugs, reading a stale or even garbage value is not going to hurt more. +20160107 dholland in deadlock-detector + - Add a deadlock detector. For now this will be supplied to + instructors as a supplementary patch, because it intrudes into + the synchronization primitives and affects what students do + there. We are planning to try it on our students this coming + semester; if that works out well, I'll probably merge it into + base. + 20160107 dholland in base - In testbin/multiexec, if fork fails partway through, continue with the forks we got. Otherwise the subprocesses we started hang diff --git a/kern/arch/mips/include/trapframe.h b/kern/arch/mips/include/trapframe.h index 19d08bb..931ed89 100644 --- a/kern/arch/mips/include/trapframe.h +++ b/kern/arch/mips/include/trapframe.h @@ -69,8 +69,6 @@ struct trapframe { uint32_t tf_s7; uint32_t tf_t8; uint32_t tf_t9; - uint32_t tf_k0; /* dummy (see exception-mips1.S comments) */ - uint32_t tf_k1; /* dummy */ uint32_t tf_gp; uint32_t tf_sp; uint32_t tf_s8; diff --git a/kern/arch/mips/locore/exception-mips1.S b/kern/arch/mips/locore/exception-mips1.S index ad99c2a..f0fec13 100644 --- a/kern/arch/mips/locore/exception-mips1.S +++ b/kern/arch/mips/locore/exception-mips1.S @@ -101,6 +101,8 @@ mips_general_end: .text .type common_exception,@function .ent common_exception + .cfi_startproc + .cfi_signal_frame common_exception: mfc0 k0, c0_status /* Get status register */ andi k0, k0, CST_KUp /* Check the we-were-in-user-mode bit */ @@ -130,11 +132,12 @@ common_exception: */ /* - * Allocate stack space for 37 words to hold the trap frame, + * Allocate stack space for 35 words to hold the trap frame, * plus four more words for a minimal argument block, plus * one more for proper (64-bit) stack alignment. */ - addi sp, sp, -168 + addi sp, sp, -160 + .cfi_def_cfa sp, 0 /* * Save general registers. @@ -143,70 +146,89 @@ common_exception: * * The order here must match mips/include/trapframe.h. * - * gdb disassembles this code to try to figure out what registers - * are where, and it isn't very bright. So in order to make gdb be - * able to trace the stack back through here, we play some silly - * games. + * gdb uses the .cfi_offset assembler directives inserted below to + * to figure out where each register is stored. Since we've marked + * this function as a "signal handler" with the .cfi_signal_frame + * directive, gdb won't complain about the fact that the stack + * is noncontiguous (if we're coming from userland). * - * In particular: - * (1) We store the return address register into the epc slot, - * which makes gdb think it's the return address slot. Then - * we store the real epc value over that. - * (2) We store the current sp into the sp slot, which makes gdb - * think it's the stack pointer slot. Then we store the real - * value. - * (3) gdb also assumes that saved registers in a function are - * saved in order. This is why we put epc where it is, and - * handle the real value of ra afterwards. - * (4) Because gdb will think we're saving k0 and k1, we need to - * leave slots for them in the trap frame, even though the - * stuff we save there is useless. + * We also play a trick with the return address: we mark the ra + * register as stored to the stack normally and then mark the + * return address for *this* function as being in the k1 register + * using the .cfi_return_column directive. gdb is then able to + * recognize that the ra we've stored here is the return address + * for the function that was executing when this exception was + * taken. * - * This logic has not been tested against a recent gdb and has - * probably bitrotted. Someone(TM) should figure out what gdb - * currently expects -- or maybe even patch gdb to understand a - * better form of this that doesn't waste so many cycles. + * All of the cfi (call frame information) material is compiled + * into the .eh_frame section of the compiled kernel. */ - sw ra, 160(sp) /* dummy for gdb */ - sw s8, 156(sp) /* save s8 */ - sw sp, 152(sp) /* dummy for gdb */ - sw gp, 148(sp) /* save gp */ - sw k1, 144(sp) /* dummy for gdb */ - sw k0, 140(sp) /* dummy for gdb */ - - sw k1, 152(sp) /* real saved sp */ + sw s8, 148(sp) /* save s8 */ + .cfi_offset s8, 148 + sw k1, 144(sp) /* real saved sp */ + .cfi_offset sp, 144 + sw gp, 140(sp) /* save gp */ nop /* delay slot for store */ + .cfi_offset gp, 140 + .cfi_return_column k1 mfc0 k1, c0_epc /* Copr.0 reg 13 == PC for exception */ - sw k1, 160(sp) /* real saved PC */ + sw k1, 152(sp) /* real saved PC */ + .cfi_offset k1, 152 sw t9, 136(sp) + .cfi_offset t9, 136 sw t8, 132(sp) + .cfi_offset t8, 132 sw s7, 128(sp) + .cfi_offset s7, 128 sw s6, 124(sp) + .cfi_offset s6, 124 sw s5, 120(sp) + .cfi_offset s5, 120 sw s4, 116(sp) + .cfi_offset s4, 116 sw s3, 112(sp) + .cfi_offset s3, 112 sw s2, 108(sp) + .cfi_offset s2, 108 sw s1, 104(sp) + .cfi_offset s1, 104 sw s0, 100(sp) + .cfi_offset s0, 100 sw t7, 96(sp) + .cfi_offset t7, 96 sw t6, 92(sp) + .cfi_offset t6, 92 sw t5, 88(sp) + .cfi_offset t5, 88 sw t4, 84(sp) + .cfi_offset t4, 84 sw t3, 80(sp) + .cfi_offset t3, 80 sw t2, 76(sp) + .cfi_offset t2, 76 sw t1, 72(sp) + .cfi_offset t1, 72 sw t0, 68(sp) + .cfi_offset t0, 68 sw a3, 64(sp) + .cfi_offset a3, 64 sw a2, 60(sp) + .cfi_offset a2, 60 sw a1, 56(sp) + .cfi_offset a1, 56 sw a0, 52(sp) + .cfi_offset a0, 52 sw v1, 48(sp) + .cfi_offset v1, 48 sw v0, 44(sp) + .cfi_offset v0, 44 sw AT, 40(sp) + .cfi_offset AT, 40 sw ra, 36(sp) + .cfi_offset ra, 36 /* * Save special registers. @@ -227,11 +249,6 @@ common_exception: mfc0 t4, c0_cause sw t4, 24(sp) /* Copr.0 reg 13 == exception cause */ - /* - * Pretend to save $0 for gdb's benefit. - */ - sw $0, 12(sp) - /* * Load the curthread register if coming from user mode. */ @@ -260,9 +277,6 @@ common_exception: jal mips_trap /* call it */ nop /* delay slot */ - /* Something must be here or gdb doesn't find the stack frame. */ - nop - /* * Now restore stuff and return from the exception. * Interrupts should be off. @@ -309,20 +323,17 @@ exception_return: lw s7, 128(sp) lw t8, 132(sp) lw t9, 136(sp) + lw gp, 140(sp) /* restore gp */ + /* 144(sp) stack pointer - below */ + lw s8, 148(sp) /* restore s8 */ + lw k1, 152(sp) /* fetch exception return PC into k1 */ - /* 140(sp) "saved" k0 was dummy garbage anyway */ - /* 144(sp) "saved" k1 was dummy garbage anyway */ - - lw gp, 148(sp) /* restore gp */ - /* 152(sp) stack pointer - below */ - lw s8, 156(sp) /* restore s8 */ - lw k0, 160(sp) /* fetch exception return PC into k0 */ - - lw sp, 152(sp) /* fetch saved sp (must be last) */ + lw sp, 144(sp) /* fetch saved sp (must be last) */ /* done */ - jr k0 /* jump back */ + jr k1 /* jump back */ rfe /* in delay slot */ + .cfi_endproc .end common_exception /* diff --git a/kern/arch/mips/locore/trap.c b/kern/arch/mips/locore/trap.c index d3873eb..69ae12c 100644 --- a/kern/arch/mips/locore/trap.c +++ b/kern/arch/mips/locore/trap.c @@ -130,8 +130,8 @@ mips_trap(struct trapframe *tf) bool iskern; int spl; - /* The trap frame is supposed to be 37 registers long. */ - KASSERT(sizeof(struct trapframe)==(37*4)); + /* The trap frame is supposed to be 35 registers long. */ + KASSERT(sizeof(struct trapframe)==(35*4)); /* * Extract the exception code info from the register fields. diff --git a/kern/arch/mips/thread/cpu.c b/kern/arch/mips/thread/cpu.c index 5acce5b..d43a4ec 100644 --- a/kern/arch/mips/thread/cpu.c +++ b/kern/arch/mips/thread/cpu.c @@ -54,6 +54,14 @@ * * These arrays are also used to start up new CPUs, for roughly the * same reasons. + * + * The values in the current cpu's slots in these arrays are updated + * with the current thread's information in trap.c before heading to + * userlevel, as well as being initialized in cpu_machdep_init below. + * This means that (unless something really horrible happens) on entry + * to the kernel, and when a new CPU starts up in cpu_start_secondary, + * they will have the information needed to figure out who we are and + * proceed. */ vaddr_t cpustacks[MAXCPUS]; diff --git a/kern/arch/sys161/dev/lamebus_machdep.c b/kern/arch/sys161/dev/lamebus_machdep.c index c6c62ce..0cc1fb2 100644 --- a/kern/arch/sys161/dev/lamebus_machdep.c +++ b/kern/arch/sys161/dev/lamebus_machdep.c @@ -42,6 +42,7 @@ #include #include #include +#include #include "autoconf.h" /* @@ -275,6 +276,15 @@ mainbus_send_ipi(struct cpu *target) lamebus_assert_ipi(lamebus, target); } +/* + * Trigger the debugger. + */ +void +mainbus_debugger(void) +{ + ltrace_stop(0); +} + /* * Interrupt dispatcher. */ diff --git a/kern/conf/DUMBVM b/kern/conf/DUMBVM index 3216e1f..d658cf6 100644 --- a/kern/conf/DUMBVM +++ b/kern/conf/DUMBVM @@ -3,7 +3,9 @@ include conf/conf.kern # get definitions of available options -debug # Compile with debug info. +debug # Compile with debug info and -Og. +#debugonly # Compile with debug info only (no -Og). +options hangman # Deadlock detection. (off by default) # # Device drivers for hardware. diff --git a/kern/conf/DUMBVM-OPT b/kern/conf/DUMBVM-OPT index e878c35..115016f 100644 --- a/kern/conf/DUMBVM-OPT +++ b/kern/conf/DUMBVM-OPT @@ -7,6 +7,7 @@ include conf/conf.kern # get definitions of available options #debug # Optimizing compile (no debug). +#debugonly options noasserts # Disable assertions. # diff --git a/kern/conf/GENERIC b/kern/conf/GENERIC index 13ff40e..a726c3c 100644 --- a/kern/conf/GENERIC +++ b/kern/conf/GENERIC @@ -5,6 +5,8 @@ include conf/conf.kern # get definitions of available options debug # Compile with debug info. +#debugonly # Compile with debug info only (no -Og). +#options hangman # Deadlock detection. (off by default) # # Device drivers for hardware. diff --git a/kern/conf/GENERIC-OPT b/kern/conf/GENERIC-OPT index f78637d..7bdc740 100644 --- a/kern/conf/GENERIC-OPT +++ b/kern/conf/GENERIC-OPT @@ -7,6 +7,7 @@ include conf/conf.kern # get definitions of available options #debug # Optimizing compile (no debug). +#debugonly options noasserts # Disable assertions. # diff --git a/kern/conf/conf.kern b/kern/conf/conf.kern index 5e5f927..57b699d 100644 --- a/kern/conf/conf.kern +++ b/kern/conf/conf.kern @@ -334,6 +334,9 @@ file thread/synch.c file thread/thread.c file thread/threadlist.c +defoption hangman +optfile hangman thread/hangman.c + # # Process system # diff --git a/kern/conf/config b/kern/conf/config index ddbb50a..2946a1e 100755 --- a/kern/conf/config +++ b/kern/conf/config @@ -12,7 +12,8 @@ # Recognized directives: # # file use source file -# debug turn on debug info +# debug turn on debug info and -Og +# debugonly turn on debug info without -Og # defoption define an option # optfile if option is enabled, use file # optofffile if option is disabled, use file @@ -97,6 +98,7 @@ echo "$CONFNAME" $CONFTMP | awk ' nfields["include"] = 2; nfields["file"] = 2; nfields["debug"] = 1; + nfields["debugonly"] = 1; nfields["defoption"] = 2; nfields["optfile"] = 3; nfields["optofffile"] = 3; @@ -776,6 +778,9 @@ echo -n ' files.mk' # Default: optimize. BEGIN { debugflags="-O2"; } $1=="debug" { + debugflags="-g -Og"; + } + $1=="debugonly" { debugflags="-g"; } diff --git a/kern/gdbscripts/mips-userland b/kern/gdbscripts/mips-userland new file mode 100644 index 0000000..0485261 --- /dev/null +++ b/kern/gdbscripts/mips-userland @@ -0,0 +1,40 @@ +# commands to interact with userland + +define load-userprogram + init-if-undefined $userprog_loaded = 0 + if !$userprog_loaded || !$_streq($arg0, $cur_userprog) + add-symbol-file $arg0 0x4000b0 + # remove after adding the new file in case add-symbol-file fails + if $userprog_loaded + remove-symbol-file -a 0x4000b0 + end + set $userprog_loaded = 1 + set $cur_userprog = $arg0 + end +end +document load-userprogram +Loads a single user program into gdb's symbol table, removing +the previous user program loaded if one exists. +Usage: load-userprogram +end +alias -a lu = load-userprogram + +define auto-userprogram + set $pname = curthread->t_proc->p_name + if $pname != 0 && $pname[0] != 0 && !$_streq($pname, "[kernel]") + if $pname[0] == '/' + set $pname = $pname + 1 + end + # This dumbness works around the fact that gdb doesn't + # expand convenience variables when you execute commands. + # And setting the third parameter to True silences the command. + python gdb.execute("load-userprogram " + str(gdb.parse_and_eval("$pname")).split()[1], False, True) + end +end +document auto-userprogram +Calls load-userprogram on whatever is curproc->p_name. It is meant +to work with kernels that set p_name to an absolute path to the +program, and it is designed to be called from hook-stop. +Usage: auto-userprogram +end + diff --git a/kern/include/cpu.h b/kern/include/cpu.h index d628688..dbcb717 100644 --- a/kern/include/cpu.h +++ b/kern/include/cpu.h @@ -88,6 +88,11 @@ struct cpu { struct tlbshootdown c_shootdown[TLBSHOOTDOWN_MAX]; unsigned c_numshootdown; struct spinlock c_ipi_lock; + + /* + * Accessed by other cpus. Protected inside hangman.c. + */ + HANGMAN_ACTOR(c_hangman); }; /* diff --git a/kern/include/hangman.h b/kern/include/hangman.h new file mode 100644 index 0000000..91f3de2 --- /dev/null +++ b/kern/include/hangman.h @@ -0,0 +1,84 @@ +/* + * Copyright (c) 2015 + * The President and Fellows of Harvard College. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the University nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE UNIVERSITY AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE UNIVERSITY OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#ifndef HANGMAN_H +#define HANGMAN_H + +/* + * Simple deadlock detector. Enable with "options hangman" in the + * kernel config. + */ + +#include "opt-hangman.h" + +#if OPT_HANGMAN + +struct hangman_actor { + const char *a_name; + const struct hangman_lockable *a_waiting; +}; + +struct hangman_lockable { + const char *l_name; + const struct hangman_actor *l_holding; +}; + +void hangman_wait(struct hangman_actor *a, struct hangman_lockable *l); +void hangman_acquire(struct hangman_actor *a, struct hangman_lockable *l); +void hangman_release(struct hangman_actor *a, struct hangman_lockable *l); + +#define HANGMAN_ACTOR(sym) struct hangman_actor sym +#define HANGMAN_LOCKABLE(sym) struct hangman_lockable sym + +#define HANGMAN_ACTORINIT(a, n) ((a)->a_name = (n), (a)->a_waiting = NULL) +#define HANGMAN_LOCKABLEINIT(l, n) ((l)->l_name = (n), (l)->l_holding = NULL) + +#define HANGMAN_LOCKABLE_INITIALIZER { "spinlock", NULL } + +#define HANGMAN_WAIT(a, l) hangman_wait(a, l) +#define HANGMAN_ACQUIRE(a, l) hangman_acquire(a, l) +#define HANGMAN_RELEASE(a, l) hangman_release(a, l) + +#else + +#define HANGMAN_ACTOR(sym) +#define HANGMAN_LOCKABLE(sym) + +#define HANGMAN_ACTORINIT(a, name) +#define HANGMAN_LOCKABLEINIT(a, name) + +#define HANGMAN_LOCKABLE_INITIALIZER + +#define HANGMAN_WAIT(a, l) +#define HANGMAN_ACQUIRE(a, l) +#define HANGMAN_RELEASE(a, l) + +#endif + +#endif /* HANGMAN_H */ diff --git a/kern/include/lib.h b/kern/include/lib.h index 5374a96..2c6e70f 100644 --- a/kern/include/lib.h +++ b/kern/include/lib.h @@ -194,7 +194,7 @@ void kprintf_bootstrap(void); */ #define DIVROUNDUP(a,b) (((a)+(b)-1)/(b)) -#define ROUNDUP(a,b) (DIVROUNDUP(a,b)*b) +#define ROUNDUP(a,b) (DIVROUNDUP(a,b)*(b)) #endif /* _LIB_H_ */ diff --git a/kern/include/mainbus.h b/kern/include/mainbus.h index 3ebbdea..4d7fff0 100644 --- a/kern/include/mainbus.h +++ b/kern/include/mainbus.h @@ -55,6 +55,9 @@ size_t mainbus_ramsize(void); /* Switch on an inter-processor interrupt. (Low-level.) */ void mainbus_send_ipi(struct cpu *target); +/* Request breaking into the debugger, where available. */ +void mainbus_debugger(void); + /* * The various ways to shut down the system. (These are very low-level * and should generally not be called directly - md_poweroff, for diff --git a/kern/include/spinlock.h b/kern/include/spinlock.h index d3b0ca0..e28ec90 100644 --- a/kern/include/spinlock.h +++ b/kern/include/spinlock.h @@ -36,6 +36,7 @@ */ #include +#include /* Inlining support - for making sure an out-of-line copy gets built */ #ifndef SPINLOCK_INLINE @@ -57,12 +58,18 @@ struct spinlock { volatile spinlock_data_t splk_lock; /* Memory word where we spin. */ struct cpu *splk_holder; /* CPU holding this lock. */ + HANGMAN_LOCKABLE(splk_hangman); /* Deadlock detector hook. */ }; /* * Initializer for cases where a spinlock needs to be static or global. */ +#ifdef OPT_HANGMAN +#define SPINLOCK_INITIALIZER { SPINLOCK_DATA_INITIALIZER, NULL, \ + HANGMAN_LOCKABLE_INITIALIZER } +#else #define SPINLOCK_INITIALIZER { SPINLOCK_DATA_INITIALIZER, NULL } +#endif /* * Spinlock functions. diff --git a/kern/include/synch.h b/kern/include/synch.h index 59058d0..a5a4681 100644 --- a/kern/include/synch.h +++ b/kern/include/synch.h @@ -74,6 +74,7 @@ void V(struct semaphore *); */ struct lock { char *lk_name; + HANGMAN_LOCKABLE(lk_hangman); /* Deadlock detector hook. */ // add what you need here // (don't forget to mark things volatile as needed) }; diff --git a/kern/include/thread.h b/kern/include/thread.h index 04a46d3..ff07544 100644 --- a/kern/include/thread.h +++ b/kern/include/thread.h @@ -93,6 +93,7 @@ struct thread { struct switchframe *t_context; /* Saved register context (on stack) */ struct cpu *t_cpu; /* CPU thread runs on */ struct proc *t_proc; /* Process thread belongs to */ + HANGMAN_ACTOR(t_hangman); /* Deadlock detector hook */ /* * Interrupt state fields. diff --git a/kern/include/version.h b/kern/include/version.h index 43f66be..4d9fa1c 100644 --- a/kern/include/version.h +++ b/kern/include/version.h @@ -34,7 +34,7 @@ * Leave this alone, so we can tell what version of the OS/161 base * code we gave you. */ -#define BASE_VERSION "2.0.2" +#define BASE_VERSION "2.0.3" /* * Change this as you see fit in the course of hacking the system. diff --git a/kern/main/menu.c b/kern/main/menu.c index 2451b13..c104187 100644 --- a/kern/main/menu.c +++ b/kern/main/menu.c @@ -35,6 +35,8 @@ #include #include #include +#include +#include #include #include #include @@ -247,6 +249,21 @@ cmd_sync(int nargs, char **args) return 0; } +/* + * Command for dropping to the debugger. + */ +static +int +cmd_debug(int nargs, char **args) +{ + (void)nargs; + (void)args; + + mainbus_debugger(); + + return 0; +} + /* * Command for doing an intentional panic. */ @@ -261,6 +278,81 @@ cmd_panic(int nargs, char **args) return 0; } +/* + * Subthread for intentially deadlocking. + */ +struct deadlock { + struct lock *lock1; + struct lock *lock2; +}; + +static +void +cmd_deadlockthread(void *ptr, unsigned long num) +{ + struct deadlock *dl = ptr; + + (void)num; + + /* If it doesn't wedge right away, keep trying... */ + while (1) { + lock_acquire(dl->lock2); + lock_acquire(dl->lock1); + kprintf("+"); + lock_release(dl->lock1); + lock_release(dl->lock2); + } +} + +/* + * Command for doing an intentional deadlock. + */ +static +int +cmd_deadlock(int nargs, char **args) +{ + struct deadlock dl; + int result; + + (void)nargs; + (void)args; + + dl.lock1 = lock_create("deadlock1"); + if (dl.lock1 == NULL) { + kprintf("lock_create failed\n"); + return ENOMEM; + } + dl.lock2 = lock_create("deadlock2"); + if (dl.lock2 == NULL) { + lock_destroy(dl.lock1); + kprintf("lock_create failed\n"); + return ENOMEM; + } + + result = thread_fork(args[0] /* thread name */, + NULL /* kernel thread */, + cmd_deadlockthread /* thread function */, + &dl /* thread arg */, 0 /* thread arg */); + if (result) { + kprintf("thread_fork failed: %s\n", strerror(result)); + lock_release(dl.lock1); + lock_destroy(dl.lock2); + lock_destroy(dl.lock1); + return result; + } + + /* If it doesn't wedge right away, keep trying... */ + while (1) { + lock_acquire(dl.lock1); + lock_acquire(dl.lock2); + kprintf("."); + lock_release(dl.lock2); + lock_release(dl.lock1); + } + /* NOTREACHED */ + return 0; +} + /* * Command for shutting down. */ @@ -463,7 +555,9 @@ static const char *opsmenu[] = { "[cd] Change directory ", "[pwd] Print current directory ", "[sync] Sync filesystems ", + "[debug] Drop to debugger ", "[panic] Intentional panic ", + "[deadlock] Intentional deadlock ", "[q] Quit and shut down ", NULL }; @@ -614,7 +708,9 @@ static struct { { "cd", cmd_chdir }, { "pwd", cmd_pwd }, { "sync", cmd_sync }, + { "debug", cmd_debug }, { "panic", cmd_panic }, + { "deadlock", cmd_deadlock }, { "q", cmd_quit }, { "exit", cmd_quit }, { "halt", cmd_quit }, diff --git a/kern/test/arraytest.c b/kern/test/arraytest.c index a63405f..b55d3a7 100644 --- a/kern/test/arraytest.c +++ b/kern/test/arraytest.c @@ -156,6 +156,9 @@ arraytest2(int nargs, char **args) (void)nargs; (void)args; + /* Silence warning with gcc 4.8 -Og (but not -O2) */ + x = 0; + kprintf("Beginning large array test...\n"); a = array_create(); KASSERT(a != NULL); diff --git a/kern/test/fstest.c b/kern/test/fstest.c index 9120920..1617027 100644 --- a/kern/test/fstest.c +++ b/kern/test/fstest.c @@ -681,7 +681,7 @@ checkfilesystem(int nargs, char **args) char *device; if (nargs != 2) { - kprintf("Usage: fs[12345] filesystem:\n"); + kprintf("Usage: fs[123456] filesystem:\n"); return EINVAL; } diff --git a/kern/thread/hangman.c b/kern/thread/hangman.c new file mode 100644 index 0000000..127167a --- /dev/null +++ b/kern/thread/hangman.c @@ -0,0 +1,182 @@ +/* + * Copyright (c) 2015 + * The President and Fellows of Harvard College. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the University nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE UNIVERSITY AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE UNIVERSITY OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +/* + * Simple deadlock detector. + */ + +#include +#include +#include +#include +#include + +static struct spinlock hangman_lock = SPINLOCK_INITIALIZER; + +/* + * Look for a path through the waits-for graph that goes from START to + * TARGET. + * + * Because lockables can only be held by one actor, and actors can + * only be waiting for one thing at a time, this turns out to be + * quite simple. + */ +static +void +hangman_check(const struct hangman_lockable *start, + const struct hangman_actor *target) +{ + const struct hangman_actor *cur; + + cur = start->l_holding; + while (cur != NULL) { + if (cur == target) { + goto found; + } + if (cur->a_waiting == NULL) { + break; + } + cur = cur->a_waiting->l_holding; + } + return; + + found: + /* + * None of this can change while we print it (that's the point + * of it being a deadlock) so drop hangman_lock while + * printing; otherwise we can come back via kprintf_spinlock + * and that makes a mess. But force splhigh() explicitly so + * the console prints in polled mode and to discourage other + * things from running in the middle of the printout. + */ + splhigh(); + spinlock_release(&hangman_lock); + + kprintf("hangman: Detected lock cycle!\n"); + kprintf("hangman: in %s (%p);\n", target->a_name, target); + kprintf("hangman: waiting for %s (%p), but:\n", start->l_name, start); + kprintf(" lockable %s (%p)\n", start->l_name, start); + cur = start->l_holding; + while (cur != target) { + kprintf(" held by actor %s (%p)\n", cur->a_name, cur); + kprintf(" waiting for lockable %s (%p)\n", + cur->a_waiting->l_name, cur->a_waiting); + cur = cur->a_waiting->l_holding; + } + kprintf(" held by actor %s (%p)\n", cur->a_name, cur); + panic("Deadlock.\n"); +} + +/* + * Note that a is about to wait for l. + * + * Note that there's no point calling this if a isn't going to wait, + * because in that case l->l_holding will be null and the check + * won't go anywhere. + * + * One could also maintain in memory a graph of all requests ever + * seen, in order to detect lock order inversions that haven't + * actually deadlocked; but there are a lot of ways in which this is + * tricky and problematic. For now we'll settle for just detecting and + * reporting deadlocks that do happen. + */ +void +hangman_wait(struct hangman_actor *a, + struct hangman_lockable *l) +{ + if (l == &hangman_lock.splk_hangman) { + /* don't recurse */ + return; + } + + spinlock_acquire(&hangman_lock); + + if (a->a_waiting != NULL) { + spinlock_release(&hangman_lock); + panic("hangman_wait: already waiting for something?\n"); + } + + hangman_check(l, a); + a->a_waiting = l; + + spinlock_release(&hangman_lock); +} + +void +hangman_acquire(struct hangman_actor *a, + struct hangman_lockable *l) +{ + if (l == &hangman_lock.splk_hangman) { + /* don't recurse */ + return; + } + + spinlock_acquire(&hangman_lock); + + if (a->a_waiting != l) { + spinlock_release(&hangman_lock); + panic("hangman_acquire: not waiting for lock %s (%p)\n", + l->l_name, l); + } + if (l->l_holding != NULL) { + spinlock_release(&hangman_lock); + panic("hangman_acquire: lock %s (%p) still held by %s (%p)\n", + l->l_name, l, a->a_name, a); + } + + l->l_holding = a; + a->a_waiting = NULL; + + spinlock_release(&hangman_lock); +} + +void +hangman_release(struct hangman_actor *a, + struct hangman_lockable *l) +{ + if (l == &hangman_lock.splk_hangman) { + /* don't recurse */ + return; + } + + spinlock_acquire(&hangman_lock); + + if (a->a_waiting != NULL) { + spinlock_release(&hangman_lock); + panic("hangman_release: waiting for something?\n"); + } + if (l->l_holding != a) { + spinlock_release(&hangman_lock); + panic("hangman_release: not the holder\n"); + } + + l->l_holding = NULL; + + spinlock_release(&hangman_lock); +} diff --git a/kern/thread/spinlock.c b/kern/thread/spinlock.c index a7b50b1..0cdf9aa 100644 --- a/kern/thread/spinlock.c +++ b/kern/thread/spinlock.c @@ -52,6 +52,7 @@ spinlock_init(struct spinlock *splk) { spinlock_data_set(&splk->splk_lock, 0); splk->splk_holder = NULL; + HANGMAN_LOCKABLEINIT(&splk->splk_hangman, "spinlock"); } /* @@ -85,6 +86,8 @@ spinlock_acquire(struct spinlock *splk) panic("Deadlock on spinlock %p\n", splk); } mycpu->c_spinlocks++; + + HANGMAN_WAIT(&curcpu->c_hangman, &splk->splk_hangman); } else { mycpu = NULL; @@ -112,6 +115,10 @@ spinlock_acquire(struct spinlock *splk) membar_store_any(); splk->splk_holder = mycpu; + + if (CURCPU_EXISTS()) { + HANGMAN_ACQUIRE(&curcpu->c_hangman, &splk->splk_hangman); + } } /* @@ -125,6 +132,7 @@ spinlock_release(struct spinlock *splk) KASSERT(splk->splk_holder == curcpu->c_self); KASSERT(curcpu->c_spinlocks > 0); curcpu->c_spinlocks--; + HANGMAN_RELEASE(&curcpu->c_hangman, &splk->splk_hangman); } splk->splk_holder = NULL; diff --git a/kern/thread/synch.c b/kern/thread/synch.c index b7a08d9..94960a7 100644 --- a/kern/thread/synch.c +++ b/kern/thread/synch.c @@ -154,6 +154,8 @@ lock_create(const char *name) return NULL; } + HANGMAN_LOCKABLEINIT(&lock->lk_hangman, lock->lk_name); + // add stuff here as needed return lock; @@ -173,14 +175,23 @@ lock_destroy(struct lock *lock) void lock_acquire(struct lock *lock) { + /* Call this (atomically) before waiting for a lock */ + //HANGMAN_WAIT(&curthread->t_hangman, &lock->lk_hangman); + // Write this (void)lock; // suppress warning until code gets written + + /* Call this (atomically) once the lock is acquired */ + //HANGMAN_ACQUIRE(&curthread->t_hangman, &lock->lk_hangman); } void lock_release(struct lock *lock) { + /* Call this (atomically) when the lock is released */ + //HANGMAN_RELEASE(&curthread->t_hangman, &lock->lk_hangman); + // Write this (void)lock; // suppress warning until code gets written diff --git a/kern/thread/thread.c b/kern/thread/thread.c index 36e7e24..062cf82 100644 --- a/kern/thread/thread.c +++ b/kern/thread/thread.c @@ -145,6 +145,7 @@ thread_create(const char *name) thread->t_context = NULL; thread->t_cpu = NULL; thread->t_proc = NULL; + HANGMAN_ACTORINIT(&thread->t_hangman, thread->t_name); /* Interrupt state fields */ thread->t_in_interrupt = false; @@ -244,6 +245,8 @@ cpu_create(unsigned hardware_number) curcpu->c_curthread = curthread; } + HANGMAN_ACTORINIT(&c->c_hangman, "cpu"); + result = proc_addthread(kproc, c->c_curthread); if (result) { panic("cpu_create: proc_addthread:: %s\n", strerror(result)); diff --git a/kern/vfs/vfslist.c b/kern/vfs/vfslist.c index 9cd2f4a..64f6a9d 100644 --- a/kern/vfs/vfslist.c +++ b/kern/vfs/vfslist.c @@ -129,6 +129,23 @@ vfs_biglock_acquire(void) if (!lock_do_i_hold(vfs_biglock)) { lock_acquire(vfs_biglock); } + else if (vfs_biglock_depth == 0) { + /* + * Supposedly we hold it, but the depth is 0. This may + * mean: (1) the count is messed up, or (2) + * lock_do_i_hold is lying. Since OS/161 ships out of + * the box with unimplemented locks (students + * implement them) that always return true, assume + * situation (2). In this case acquire the lock + * anyway. + * + * Once you have working locks, this won't be the + * case, and if you get here it should be situation + * (1), in which case the count is messed up and one + * can panic. + */ + lock_acquire(vfs_biglock); + } vfs_biglock_depth++; } @@ -381,6 +398,9 @@ vfs_doadd(const char *dname, int mountable, struct device *dev, struct fs *fs) unsigned index; int result; + /* Silence warning with gcc 4.8 -Og (but not -O2) */ + index = 0; + vfs_biglock_acquire(); name = kstrdup(dname); diff --git a/kern/vfs/vfslookup.c b/kern/vfs/vfslookup.c index 773cbcd..afeacea 100644 --- a/kern/vfs/vfslookup.c +++ b/kern/vfs/vfslookup.c @@ -135,6 +135,13 @@ getdevice(char *path, char **subpath, struct vnode **startvn) KASSERT(vfs_biglock_do_i_hold()); + /* + * Entirely empty filenames aren't legal. + */ + if (path[0] == 0) { + return EINVAL; + } + /* * Locate the first colon or slash. */ diff --git a/kern/vm/kmalloc.c b/kern/vm/kmalloc.c index 13684a7..23bf72e 100644 --- a/kern/vm/kmalloc.c +++ b/kern/vm/kmalloc.c @@ -1113,6 +1113,10 @@ subpage_kfree(void *ptr) checksubpages(); + /* Silence warnings with gcc 4.8 -Og (but not -O2) */ + prpage = 0; + blktype = 0; + for (pr = allbase; pr; pr = pr->next_all) { prpage = PR_PAGEADDR(pr); blktype = PR_BLOCKTYPE(pr); diff --git a/mk/os161.mkdirs.mk b/mk/os161.mkdirs.mk index 8e4fe40..370eb87 100644 --- a/mk/os161.mkdirs.mk +++ b/mk/os161.mkdirs.mk @@ -1,5 +1,5 @@ # -# MKDIRS logic +# OS/161 build environment: MKDIRS logic # # This generates rules for all intermediate directories as well as # the directories listed. (That is, if you say MKDIRS=/usr/bin/foo diff --git a/mk/os161.script.mk b/mk/os161.script.mk new file mode 100644 index 0000000..6d579c8 --- /dev/null +++ b/mk/os161.script.mk @@ -0,0 +1,92 @@ +# +# OS/161 build environment: install scripts +# +# Usage: +# TOP=../.. +# .include "$(TOP)/mk/os161.config.mk" +# [defs go here] +# .include "$(TOP)/mk/os161.man.mk" +# [any extra rules go here] +# +# Variables controlling this file: +# +# EXECSCRIPTS Executable files to install. +# NONEXECSCRIPTS Non-executable files to install. +# +# SCRIPTDIR Directory under $(OSTREE) to install into, +# e.g. /hostbin. Should have a leading slash. +# + +ALLSCRIPTS=$(NONEXECSCRIPTS) + +# We may want these directories created. (Used by os161.baserules.mk.) +MKDIRS+=$(MYBUILDDIR) +MKDIRS+=$(INSTALLTOP)$(SCRIPTDIR) +MKDIRS+=$(OSTREE)$(SCRIPTDIR) + +# Default rule: "build" the executable scripts. This sets the hash-bang +# header with the interpreter path. +# (In make the first rule found is the default.) +all: all-local +all-local: $(MYBUILDDIR) .WAIT +.for S in $(EXECSCRIPTS) +ALLSCRIPTS+=$(MYBUILDDIR)/$(S) +all-local: $(MYBUILDDIR)/$(S) +$(MYBUILDDIR)/$(S): $(S) +# This must test $@, or assign another variable; it can't test $(S). +# (This is a bmake bug.) +.if !empty(@:M*.py) + echo '#!'"$(PYTHON_INTERPRETER)" > $@.new +.else + sed -n -e '1p' < $(S) > $@.new +.endif + sed -e '1d' < $(S) >> $@.new + chmod 755 $@.new + mv -f $@.new $@ +.endfor + +# depend doesn't need to do anything +depend-local: ; + +# +# Install: we can install into either $(INSTALLTOP) or $(OSTREE). +# When building the whole system, we always install into the staging +# area. However, if you're working on a particular program it is +# usually convenient to be able to install it directly to $(OSTREE) +# instead of doing a complete top-level install. +# +# Note that we make a hard link instead of a copy by default to reduce +# overhead. +# +install-staging-local: $(INSTALLTOP)$(SCRIPTDIR) .WAIT + +.for _F_ in $(ALLSCRIPTS) +install-staging-local: $(INSTALLTOP)$(SCRIPTDIR)/$(_F_:T) +$(INSTALLTOP)$(SCRIPTDIR)/$(_F_:T): $(_F_) + rm -f $(.TARGET) + ln $(_F_) $(.TARGET) || cp $(_F_) $(.TARGET) +.endfor + +install-local: $(OSTREE)$(SCRIPTDIR) .WAIT installscripts +installscripts: +.for _F_ in $(ALLSCRIPTS) + rm -f $(OSTREE)$(SCRIPTDIR)/$(_F_:T) + ln $(_F_) $(OSTREE)$(SCRIPTDIR)/$(_F_:T) || \ + cp $(_F_) $(OSTREE)$(SCRIPTDIR)/$(_F_:T) +.endfor + +# clean: remove build products +clean-local: +.for S in $(EXECSCRIPTS) + rm -f $(MYBUILDDIR)/$(S) +.endfor + +# Mark targets that don't represent files PHONY, to prevent various +# lossage if files by those names appear. +.PHONY: all all-local install-staging-local install-local installscripts +.PHONY: clean-local + +# Finally, get the shared definitions for the most basic rules. +.include "$(TOP)/mk/os161.baserules.mk" + +# End. diff --git a/userland/bin/ls/ls.c b/userland/bin/ls/ls.c index 6dd8c88..2041bc8 100644 --- a/userland/bin/ls/ls.c +++ b/userland/bin/ls/ls.c @@ -177,7 +177,7 @@ print(const char *path) typech = '?'; } - printf("%crwx------ %2d root %-8llu", + printf("%crwx------ %2d root %-7llu ", typech, statbuf.st_nlink, statbuf.st_size); diff --git a/userland/bin/tac/tac.c b/userland/bin/tac/tac.c index 3e0a260..edaa48d 100644 --- a/userland/bin/tac/tac.c +++ b/userland/bin/tac/tac.c @@ -197,11 +197,9 @@ dumpdata(void) indexsize = dolseek(indexfd, indexname, 0, SEEK_CUR); pos = indexsize; - while (1) { + assert(pos % sizeof(x) == 0); + while (pos > 0) { pos -= sizeof(x); - if (pos == 0) { - break; - } assert(pos >= 0); dolseek(indexfd, indexname, pos, SEEK_SET); diff --git a/userland/testbin/badcall/bad_pipe.c b/userland/testbin/badcall/bad_pipe.c index 27f3f68..c059361 100644 --- a/userland/testbin/badcall/bad_pipe.c +++ b/userland/testbin/badcall/bad_pipe.c @@ -69,6 +69,11 @@ pipe_unaligned(void) rv = pipe((int *)ptr); report_survival(rv, errno, &result); + if (rv == 0) { + memmove(fds, ptr, 2*sizeof(int)); + close(fds[0]); + close(fds[1]); + } return result; } diff --git a/userland/testbin/filetest/filetest.c b/userland/testbin/filetest/filetest.c index 7d7d4df..f013470 100644 --- a/userland/testbin/filetest/filetest.c +++ b/userland/testbin/filetest/filetest.c @@ -46,7 +46,7 @@ int main(int argc, char *argv[]) { - static char writebuf[40] = "Twiddle dee dee, Twiddle dum dum.......\n"; + static char writebuf[41] = "Twiddle dee dee, Twiddle dum dum.......\n"; static char readbuf[41]; const char *file; diff --git a/userland/testbin/forktest/forktest.c b/userland/testbin/forktest/forktest.c index bcefe62..a76acb1 100644 --- a/userland/testbin/forktest/forktest.c +++ b/userland/testbin/forktest/forktest.c @@ -152,12 +152,20 @@ void test(int nowait) { int pid0, pid1, pid2, pid3; + int depth = 0; /* * Caution: This generates processes geometrically. * * It is unrolled to encourage gcc to registerize the pids, * to prevent wait/exit problems if fork corrupts memory. + * + * Note: if the depth prints trigger and show that the depth + * is too small, the most likely explanation is that the fork + * child is returning from the write() inside putchar() + * instead of from fork() and thus skipping the depth++. This + * is a fairly common problem caused by races in the kernel + * fork code. */ /* @@ -184,18 +192,37 @@ test(int nowait) pid0 = dofork(); nprintf("."); write(fd, "A", 1); + depth++; + if (depth != 1) { + warnx("depth %d, should be 1", depth); + } check(); + pid1 = dofork(); nprintf("."); write(fd, "B", 1); + depth++; + if (depth != 2) { + warnx("depth %d, should be 2", depth); + } check(); + pid2 = dofork(); nprintf("."); write(fd, "C", 1); + depth++; + if (depth != 3) { + warnx("depth %d, should be 3", depth); + } check(); + pid3 = dofork(); nprintf("."); write(fd, "D", 1); + depth++; + if (depth != 4) { + warnx("depth %d, should be 4", depth); + } check(); /* diff --git a/userland/testbin/hog/hog.c b/userland/testbin/hog/hog.c index 7b69f15..9c6e2b5 100644 --- a/userland/testbin/hog/hog.c +++ b/userland/testbin/hog/hog.c @@ -31,7 +31,7 @@ * hog.c * Spawned by several other user programs to test time-slicing. * - * This does not differ from guzzle in any important way. + * Loops consuming CPU cycles. */ int diff --git a/userland/testbin/multiexec/multiexec.c b/userland/testbin/multiexec/multiexec.c index 2eb7931..af40677 100644 --- a/userland/testbin/multiexec/multiexec.c +++ b/userland/testbin/multiexec/multiexec.c @@ -134,6 +134,7 @@ semP(struct usem *sem, size_t num) if (read(sem->fd, c, num) < 0) { err(1, "%s: read", sem->name); } + (void)c; } static diff --git a/userland/testbin/parallelvm/parallelvm.c b/userland/testbin/parallelvm/parallelvm.c index 31b2e58..c735338 100644 --- a/userland/testbin/parallelvm/parallelvm.c +++ b/userland/testbin/parallelvm/parallelvm.c @@ -273,16 +273,24 @@ static void semP(struct usem *sem, size_t num) { - if (read(sem->fd, NULL, num) < 0) { + char c[num]; + + if (read(sem->fd, c, num) < 0) { err(1, "%s: read", sem->name); } + (void)c; } static void semV(struct usem *sem, size_t num) { - if (write(sem->fd, NULL, num) < 0) { + char c[num]; + + /* semfs does not use these values, but be conservative */ + memset(c, 0, num); + + if (write(sem->fd, c, num) < 0) { err(1, "%s: write", sem->name); } } @@ -326,7 +334,12 @@ makeprocs(bool dowait) for (i=0; i #include +#include #include #include @@ -84,9 +85,9 @@ void Pn(struct usem *sem, unsigned count) { ssize_t r; - char c; + char c[count]; - r = read(sem->fd, &c, count); + r = read(sem->fd, c, count); if (r < 0) { err(1, "%s: read", sem->name); } @@ -105,9 +106,12 @@ void Vn(struct usem *sem, unsigned count) { ssize_t r; - char c; + char c[count]; - r = write(sem->fd, &c, count); + /* semfs does not use these values, but be conservative */ + memset(c, 0, count); + + r = write(sem->fd, c, count); if (r < 0) { err(1, "%s: write", sem->name); }