Tcl Source Code

I found this odd behavior first in an old tclkit.  In
that case it was an 8.5a4-based version (sometime
before the BigNum code was integrated) and it caused a
segfault.

I just tried today with the HEAD and it's broken, but
doesn't segfault for me.  Steve Redler IV confirmed
that it's broken in Slackware Linux.  Steve was able to
get a segfault with a tclkit built with the Tcl HEAD as
of about 2 weeks ago -- using the patterns I list below.

I haven't yet tried todays HEAD with a tclkit, but it
could be that tclkit introduces a segfault rather than
just the strange behavior.

In the tclkit the pattern is:
# cd /tmp
# tclkit8.5 
% cd ..
% pwd
% cd ..
% pwd
Memory fault (core dumped) 

Note: I had to be root to get the /tclkit8.5.core created.

The backtrace on the core indicates a loop of patterns
that goes on for quite a while (endless recursion? or
possibly the debugger is confused).
#0  0x080811bf in TclDefaultBgErrorHandlerObjCmd ()
[this is where it repeats for thousands of times]
#1  0x08094ca7 in Tcl_FSLstat ()
#2  0x080a593e in Tcl_FSLstat ()
#3  0x080a58dc in Tcl_FSLstat ()
#4  0x080a7bfb in Tcl_FSLstat ()
#5  0x080d220f in TclpUnloadFile ()
#6  0x080952a9 in Tcl_FSLstat ()
#7  0x08094248 in Tcl_FSLstat ()
#8  0x080a7c0e in Tcl_FSLstat ()
#9  0x080d220f in TclpUnloadFile ()
#10 0x080952a9 in Tcl_FSLstat ()
#11 0x08094248 in Tcl_FSLstat ()
#12 0x080a7c0e in Tcl_FSLstat ()
#13 0x080d220f in TclpUnloadFile ()
...


This demonstrates the broken behavior with today's HEAD:
$ make shell
LD_LIBRARY_PATH=`pwd`:${LD_LIBRARY_PATH}; export
LD_LIBRARY_PATH;  TCL_LIBRARY="
/gps/src/cvs/HEAD/tcl/library"; export TCL_LIBRARY; 
./tclsh 
% cd /gps
% cd ..
% pwd
% cd ..
% pwd
..
% cd ..
% pwd
..
% cd ..
% pwd
..
% cd ..
% pwd
..
% cd /
% pwd
/
% cd ..
% pwd
% cd ..
% pwd
..

Logged In: YES 
user_id=1312539

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

Logged In: YES 
user_id=80530


another revision committed.
please test.

Logged In: YES 
user_id=585068


$ make shell
LD_LIBRARY_PATH=`pwd`:${LD_LIBRARY_PATH}; export
LD_LIBRARY_PATH; 
TCL_LIBRARY="/gps/src/cvs/HEAD/tcl/library"; export
TCL_LIBRARY;  ./tclsh 
% file normalize ../..
/gps/src/cvs
% file normalize /tmp/../..
% file normalize /.. 
/
% file normalize /../..
%

So the / result isn't occuring here.

Now notice the cd behavior here after a fresh start:
$ make shell
LD_LIBRARY_PATH=`pwd`:${LD_LIBRARY_PATH}; export
LD_LIBRARY_PATH; 
TCL_LIBRARY="/gps/src/cvs/HEAD/tcl/library"; export
TCL_LIBRARY;  ./tclsh 
% cd /tmp/../..
couldn't change working directory to "/tmp/../..": no such
file or directory
%

Now here is a similar pattern, but the cd doesn't fail
(perhaps from the failed normalize):

$ make shell
LD_LIBRARY_PATH=`pwd`:${LD_LIBRARY_PATH}; export
LD_LIBRARY_PATH; 
TCL_LIBRARY="/gps/src/cvs/HEAD/tcl/library"; export
TCL_LIBRARY;  ./tclsh 
% file normalize /tmp/../..
% file normalize ../..
/gps/src/cvs
% cd /tmp/../..
% pwd

