Re: one other coreutils test failure

[Jim Meyering]

Jim Meyering wrote:
> Michael Stone wrote:
>> On Fri, Sep 16, 2011 at 06:54:32PM +0200, Jim Meyering wrote:
>>>Michael Stone wrote:
>>>> This one was actually in gnulib; what's the best way to pursue it?
>>>>
>>>> FAIL: test-getcwd (exit: 16)
>>>> ============================
>>>
>>>Hi Michael,
>>>
>>>Thanks for the report.
>>>I'll Cc bug-gnulib, but to start with, note that test-getcwd.c's main
>>>does this:
>>>
>>>  return test_abort_bug () + test_long_name ();
>>>
>>>and the first can return 0 or 2,3,4 or 5, while
>>>the latter can return only two values smaller than 16:
>>>10 and 11.
>>
>> or 4 & 12, or am I missing something?
>
> No.  I missed the 12.
>
>> at the point where it sets
>> fail=12, AT_FDCWD is -100 and errno is 2 (ENOENT)
>
> Then maybe your system has problems with long names?

Yep, that is the problem.


[remember, this is on a kFreeBSD system]

I used one of debian.net's systems and debugged it.
This was an interesting one.

None of this would have happened if kFreeBSD had openat support,
but it doesn't, so...

First, here's an expedient work-around:

  Let gnulib/lib/getcwd.c use gnulib's openat emulation.
  Since kFreeBSD also lacks /proc/self/fd support, this has the
  drawback of making getcwd non-thread-safe, and making it possible
  that getcwd will exit under some unusual conditions.  But neither
  of those matter to coreutils, since getcwd is not used in the sole
  multi-threaded application (sort), and it's fine if the function
  exits from the very few callers.

To do that, you could change this in lib/getcwd.c:

    /* If this host provides the openat function, then enable
       code below to make getcwd more efficient and robust.  */
    #ifdef HAVE_OPENAT
    # define HAVE_OPENAT_SUPPORT 1
    #else
    # define HAVE_OPENAT_SUPPORT 0
    #endif

to this:

    #define HAVE_OPENAT_SUPPORT 1

I'm still debating how to deal with this.
At worst, (i.e., if we do nothing in gnulib), we can always
patch coreutils' version of getcwd.c as suggested above.

--------------------------------------------------------------
What happened to make the test_abort_bug function fail?
First, it constructs a directory whose absolute name is 5 * 1024 bytes
long, cd's into the deepest directory, and then calls getcwd from there.

Without *at functions (which handle this case just fine),
getcwd has to resort to considering directories ".", "..", "../..",
"../../..", etc., until it reaches the root.  Since each of the
directories created initially had length 14, that means there will be
approximately 5 * 1024 / 15 = 341.33... of them (modulo the initial $PWD).

As luck would have it, 341 is a sort of magic number in this context,
since each "../" takes 3 bytes.  3*341=1023.  That means one of the final
"../../ ... /.." paths for our example may well end up having length 1025,
which is one greater than PATH_MAX.  Oops.

It just so happened that our attempt to trigger the ia64 abort-inducing
glibc/getcwd bug just *barely* triggers a different bug here.  I could
have chosen any target length between 4k and 16k, but I chose 5k.
What luck.

Anyway, this exercise shows that without openat, gnulib's getcwd.c
fails for any directory of depth greater than PATH_MAX / 3.

Which do you prefer?  Such a limitation (existing code), or the potential
limitation of a cwd-changing openat implementation[*] that may exit when
it fails to restore the working directory.  The latter is what you get
when using the openat emulation.

There's also the possibility of using more than one implementation.
E.g., using the "../.."-opening lame-but-thread/library-safe version
until it fails, and if/when it fails, resort to the implementation
that uses the emulated openat.  Though note that I am not interested
in accommodating systems without openat in this regard.  IMHO, either
they are still evolving enough that they will add openat support, or
they will not be relevant in a few years, so aren't worth the investment.


Jim

[*] Remember that we end up using the cwd-changing and possibly-exiting
emulation code only on systems that lack both openat and /proc/self/fd
support.