|
From: Julian S. <js...@ac...> - 2015-04-08 07:26:54
|
What would happen if, for mips, we added to SysRes, a boolean field that indicates whether or not _valEx contains a live value? It seems to me that at the moment we have "unclean" semantics because we can't know, from a SysRes in isolation, whether or not _valEx needs to be taken into account. It sounds, from Petar's earlier mail, that forcing _valEx to always be zero in cases where it doesn't carry a live value, breaks stuff. And that doesn't really reflect the kernel interface's semantics anyway, since that implicitly seems to require that a SysRes is interpreted in the context of whether the syscall that created it is or isn't sys_pipe. Hence my proposal to encode that information (as a bool) inside SysRes itself. I agree it would be nice to get this fixed cleanly, on the whole. J On 08/04/15 04:40, Petar Jovanovic wrote: > Hi Florian, > > I have just given a second look to this issue. > It seems to me that if we want to make sure SysRes is always correctly > initialized (i.e. to make sure _valEx and v1 in guest state are set to zero > for non-pipe calls or for unsuccessful pipe call, we need to modify several > spots for MIPS: > > - within VG_(do_syscall), after syscall is executed > - ML_(do_syscall_for_client_WRK) in syscall-mips64-linux.S > - putSyscallStatusIntoGuestState > - hack around vki_ucontext and VG_UCONTEXT_SYSCALL_SYSRES > > And we would be doing it to satisfy condition sr_EQ introduced in r15060 > that's used for assert only. Do you think it is worth adding these changes > and workarounds? If you believe so, I will make the necessary changes. > > Regards, > Petar > > > On Sat, Apr 4, 2015 at 5:15 AM, Petar Jovanovic <mip...@gm...> wrote: > >> >> On Fri, Apr 3, 2015 at 4:29 PM, Florian Krohm <fl...@ei...> >> wrote: >> >>> Yes. But comparing _valEx makes sense when it has a deterministic value >>> (i.e. after a pipe call). So not comparing the _valEx value at all does >>> not sound right either. >> >> >> I agree. >> >> >>> How is this (for mips64): >>> >>> Index: coregrind/m_syscall.c >>> =================================================================== >>> --- coregrind/m_syscall.c (revision 15062) >>> +++ coregrind/m_syscall.c (working copy) >>> @@ -859,6 +859,7 @@ >>> ULong V0 = do_syscall_WRK(a1,a2,a3,a4,a5,a6,sysno,v1_a3); >>> ULong V1 = (ULong)v1_a3[0]; >>> ULong A3 = (ULong)v1_a3[1]; >>> + if (sysno != _NR_pipe) V1 = 0; // V1 is unused for this syscall >>> return VG_(mk_SysRes_mips64_linux)( V0, V1, A3 ); >>> >>> #else >>> >>> Does that help? >>> >>> >> I have tested a similar change right away, but it was not sufficient. >> I believe I need to modify at least putSyscallStatusIntoGuestState(), >> will see if anything else is missing after more testing. >> >> Petar >> > > > > ------------------------------------------------------------------------------ > BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT > Develop your own process in accordance with the BPMN 2 standard > Learn Process modeling best practices with Bonita BPM through live exercises > http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_ > source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF > > > > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers > |