mongodb / mongo-php-driver

The Official MongoDB PHP driver
https://pecl.php.net/package/mongodb
Apache License 2.0
888 stars 204 forks source link

libbson Bug Report : variable ret value type error #1445

Open jingjingxyk opened 1 year ago

jingjingxyk commented 1 year ago

Bug Report

https://github.com/mongodb/mongo-c-driver/blob/6b7caf9da30eeae09c8eb0c539ebacbb31b9e520/src/libbson/src/bson/bson-error.c#L113


/src/libmongoc/src/libbson/src/bson/bson-error.c -o ext/mongodb/src/libmongoc/src/libbson/src/bson/bson-error.lo  -MMD -MF ext/mongodb/src/libmongoc/src/libbson/src/bson/bson-error.dep -MT ext/mongodb/src/libmongoc/src/libbson/src/bson/bson-error.lo
/tmp/t/php-src/ext/mongodb/src/libmongoc/src/libbson/src/bson/bson-error.c:113:8: error: incompatible integer to pointer conversion assigning to 'char *' from 'int' [-Wint-conversion]
   ret = strerror_r (err_code, buf, buflen);
       ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [Makefile:806: ext/mongodb/src/libmongoc/src/libbson/src/bson/bson-error.lo] Error 1
make: *** Waiting for unfinished jobs....
4 warnings generated.

image

test php static link on alpine 3.17


set -uex
mkdir -p /tmp/t
cd /tmp/t

test -f php-8.2.7.tar.gz || wget  -O php-8.2.7.tar.gz  https://github.com/php/php-src/archive/refs/tags/php-8.2.7.tar.gz
test -d php-src && rm -rf php-src
mkdir -p  php-src
tar --strip-components=1 -C php-src -xf  php-8.2.7.tar.gz

test -f mongodb-1.16.1.tgz || wget -O mongodb-1.16.1.tgz   https://github.com/mongodb/mongo-php-driver/releases/download/1.16.1/mongodb-1.16.1.tgz
mkdir -p mongodb
tar --strip-components=1 -C mongodb  -xf  mongodb-1.16.1.tgz

test -d php-src/ext/mongodb && rm -rf php-src/ext/mongodb
mv mongodb php-src/ext/

export CC=clang
export CXX=clang++
export LD=ld.lld

cd php-src

./buildconf --force

./configure \
--disable-all \
--disable-cgi  \
--enable-shared=no \
--enable-static=yes \
--enable-cli  \
--enable-mongodb \
--with-mongodb-system-libs=no \
--with-mongodb-ssl=no  \
--with-mongodb-sasl=no  \
--with-mongodb-icu=no  \
--with-mongodb-client-side-encryption=no

make -j $(nproc)
jmikola commented 1 year ago

@jingjingxyk: Can you confirm that the error also appears when compiling the extension on its own (i.e. not statically linked to PHP)? I just want to rule that out.

Looking at the compiler flags in the screenshot, I see the following:

-Wstrict-prototypes
-Wall
-Wextra
-Wno-strict-aliasing
-Wno-unused-parameter
-Wno-sign-compare

According to the Clang docs, -Wint-conversion is "an error by default", so I don't think this issue is related to the compiler flags above. libmongoc and libbson are tested with clang in CI, although they are not tested with Alpine.

Looking at the bson_strerror_r() definition, the following stands out:

#elif defined(__GNUC__) && defined(_GNU_SOURCE)
   ret = strerror_r (err_code, buf, buflen);
#else /* XSI strerror_r */
   if (strerror_r (err_code, buf, buflen) == 0) {
      ret = buf;
   }
#endif

And then consider the following from strerror_r(3):

int strerror_r(int errnum, char *buf, size_t buflen);
            /* XSI-compliant */

char *strerror_r(int errnum, char *buf, size_t buflen);
            /* GNU-specific */

