portable: two fixes - #620
Open
cyclistmass wants to merge 2 commits into
Open
Conversation
library/core-files.lisp is compiled only under #+:linuxx8664-target
today, but its central tag dispatch is written against target::
constants as if it were shared code. Two of them exist only in the
x8664 tag model: TAG-TRA (x8664-arch.lisp is the only definer in the
whole tree) and FULLTAG-FUNCTION (on arm64 and ppc64 a function is an
ordinary uvector reached through the FULLTAG-MISC clause; the arm64
tag assignment deliberately has no function fulltag). Guard those two
clauses with #+x8664-target, the idiom l1-clos-boot.lisp already uses
for the same per-target dispatch in the %class-of table.
uvheader-type, in the same file, hard-codes x8664:: for the five
header-class fulltags. Those values differ across targets -- x8664
has fulltag-nodeheader-1 = 6 while arm64 has fulltag-nodeheader-0 = 6
-- so on any other target every uvector header class would be looked
up in the wrong table and the reported type would be silently wrong:
an arm64 function header (subtag #x96, low nibble 6) would be filed
under *nodeheader-1-types* instead of *nodeheader-0-types*. Use
target:: so the same-architecture assumption the file already states
("Assumes we're running on same architecture as core file") holds.
Both changes are no-ops on linuxx8664, where TARGET is the X8664
package. They do not enable core-files anywhere new by themselves:
off linuxx8664 the file still needs, at least, %%raw-obj (a
defx86lapfunction), uvheader-byte-size (x8664-misc-byte-count),
uvheader-p's numeric header-class range, the recover-fn-from-rip-*
function recovery, and an xcf layout on targets that lack one. Note
also that level-1/l1-boot-2.lisp boot-provides "CORE-FILES" on every
platform, so off linuxx8664 (require 'core-files) silently no-ops
instead of reporting that the facility does not exist.
…tring
CLHS says *compile-file-pathname* is "the pathname of the file being compiled,
merged with the defaults", and its value type is a pathname or nil. In CCL it
is a STRING whenever the source pathname has a directory component, which is to
say almost always outside a bare `(compile-file "foo.lisp")' from the current
directory.
ansi-tests COMPILE-FILE.16 has been failing on this for as long as we have been
running the suite. It is not architecture-specific -- it reproduces identically
on x86-64 at 1.12.2 and at 1.13, and the source is portable.
=== THE PATH THE VALUE TAKES
lib/nfcomp.lisp starts with a pathname, and every step keeps it one:
(let* ((orig-src (merge-pathnames src)) ; a PATHNAME
...
(when (physical-pathname-p orig-src)
(setq orig-src (back-translate-pathname orig-src '("home" "ccl"))))
...
(*compile-file-pathname* orig-src)
back-translate-pathname is where the type is lost. lib/pathnames.lisp:
(defun back-translate-pathname (path &optional hosts)
(let ((newpath (back-translate-pathname-1 path hosts)))
(cond ((equalp path newpath)
;; (fcomp-standard-source path)
(namestring (pathname path))) ; <- a STRING
(t newpath)))) ; <- a PATHNAME
The function returns a string or a pathname depending on whether a logical-host
translation happened to match. back-translate-pathname-1 returns its argument
unchanged when nothing matches, which is the common case, so the string branch
is the one almost every caller gets. A function named back-translate-PATHNAME
returning a namestring half the time is the defect; %compile-file is just the
caller that stores the result somewhere the standard constrains.
The commented-out `(fcomp-standard-source path)' on the line above is the
vestige of whatever this once did.
=== MEASURED, both branches, on 1.13 (v1.13-309-g7cff8ca5) x86-64
*compile-file-pathname* after compiling the same file, varying only
*default-pathname-defaults*:
DPD = #P"" -> #P"compile-file-test-file-5.lsp"
type PATHNAME COMPILE-FILE.16 passes
DPD = #P"<dir>/ansi-tests/" -> "<dir>/ansi-tests/compile-file-test-file-5.lsp"
type SIMPLE-BASE-STRING COMPILE-FILE.16 fails
physical-pathname-p (lib/pathnames.lisp) is the gate, and it requires a
directory component:
#P"<dir>/ansi-tests/compile-file-test-file-5.lsp" physical-pathname-p = T branch RUNS
#P"tests:ansi-tests;compile-file-test-file-5.lsp" physical-pathname-p = NIL branch SKIPPED
That is also why the failure looks intermittent across harnesses rather than
constant: a suite driver that sets *default-pathname-defaults* to a directory
exposes it, and one that only chdir's does not, because CCL's `cwd'
(level-1/l1-files.lisp) calls %chdir and never assigns
*default-pathname-defaults*.
Red-then-green control, in one process, redefining only this function:
BEFORE *compile-file-pathname* = "<dir>/ansi-tests/compile-file-test-file-5.lsp"
(SIMPLE-BASE-STRING 57) COMPILE-FILE.16 value 2 -> FAIL
AFTER *compile-file-pathname* = #P"<dir>/ansi-tests/compile-file-test-file-5.lsp"
PATHNAME COMPILE-FILE.16 value 2 -> PASS
*compile-file-truename* was correct throughout (nfcomp.lisp is
`(truename src)'), and passes in both halves -- only the second value of the
test moves.
=== WHY THE FIX IS AT THE FUNCTION AND NOT AT THE BINDING
Coercing at nfcomp.lisp would fix COMPILE-FILE.16 and leave the
type-inconsistent function in place for the next caller. Every other caller in
the tree already wraps the result in `namestring', so none of them can observe
the change:
lib/source-files.lisp (namestring (back-translate-pathname file-name))
cocoa-ide/search-files.lisp (namestring (ccl::back-translate-pathname ...))
cocoa-ide/search-files-pre-lion.lisp (namestring (ccl::back-translate-pathname ...))
and `(namestring <pathname>)' is the same string it returns today. Inside
%compile-file the only other use of orig-src is
`(namestring orig-src)', which is likewise unaffected.
The fix site is byte-identical on origin/master and origin/arm64.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1.
*compile-file-pathname*must hold a pathname, not a namestring.CLHS gives it a value type of pathname or nil. CCL binds it to a STRING
whenever
physical-pathname-pis true of the merged source, which needs adirectory component and is therefore the common case.
back-translate-pathnameloses the type atlib/pathnames.lisp:50-55:back-translate-pathname-1returns its argument unchanged when no logical-hosttranslation matches, so almost every caller gets the string branch.
%compile-fileis the caller that stores the result where the standardconstrains it:
lib/nfcomp.lisp:204gates onphysical-pathname-p,:205back-translates, and
:221binds*compile-file-pathname*to the result.Measured on x86-64 at 1.13 (
v1.13-309-g7cff8ca5), varying only*default-pathname-defaults*:ansi-tests COMPILE-FILE.16 fails on stock x86-64 CCL at 1.12.2 and at 1.13,
21679 tests with that one failure. A red-then-green in one process, redefining
only this function, moves value 2 of the test from FAIL to PASS.
*compile-file-truename*is correct throughout and passes in both halves.That gate is also where the failure hides: a suite driver that sets
*default-pathname-defaults*exposes the defect, and one that only chdirs doesnot. Our own arm64 suite reports no failure for that second reason, so it is
not evidence either way.
The fix goes at the function, not at the binding. The three other callers in
the tree, at
lib/source-files.lisp:745,cocoa-ide/search-files.lisp:542andcocoa-ide/search-files-pre-lion.lisp:188, all wrap the result innamestringalready, so none of them can observe the change. The fix site is byte-identical
on
masterandarm64.2. Keep x8664-only tag constants out of the shared dispatch.
library/core-files.lispcompiles only under#+:linuxx8664-targettoday, andits central tag dispatch reads as if it were shared, because it uses
target::constants. Two of those exist only in the x8664 tag model:
TAG-TRAandFULLTAG-FUNCTION. On arm64 and ppc64 a function is an ordinary uvectorreached through the
FULLTAG-MISCclause. Those two clauses get#+x8664-target, the idioml1-clos-boot.lispalready uses for the sameper-target dispatch.
uvheader-typein the same file hard-codesx8664::for the fiveheader-class fulltags, and those differ across targets. x8664 has
fulltag-nodeheader-1 = 6while arm64 hasfulltag-nodeheader-0 = 6. On anyother target the code would look up every uvector header class in the wrong
table and report the wrong type in silence. An arm64 function header, subtag
#x96with low nibble 6, would land under*nodeheader-1-types*.Both changes are no-ops on linuxx8664, where
TARGETis theX8664package.Neither one enables core-files anywhere new on its own.
Verified at this base. Built and run on linuxarm64 at
ec578745with allten patches applied: ANSI 21679 tests, 0 failures.
tests/ccl.lsp243 tests, 0failures. Image
281a49e5, kernel67bb66b4.