svn commit: r186519 - head

M. Warner Losh imp at bsdimp.com
Thu Jan 1 06:38:11 UTC 2009


In message: <86r63p5334.fsf at ds4.des.no>
            Dag-Erling_Smørgrav <des at des.no> writes:
: "M. Warner Losh" <imp at bsdimp.com> 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 be
: 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
: ===================================================================
: --- /home/des/freebsd/base/head/lib/libpam/modules/Makefile.inc	(revision 186063)
: +++ /home/des/freebsd/base/head/lib/libpam/modules/Makefile.inc	(working copy)
: @@ -19,4 +19,7 @@
:  LDADD+=		-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_MODULES)
/* gcc, static linking */
# include <sys/cdefs.h>
# include <linker_set.h>
# define OPENPAM_STATIC_MODULES
# define PAM_EXTERN static
# define PAM_MODULE_ENTRY(name)						\
	static char _pam_name[] = name PAM_SOEXT;			\
	static struct pam_module _pam_module = {			\
		.path = _pam_name,					\
		.func = {						\
			[PAM_SM_AUTHENTICATE] = _PAM_SM_AUTHENTICATE,	\
			[PAM_SM_SETCRED] = _PAM_SM_SETCRED,		\
			[PAM_SM_ACCT_MGMT] = _PAM_SM_ACCT_MGMT,		\
			[PAM_SM_OPEN_SESSION] = _PAM_SM_OPEN_SESSION,	\
			[PAM_SM_CLOSE_SESSION] = _PAM_SM_CLOSE_SESSION, \
			[PAM_SM_CHAUTHTOK] = _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 defined 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

<etc>

The only thing that I've seen that really works is the following
unsatisfying kludge:

Index: include/security/openpam.h
===================================================================
--- 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
===================================================================
--- 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_MODULES)
+#if defined(__GNUC__) && defined(OPENPAM_STATIC_MODULES) && !defined(NO_STATIC_MODULES)
 /* gcc, static linking */
 # include <sys/cdefs.h>
 # include <linker_set.h>
-# define OPENPAM_STATIC_MODULES
 # define PAM_EXTERN static
 # define PAM_MODULE_ENTRY(name)						\
 	static char _pam_name[] = name PAM_SOEXT;			\
Index: lib/libpam/modules/Makefile.inc
===================================================================
--- lib/libpam/modules/Makefile.inc	(revision 186639)
+++ lib/libpam/modules/Makefile.inc	(working copy)
@@ -19,4 +19,7 @@
 LDADD+=		-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...


More information about the freebsd-arch mailing list