libressl / portable

LibreSSL Portable itself. This includes the build scaffold and compatibility layer that builds portable LibreSSL from the OpenBSD source code. Pull requests or patches sent to tech@openbsd.org are welcome.
https://www.libressl.org
1.37k stars 266 forks source link

ssl_tlsext.c:1608:30: warning: 'client_preferred_group' may be used uninitialized in this function #1066

Closed chipitsine closed 4 months ago

chipitsine commented 4 months ago
  CC       libssl_la-ssl_tlsext.lo
  CC       libssl_la-ssl_transcript.lo
ssl_tlsext.c: In function 'tlsext_keyshare_server_process':
ssl_tlsext.c:1608:30: warning: 'client_preferred_group' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1608 |   if (!preferred_group_found || group != client_preferred_group)
      |       ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  CC       libssl_la-ssl_txt.lo
  CC       libssl_la-ssl_versions.lo
botovq commented 4 months ago

It's gcc being stupid about conditionally initialized variables again. I really wish they would finally fix their warning story. It's just so terribly bad and initializing variables just to shut the compiler up really doesn't help improving the code at all.

I knew someone would eventually paste this, so I worked around it a while back in OpenBSD: it hasn't made it to libressl/openbsd yet. I will sync it once I'm back to having a decent connection.

https://github.com/openbsd/src/commits/master/876d5c2559afc31e2324b480e373ab12ef44f234

botovq commented 4 months ago

Here's a working link:

https://github.com/openbsd/src/commit/876d5c2559afc31e2324b480e373ab12ef44f234

chipitsine commented 4 months ago

thanks!

job commented 4 months ago

@botovq do you know what aspect exactly in gcc arrives at these false positives? how does the detection work and why does it mess up in situations like these?

botovq commented 4 months ago

I have no idea about the inner workings of gcc. Reading gcc code is really not something I like doing...

I think what spawns the warning is just too simple minded. It sees that client_preferred_group is only initialized in a certain branch, hence it could potentially be used uninitialized. Some simple logic like that flags the variable and triggers the warning.

It fails to understand two points: first, preferred_group_found is initialized to 0 and second it's the same (and only) branch that sets preferred_group_found = 1 that alos initializes client_preferred_group. So what it should figure out (and the compiler will have to understand that on some level) is that

if (!preferred_group_found || group != client_preferred_group)

will always short circuit unless preferred_group_found = 1 and client_preferred_group is initialized in that case.

llvm understand this and keeps quiet.