On 20/07/2023 16: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?

Yes, this works.

This is how I wrote it initially, in fact. This is perhaps slightly less good, because it still has that leak if the fh method calls throw an exception (which should never happen :)), but has the advantage of actually working.

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));
+
+         if (!(fh = build_fh_pc (pc)))
+           __leave;
+
          res = fh->fstat (buf);
          if (!res)
            fh->stat_fixup (buf);


Reply via email to