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

http: gcc 14 compiler warning on mac arm64 (cmake, unity) #14168

Closed
vszakats opened this issue Jul 12, 2024 · 8 comments
Closed

http: gcc 14 compiler warning on mac arm64 (cmake, unity) #14168

vszakats opened this issue Jul 12, 2024 · 8 comments
Labels

Comments

@vszakats
Copy link
Member

vszakats commented Jul 12, 2024

I did this

Built curl-for-win with gcc for mac (arm64), and this compiler warning happened:

curl/lib/http.c: In function 'Curl_add_custom_headers':
curl/lib/http.c:1528:42: warning: '%s' directive argument is null [-Wformat-overflow=]
 1528 |             result = Curl_dyn_addf(req, "%s\r\n", compare);
      |                                          ^~

https://github.com/curl/curl-for-win/actions/runs/9906570816/job/27368394282#step:3:4688

I expected the following

No warnings.

curl/libcurl version

master f94aa3d

operating system

macOS 14 arm64 GHA runner

@vszakats vszakats added the build label Jul 12, 2024
@bagder
Copy link
Member

bagder commented Jul 19, 2024

That's a silly warning for us since a NULL there is not a problem.

But also: how is it NULL there?

@bagder
Copy link
Member

bagder commented Aug 2, 2024

I build with gcc 14 locally on my x86_64 debian and this is not a warning I see. Does it only happen on arm64? That seems weird.

How about just silence the (false positive) warning for that compiler + platform combo?

@vszakats
Copy link
Member Author

vszakats commented Aug 2, 2024

Are you building with cmake/unity=on?

@vszakats
Copy link
Member Author

vszakats commented Aug 2, 2024

Good question regarding the CPU. There is no Intel gcc test in CI, but could replicate it locally:

curl/lib/http.c: In function 'Curl_add_custom_headers':
curl/lib/http.c:1528:42: warning: '%s' directive argument is null [-Wformat-overflow=]
 1528 |             result = Curl_dyn_addf(req, "%s\r\n", compare);
      |                                          ^~

cmake:

cmake -B _x64-mac-sys-bld -Wno-dev -DCMAKE_BUILD_TYPE=Release 
-DCMAKE_OSX_DEPLOYMENT_TARGET=10.9 -DCMAKE_INSTALL_MESSAGE=NEVER 
-DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_C_COMPILER=gcc-14 
-DCMAKE_IGNORE_PREFIX_PATH=/usr/local 
-DCMAKE_OSX_SYSROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk 
-DCMAKE_C_COMPILER_TARGET=x86_64-apple-darwin -DBUILD_TESTING=OFF 
-DENABLE_WEBSOCKETS=ON -DENABLE_THREADED_RESOLVER=ON 
-DZLIB_INCLUDE_DIR= -DCURL_BROTLI=OFF -DCURL_ZSTD=OFF -DCURL_USE_OPENSSL=ON 
-DOPENSSL_ROOT_DIR=.../libressl/_x64-mac-sys/usr -DCURL_DISABLE_OPENSSL_AUTO_LOAD_CONFIG=ON 
-DHAVE_BORINGSSL=0 -DHAVE_AWSLC=0 -DHAVE_SSL_CTX_SET_QUIC_METHOD=1 -DCURL_USE_SECTRANSP=OFF 
-DCURL_DISABLE_SRP=ON -DCURL_USE_LIBSSH=OFF -DCURL_USE_LIBSSH2=OFF -DUSE_NGHTTP2=OFF 
-DUSE_NGHTTP3=OFF -DUSE_NGTCP2=OFF -DUSE_LIBIDN2=OFF -DUSE_APPLE_IDN=ON 
-DCURL_USE_LIBPSL=OFF -DENABLE_CURL_MANUAL=ON -DBUILD_LIBCURL_DOCS=OFF 
-DSHARE_LIB_OBJECT=ON --no-warn-unused-cli -DCMAKE_UNITY_BUILD=ON -DBUILD_CURL_EXE=ON 
-DBUILD_STATIC_CURL=ON -DCURL_CA_PATH=none -DCURL_CA_BUNDLE=none -DBUILD_SHARED_LIBS=ON 
-DBUILD_STATIC_LIBS=ON -DCURL_HIDDEN_SYMBOLS=ON -DCMAKE_RC_FLAGS= '-DCMAKE_C_FLAGS=  
-mmacosx-version-min=10.9 -fno-omit-frame-pointer -fcf-protection=full -fvisibility=hidden 
-fno-unwind-tables -fno-asynchronous-unwind-tables -fno-ident   -DHAS_ALPN ' 
'-DCMAKE_EXE_LINKER_FLAGS= -v  -Wl,-map,curl.map ' 
'-DCMAKE_SHARED_LINKER_FLAGS= -v  -Wl,-map,libcurl.map '

not minimal, but straight of a curl-for-win with curl + libressl only:

export CW_CONFIG=test-x64-gcc-mac; export CW_CCSUFFIX='-14'

With riscv64 Linux and gcc 14 this warning is not happening.

Instead there is this one, where the code look correct but non-trivial for a compiler to see:

/home/runner/work/curl-for-win/curl-for-win/curl/lib/smb.c: In function 'smb_connection_state':
/home/runner/work/curl-for-win/curl-for-win/curl/lib/smb.c:895:5: warning: 'memcpy' offset [74, 80] from the object at 'buf' is out of the bounds of referenced subobject 'bytes' with type 'char[1]' at offset 73 [-Warray-bounds=]
  895 |     memcpy(smbc->challenge, nrsp->bytes, sizeof(smbc->challenge));
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/curl-for-win/curl-for-win/curl/lib/smb.c:130:8: note: subobject 'bytes' declared here
  130 |   char bytes[1];
      |        ^~~~~

@vszakats
Copy link
Member Author

vszakats commented Aug 2, 2024

Confirmed this is unity-specific, and thus needs cmake.

@bagder
Copy link
Member

bagder commented Aug 17, 2024

But we don't see these in any CI do we?

@vszakats
Copy link
Member Author

vszakats commented Aug 17, 2024

It's on the curl-for-win daily CI run.

Edit: no sorry, not the daily but the other, extended workflow.

@bagder
Copy link
Member

bagder commented Aug 19, 2024

-Wformat-overflow is not a warning that we want enabled as it does not help us. It can only bring us false positives:

Warn about calls to formatted input/output functions such as sprintf and vsprintf that might overflow the destination buffer

We explicitly never use sprintf and vsprintf in code (they are banned by checksrc).

I will file a PR to remove it.

bagder added a commit that referenced this issue Aug 19, 2024
-Wformat-overflow is not a warning that we want enabled as it does not
help us. It can only bring us false positives since it warng on bad uses
of sprintf and vsprintf ("that might overflow the destination buffer").
Two functions we explictly ban in curlcode.

The only way this flag triggers warnings in curl code is false positives
for functions we have marked with the CURL_PRINTF() macro.

Further: it seems -Wformat-trunaction option might in turn also enable
-Wformat-overflow, so if this second option is used, we need to
explcitly set -Wno-format-overflow - not just skip setting
-Wformat-overflow.

Reported-by: Viktor Szakats
Fixes #14168
@bagder bagder closed this as completed in 145f87b Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants