`pwd -P` with systemd-homed causes inconsistent cwd state

`pwd -P` with systemd-homed causes inconsistent cwd state

[Kyle Laker]

I have found a strange behavior with ZSH when working with 
systemd-homed. After running `pwd -P` within my home directory, it seems 
as if the current working directory effectively changes to `/`. I am 
currently using ZSH 5.9 and systemd 254.

systemd-homed is configured using a BTRFS subvolume. My home directory 
is `/home/kyle` whereas the systemd-homed image is located at 
`/home/kyle.homedir`. Running `pwd -P` prints the current directory 
under the homed image path which isn't entirely unexpected (though this 
behavior differs from that of other shells such as bash and fish which 
still print `/home/kyle/...`). What is unexpected is that after running 
`pwd -P` all other subprocesses act as if the `cwd` is `/` (though the 
`pwd` builtin and `$PWD` env var do not show any chagne).

To summarize what I am seeing:

```
% ls
test.txt
% pwd
/home/kyle/test
% pwd -P
/home/kyle.homedir/test
% ls
bin  boot  dev	etc  home  lib	lib64  mnt  opt  proc  root  run  sbin	srv 
  sys  tmp  usr  var
% pwd
/home/kyle/test
% /bin/pwd
/
% zsh -c "pwd"
/
% cd .
% ls
test.txt
% /bin/pwd
/home/kyle/test
% pwd
/home/kyle/test
```

Is there a configuration change that I need to make for ZSH or for 
systemd-homed to not end up with an inconsistent cwd state after running 
the `pwd` command?

Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Bart Schaefer]

On Fri, Oct 20, 2023 at 8:18 PM Kyle Laker <kyle@laker.email> wrote:
>
> I have found a strange behavior with ZSH when working with
> systemd-homed. After running `pwd -P` within my home directory, it seems
> as if the current working directory effectively changes to `/`. I am
> currently using ZSH 5.9 and systemd 254.
>
> systemd-homed is configured using a BTRFS subvolume.

This sounds like it must be the same issue as workers/52213 which also
involves doing operations on BTRFS mounts.

> What is unexpected is that after running
> `pwd -P` all other subprocesses act as if the `cwd` is `/`

When you run `pwd -P`, zsh invokes the getcwd() system call and prints
whatever it returns.  That's literally all it does.  So, although I
don't have BTRFS to test with, this looks from here like a problem
with getcwd() internally changing the process current directory to the
root when crossing a BTRFS link.  That would be invisible to $PWD and
therefore to the pwd command (without -P).

There are some cases where zsh itself would change the current
directory, e.g., if the parent of the current directory is removed
from the filesystem and a few other "catastrophic failure" cases, but
zsh would display a warning message in those circumstances and in your
example its happening silently ... another clue that whatever it is,
is behind zsh's back, so to speak.

Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Kyle Laker]

On 10/21/23 00:05, Bart Schaefer wrote:
> On Fri, Oct 20, 2023 at 8:18 PM Kyle Laker <kyle@laker.email> wrote:
>>
> When you run `pwd -P`, zsh invokes the getcwd() system call and prints
> whatever it returns.  That's literally all it does.  So, although I
> don't have BTRFS to test with, this looks from here like a problem
> with getcwd() internally changing the process current directory to the
> root when crossing a BTRFS link.  That would be invisible to $PWD and
> therefore to the pwd command (without -P).

Thanks for the context! So it seems like the issue is in the 
non-getcwd() zgetdir() code path. Checking against master, this issue is 
no longer present with a default configure after workers/50287 enabled 
using getcwd() by default just after the 5.9 release. getcwd() seems to 
handle this case correctly.

For situations where getcwd() isn't used, the actual errant `cd`ing 
happens at the end of the `while` loop in zgetdir(). This continuously 
calls `chdir("..")`. Eventually, this puts us at `/`. Typically, the 
call to `zchdir(buf + pos)` would take us back to the directory we 
started in; however, because ZSH determined the path was 
`/home/kyle.homedir` (using the homed image path) instead of 
`/home/kyle` (the actual mounted home directory), that fails. The 
`.homedir` folder is owned by nobody:nobody so when zchdir() calls 
chdir(3) the latter fails with an EACCES error. This leaves the process 
in `/`, where it was put by zgetdir() but without any of the other 
environment variables updated.

I'm not sure how these btrfs homedir mounts can be determined in 
situations like this (or how valuable it would be since nearly everyone 
with btrfs probably has a working getcwd(3)). But perhaps if the final 
zchdir() fails, zgetdir() could chdir(3) back to the original path 
rather than the resolved path?

Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Bart Schaefer]

On Sat, Oct 21, 2023 at 9:26 AM Kyle Laker <kyle@laker.email> wrote:
>
> Thanks for the context! So it seems like the issue is in the
> non-getcwd() zgetdir() code path. Checking against master, this issue is
> no longer present with a default configure after workers/50287

Aha.  I should have thought to look at the older source.

> I'm not sure how these btrfs homedir mounts can be determined in
> situations like this (or how valuable it would be since nearly everyone
> with btrfs probably has a working getcwd(3)). But perhaps if the final
> zchdir() fails, zgetdir() could chdir(3) back to the original path
> rather than the resolved path?

So this reaches the "return NULL" at the end of zgetdir(), after the
comment "Something bad happened." ?

Or conversely this is a consequence of zgetdir() not checking the
return value from zchdir(), which it doesn't in a couple of places?

[PATCH] Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Bart Schaefer]

[-- Attachment #1: Type: text/plain, Size: 549 bytes --]

On Sun, Oct 22, 2023 at 11:59 AM Bart Schaefer
<schaefer@brasslantern.com> wrote:
>
> So this reaches the "return NULL" at the end of zgetdir(), after the
> comment "Something bad happened." ?
>
> Or conversely this is a consequence of zgetdir() not checking the
> return value from zchdir(), which it doesn't in a couple of places?

Lacking a response to this and unable to test this directly, I offer
the following patch, which attempts to cover all those cases.

I forced HAVE_GETCWD to be false and ran make check with no issues.

[-- Attachment #2: zgetdir-lost.txt --]
[-- Type: text/plain, Size: 1661 bytes --]

diff --git a/Src/compat.c b/Src/compat.c
index 817bb4aaf..fbed7d80f 100644
--- a/Src/compat.c
+++ b/Src/compat.c
@@ -403,12 +403,25 @@ zgetdir(struct dirsav *d)
 		buf[--pos] = '/';
 	    if (d) {
 #ifndef HAVE_FCHDIR
-		zchdir(buf + pos);
+		if (zchdir(buf + pos) < 0) {
+		    /*
+		     * The reason for all this chdir-ing is to get the
+		     * absolute name of the current directory, so if we
+		     * cannot chdir to that name we have to assume our
+		     * directory is lost.  See "something bad" below.
+		     */
+		    noholdintr();
+		    return NULL;
+		}
 		noholdintr();
 #endif
 		return d->dirname = ztrdup(buf + pos);
 	    }
-	    zchdir(buf + pos);
+	    if (zchdir(buf + pos) < 0) {
+		/* Current directory lost */
+		noholdintr();
+		return NULL;
+	    }
 	    noholdintr();
 	    return buf + pos;
 	}
@@ -477,14 +490,22 @@ zgetdir(struct dirsav *d)
     if (d) {
 #ifndef HAVE_FCHDIR
 	if (buf[pos])
-	    zchdir(buf + pos + 1);
+	    if (zchdir(buf + pos + 1) < 0) {
+		/* Current directory lost */
+		noholdintr();
+		return NULL;
+	    }
 	noholdintr();
 #endif
 	return NULL;
     }
 
     if (buf[pos])
-	zchdir(buf + pos + 1);
+	if (zchdir(buf + pos + 1) < 0) {
+	    /* Current directory lost */
+	    noholdintr();
+	    return NULL;
+	}
     noholdintr();
 
 #else  /* __CYGWIN__, USE_GETCWD cases */
@@ -544,7 +565,11 @@ zgetcwd(void)
     }
 #endif /* HAVE_GETCWD */
     if (!ret)
-	ret = unmeta(pwd);
+	if (chdir(ret = unmeta(pwd)) < 0) {
+	    zwarn("%e: current directory %s lost, using /",
+		  errno, ret);
+	    chdir(ret = dupstring("/"));
+	}
     if (!ret || *ret == '\0')
 	ret = dupstring(".");
     return ret;

Re: [PATCH] Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Kyle Laker]

On 10/30/23 23:46, Bart Schaefer wrote:
> On Sun, Oct 22, 2023 at 11:59 AM Bart Schaefer 
> <schaefer@brasslantern.com> wrote:
>> 
>> So this reaches the "return NULL" at the end of zgetdir(), after 
>> the comment "Something bad happened." ?

>> Or conversely this is a consequence of zgetdir() not checking the 
>> return value from zchdir(), which it doesn't in a couple of 
>> places?

This is caused by not checking the return value. zgetdir() is returning
`buf + pos` so the error handling logic for `!ret` in zgetcwd() isn't hit.

> Lacking a response to this and unable to test this directly, I offer
>  the following patch, which attempts to cover all those cases.

Sorry for the delay. This patch does ensure that zgetdir() returns NULL
when the current working directory is clobbered. So it does fix that
part of the issue.

> I forced HAVE_GETCWD to be false and ran make check with no issues.

It seems like this does in fact fix the case where _both_ HAVE_GETCWD
and USE_GETCWD are false; however, in 5.9, even if HAVE_GETCWD is true,
USE_GETCWD is false (and this same setup can be induced on master where
the same behavior happens). In that case, the new error handling is
never hit because after zgetdir() now returns NULL, getcwd() is called
as a fallback (which sets ret so the subsequent logic to use pwd isnt
hit). But because at that point cwd has been lost by zgetdir()/zchdir(),
calling getcwd() just returns "/" so the value isn't useful. `pwd -P`
will print "/" but no error message.

With this patch so long as HAVE_GETCWD == USE_GETCWD it seems like
things work as expected; however, if USE is false and HAVE is true,
there are still issues with losing the working directory without
printing the error message.

Re: [PATCH] Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Bart Schaefer]

On Wed, Nov 1, 2023 at 7:54 PM Kyle Laker <kyle@laker.email> wrote:
>
> It seems like this does in fact fix the case where _both_ HAVE_GETCWD
> and USE_GETCWD are false; however, in 5.9, even if HAVE_GETCWD is true,
> USE_GETCWD is false (and this same setup can be induced on master where
> the same behavior happens). In that case, the new error handling is
> never hit because after zgetdir() now returns NULL, getcwd() is called
> as a fallback (which sets ret so the subsequent logic to use pwd isnt
> hit).

The only possible case I can think of where we'd want to fall back on
getcwd() after zgetdir() returns NULL, is here:

363    if (stat(".", &sbuf) < 0) {
364        return NULL;
365    }

Somehow getcwd() might bypass whatever restriction caused this to
fail?  In all other cases we've chdir'd away from the current
directory and can't get back.  I'm tempted to say we should just
delete that entire fallback, but then we never use getcwd() at all.
Maybe we should be checking errno (for what?) as well as ret before
falling back to getcwd()?  Or maybe when USE_GETCWD, we should use it
preferentially, and zgetdir() should be the fallback?

Or maybe zgetdir() itself should USE_GETCWD there at line 364?

The whole situation seems a bit strange.  In utils.c:lchdir() we have
calls to zgetdir() in both an #ifndef HAVE_FCHDIR and later an #ifdef
HAVE_CHDIR branch.

Re: [PATCH] Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Kyle Laker]

