MySQL Bugs: #53916: storage/innodb_plugin does not compile on NetBSD/sparc64

Bug #53916	storage/innodb_plugin does not compile on NetBSD/sparc64
Submitted:	22 May 2010 21:44	Modified:	14 Dec 2010 19:40
Reporter:	Martin Husemann	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: InnoDB Plugin storage engine	Severity:	S3 (Non-critical)
Version:	5.1.46	OS:	Other (NetBSD/sparc64)
Assigned to:	Vasil Dimov	CPU Architecture:	Any

Description:
When trying to compile mysql on NetBSD/sparc64 it failse with:

libtool: compile:  c++ -DHAVE_CONFIG_H -I. -I../../include -I../../regex -I./include -I../../sql -Dunix -I/usr/pkgobj/databases/mysql51-server/work/.buildlink/include/mysql -DMYSQL_DYNAMIC_PLUGIN -O2 -pipe -DUSE_OLD_FUNCTIONS -fPIC -DPIC -fPIC -fno-implicit-templates -fno-exceptions -fno-rtti -c handler/ha_innodb.cc  -fPIC -DPIC -o .libs/ha_innodb_plugin_la-ha_innodb.o
./include/sync0sync.ic: In function 'unsigned char mutex_test_and_set(ib_mutex_t*)':
./include/sync0sync.ic:83: error: 'atomic_swap_uchar' was not declared in this scope
./include/sync0sync.ic: In function 'void mutex_reset_lock_word(ib_mutex_t*)':
./include/sync0sync.ic:114: error: 'atomic_swap_uchar' was not declared in this scope
./include/sync0rw.ic: In function 'void rw_lock_set_writer_id_and_recursion_flag(rw_lock_t*, ulint)':
./include/sync0rw.ic:281: error: cannot convert '__pthread_st* volatile*' to 'volatile __uint64_t*' for argument '1' to '__uint64_t atomic_cas_64(volatile __uint64_t*, __uint64_t, __uint64_t)'
gmake[2]: *** [ha_innodb_plugin_la-ha_innodb.lo] Error 1
gmake[2]: Leaving directory `/usr/pkgobj/databases/mysql51-server/work/mysql-5.1.46/storage/innodb_plugin'
gmake[1]: *** [all-recursive] Error 1

The code in question is:

UNIV_INLINE
byte
mutex_test_and_set(
/*===============*/
        mutex_t*        mutex)  /*!< in: mutex */
{
#if defined(HAVE_ATOMIC_BUILTINS)
        return(os_atomic_test_and_set_byte(&mutex->lock_word, 1));
#else
..

I'm a bit puzzled by the idea of  a "atomic_test_and_set_byte" operation (I think m68k is the last architecture I saw supporting that), all modern archs only support 32 bit or 64 bit atomic operations.

How to repeat:
This does not hit when configure finds the (CISCy) builtin atomic operations, for example building the same source on NetBSD/amd64 works fine.

Suggested fix:
Use 32bit atomic operations for mutices, or just use the pthread ones?

Thank you for the report.

I can not repeat described behavior. Please provide configure options you used.

Stock pkgsrc:

[/usr/pkgsrc/databases/mysql5-client] martin@nelly > make -VCONFIGURE_ARGS
--with-openssl=${BUILDLINK_PREFIX.openssl} --localstatedir=${MYSQL_DATADIR:Q} --with-named-z-libs=z --without-libwrap --without-readline --without-libedit --disable-dependency-tracking --without-debug --without-bench --with-low-memory --with-zlib-dir=${BUILDLINK_PREFIX.zlib:Q} --with-vio --with-charset=${MYSQL_CHARSET:Q} --with-extra-charsets=${MYSQL_EXTRA_CHARSET:Q} --without-extra-tools --without-server --enable-thread-safe-client --prefix=${GNU_CONFIGURE_PREFIX:Q} --build=${MACHINE_GNU_PLATFORM:Q} --host=${MACHINE_GNU_PLATFORM:Q} --infodir=${GNU_CONFIGURE_INFODIR:Q} --mandir=${GNU_CONFIGURE_MANDIR:Q}

Thank you for the feedback.

I still can not repeat described behavior. Please expand variables, especially $MYSQL_*CHARSET

Sorry - here is the full expanded version:

[/usr/pkgsrc/databases/mysql5-client] martin@nelly > make -V '${CONFIGURE_ARGS}' 
--with-openssl=/usr --localstatedir=/var/mysql --with-named-z-libs=z --without-libwrap --without-readline --without-libedit --disable-dependency-tracking --without-debug --without-bench --with-low-memory --with-zlib-dir= --with-vio --with-charset=latin1 --with-extra-charsets=all --without-extra-tools --without-server --enable-thread-safe-client --prefix=/usr/pkg --build=sparc64--netbsd --host=sparc64--netbsd --infodir=/usr/pkg/info --mandir=/usr/pkg/man

Please try to use gmake instead of "BSD"-make.

Verified, this is specific to NetBSD/sparc64.  Assigning over to InnoDB team to take a look.

Valid workaround in pkgsrc is to disable the plugin, for now.  Would be interesting to see if the builtin InnoDB in 5.5+ has the same issue.

This might be considered a simple configure problem: NetBSD/sparc64 does have most of the same atomic_* functions that Solaris does provide (i.e. it has atomic_swap_32(), atomic_swap_ptr(), ...) - but not for data types where the hardware does not natively support atomic operations, like single bytes.

From this POV, the configure magic should have rejected the atomic.h interface for this platform.

From a different point of view (and I have no idea what InnoDB is all about) it might be a valid aproach to make all the mutices just sane types (like uint32) - as the shift/mask/cas-32-bit-value aproach Solaris takes for atomic_swap_uchar() is pretty dangerous if used w/o proper care (not implying that InnoDB falls into this trap - I don't know).

Hello,

from os0sync.h:

--- cut ---
#elif defined(HAVE_IB_SOLARIS_ATOMICS)

#define HAVE_ATOMIC_BUILTINS

...

# define os_atomic_test_and_set_byte(ptr, new_val) \
        atomic_swap_uchar(ptr, new_val)
--- cut ---

The test that defines HAVE_IB_SOLARIS_ATOMICS is in storage/innodb_plugin/plug.in:

--- cut ---
  AC_CHECK_FUNCS(atomic_add_long \
                 atomic_cas_32 \
                 atomic_cas_64 \
                 atomic_cas_ulong,

                 AC_DEFINE([HAVE_IB_SOLARIS_ATOMICS], [1],
                           [Define to 1 if Solaris libc atomic functions \
                            are available])
  )
--- cut ---

It does not check for all the functions it is going to use. It assumes that if a bunch of functions are present then all of them are.

The simple fix is to check for all the atomc_* functions that are going to be used.

(this mostly repeats what Martin already said).

Patch that fixes this bug by checking for all necessary atomic_* functions

Attachment: bug53916-1.diff (application/octet-stream, text), 745 bytes.

Hi Martin,

Can you test whether this patch fixes the problem:
http://bugs.mysql.com/file.php?id=15862

Note that you will have to recreate the ./configure script (e.g. by running ./BUILD/autorun.sh).

Thanks!

Martin, I assume you used GCC, but for some reason 
HAVE_IB_GCC_ATOMIC_BUILTINS was not defined for you. Can you look inside config.log for "whether GCC atomic builtins are available", there should be some undefined references to GCC functions like __sync_lock_test_and_set_4().

I am just curious, this is not relevant to the bug.

From the GCC documentation:

 The definition given in the Intel documentation allows only for the
use of the types `int', `long', `long long' as well as their unsigned
counterparts.  GCC will allow any integral scalar or pointer type that
is 1, 2, 4 or 8 bytes in length.

 Not all operations are supported by all target processors.  If a
particular operation cannot be implemented on the target processor, a
warning will be generated and a call an external function will be
generated.  The external function will carry the same name as the
builtin, with an additional suffix `_N' where N is the size of the data
type.

