From owner-freebsd-arch@FreeBSD.ORG Thu Jan 1 06:38:11 2009 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BD039106564A; Thu, 1 Jan 2009 06:38:11 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id 715D18FC08; Thu, 1 Jan 2009 06:38:11 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.2/8.14.1) with ESMTP id n016aUN5028383; Wed, 31 Dec 2008 23:36:30 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Wed, 31 Dec 2008 23:36:44 -0700 (MST) Message-Id: <20081231.233644.1974810120.imp@bsdimp.com> To: des@des.no, ru@freebsd.org From: "M. Warner Losh" In-Reply-To: <86r63p5334.fsf@ds4.des.no> References: <26259E4E-6E26-4DAE-8046-80C7C46B7CD5@gmail.com> <20081227.193308.255407637.imp@bsdimp.com> <86r63p5334.fsf@ds4.des.no> X-Mailer: Mew version 5.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Cc: yanefbsd@gmail.com, bz@freebsd.org, arch@freebsd.org Subject: Re: svn commit: r186519 - head X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Jan 2009 06:38:11 -0000 In message: <86r63p5334.fsf@ds4.des.no> Dag-Erling_Sm=F8rgrav writes: : "M. Warner Losh" writes: : > Still working on a solution that's correct and mutually acceptable = to : > DES. i think the ball is in my court. : = : No, it's a build system issue. If it's in anyone's court, it would b= e : ru@'s. : = : The problem is that we need to use different CFLAGS for .o and .So. = The : quick and dirty fix is to override the .c.o rule for PAM modules: : = : Index: /home/des/freebsd/base/head/lib/libpam/modules/Makefile.inc : =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D : --- /home/des/freebsd/base/head/lib/libpam/modules/Makefile.inc (revi= sion 186063) : +++ /home/des/freebsd/base/head/lib/libpam/modules/Makefile.inc (work= ing copy) : @@ -19,4 +19,7 @@ : LDADD+=3D -lpam : .endif : = : +.c.o: : + ${CC} ${CFLAGS} -DOPENPAM_STATIC_MODULES -c ${.IMPSRC} : + : .include "../Makefile.inc" I just tried this, and it doesn't work for me. I still get the errors. The problem, I think, is that this doesn't get to the root-cause of the issues I'm seeing. This define doesn't affect enough things. If we look at contrib/openpam/include/security/openpam.h, we see: /* * Infrastructure for static modules using GCC linker sets. * You are not expected to understand this. */ #if defined(__FreeBSD__) # define PAM_SOEXT ".so" #else # undef NO_STATIC_MODULES # define NO_STATIC_MODULES #endif #if defined(__GNUC__) && !defined(__PIC__) && !defined(NO_STATIC_MODULE= S) /* gcc, static linking */ # include # include # define OPENPAM_STATIC_MODULES # define PAM_EXTERN static # define PAM_MODULE_ENTRY(name) \ static char _pam_name[] =3D name PAM_SOEXT; \ static struct pam_module _pam_module =3D { \ .path =3D _pam_name, \ .func =3D { \ [PAM_SM_AUTHENTICATE] =3D _PAM_SM_AUTHENTICATE, \ [PAM_SM_SETCRED] =3D _PAM_SM_SETCRED, \ [PAM_SM_ACCT_MGMT] =3D _PAM_SM_ACCT_MGMT, \ [PAM_SM_OPEN_SESSION] =3D _PAM_SM_OPEN_SESSION, \ [PAM_SM_CLOSE_SESSION] =3D _PAM_SM_CLOSE_SESSION, \ [PAM_SM_CHAUTHTOK] =3D _PAM_SM_CHAUTHTOK \ }, \ }; \ DATA_SET(_openpam_static_modules, _pam_module) #else /* normal case */ # define PAM_EXTERN # define PAM_MODULE_ENTRY(name) #endif so defining OPENPAM_STATIC_MODULES doesn't affect the PAM_EXTERN define, so I get multiple defined things because __PIC__ is always defined for mips, both for .o and .so compilation (because MIPS always does PIC code for ABI compliance): ../modules/pam_deny/libpam_deny.a(pam_deny.o)(.text+0x3c): In function = `pam_sm_open_session': : multiple definition of `pam_sm_open_session' ../modules/pam_chroot/libpam_chroot.a(pam_chroot.o)(.text+0x14): first = defined here ../modules/pam_deny/libpam_deny.a(pam_deny.o)(.text+0x50): In function = `pam_sm_close_session': : multiple definition of `pam_sm_close_session' ../modules/pam_chroot/libpam_chroot.a(pam_chroot.o)(.text+0x0): first d= efined here ../modules/pam_echo/libpam_echo.a(pam_echo.o)(.text+0x0): In function `= pam_sm_setcred': : multiple definition of `pam_sm_setcred' ../modules/pam_deny/libpam_deny.a(pam_deny.o)(.text+0x0): first defined= here The only thing that I've seen that really works is the following unsatisfying kludge: Index: include/security/openpam.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- include/security/openpam.h (revision 186587) +++ include/security/openpam.h (working copy) @@ -308,8 +308,9 @@ /* * Infrastructure for static modules using GCC linker sets. * You are not expected to understand this. + * And it doesn't work on mips, so is disabled there. */ -#if defined(__FreeBSD__) +#if defined(__FreeBSD__) && !defined(__mips__) # define PAM_SOEXT ".so" #else # undef NO_STATIC_MODULES Or the following also works, which seems less kludgy. Note, this hasn't been run-time tested yet, just compile time. It resolves the overloading of __PIC__ to mean 'compiling dynamic libraries' by relying on the OPENPAM_STATIC_MODULES being defined. It also has the advantage of not disabling this functionality on mips. It has the very mild disadvantage of not wrapping at 80 columns too :) Index: contrib/openpam/include/security/openpam.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- contrib/openpam/include/security/openpam.h (revision 186639) +++ contrib/openpam/include/security/openpam.h (working copy) @@ -316,11 +316,10 @@ # define NO_STATIC_MODULES #endif = -#if defined(__GNUC__) && !defined(__PIC__) && !defined(NO_STATIC_MODUL= ES) +#if defined(__GNUC__) && defined(OPENPAM_STATIC_MODULES) && !defined(N= O_STATIC_MODULES) /* gcc, static linking */ # include # include -# define OPENPAM_STATIC_MODULES # define PAM_EXTERN static # define PAM_MODULE_ENTRY(name) \ static char _pam_name[] =3D name PAM_SOEXT; \ Index: lib/libpam/modules/Makefile.inc =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- lib/libpam/modules/Makefile.inc (revision 186639) +++ lib/libpam/modules/Makefile.inc (working copy) @@ -19,4 +19,7 @@ LDADD+=3D -lpam .endif = +.c.o: + ${CC} ${CFLAGS} -DOPENPAM_STATIC_MODULES -c ${.IMPSRC} + .include "../Makefile.inc" btw, I suspect that if we have a #define that's really true only for .so's, then we should use that. My quick look at the spec file didn't show one. I think one will have to come from the build system, and I'm a little hesitant to innovate there too much without first having a discussion about the right way to go. Looking at your patch here, it seems to be along the same lines that I'm thinking, but I'm not sure I like the new variable names. Comments? Warner P.S. I've moved this discussion to arch@, which appears to be our best, high S/N mailing list these days...