> On Jan. 18, 2017, 4:06 p.m., Jason Lowe-Power wrote: > > src/arch/riscv/process.cc, line 222 > > <http://reviews.gem5.org/r/3780/diff/1/?file=64366#file64366line222> > > > > Two comments: > > 1. Shouldn't this at least have a warning? If the syscall arg register > > > 3 there is definitely a bug, right? > > 2. Can you return "0" instead of the last register? I think that's a > > better failover case. > > Alec Roelke wrote: > Returning 0 would make more sense, so I'll do that. I can also add a > warning, but that causes a minimum of two warnings to print every system call > since the doSyscall() function gathers the values of the system call > registers from indices 0 to 5 every time a system call is made. This is only > used for a debug statement, though; is there a reason it does this even when > debugging is off?
Ah. In this case, don't worry about adding a "warn". We'll just have to assume that anyone who implements a syscall in RISC-V will know not to use the values in registers > 3. It would be good to add a comment when you update to return 0. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3780/#review9261 ----------------------------------------------------------- On Jan. 12, 2017, 9 p.m., Alec Roelke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3780/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2017, 9 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11795:44b4eec15cd7 > --------------------------- > riscv: Fix crash when syscall argument reg index is too high > > By default, doSyscall gets the values of six registers to be used for > system call arguments. RISC-V, by convention, only has four. Because > RISC-V's implementation of these indices is as arrays of integers rather > than as base indices plus offsets, trying to get the fifth argument > register's value will cause a crash. This patch fixes that by providing > the fourth register's value for any index higher than 3. > > > Diffs > ----- > > src/arch/riscv/process.cc 97eebddaae84 > > Diff: http://reviews.gem5.org/r/3780/diff/ > > > Testing > ------- > > > Thanks, > > Alec Roelke > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
