I'm contemplating a significant change to getSyscallArg(), but there are two different ways I could go, and it's a big enough change that I really don't want to implement one approach and post a patch just to have people not like it and tell me I should do the other thing (or nothing at all). So here goes...
A few years ago we ran into a problem where syscall arguments weren't cleanly indexable. The issue apparently centered around truncate64/ftruncate64 on i386, where the second arg (the 64-bit offset) is passed in two 32-bit registers instead of a single 64-bit register. Thus using the generic "getSyscallArg(tc, 1)" like you might normally expect wouldn't work in that case. In addition, in the (theoretical?) case where there were additional arguments after the offset, whether or not those additional arguments would be found in the 3rd and following or 4th and following registers would depend on the architecture. So as a result it was decided that using context-free integers as arg indices wouldn't work, and we needed to revamp getSyscallArg(). The solution we ended up with is described here: http://repo.gem5.org/gem5/rev/4842482e1bd1 I won't bother trying to recap it; if you're reading this email, please go and look at that changeset comment before continuing. I'm in the middle of contemplating some refactoring of the syscall code, and I've found that this setup is a little awkward and definitely confusing. The "index" variable is no longer an index (conceptually), it's really an opaque state variable that's tracking your progress through the stack of arguments. The only truly valid thing a user of the interface should be able to do is to set it to 0 to restart the stack traversal. But the interface (and the naming of the variable) don't indicate this, so people unsurprisingly get confused, and we end up with code like this: http://repo.gem5.org/gem5/diff/6e854ea87bab/src/sim/syscall_emul.hh which (I believe) works because all of the arguments are fixed size on all of the platforms where we use that syscall, but obviously is in total violation of how the interface is intended to be used. (I'm not blaming anyone, since that intention isn't really documented anywhere other than in the changeset notice.) My initial reaction is that if we're going to have an implicit opaque state variable we really should make it implicit and opaque. One way to do that would be to make the index a private member of ThreadContext, change getSyscallArg(ThreadContext *tc, int &index) to getNextSyscallArg(ThreadContext *tc), and add a ThreadContext method like resetSyscallArgStack() that sets the internal index to 0. Then we'd call resetSyscallArgStack() in doSyscall() and just get rid of the index variables everywhere. I think that would work fine and would be an improvement over the status quo. However, it appears that, after almost three years, the only situation that currently makes use of all this functionality is truncate64 and ftruncate64 on i386. So another solution is just to special-case those two syscalls on that architecture and go back to the old scheme where we had fixed integer indices. The big question here is whether anyone knows of any other syscalls or architectures that might need this more powerful stack handling scheme; if so, we should probably go with the first approach. If not, I'm open to the second approach. If someone feels like the current scheme is good enough, or there's a third approach I haven't considered, please speak up. I'd like to get some initial consensus here before I go ripping things up. Thanks, Steve _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
