From: Dag-Erling S. <de...@of...> - 2002-11-26 22:31:15
Attachments:
sbcl.diff
|
Attached is a patch that fixes some problems in src/runtime (relative to the SBCL 0.7.9 sources from SourceForge) which I ran across while writing a FreeBSD port for SBCL. The second hunk, which adds #include <sys/ucontext.h> to bsd-os.h, is a showstopper for FreeBSD 5.0; the rest just fix warnings. While I'm here, I might as well point out that the SBCL sources in general seem to make incorrect use of many headers: - when sys headers are needed, they should always be included before non-sys headers (except for osreldate.h which may be needed to determine which headers to include) - several files lack <string.h>, probably because old versions of GCC don't warn about it because of a bug. (adressed in this patch) - some files lack <unistd.h>. (adressed in this patch) I have a SourceForge account (decs), so if you feel like it, give me commit access and I'll fix these and other style issues myself :) DES -- Dag-Erling Smorgrav - de...@of... |
From: Christophe R. <cs...@ca...> - 2002-11-27 12:42:15
|
On Tue, Nov 26, 2002 at 11:31:06PM +0100, Dag-Erling Smorgrav wrote: > Attached is a patch that fixes some problems in src/runtime (relative > to the SBCL 0.7.9 sources from SourceForge) which I ran across while > writing a FreeBSD port for SBCL. The second hunk, which adds #include > <sys/ucontext.h> to bsd-os.h, is a showstopper for FreeBSD 5.0; the > rest just fix warnings. Thanks -- can you say whether this change affects the ability to compile on earlier versions of FreeBSD? > While I'm here, I might as well point out that the SBCL sources in > general seem to make incorrect use of many headers: > > - when sys headers are needed, they should always be included before > non-sys headers (except for osreldate.h which may be needed to > determine which headers to include) By sys headers, do you mean "header files included with the system" or "header files in <sys/*.h>"? I'm perfectly willing to believe that the runtime's C coding style isn't wonderful -- can you explain the motivation behind this rule, though? > - several files lack <string.h>, probably because old versions of GCC > don't warn about it because of a bug. (adressed in this patch) > > - some files lack <unistd.h>. (adressed in this patch) Thank you. > --- gc-common.c.orig Tue Nov 26 23:10:44 2002 > +++ gc-common.c Tue Nov 26 23:11:55 2002 > @@ -1393,7 +1393,7 @@ > { > struct weak_pointer *wp; > for (wp = weak_pointers; wp != NULL; > - wp=(struct weak_pointer *)native_pointer(wp->next)) { > + wp=(struct weak_pointer *)native_pointer(LISPOBJ(wp->next))) { > lispobj value = wp->value; > lispobj *first_pointer; > gc_assert(widetag_of(wp->header)==WEAK_POINTER_WIDETAG); As part of the general bletcherousness of the runtime's source, there are a number of 32/64 bit issues; this one might be one of them. In particular, applying this bit of the patch without testing on the Alpha (and in general without having a certain amount of confidence that I understood all the nuances of casting to and from u32s) would worry me. (on the Alpha, wp->next is u32, not struct weak_pointer *) > --- sbcl.h.orig Tue Nov 26 23:06:27 2002 > +++ sbcl.h Tue Nov 26 23:08:21 2002 > @@ -134,7 +134,7 @@ > #define CONTROL_STACK_END LISPOBJ(1140846592) /* 0x43FFF000 */ > #define ALTERNATE_SIGNAL_STACK_START LISPOBJ(1140850688) /* 0x44000000 */ > #define DYNAMIC_SPACE_START LISPOBJ(1207959552) /* 0x48000000 */ > -#define DYNAMIC_SPACE_END LISPOBJ(2281701376) /* 0x88000000 */ > +#define DYNAMIC_SPACE_END LISPOBJ(2281701376U) /* 0x88000000 */ > > #define END_CORE_ENTRY_TYPE_CODE 3840 /* 0xF00 */ > #define VERSION_CORE_ENTRY_TYPE_CODE 3860 /* 0xF14 */ I should warn you that sbcl.h is a generated file (by the invocation of SB!VM::GENESIS at the end of the first stage of compilation -- see make-host-1.sh in the top-level directory). So if this is necessary, the change needs to take place elsewhere. Thanks for the patch, at least some of which will probably be merged post-0.7.10-release :-) Cheers, Christophe -- http://www-jcsu.jesus.cam.ac.uk/~csr21/ +44 1223 510 299/+44 7729 383 757 (set-pprint-dispatch 'number (lambda (s o) (declare (special b)) (format s b))) (defvar b "~&Just another Lisp hacker~%") (pprint #36rJesusCollegeCambridge) |
From: Dag-Erling S. <de...@of...> - 2002-11-27 13:50:59
|
Christophe Rhodes <cs...@ca...> writes: > Thanks -- can you say whether this change affects the ability to compile > on earlier versions of FreeBSD? It shouldn't. FreeBSD 4's sys/signal.h includes sys/ucontext.h, so the patch is a no-op for FreeBSD 4. > > - when sys headers are needed, they should always be included before > > non-sys headers (except for osreldate.h which may be needed to > > determine which headers to include) > > By sys headers, do you mean "header files included with the system" or > "header files in <sys/*.h>"? The latter. > I'm perfectly willing to believe that the > runtime's C coding style isn't wonderful -- can you explain the > motivation behind this rule, though? While some non-sys headers depend on sys headers, the reverse is never true. Also, the dependencies between sys headers are intricate and great care should be taken with regard to their relative ordering. > As part of the general bletcherousness of the runtime's source, there > are a number of 32/64 bit issues; this one might be one of them. In > particular, applying this bit of the patch without testing on the Alpha > (and in general without having a certain amount of confidence that > I understood all the nuances of casting to and from u32s) would worry > me. (on the Alpha, wp->next is u32, not struct weak_pointer *) native_pointer() expects a lispobj, so casting its argument to lispobj shouldn't change anything - at best, it silences a warning and makes the cast explicit to the reader, and at worst, it breaks already broken code. Regarding 32/64-bit issues and the use of u32: if lispobj were defined to intptr_t instead of uXX, portability might be improved. I have an Alpha running FreeBSD 5 and will test my port on it before committing it to the FreeBSD ports tree. > >> --- sbcl.h.orig Tue Nov 26 23:06:27 2002 >> +++ sbcl.h Tue Nov 26 23:08:21 2002 >> @@ -134,7 +134,7 @@ >> #define CONTROL_STACK_END LISPOBJ(1140846592) /* 0x43FFF000 */ >> #define ALTERNATE_SIGNAL_STACK_START LISPOBJ(1140850688) /* 0x44000000 */ >> #define DYNAMIC_SPACE_START LISPOBJ(1207959552) /* 0x48000000 */ >> -#define DYNAMIC_SPACE_END LISPOBJ(2281701376) /* 0x88000000 */ >> +#define DYNAMIC_SPACE_END LISPOBJ(2281701376U) /* 0x88000000 */ >> >> #define END_CORE_ENTRY_TYPE_CODE 3840 /* 0xF00 */ >> #define VERSION_CORE_ENTRY_TYPE_CODE 3860 /* 0xF14 */ > > I should warn you that sbcl.h is a generated file (by the invocation of > SB!VM::GENESIS at the end of the first stage of compilation -- see > make-host-1.sh in the top-level directory). So if this is necessary, > the change needs to take place elsewhere. OK. This is beyond me though, as I never looked at SBCL or CMUCL source code before yesterday. > Thanks for the patch, at least some of which will probably be merged > post-0.7.10-release :-) IWBNI the <sys/ucontext.h> patch were merged *before* the next release. When's it due, BTW? DES -- Dag-Erling Smorgrav - de...@of... |
From: Christophe R. <cs...@ca...> - 2002-11-27 14:03:14
|
On Wed, Nov 27, 2002 at 02:50:48PM +0100, Dag-Erling Smorgrav wrote: > I have an Alpha running FreeBSD 5 and will test my port on it before > committing it to the FreeBSD ports tree. Cool -- though I'm not sure if anyone's actually tried sbcl on FreeBSD/Alpha... you might be slightly in uncharted territory here. > > Thanks for the patch, at least some of which will probably be merged > > post-0.7.10-release :-) > > IWBNI the <sys/ucontext.h> patch were merged *before* the next release. > > When's it due, BTW? Sometime later today... I think you're in the hands of William Newman here (we've been in code freeze since Monday)... Bill? Christophe -- http://www-jcsu.jesus.cam.ac.uk/~csr21/ +44 1223 510 299/+44 7729 383 757 (set-pprint-dispatch 'number (lambda (s o) (declare (special b)) (format s b))) (defvar b "~&Just another Lisp hacker~%") (pprint #36rJesusCollegeCambridge) |
From: William H. N. <wil...@ai...> - 2002-11-27 14:27:42
|
On Wed, Nov 27, 2002 at 02:01:59PM +0000, Christophe Rhodes wrote: > On Wed, Nov 27, 2002 at 02:50:48PM +0100, Dag-Erling Smorgrav wrote: > > I have an Alpha running FreeBSD 5 and will test my port on it before > > committing it to the FreeBSD ports tree. > > Cool -- though I'm not sure if anyone's actually tried sbcl on > FreeBSD/Alpha... you might be slightly in uncharted territory here. > > > > Thanks for the patch, at least some of which will probably be merged > > > post-0.7.10-release :-) > > > > IWBNI the <sys/ucontext.h> patch were merged *before* the next release. > > > > When's it due, BTW? > > Sometime later today... I think you're in the hands of William Newman > here (we've been in code freeze since Monday)... Bill? Yes, 0.7.10 should probably be released within a few hours. I plan to do with the sys/ucontext.h fix what I have sometimes done before with other late-breaking stuff: I'll mention in the release announcement that the patch exists and will soon be in CVS. After that, we typically release late each month, so the patch will probably be in the 0.7.11 release by around Christmas. -- William Harold Newman <wil...@ai...> "If you can't remember what mnemonic means, you've got a problem." -- Wall, Christiansen, and Schwartz, _Programming Perl_, 2d edn., p. 548. PGP key fingerprint 85 CE 1C BA 79 8D 51 8C B9 25 FB EE E0 C3 E5 7C |
From: Christophe R. <cs...@ca...> - 2002-12-06 15:03:54
|
On Tue, Nov 26, 2002 at 11:31:06PM +0100, Dag-Erling Smorgrav wrote: > Attached is a patch that fixes some problems in src/runtime (relative > to the SBCL 0.7.9 sources from SourceForge) which I ran across while > writing a FreeBSD port for SBCL. The second hunk, which adds #include > <sys/ucontext.h> to bsd-os.h, is a showstopper for FreeBSD 5.0; the > rest just fix warnings. Thanks. I've committed the inclusion of sys/ucontext.h into sbcl-0.7.10.13; I've left the rest of the problems (warning fixes) alone, because I don't have a good testing environment for the *BSDs. Many thanks, Cheers, Christophe -- http://www-jcsu.jesus.cam.ac.uk/~csr21/ +44 1223 510 299/+44 7729 383 757 (set-pprint-dispatch 'number (lambda (s o) (declare (special b)) (format s b))) (defvar b "~&Just another Lisp hacker~%") (pprint #36rJesusCollegeCambridge) |