Feature Test Macro Requirements for glibc (see feature_test_macros(7)): The XSI-compliant version of strerror_r() is provided if: (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE Otherwise, the GNU-specific version is provided.

My guess is that Alpine is also defining _POSIX_C_SOURCE and libbson fails to detect that the XSI-compliant version is used due to its #elif condition. I've reported this in CDRIVER-4679.

Note: I do not suggest using -Wno-int-conversion as a workaround because the XSI-compliant version of strerror_r() has a very different return value. Allowing its integer result, which is an error code, to be cast to a char* will likely lead to a segfault down the line.

A workaround may require either manually changing defined constants or patching the code for now.

jingjingxyk commented 1 year ago

@jmikola According to your suggestion, after verification, I found that this error only appears in alpine linux and clang compilers and PHP versions greater than 8.2.0. static links and non-static links also report errors .

verify result:

OS OS VERSION PHP VERSION C COMPILER static linked shared linked build result
alpine 3.17 8.2.1 clang yes no compiler error
alpine 3.17 8.2.1 clang no yes compiler error
alpine 3.17 8.2.1 gcc yes no compiler sucess
alpine 3.17 8.2.1 gcc no yes compiler sucess
alpine 3.17 8.1.21 clang yes no compiler sucess
alpine 3.17 8.1.21 clang no yes compiler sucess
alpine 3.17 8.1.21 gcc yes no compiler sucess
alpine 3.17 8.1.21 gcc no yes compiler sucess
debian 11 8.2.1 clang yes no compiler sucess
debian 11 8.2.1 clang no yes compiler sucess
debian 11 8.2.1 gcc yes no compiler sucess
debian 11 8.2.1 gcc no yes compiler sucess

PHP 8.2 New features: https://github.com/php/php-src/blob/72e2e25066e2f50ce1c1b1da4fc5721f3460510c/configure.ac#L273C37-L273C37

jingjingxyk commented 1 year ago

verify command

build script

vi test.sh


set -uex

mkdir -p /tmp/t cd /tmp/t

PHP_VERSION=8.1.21 PHP_VERSION=8.2.1

test -f php-${PHP_VERSION}.tar.gz || wget -O php-${PHP_VERSION}.tar.gz https://github.com/php/php-src/archive/refs/tags/php-${PHP_VERSION}.tar.gz test -d php-src && rm -rf php-src mkdir -p php-src tar --strip-components=1 -C php-src -xf php-${PHP_VERSION}.tar.gz

test -f mongodb-1.16.1.tgz || wget -O mongodb-1.16.1.tgz https://github.com/mongodb/mongo-php-driver/releases/download/1.16.1/mongodb-1.16.1.tgz mkdir -p mongodb tar --strip-components=1 -C mongodb -xf mongodb-1.16.1.tgz

test -d php-src/ext/mongodb && rm -rf php-src/ext/mongodb mv mongodb php-src/ext/

export CC=gcc export CXX=g++ export LD=ld

export CC=clang export CXX=clang++ export LD=ld.lld

cd php-src

./buildconf --force

./configure \ --disable-all \ --disable-cgi \ --enable-shared=no \ --enable-static=yes \ --enable-cli \ --disable-phpdbg \ --without-valgrind \ --enable-mongodb \ --with-mongodb-system-libs=no \ --with-mongodb-ssl=no \ --with-mongodb-sasl=no \ --with-mongodb-icu=no \ --with-mongodb-client-side-encryption=no

make -j $(nproc)

file sapi/cli/php readelf -h sapi/cli/php


## debian  environment
```shell
docker run --rm   -ti --init -v .:/work -w /work debian:11

apt update -y

apt install -y git curl wget ca-certificates \
xz-utils autoconf automake clang-tools clang lld libtool cmake bison re2c gettext coreutils lzip zip unzip \
pkg-config bzip2 flex p7zip \
gcc g++ vim 

alpine environment


docker run --rm   -ti --init -v .:/work -w /work alpine:3.17

apk update

apk add vim alpine-sdk xz autoconf automake linux-headers clang-dev clang lld libtool cmake  \
bison re2c gettext coreutils gcc g++ \
bash p7zip zip unzip flex pkgconf ca-certificates \
wget git curl \
libc++-static libltdl-static
jingjingxyk commented 1 year ago

not statically linked PHP. on aline 3.17 clang image

jingjingxyk commented 1 year ago

I found that there is a difference between php8.1.21 and php8.2.0 compilation parameters .

# php 8.1.21
:<<'EOF'
 -fno-common
 -Wstrict-prototypes
 -Wall
 -Wextra
 -Wno-strict-aliasing
 -Wno-unused-parameter
 -Wno-sign-compare -g -O2
 -fvisibility=hidden
 -DZEND_SIGNALS
EOF

# php 8.2.1
:<<'EOF'
-D_GNU_SOURCE
-fno-common
-Wstrict-prototypes
-Wall
-Wextra
-Wno-strict-aliasing
-Wno-unused-parameter
-Wno-sign-compare -g -O2
-fvisibility=hidden
-DZEND_SIGNALS
-DBSON_COMPILATION
-DMONGOC_COMPILATION
-D_DEFAULT_SOURCE
-DKMS_MESSAGE_LITTLE_ENDIAN=1
-DMONGOCRYPT_LITTLE_ENDIAN=1

EOF
jmikola commented 1 year ago

@jingjingxyk: Thanks for the extensive testing. The addition of -D_GNU_SOURCE in https://github.com/php/php-src/commit/067df263448ee26013cddee1065bc9c1f028bd23 (for PHP 8.2+) definitely looks related. I've left a note about this in CDRIVER-4679.

Dev-WBAP commented 6 months ago

/bin/sh /private/tmp/pear/temp/pear-build-rootP1ik7m/mongodb-1.15.1/libtool --mode=compile cc -Isrc/libmongoc/src/libbson/src/bson/ -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/libbson/src/bson/ -I/private/tmp/pear/temp/pear-build-rootP1ik7m/mongodb-1.15.1/include -I/private/tmp/pear/temp/pear-build-rootP1ik7m/mongodb-1.15.1/main -I/private/tmp/pear/temp/mongodb -I/Applications/MAMP/bin/php/php8.2.0/include/php -I/Applications/MAMP/bin/php/php8.2.0/include/php/main -I/Applications/MAMP/bin/php/php8.2.0/include/php/TSRM -I/Applications/MAMP/bin/php/php8.2.0/include/php/Zend -I/Applications/MAMP/bin/php/php8.2.0/include/php/ext -I/Applications/MAMP/bin/php/php8.2.0/include/php/ext/date/lib -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/common/ -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/libbson/src/ -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/libbson/src/jsonsl/ -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/libmongoc/src/ -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/zlib-1.2. 12/ -I/private/tmp/pear/temp/mongodb/src/libmongocrypt/src/ -I/private/tmp/pear/temp/mongodb/src/libmongocrypt/kms-message/src/ -I/private/tmp/pear/temp/mongodb/src/libmongocrypt-compat/ -I/private/tmp/pear/temp/mongodb/src/ -I/private/tmp/pear/temp/mongodb/src/BSON/ -I/private/tmp/pear/temp/mongodb/src/MongoDB/ -I/private/tmp/pear/temp/mongodb/src/MongoDB/Exception/ -I/private/tmp/pear/temp/mongodb/src/MongoDB/Monitoring/ -I/private/tmp/pear/temp/mongodb/src/contrib/ -DHAVE_CONFIG_H -g -O2 -D_GNU_SOURCE -DBSON_COMPILATION -DMONGOC_COMPILATION -D_DEFAULT_SOURCE -D_THREAD_SAFE -DKMS_MESSAGE_ENABLE_CRYPTO=1 -DKMS_MESSAGE_ENABLE_CRYPTO_COMMON_CRYPTO=1 -D_THREAD_SAFE -D_THREAD_SAFE -DKMS_MESSAGE_LITTLE_ENDIAN=1 -DMONGOCRYPT_LITTLE_ENDIAN=1 -c /private/tmp/pear/temp/mongodb/src/libmongoc/src/libbson/src/bson/bson-error.c -o src/libmongoc/src/libbson/src/bson/bson-error.lo -MMD -MF src/libmongoc/src/libbson/src/bson/bson-error.dep -MT src/libmongoc/src/libbson/src/bson/bson-error.lo cc -Isrc/libmongoc/src/libbson/src/bson/ -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/libbson/src/bson/ -I/private/tmp/pear/temp/pear-build-rootP1ik7m/mongodb-1.15.1/include -I/private/tmp/pear/temp/pear-build-rootP1ik7m/mongodb-1.15.1/main -I/private/tmp/pear/temp/mongodb -I/Applications/MAMP/bin/php/php8.2.0/include/php -I/Applications/MAMP/bin/php/php8.2.0/include/php/main -I/Applications/MAMP/bin/php/php8.2.0/include/php/TSRM -I/Applications/MAMP/bin/php/php8.2.0/include/php/Zend -I/Applications/MAMP/bin/php/php8.2.0/include/php/ext -I/Applications/MAMP/bin/php/php8.2.0/include/php/ext/date/lib -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/common/ -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/libbson/src/ -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/libbson/src/jsonsl/ -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/libmongoc/src/ -I/private/tmp/pear/temp/mongodb/src/libmongoc/src/zlib-1.2.12/ -I/private/tmp/pear/temp/mongodb/src/libmongocrypt/src/ -I/private/tmp/pear/temp/mongo db/src/libmongocrypt/kms-message/src/ -I/private/tmp/pear/temp/mongodb/src/libmongocrypt-compat/ -I/private/tmp/pear/temp/mongodb/src/ -I/private/tmp/pear/temp/mongodb/src/BSON/ -I/private/tmp/pear/temp/mongodb/src/MongoDB/ -I/private/tmp/pear/temp/mongodb/src/MongoDB/Exception/ -I/private/tmp/pear/temp/mongodb/src/MongoDB/Monitoring/ -I/private/tmp/pear/temp/mongodb/src/contrib/ -DHAVE_CONFIG_H -g -O2 -D_GNU_SOURCE -DBSON_COMPILATION -DMONGOC_COMPILATION -D_DEFAULT_SOURCE -D_THREAD_SAFE -DKMS_MESSAGE_ENABLE_CRYPTO=1 -DKMS_MESSAGE_ENABLE_CRYPTO_COMMON_CRYPTO=1 -D_THREAD_SAFE -D_THREAD_SAFE -DKMS_MESSAGE_LITTLE_ENDIAN=1 -DMONGOCRYPT_LITTLE_ENDIAN=1 -c /private/tmp/pear/temp/mongodb/src/libmongoc/src/libbson/src/bson/bson-error.c -MMD -MF src/libmongoc/src/libbson/src/bson/bson-error.dep -MT src/libmongoc/src/libbson/src/bson/bson-error.lo -fno-common -DPIC -o src/libmongoc/src/libbson/src/bson/.libs/bson-error.o /private/tmp/pear/temp/mongodb/src/libmongoc/src/libbson/src/bson/bson-error.c:113:8: error: incompatible integer to pointer conversion assigning to 'char *' from 'int' [-Wint-conversion] ret = strerror_r (err_code, buf, buflen); ^ ~~~~~~~~~~ 1 error generated.

jmikola commented 6 months ago

Note: above comment is a duplicate of https://github.com/mongodb/mongo-php-driver/issues/1541.