Thanks!

Reducing the effort estimation in triage since a fix already exists.

Side remark re: why does it not use gcc atomic ops: I noticed that it should detect those, but it fails to link the test programs missing some internal symbols - I blamed this on a NetBSD bug when building libgcc and will investigate it.

Hi,

try to compile and link this simple prog:

--- cut ---
int
main(int argc, char **argv)
{
        char    a;

        a = 10;

        __sync_lock_test_and_set(&a, 1);

        return(0);
}
--- cut ---

If it fails to link with undefined reference to __sync_lock_test_and_set_1() then:

"Not all operations are supported by all target processors.  If a
particular operation cannot be implemented on the target processor, a
warning will be generated and a call an external function will be
generated.  The external function will carry the same name as the
builtin, with an additional suffix `_N' where N is the size of the data
type."

yes, exactly, it fails with __sync_lock_test_and_set_1() unresolved

Hi Martin,

So does this patch fix the problem?
http://bugs.mysql.com/file.php?id=15862

Note that you will have to recreate the ./configure script (e.g. by running
./BUILD/autorun.sh).

Thanks!

Sorry, can't realy tell yet - the quick test did not work, as my usual build environment (NetBSD/pkgsrc) interfered with the rebuilding of the autoconfig scripts in some way not obvious to me.

Jonathan pointed me at build instructions to test a "native" build, but I have been too busy - will test and report back within a few days.

Hello?

Sorry for the delay - with the patch it works fine:

-rw-r--r--   1 martin  users  169744686 Oct 19 10:56 mysql-5.5.6-rc-netbsd-sparc64.tar.gz

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/121252

3190 Vasil Dimov	2010-10-19
      Fix Bug#53916 storage/innodb_plugin does not compile on NetBSD/sparc64
      
      Just check for all the functions that we are going to use, not a subset
      of them.
      
      Reviewed by:	Davi (via IRC)

Pushed into mysql-trunk 5.6.99-m5 (revid:alexander.nozdrin@oracle.com-20101113155825-czmva9kg4n31anmu) (version source revid:alexander.nozdrin@oracle.com-20101113152450-2zzcm50e7i4j35v7) (merge vers: 5.6.1-m4) (pib:21)

Pushed into mysql-next-mr (revid:alexander.nozdrin@oracle.com-20101113160336-atmtmfb3mzm4pz4i) (version source revid:vasil.dimov@oracle.com-20100629074804-359l9m9gniauxr94) (pib:21)

Pushed into mysql-5.1 5.1.54 (revid:build@mysql.com-20101118153531-693taxtxyxpt037i) (version source revid:build@mysql.com-20101118153531-693taxtxyxpt037i) (merge vers: 5.1.54) (pib:21)

Added to change log:

A compilation problem affected the InnoDB source code on
NetBSD/sparc64.

Pushed into mysql-5.5 5.5.9 (revid:jonathan.perkin@oracle.com-20101216101358-fyzr1epq95a3yett) (version source revid:jonathan.perkin@oracle.com-20101216101358-fyzr1epq95a3yett) (merge vers: 5.5.9) (pib:24)

Removed the 5.5.x changelog entry. The push to 5.5.x was presumably a null merge because 5.5 is CMake-configured, not autoconf-configured.