So we try this:
$ make shell
LD_LIBRARY_PATH=`pwd`:${LD_LIBRARY_PATH}; export
LD_LIBRARY_PATH; 
TCL_LIBRARY="/gps/src/cvs/HEAD/tcl/library"; export
TCL_LIBRARY;  ./tclsh 
% file normalize /tmp/../..
% cd /tmp/../..
couldn't change working directory to "/tmp/../..": no such
file or directory
% 

So some internal state is being corrupt, and causing other
failures :/  

FWIW the tclkit built with the same sources has this problem:
$ ./tclkit85-magichimo.localdomain 
% cd /tmp/../.. 
% pwd
%

Logged In: YES 
user_id=80530

commited revised fix for
this family of bugs.

Please test on Windows.

Logged In: YES 
user_id=72656

Specific failures:

==== filesystem-1.19 file normalisation FAILED
==== Contents of test case:

    file normalize ${drive}:/./../../..

---- Result was:
B:
---- Result should have been (exact matching):
B:/
==== filesystem-1.19 FAILED


==== filesystem-1.22 file normalisation FAILED
==== Contents of test case:

    file normalize //name/foo/.

---- Result was:
//name/foo/
---- Result should have been (exact matching):
//name/foo
==== filesystem-1.22 FAILED


==== filesystem-1.23 file normalisation FAILED
==== Contents of test case:

    file normalize c:/./foo

---- Result was:
C://foo
---- Result should have been (exact matching):
C:/foo
==== filesystem-1.23 FAILED

Logged In: YES 
user_id=148712

tclguy reports on XP: filesystem-1.19,22,23 fail, all with
extra or missing /

Logged In: YES 
user_id=148712

Tests ok on linux

Logged In: YES 
user_id=80530


fix committed to HEAD
please test on all platforms.

Logged In: YES 
user_id=148712

Today's HEAD on linux: 'make test' is clean, but
% cd /; pwd
/
% cd ..; pwd
% cd ..; pwd
..

Logged In: YES 
user_id=1312539

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

Logged In: YES 
user_id=80530


another attempt committed.
Windows testers needed to
check fileSystem.test

Logged In: YES 
user_id=80530

still same failure on Windows.

What a garbage routine this
is to debug.

File Added - 169653: 1379287-amend.patch

Logged In: YES 
user_id=80530


Here's an additional patch
I'm committing to HEAD.

Windows testers please?

Logged In: YES 
user_id=80530


Test failure on Windows:

---- Result was:
Paths should be equal: /bar , C:/bar
---- Result should have been (exact matching):
ok
==== filesystem-1.45 FAILED

Logged In: YES 
user_id=32170

Thanks dgp!  I've just had a kid so will not have much time
to examine this sort of thing for a while... Vince.

File Deleted - 169495:

File Added - 169576: 1379287.patch

Logged In: YES 
user_id=80530


Revised patch now works on
OSX and any other system 
where /tmp is a symlink.
Tests broken if /xxx is a
symlink, but that ought to
be rare enough to not worry about.

Committing so that someone
will test on Windows.

File Added - 169495: 1379287.patch

Logged In: YES 
user_id=80530


I think the attached patch
solves the problem, but review
and testing is welcome.

Logged In: YES 
user_id=80530


revised subject.

Note that the lexical processing
of ".." by TclFSNormalizeAbsolutePath
is already a point of controversy.
See Bug 961646.

Logged In: YES 
user_id=80530


The problem is with
TclFSNormalizeAbsolutePath().
In particular the call around
line 1884.

The "absolutePath" value passed
in is "/tmp/.." and the resulting
normalized form is "".

Logged In: YES 
user_id=80530

hmmm.. ok, several lines later
around lline 1849-1851, the
TCL_PATH_RELATIVE case is handled,
so this is just a bad var name.

