On Jul 20 17:14, Corinna Vinschen wrote:
> On Jul 20 13:55, Jon Turney wrote:
> > On 19/07/2023 16:33, Corinna Vinschen wrote:
> > > On Jul 19 13:41, Jon Turney wrote:
> > > > [1/2] has the side effect of flipping test stat06 from working to 
> > > > failing.
> > > > [2/2] fixes that
> > > > 
> > > > When run with TDIRECTORY set, libltp just uses that directory and 
> > > > assumes
> > > > something else will clean it up.
> > > > 
> > > > When TDIRECTORY is not set, libltp creates a subdirectory under /tmp, 
> > > > and when
> > > > the test is completed, removes the expected files and verifies that the
> > > > directory is empty.
> > > > 
> > > > stat06 fails that check, because it creates the a file named "file" 
> > > > there, and
> > > > tries stat("file", -1), testing that it returns the expected value 
> > > > EFAULT.
> > > > 
> > > > "file" is removed, but lingers in the STATUS_DELETE_PENDING state until 
> > > > the
> > > > Windows handle which stat_worker() leaks when an exception occurs is 
> > > > closed
> > > > (when the processes exits).
> > > 
> > > Great find. Please push.
> > 
> > So, it seems this doesn't work in an optimized build, as fh is always NULL
> > when we get around to deleting it after a fault.
> > 
> > I'm thinking that I've written this wrong somehow (horses), rather than it
> > being some complex problem with how the optimizer interacts with all the
> > memory and register barriers the exception handling uses (zebras)
> 
> What if you turn around the order instead?
> 
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 73343ecc1f07..32ace4d38943 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -1967,12 +1967,13 @@ stat_worker (path_conv &pc, struct stat *buf)
>       {
>         fhandler_base *fh;
>  
> -       if (!(fh = build_fh_pc (pc)))
> -         __leave;
> -
>         debug_printf ("(%S, %p, %p), file_attributes %d",
>                       pc.get_nt_native_path (), buf, fh, (DWORD) *fh);
>         memset (buf, 0, sizeof (*buf));

Maybe adding a MemoryBarrier() call here if all else fails...

> +
> +       if (!(fh = build_fh_pc (pc)))
> +         __leave;
> +
>         res = fh->fstat (buf);
>         if (!res)
>           fh->stat_fixup (buf);

Reply via email to