George Koehler <kern...@gmail.com> writes: > On Wed, 23 Feb 2022 00:08:34 +0100 > Omar Polo <o...@openbsd.org> wrote: > >> I think I've finally solved it. The problem is that contrib/sb-capstone >> hardcodes some values from capstone.h so lisp can call the C functions. >> This "transcription" however is not accurate and worked until now by >> pure chance on 64 bit arches due to how arguments are passed and struct >> padding, pure luck! > > I tried to patch capstone.lisp but didn't fix my build. Your patch > looks better, but I haven't built it. I have comments below. > > The build deletes every *.orig and breaks "make update-patches", so > I started using "PATCHORIG= .orig.port". Joshua Elsasser, as the > maintainer, do you have an opinion about PATCHORIG? > > Go down for patch comments. > >> [...] >> Index: patches/patch-contrib_sb-capstone_capstone_lisp >> =================================================================== >> RCS file: patches/patch-contrib_sb-capstone_capstone_lisp >> diff -N patches/patch-contrib_sb-capstone_capstone_lisp >> --- /dev/null 1 Jan 1970 00:00:00 -0000 >> +++ patches/patch-contrib_sb-capstone_capstone_lisp 22 Feb 2022 23:07:25 >> -0000 >> @@ -0,0 +1,26 @@ >> +$OpenBSD$ >> + >> +sync a couple of size mismatches with capstone.h; fixes the build on >> +32bit arches (i386, powerpc.) >> + >> +Index: contrib/sb-capstone/capstone.lisp >> +--- contrib/sb-capstone/capstone.lisp.orig >> ++++ contrib/sb-capstone/capstone.lisp >> +@@ -285,7 +285,7 @@ >> + (define-alien-type cs-insn >> + (struct nil >> + (insn-id int) >> +- (insn-addr unsigned) >> ++ (insn-addr (integer 64)) >> + (insn-size short) >> + (insn-bytes (array char 16)) >> + (insn-mnemonic (array char 32)) > > I guess that it should be (insn-addr (unsigned 64)), to match the > capstone.h's struct cs_insn's uint64_t address.
agreed, sorry for the typo, i meant unsigned > I have not called C from Lisp before now, so sb-alien confused me. > I now believe that "integer" and "unsigned" have pointer size, but > "int" and "unsigned-int" have 32 bits. So "unsigned" had 64 bits > on amd64, or 32 bits on i386|powerpc. I've yet to find a list of types and their counterpart in C in the sbcl documentation, I'm only finding stuff for sb-groveler which uses sb-alien. Anyway, I *think* that `integer' and `unsigned' should mirror their C counterpart (int and unsigned int), so 32 bit on amd64, i386 and powerpc. http://www.lichteblau.com/sbcl/doc/manual/sbcl/Foreign-Type-Specifiers.html My (unconfirmed) guess is that on amd64 the compiler aligns the short at 64 bit boundaries and so there are 32 bits of padding between ins-addr and ins-size, which makes the difference between `unsigned' and (integer 64) not noticeable for small values -- at least on LE. test.lisp uses small chunks that output only a few bytes so the truncation due to the wrong size went unseen. Testing on a 64bit BE arch would have discovered this issue too probably. >> +@@ -310,7 +310,7 @@ >> + >> + ;; The handle returned by cs-open will be represented as being of type >> unsigned >> + >> +-(define-alien-routine cs-open int (arch int) (mode (integer 64)) (handle >> unsigned :out)) >> ++(define-alien-routine cs-open int (arch int) (mode unsigned) (handle >> unsigned :out)) >> + >> + (define-alien-routine cs-version unsigned (major int :out) (minor int >> :out)) >> + > > cs-open should have "(mode int)". <capstone.h> has "cs_mode mode" and > powerpc clang says that "(cs_mode)-1 > 0" is false, implying that > cs_mode is a signed type. (i forgot to explain this in the previous mail, sorry) That was my first thought as well, but the third test fails if `mode' is `int': Test SB-CAPSTONE-TESTS::PPC-BIG-ENDIAN failed Form: (SB-CAPSTONE-TESTS::CAPSTONE-CHECK '(124 8 2 166) '(:PPC64 :BIG-ENDIAN) '("MFLR R0")) Expected value: T Actual value: #<TYPE-ERROR expected-type: (SIGNED-BYTE 32) datum: 2147483656>. The value 2147483656 is not of type (SIGNED-BYTE 32) my reading is that capstone.lisp:get-cs-mode tries to set the most significant bit which is fine in C but sbcl begs to differ. (2147483656 is actually -2147483640 in base 2 complement if I'm doing the math right, or 0x80000008) This probably explain why upstream went for (integer 64), sbcl doesn't warn and the value is still correctly passed to and read by the library on amd64 (don't know the details of the calling convention but I guess 32 and 64 bits integers are passed on registers.) > I also think that cs-version should return "unsigned-int" (32 bits), > not "unsigned" (64 bits on amd64), though I suspect that amd64 > tolerates the wrong size. It's true that the wrong size is tolerated but I think that `unsigned' mirrors C' `unsigned int', so it's already correct? > --George Updated diff below with the type fixed for insn-addr and PATCHORIG for our sanity :) (note that I haven't finished to build with the following patch, I just started it, but I'm quite confident it'll work) Thanks! Index: Makefile =================================================================== RCS file: /home/cvs/ports/lang/sbcl/Makefile,v retrieving revision 1.47 diff -u -p -r1.47 Makefile --- Makefile 31 Dec 2021 09:53:11 -0000 1.47 +++ Makefile 23 Feb 2022 09:03:58 -0000 @@ -1,35 +1,17 @@ # $OpenBSD: Makefile,v 1.47 2021/12/31 09:53:11 solene Exp $ -BROKEN-i386 = build fails in "Compiling file [...]/src/compiler/generic/genesis.lisp" -# ;; Compiling file /pobj/sbcl-2.0.1/sbcl-2.0.1/src/compiler/generic/genesis.lisp ... -# ;; Wrote file /pobj/sbcl-2.0.1/sbcl-2.0.1/obj/from-host/src/compiler/generic/genesis.fas-tmp -# 0 errors, 0 warnings -# ;; Loading file obj/from-host/src/compiler/generic/genesis.fas ... -# ;; Loaded file obj/from-host/src/compiler/generic/genesis.fas -# *** - OPEN: File -# #P"/pobj/sbcl-2.0.1/sbcl-2.0.1/obj/from-xc/tls-init.lisp-expr" does not -# exist -# The following restarts are available: -# SKIP :R1 skip (GENESIS OBJECT-FILE-NAMES # ...) -# RETRY :R2 retry (GENESIS OBJECT-FILE-NAMES # ...) -# STOP :R3 stop loading file /pobj/sbcl-2.0.1/sbcl-2.0.1/make-genesis-2.lisp -# ABORT-BUILD :R4 Abort building SBCL. -# ABORT :R5 Abort main loop -# //testing for consistency of first and second GENESIS passes -# diff: output/genesis-2: No such file or directory -# error: header files do not match between first and second GENESIS - # not yet ported to other arches ONLY_FOR_ARCHS = amd64 i386 powerpc USE_WXNEEDED = Yes COMMENT= compiler and runtime system for ANSI Common Lisp -V = 2.1.11 +V = 2.2.1 DISTNAME= sbcl-${V}-source PKGNAME= sbcl-${V} WRKDIST= ${WRKDIR}/sbcl-${V} EXTRACT_SUFX= .tar.bz2 +PATCHORIG = .orig.port CATEGORIES= lang HOMEPAGE= http://www.sbcl.org/ @@ -58,10 +40,17 @@ WANTLIB+= pthread MAKE_PARAMS += --with-sb-core-compression \ --with-sb-xref-for-internals +# contrib/sb-capstone/test.lisp uses it at build-time if present +BUILD_DEPENDS = devel/capstone/main + .if ${FLAVOR:Mnative_bootstrap} BUILD_DEPENDS+= lang/sbcl BOOTSTRAP_CMD= ${LOCALBASE}/bin/sbcl \ --disable-debugger --no-sysinit --no-userinit +.elif ${MACHINE_ARCH:Mi386} +# ecl is slower but lang/clips fails to build sbcl on i386 +BUILD_DEPENDS += lang/ecl +BOOTSTRAP_CMD = ${LOCALBASE}/bin/ecl -q --norc .else BUILD_DEPENDS += lang/clisp BOOTSTRAP_CMD = ${LOCALBASE}/bin/clisp -q -norc Index: distinfo =================================================================== RCS file: /home/cvs/ports/lang/sbcl/distinfo,v retrieving revision 1.21 diff -u -p -r1.21 distinfo --- distinfo 31 Dec 2021 09:53:11 -0000 1.21 +++ distinfo 22 Feb 2022 23:07:36 -0000 @@ -1,2 +1,2 @@ -SHA256 (sbcl-2.1.11-source.tar.bz2) = v8FIHef9vfru8qsPDo6E79NDQz3qjSHPvqiwFGy9/v0= -SIZE (sbcl-2.1.11-source.tar.bz2) = 6687529 +SHA256 (sbcl-2.2.1-source.tar.bz2) = Xdbm4/CLfG7fJioOhEqfi15WLMoIFVA0wfLAFPyQh9o= +SIZE (sbcl-2.2.1-source.tar.bz2) = 6701705 Index: patches/patch-contrib_sb-capstone_capstone_lisp =================================================================== RCS file: patches/patch-contrib_sb-capstone_capstone_lisp diff -N patches/patch-contrib_sb-capstone_capstone_lisp --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ patches/patch-contrib_sb-capstone_capstone_lisp 23 Feb 2022 08:55:36 -0000 @@ -0,0 +1,26 @@ +$OpenBSD$ + +sync a couple of size mismatches with capstone.h; fixes the build on +32bit arches (i386, powerpc.) + +Index: contrib/sb-capstone/capstone.lisp +--- contrib/sb-capstone/capstone.lisp.orig ++++ contrib/sb-capstone/capstone.lisp +@@ -285,7 +285,7 @@ + (define-alien-type cs-insn + (struct nil + (insn-id int) +- (insn-addr unsigned) ++ (insn-addr (unsigned 64)) + (insn-size short) + (insn-bytes (array char 16)) + (insn-mnemonic (array char 32)) +@@ -310,7 +310,7 @@ + + ;; The handle returned by cs-open will be represented as being of type unsigned + +-(define-alien-routine cs-open int (arch int) (mode (integer 64)) (handle unsigned :out)) ++(define-alien-routine cs-open int (arch int) (mode unsigned) (handle unsigned :out)) + + (define-alien-routine cs-version unsigned (major int :out) (minor int :out)) +
