Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libbson Bug Report : variable ret value type error #1445

Open
jingjingxyk opened this issue Jul 12, 2023 · 8 comments
Open

libbson Bug Report : variable ret value type error #1445

jingjingxyk opened this issue Jul 12, 2023 · 8 comments
Labels
tracked-in-jira Ticket filed in Mongo's Jira system waiting-on-jira

Comments

@jingjingxyk
Copy link

jingjingxyk commented Jul 12, 2023

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)
@jingjingxyk jingjingxyk changed the title Bug Report : variable ret value type error libbson Bug Report : variable ret value type error Jul 12, 2023
@jmikola
Copy link
Member

jmikola commented Jul 13, 2023

@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.

@jmikola jmikola added waiting-on-jira tracked-in-jira Ticket filed in Mongo's Jira system labels Jul 13, 2023
@jingjingxyk
Copy link
Author

jingjingxyk commented Jul 14, 2023

@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
Copy link
Author

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

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
Copy link
Author

not statically linked PHP. on aline 3.17 clang
image

@jingjingxyk
Copy link
Author

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 referenced this issue in php/php-src Jul 14, 2023
On x86_64 glibc memrchr() uses SSE/AVX CPU extensions and works much
faster then naive loop. On x86 32-bit we still use inlined version.

memrchr() is a GNU extension. Its prototype  becomes available when
<string.h> is included with defined _GNU_SOURCE macro. Previously, we
defined it in "php_config.h", but some sources may include <string.h>
befire it. To avod mess we also pass -D_GNU_SOURCE to C compiler.
@jmikola
Copy link
Member

jmikola commented Jul 14, 2023

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

@Dev-WBAP
Copy link

Dev-WBAP commented May 2, 2024

/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
Copy link
Member

jmikola commented May 2, 2024

Note: above comment is a duplicate of #1541.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracked-in-jira Ticket filed in Mongo's Jira system waiting-on-jira
Projects
None yet
Development

No branches or pull requests

3 participants