[-- Attachment #1: Type: text/plain, Size: 1626 bytes --]

On 11/3/23 00:28, Bart Schaefer wrote:> Somehow getcwd() might bypass 
whatever restriction caused this to
> fail?  In all other cases we've chdir'd away from the current
> directory and can't get back.  I'm tempted to say we should just
> delete that entire fallback, but then we never use getcwd() at all.

If that fallback gets deleted, it seems like HAVE_GETCWD in its entirety 
isn't particularly useful anymore; this is the only `#ifdef` for it. It 
might simplify the logic a bit; if USE_GETCWD then do so, otherwise, 
rely on fallbacks (especially since zgetdir() itself may be calling 
getcwd()).

> Maybe we should be checking errno (for what?) as well as ret before
> falling back to getcwd()?  Or maybe when USE_GETCWD, we should use it
> preferentially, and zgetdir() should be the fallback?

I think zgetdir() does use getcwd() preferentially when USE_GETCWD. It's 
awkwardly at the bottom of zgetdir(). When USE_GETCWD, all the internal 
pathwalking logic of zgetdir() is skipped.

> Or maybe zgetdir() itself should USE_GETCWD there at line 364?

That feels possibly the cleanest. That would allow removing all the 
calls to getcwd() from within zgetcwd() itself.

I've attached a patch that builds on yours. Testing it in my use case, 
it correctly works when HAVE_GETCWD && USE_GETCWD (eagerly invokes 
getcwd()), HAVE_GETCWD && !USE_GETCWD (zgetdir() returns null and the 
pwd fallback in zgetcwd() is used), and !HAVE_GETCWD && !USE_GETCWD 
(same as when HAVE && !USE). The patch also removes 
`GETCWD_CALLS_MALLOC` because it was only used in that fallback case. 
Running `make check` for me still passes.

[-- Attachment #2: zgetdir-getcwd-reorder.txt --]
[-- Type: text/plain, Size: 4477 bytes --]

diff --git a/Src/compat.c b/Src/compat.c
index 817bb4aaf..23ad703f4 100644
--- a/Src/compat.c
+++ b/Src/compat.c
@@ -359,9 +359,40 @@ zgetdir(struct dirsav *d)
     buf = zhalloc(bufsiz = PATH_MAX+1);
     pos = bufsiz - 1;
     buf[pos] = '\0';
+#if defined(USE_GETCWD) || defined(__CYGWIN__)
+    if (!getcwd(buf, bufsiz)) {
+	if (d) {
+	    return NULL;
+	}
+    } else {
+	if (d) {
+	    return d->dirname = ztrdup(buf);
+	}
+	return buf;
+    }
+#else
     strcpy(nbuf, "../");
     if (stat(".", &sbuf) < 0) {
+	/*
+	 * Fallback to getcwd() since it may be able to overcome
+	 * the failure in stat(). We haven't changed directories
+	 * to walk the path yet, so this should return the current
+	 * directory.
+	 */
+#ifdef HAVE_GETCWD
+	if (!getcwd(buf, bufsiz)) {
+	    if (d) {
+		return NULL;
+	    }
+	} else {
+	    if (d) {
+		return d->dirname = ztrdup(buf);
+	    }
+	    return buf;
+	}
+#else
 	return NULL;
+#endif
     }
 
     /* Record the initial inode and device */
@@ -369,7 +400,6 @@ zgetdir(struct dirsav *d)
     pdev = sbuf.st_dev;
     if (d)
 	d->ino = pino, d->dev = pdev;
-#if !defined(__CYGWIN__) && !defined(USE_GETCWD)
 #ifdef HAVE_FCHDIR
     else
 #endif
@@ -403,12 +433,25 @@ zgetdir(struct dirsav *d)
 		buf[--pos] = '/';
 	    if (d) {
 #ifndef HAVE_FCHDIR
-		zchdir(buf + pos);
+		if (zchdir(buf + pos) < 0) {
+		    /*
+		     * The reason for all this chdir-ing is to get the
+		     * absolute name of the current directory, so if we
+		     * cannot chdir to that name we have to assume our
+		     * directory is lost.  See "something bad" below.
+		     */
+		    noholdintr();
+		    return NULL;
+		}
 		noholdintr();
 #endif
 		return d->dirname = ztrdup(buf + pos);
 	    }
-	    zchdir(buf + pos);
+	    if (zchdir(buf + pos) < 0) {
+		/* Current directory lost */
+		noholdintr();
+		return NULL;
+	    }
 	    noholdintr();
 	    return buf + pos;
 	}
@@ -477,28 +520,23 @@ zgetdir(struct dirsav *d)
     if (d) {
 #ifndef HAVE_FCHDIR
 	if (buf[pos])
-	    zchdir(buf + pos + 1);
+	    if (zchdir(buf + pos + 1) < 0) {
+		/* Current directory lost */
+		noholdintr();
+		return NULL;
+	    }
 	noholdintr();
 #endif
 	return NULL;
     }
 
     if (buf[pos])
-	zchdir(buf + pos + 1);
-    noholdintr();
-
-#else  /* __CYGWIN__, USE_GETCWD cases */
-
-    if (!getcwd(buf, bufsiz)) {
-	if (d) {
+	if (zchdir(buf + pos + 1) < 0) {
+	    /* Current directory lost */
+	    noholdintr();
 	    return NULL;
 	}
-    } else {
-	if (d) {
-	    return d->dirname = ztrdup(buf);
-	}
-	return buf;
-    }
+    noholdintr();
 #endif
 
     /*
@@ -526,25 +564,12 @@ mod_export char *
 zgetcwd(void)
 {
     char *ret = zgetdir(NULL);
-#ifdef HAVE_GETCWD
-    if (!ret) {
-#ifdef GETCWD_CALLS_MALLOC
-	char *cwd = getcwd(NULL, 0);
-	if (cwd) {
-	    ret = dupstring(cwd);
-	    free(cwd);
-	}
-#else
-	char *cwdbuf = zalloc(PATH_MAX+1);
-	ret = getcwd(cwdbuf, PATH_MAX);
-	if (ret)
-	    ret = dupstring(ret);
-	zfree(cwdbuf, PATH_MAX+1);
-#endif /* GETCWD_CALLS_MALLOC */
-    }
-#endif /* HAVE_GETCWD */
     if (!ret)
-	ret = unmeta(pwd);
+	if (chdir(ret = unmeta(pwd)) < 0) {
+	    zwarn("%e: current directory %s lost, using /",
+		  errno, ret);
+	    chdir(ret = dupstring("/"));
+	}
     if (!ret || *ret == '\0')
 	ret = dupstring(".");
     return ret;
diff --git a/configure.ac b/configure.ac
index c5263035e..5dd6c7892 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2068,31 +2068,6 @@ if test x$zsh_cv_use_getcwd = xyes; then
   AC_DEFINE(USE_GETCWD)
 fi
 
-dnl GNU getcwd() can allocate as much space as necessary for a
-dnl directory name, preventing guessing games.
-AH_TEMPLATE([GETCWD_CALLS_MALLOC],
-[Define to 1 if getcwd() calls malloc to allocate memory.])
-if test x$ac_cv_func_getcwd = xyes; then
-  AC_CACHE_CHECK(whether getcwd calls malloc to allocate memory,
-  zsh_cv_getcwd_malloc,
-  [AC_RUN_IFELSE([AC_LANG_SOURCE([[
-#include <unistd.h>
-#include <string.h>
-int main() {
-    char buf[1024], *ptr1, *ptr2;
-    ptr1 = getcwd(buf, 1024);
-    ptr2 = getcwd(NULL, 0);
-    if (ptr1 && ptr2 && !strcmp(ptr1, ptr2)) {
-      return 0;
-    }
-    return 1;
-}
-]])],[zsh_cv_getcwd_malloc=yes],[zsh_cv_getcwd_malloc=no],[zsh_cv_getcwd_malloc=no])])
-  if test x$zsh_cv_getcwd_malloc = xyes; then
-    AC_DEFINE(GETCWD_CALLS_MALLOC)
-  fi
-fi
-
 dnl CHECK FOR setproctitle() FOR jobs -Z / ARGV0
 AH_TEMPLATE([HAVE_SETPROCTITLE],
 [Define to 1 if the system supports `setproctitle' to change process name])

Re: [PATCH] Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Bart Schaefer]

On Sun, Nov 5, 2023 at 6:17 AM Kyle Laker <kyle@laker.email> wrote:
>
> I've attached a patch that builds on yours.

Thanks.  I believe there's still some more that could be done here --
for example

+    if (!getcwd(buf, bufsiz)) {
+        if (d) {
+            return NULL;
+        }
+    } else {

The "if (d)" is extraneous here, we're just going to return NULL later
anyway.  That test implies that we'd later be attempting to use
fchdir() [via zchdir?] but in the failure case we won't.  I know you
carried this over from the previous code but it's extraneous there too
so we might as well fix it.

> The patch also removes
> `GETCWD_CALLS_MALLOC` because it was only used in that fallback case.

I think the reason for that may have been that a reason for failure is
that the buffer isn't large enough.  Rather than remove this, I think
it should actually be moved into zgetdir().

I'll put something together a bit later today, but in the meantime
would be interested in feedback from anyone else who has a better idea
how we arrived at this state (and possible what really is going on in
lchdir() ...).

Re: [PATCH] Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Bart Schaefer]

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

On Sun, Nov 5, 2023 at 8:17 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> I'll put something together a bit later today, but in the meantime
> would be interested in feedback from anyone else who has a better idea
> how we arrived at this state (and possible what really is going on in
> lchdir() ...).

The attached passes "make check" for me with all sensible**
permutations of HAVE_GETCWD and USE_GETCWD.  If someone else can check
the __CYGWIN__ case that would be great.

** That is, I didn't try defined(USE_GETCWD) && !defined(HAVE_GETCWD)

Kyle?

[-- Attachment #2: zgetdir-lost2.txt --]
[-- Type: text/plain, Size: 3031 bytes --]

diff --git a/Src/compat.c b/Src/compat.c
index 817bb4aaf..8b31ad9f4 100644
--- a/Src/compat.c
+++ b/Src/compat.c
@@ -342,36 +342,69 @@ zopenmax(void)
 mod_export char *
 zgetdir(struct dirsav *d)
 {
-    char nbuf[PATH_MAX+3];
     char *buf;
+#if defined(HAVE_GETCWD) || defined(__CYGWIN__)
+    char *cwdbuf;
+#endif
+#if defined(USE_GETCWD) || defined(__CYGWIN__)
+#ifdef GETCWD_CALLS_MALLOC
+    if ((cwdbuf = getcwd(NULL, 0))) {
+	buf = dupstring(cwdbuf);
+	free(cwdbuf);
+    } else
+	buf = NULL;
+#else
+    cwdbuf = zalloc(PATH_MAX+1);
+    if ((buf = getcwd(cwdbuf, PATH_MAX)))
+	buf = dupstring(buf);
+    zfree(cwdbuf, PATH_MAX+1);
+#endif /* GETCWD_CALLS_MALLOC */
+    if (d && buf)
+	return d->dirname = ztrdup(buf);
+    else
+	return buf;
+#else /* !USE_GETCWD && !__CYGWIN__ */
     int bufsiz, pos;
-    struct stat sbuf;
-    ino_t pino;
-    dev_t pdev;
-#if !defined(__CYGWIN__) && !defined(USE_GETCWD)
+    char nbuf[PATH_MAX+3];
     struct dirent *de;
     DIR *dir;
-    dev_t dev;
-    ino_t ino;
+    struct stat sbuf;
+    ino_t pino, ino;
+    dev_t pdev, dev;
     int len;
-#endif
 
+    strcpy(nbuf, "../");
+    if (stat(".", &sbuf) == 0) {
+	/* Record the initial inode and device */
+	pino = sbuf.st_ino;
+	pdev = sbuf.st_dev;
+	if (d)
+	    d->ino = pino, d->dev = pdev;
+    }
+#ifdef HAVE_GETCWD
+    else {
+#ifdef GETCWD_CALLS_MALLOC
+	if ((cwdbuf = getcwd(NULL, 0))) {
+	    buf = dupstring(cwdbuf);
+	    free(cwdbuf);
+	} else
+	    buf = NULL;
+#else
+	cwdbuf = zalloc(PATH_MAX+1);
+	if ((buf = getcwd(cwdbuf, PATH_MAX)))
+	    buf = dupstring(buf);
+	zfree(cwdbuf, PATH_MAX+1);
+#endif /* GETCWD_CALLS_MALLOC */
+	return buf;    /* NULL when stat() and getcwd() both failed */
+    }
+#endif
+    /* stat() succeeded */
     buf = zhalloc(bufsiz = PATH_MAX+1);
     pos = bufsiz - 1;
     buf[pos] = '\0';
-    strcpy(nbuf, "../");
-    if (stat(".", &sbuf) < 0) {
-	return NULL;
-    }
 
-    /* Record the initial inode and device */
-    pino = sbuf.st_ino;
-    pdev = sbuf.st_dev;
-    if (d)
-	d->ino = pino, d->dev = pdev;
-#if !defined(__CYGWIN__) && !defined(USE_GETCWD)
 #ifdef HAVE_FCHDIR
-    else
+    if (!d)
 #endif
 	holdintr();
 
@@ -487,18 +520,6 @@ zgetdir(struct dirsav *d)
 	zchdir(buf + pos + 1);
     noholdintr();
 
-#else  /* __CYGWIN__, USE_GETCWD cases */
-
-    if (!getcwd(buf, bufsiz)) {
-	if (d) {
-	    return NULL;
-	}
-    } else {
-	if (d) {
-	    return d->dirname = ztrdup(buf);
-	}
-	return buf;
-    }
 #endif
 
     /*
@@ -526,23 +547,6 @@ mod_export char *
 zgetcwd(void)
 {
     char *ret = zgetdir(NULL);
-#ifdef HAVE_GETCWD
-    if (!ret) {
-#ifdef GETCWD_CALLS_MALLOC
-	char *cwd = getcwd(NULL, 0);
-	if (cwd) {
-	    ret = dupstring(cwd);
-	    free(cwd);
-	}
-#else
-	char *cwdbuf = zalloc(PATH_MAX+1);
-	ret = getcwd(cwdbuf, PATH_MAX);
-	if (ret)
-	    ret = dupstring(ret);
-	zfree(cwdbuf, PATH_MAX+1);
-#endif /* GETCWD_CALLS_MALLOC */
-    }
-#endif /* HAVE_GETCWD */
     if (!ret)
 	ret = unmeta(pwd);
     if (!ret || *ret == '\0')

Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Jan Alexander Steffens (heftig)]

Hi Bart,

In case it helps you test this directly, I can reproduce the problem
(on Zsh 5.9) using just a bind mount, so this seems to be independent
of systemd-homed or even btrfs.

```  
mkdir -p x/{foo,bar}  
cd x  
sudo mount --bind foo bar  
cd bar  
zsh -c 'echo ${${:-a}:a}'  
```

should display `/.../x/bar/a` but displays `/.../x/foo/a`.

I guess it might be dependent on the directory iteration order, so if
it doesn't reproduce, try swapping `foo` and `bar`.

Greetings,  
Jan

Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Bart Schaefer]

On Mon, Nov 20, 2023 at 2:23 PM Jan Alexander Steffens (heftig)
<heftig@archlinux.org> wrote:
>
> sudo mount --bind foo bar
> cd bar
> zsh -c 'echo ${${:-a}:a}'

Confirmed this produces the correct result in the current dev
revision.  Still hoping someone can test Cygwin.