Logged In: YES 
user_id=80530


Within Tcl_FSGetNormalizedPath()
at line 1830 of tclPathObj.c:

  Tcl_Obj *absolutePath = fsPathPtr->translatedPathPtr;

The variable name "absolutePath" suggests
the code expects that path to have
pathtype TCL_PATH_ABSOLUTE.  In this example,
that is not the case.  The value is "..",
which has pathtype TCL_PATH_RELATIVE.  If
the code does in fact depend on absolutePath
being an absolutePath, then things are broken
somewhere.  If it does not depend on that, then
the variable name choice is poor.

Logged In: YES 
user_id=80530


Finally, the next relevant revision:

2004-01-22  Vince Darley  <[email protected]>

        * tests/fileSystem.test: 3 new tests
        * generic/tclPathObj.c: fix to [Bug 879555] in file
normalization.
        * doc/filename.n: small clarification to Windows
behaviour with
        filenames like '.....', 'a.....', '.....a'.

left things in their current buggy state:

% cd /tmp 
% pwd
/tmp
% cd ..
% pwd
% cd ..
% pwd
..

Logged In: YES 
user_id=80530


Next relevant revision,

2004-01-21  Vince Darley  <[email protected]>

gets us closer to what we see now:

% cd /tmp
% pwd
/tmp
% cd ..
% pwd
% cd ..
% pwd
<CRASH>

Logged In: YES 
user_id=80530


The next relevant changes:

2003-12-17  Vince Darley  <[email protected]>

        * generic/tclCmdAH.c:
        * unix/tclUnixFile.c:
        * win/tclWinFCmd.c:
        * tests/fCmd.test:
        * tests/fileSystem.test:
        * doc/file.n: final fix to support for relative
links and
        its implications on normalization and other parts of the
        filesystem code.  Fixes [Bug 859251] and some Windows
        problems with recursive file delete/copy and
symbolic links.

2003-12-17  Vince Darley  <[email protected]>

        * generic/tclPathObj.c:
        * tests/fileSystem.test: fix and tests for [Bug
860402] in new
        file normalization code.

...left things buggy in a different way:

% cd /tmp
% pwd
/tmp
% cd ..
couldn't change working directory to "..": no such file or
directory

Logged In: YES 
user_id=80530

This test case worked correctly
until:

2003-12-14  Vince Darley  <[email protected]>

        * generic/tclPathObj.c: complete rewrite of generic file
        normalization code to cope with links followed by '..'.
        [Bug 849514], and parts of [859251]


That change made things buggy in this way:

% cd /tmp
% pwd
/tmp
% cd ..
% pwd
/tmp

Logged In: YES 
user_id=80530


Ditto 8.5a1.

Working theory is this bug
arose with the 2003-04-11
refactorization that produced
tclPathObj.c.

Logged In: YES 
user_id=80530


This bug is present in
Tcl 8.5a3 and 8.5a2 releases.

Still testing...

Logged In: YES 
user_id=585068

from CdObjCmd -> FSChdir -> Tcl_FSGetNormalizedPath we find
the failure I think.  pathPtr->bytes is ".." at the start.

None of the code in Tcl_FSGetNormalizedPath that updates the
->normPathPtr used at the end is executed.  There is only
one branch that is entered and that is this:

 if (fsPathPtr->cwdPtr != NULL) {
  ...
  if (...) { 
   [not executed]
  } else if (...) {
   [not executed]
  }
 }
 
We then reach:
 return fsPathPtr->normPathPtr

I added this before the return:
printf ("fsPathPtr->normPathPtr '%s'\n",
fsPathPtr->normPathPtr);

which output this:
fsPathPtr->normPathPtr ''

So the normPathPtr should be updated by some code in
Tcl_FSGetNormalizedPath based on my interpretation of the
code and output.

Logged In: YES 
user_id=148712

Still occurs on current HEAD. Any progress?