Strict C90 CFLAGS results in sha.h:91 ISO C90 does not support long long #10547

blastwave · 2019-12-01T04:35:23Z

Somewhat loosely related to #8048 wherein we all agree that the codebase is compliant with C90. However while trying a build with strict C90 flags I see :

titan# make 
/usr/bin/perl "-I." -Mconfigdata "util/dofile.pl" \
    "-oMakefile" crypto/include/internal/bn_conf.h.in > crypto/include/internal/bn_conf.h
/usr/bin/perl "-I." -Mconfigdata "util/dofile.pl" \
    "-oMakefile" crypto/include/internal/dso_conf.h.in > crypto/include/internal/dso_conf.h
/usr/bin/perl "-I." -Mconfigdata "util/dofile.pl" \
    "-oMakefile" include/openssl/opensslconf.h.in > include/openssl/opensslconf.h
/usr/bin/make depend && /usr/bin/make _all
make[1]: Entering directory '/opt/bw/build/openssl-1.1.1d_debian_sid_i686_5.3.7-genunix.001'
make[1]: Leaving directory '/opt/bw/build/openssl-1.1.1d_debian_sid_i686_5.3.7-genunix.001'
make[1]: Entering directory '/opt/bw/build/openssl-1.1.1d_debian_sid_i686_5.3.7-genunix.001'
/usr/bin/gcc-9  -I. -Iinclude -fPIC -std=c90 -pedantic -pedantic-errors -Wpedantic -fno-builtin -march=i686 -mtune=i686 -Wl,-rpath=/opt/bw/lib,--enable-new-dtags -mpc80 -pthread -Wa,--noexecstack -std=c90 -pedantic -pedantic-errors -Wpedantic -fno-builtin -march=i686 -mtune=i686 -Wl,-rpath=/opt/bw/lib,--enable-new-dtags -mpc80 -DOPENSSL_USE_NODELETE -DOPENSSL_PIC -DOPENSSLDIR="\"/opt/bw/ssl\"" -DENGINESDIR="\"/opt/bw/lib/engines-1.1\"" -DNDEBUG -I/opt/bw/include -D_POSIX_PTHREAD_SEMANTICS -D_TS_ERRNO -D_LARGEFILE64_SOURCE -MMD -MF apps/app_rand.d.tmp -MT apps/app_rand.o -c -o apps/app_rand.o apps/app_rand.c
In file included from include/openssl/x509.h:30,
                 from apps/apps.h:26,
                 from apps/app_rand.c:10:
include/openssl/sha.h:91:36: error: ISO C90 does not support 'long long' [-Wlong-long]
   91 | #  define SHA_LONG64 unsigned long long
      |                                    ^~~~
include/openssl/sha.h:96:5: note: in expansion of macro 'SHA_LONG64'
   96 |     SHA_LONG64 h[8];
      |     ^~~~~~~~~~
include/openssl/sha.h:91:36: error: ISO C90 does not support 'long long' [-Wlong-long]
   91 | #  define SHA_LONG64 unsigned long long
      |                                    ^~~~
include/openssl/sha.h:97:5: note: in expansion of macro 'SHA_LONG64'
   97 |     SHA_LONG64 Nl, Nh;
      |     ^~~~~~~~~~
include/openssl/sha.h:91:36: error: ISO C90 does not support 'long long' [-Wlong-long]
   91 | #  define SHA_LONG64 unsigned long long
      |                                    ^~~~
include/openssl/sha.h:99:9: note: in expansion of macro 'SHA_LONG64'
   99 |         SHA_LONG64 d[SHA_LBLOCK];
      |         ^~~~~~~~~~
make[1]: *** [Makefile:708: apps/app_rand.o] Error 1
make[1]: Leaving directory '/opt/bw/build/openssl-1.1.1d_debian_sid_i686_5.3.7-genunix.001'
make: *** [Makefile:174: all] Error 2
Command exited with non-zero status 2

titan#

Seems reasonable to re-try that build with no CFLAGS of any particular compliance flavour however I am curious why C90 clean code would have any issue whatsoever?

Dennis

The text was updated successfully, but these errors were encountered:

richsalz · 2019-12-01T04:43:33Z

What platform and config args did you use? The 64bit datatype might be something other than "long long"

blastwave · 2019-12-01T04:44:23Z

Merely a follow up but if one relaxes these options a tad and stays with C90 then we hit an "asm" wall :

/usr/bin/gcc-9  -I. -Icrypto/include -Iinclude -fPIC -std=c90 -fno-builtin -march=i686 -mtune=i686 -Wl,-rpath=/opt/bw/lib,--enable-new-dtags -mpc80 -pthread -Wa,--noexecstack -std=c90 -fno-builtin -march=i686 -mtune=i686 -Wl,-rpath=/opt/bw/lib,--enable-new-dtags -mpc80 -DOPENSSL_USE_NODELETE -DOPENSSL_PIC -DOPENSSLDIR="\"/opt/bw/ssl\"" -DENGINESDIR="\"/opt/bw/lib/engines-1.1\"" -DNDEBUG -I/opt/bw/include -D_POSIX_PTHREAD_SEMANTICS -D_TS_ERRNO -D_LARGEFILE64_SOURCE -MMD -MF crypto/bn/bn_div.d.tmp -MT crypto/bn/bn_div.o -c -o crypto/bn/bn_div.o crypto/bn/bn_div.c
crypto/bn/bn_div.c: In function 'bn_div_fixed_top':
crypto/bn/bn_div.c:175:13: error: 'asm' undeclared (first use in this function)
  175 |         ({  asm volatile (                      \
      |             ^~~
crypto/bn/bn_div.c:361:17: note: in expansion of macro 'bn_div_words'
  361 |             q = bn_div_words(n0, n1, d0);
      |                 ^~~~~~~~~~~~
crypto/bn/bn_div.c:175:13: note: each undeclared identifier is reported only once for each function it appears in
  175 |         ({  asm volatile (                      \
      |             ^~~
crypto/bn/bn_div.c:361:17: note: in expansion of macro 'bn_div_words'
  361 |             q = bn_div_words(n0, n1, d0);
      |                 ^~~~~~~~~~~~
crypto/bn/bn_div.c:175:17: error: expected ';' before 'volatile'
  175 |         ({  asm volatile (                      \
      |                 ^~~~~~~~
crypto/bn/bn_div.c:361:17: note: in expansion of macro 'bn_div_words'
  361 |             q = bn_div_words(n0, n1, d0);
      |                 ^~~~~~~~~~~~
make[1]: *** [Makefile:1710: crypto/bn/bn_div.o] Error 1
make[1]: Leaving directory '/opt/bw/build/openssl-1.1.1d_debian_sid_i686_5.3.7-genunix.001'
make: *** [Makefile:174: all] Error 2
Command exited with non-zero status 2

So perhaps other flags or config options are needed in order for C90 compliance?

Dennis

blastwave · 2019-12-01T04:52:42Z

What platform and config args did you use? The 64bit datatype might be something other than "long long"

Thanks for the snap fast reply!

This is Debian Linux "sid" on Intel 32-bit with nothing particularly special at all.

Let's take a look with and without the pedantic options :

titan# uname -a 
Linux titan 5.3.7-genunix #1 SMP Fri Oct 25 15:43:22 UTC 2019 i686 GNU/Linux

titan# cat /etc/debian_version 
bullseye/sid

titan# 

titan# echo $CC
/usr/bin/gcc-9
titan# 

titan# echo $CFLAGS 
-std=iso9899:1990 -pedantic -pedantic-errors -Wpedantic -fno-builtin -march=i686 -mtune=i686 -Wl,-rpath=/opt/bw/lib,--enable-new-dtags -mpc80
titan# 

titan# echo $CPPFLAGS 
-I/opt/bw/include -D_POSIX_PTHREAD_SEMANTICS -D_TS_ERRNO -D_LARGEFILE64_SOURCE
titan# 
titan# 
titan# cat /tmp/foo.c 

#include <stdio.h>
#include <stdlib.h>

int main(int argc, char **argv)
{
    long long foo;
    printf ( "What is the sizeof(foo) ?  sizeof() says %i\n",
               (size_t)sizeof(long long) );

    return ( EXIT_SUCCESS );

}

titan# 
titan# $CC $CFLAGS $CPPFLAGS -o /tmp/foo /tmp/foo.c 
/tmp/foo.c: In function 'main':
/tmp/foo.c:7:10: error: ISO C90 does not support 'long long' [-Wlong-long]
    7 |     long long foo;
      |          ^~~~
/tmp/foo.c:9:36: error: ISO C90 does not support 'long long' [-Wlong-long]
    9 |                (size_t)sizeof(long long) );
      |                                    ^~~~
titan#

Right, so here gcc has a fit due to "pedantic" options.

Take them away ?

titan# 
titan# $CC -std=iso9899:1990 -fno-builtin -march=i686 -mtune=i686 -g $CPPFLAGS -o /tmp/foo /tmp/foo.c 
titan# 
titan# /tmp/foo
What is the sizeof(foo) ?  sizeof() says 8
titan#

Well I think 64-bit is fine here.

Dennis

richsalz · 2019-12-01T04:54:37Z

Er, no. Let's take a look at your config command. Please. OR follow the instructions and post the output of the dump command.

levitte · 2019-12-01T04:55:36Z

We're frankly not 100% strict with asm, obviously. On the other hand, the asm stuff requires specific compilers, where we know it's supported. Therefore, trying à pure C90 build can only be assumed to pass with a no-asm configuration.

(we could spend time on moving this inline asm sections, but since they are relatively small, it doesn't seem worth the effort)

levitte · 2019-12-01T05:00:47Z

.travis.yml has one configuration where we do a check of a C89 build. Look for -ansi to find it, that shows the flags we're using, maybe that helps? Or maybe they need expanding?

blastwave · 2019-12-01T05:20:07Z

OKay ... I am perhaps being a bit pedantic here ( nothing new ) and I can remove the "pedantic" options and also configure with --debug and "no-asm". This is precisely what I am doing now.

All of this is merely so that I can single step into some other code that is linked with libcrypto.so and that explains how I fell into this.

The Configuration was done with 10-main.conf slightly modified merely to slip in these CFLAGS :

titan# diff -c Configurations/10-main.conf.orig Configurations/10-main.conf 
*** Configurations/10-main.conf.orig    Tue Sep 10 13:13:07 2019
--- Configurations/10-main.conf Sun Dec  1 05:04:50 2019
***************
*** 176,182 ****
          inherit_from     => [ "BASE_unix" ],
          CC               => "gcc",
          CFLAGS           => picker(debug   => "-O0 -g",
!                                    release => "-O3"),
          thread_scheme    => "(unknown)",
          bn_ops           => "BN_LLONG",
      },
--- 176,182 ----
          inherit_from     => [ "BASE_unix" ],
          CC               => "gcc",
          CFLAGS           => picker(debug   => "-O0 -g",
!                                    release => "-O0 -g"),
          thread_scheme    => "(unknown)",
          bn_ops           => "BN_LLONG",
      },
***************
*** 636,648 ****
          inherit_from     => [ "BASE_unix" ],
          CC               => "gcc",
          CXX              => "g++",
!         CFLAGS           => picker(default => "-Wall",
                                     debug   => "-O0 -g",
!                                    release => "-O3"),
!         CXXFLAGS         => picker(default => "-Wall",
                                     debug   => "-O0 -g",
!                                    release => "-O3"),
!         cflags           => threads("-pthread"),
          cxxflags         => combine("-std=c++11", threads("-pthread")),
          lib_cppflags     => "-DOPENSSL_USE_NODELETE",
          ex_libs          => add("-ldl", threads("-pthread")),
--- 636,648 ----
          inherit_from     => [ "BASE_unix" ],
          CC               => "gcc",
          CXX              => "g++",
!         CFLAGS           => picker(default => "-std=c90 -m32 -g -fno-builtin -march=i686 -mtune=i686 -Wl,-rpath=/opt/bw/lib,--enable-new-dtags -mpc80 -Wall",
                                     debug   => "-O0 -g",
!                                    release => "-O0 -g"),
!         CXXFLAGS         => picker(default => "-std=c90 -m32 -g -fno-builtin -march=i686 -mtune=i686 -Wl,-rpath=/opt/bw/lib,--enable-new-dtags -mpc80 -Wall",
                                     debug   => "-O0 -g",
!                                    release => "-O0 -g"),
!         cflags           => threads("-std=c90 -m32 -g -fno-builtin -march=i686 -mtune=i686 -Wl,-rpath=/opt/bw/lib,--enable-new-dtags -mpc80 -pthread"),
          cxxflags         => combine("-std=c++11", threads("-pthread")),
          lib_cppflags     => "-DOPENSSL_USE_NODELETE",
          ex_libs          => add("-ldl", threads("-pthread")),





titan# 
titan# perl ./Configure --prefix=/opt/bw --openssldir=/opt/bw/ssl --debug no-asm linux-generic32 2>&1 
Configuring OpenSSL version 1.1.1d (0x1010104fL) for linux-generic32
Using os-specific seed configuration
Creating configdata.pm
Creating Makefile

**********************************************************************
***                                                                ***
***   OpenSSL has been successfully configured                     ***
***                                                                ***
***   If you encounter a problem while building, please open an    ***
***   issue on GitHub <https://github.com/openssl/openssl/issues>  ***
***   and include the output from the following command:           ***
***                                                                ***
***       perl configdata.pm --dump                                ***
***                                                                ***
***   (If you are new to OpenSSL, you might want to consult the    ***
***   'Troubleshooting' section in the INSTALL file first)         ***
***                                                                ***
**********************************************************************
titan#

That seems to be compiling fine now.

Very odd that pedantic C90 compliance throws these errors however going with no-asm may solve this neatly. Regardless here is the "dump" :

openssl_c90ish.txt

Dennis

blastwave · 2019-12-01T05:39:19Z

As I slow through this I hit other problems :

.
.
.
/usr/bin/gcc-9  -I. -Icrypto/include -Iinclude -fPIC -std=c90 -m32 -g -fno-builtin -march=i686 -mtune=i686 -Wl,-rpath=/opt/bw/lib,--enable-new-dtags -mpc80 -pthread -std=c90 -fno-builtin -g -m32 -march=i686 -mtune=i686 -Wl,-rpath=/opt/bw/lib,--enable-new-dtags -mpc80 -DOPENSSL_USE_NODELETE -DOPENSSL_PIC -DOPENSSLDIR="\"/opt/bw/ssl\"" -DENGINESDIR="\"/opt/bw/lib/engines-1.1\""  -I/opt/bw/include -D_POSIX_PTHREAD_SEMANTICS -D_TS_ERRNO -D_LARGEFILE64_SOURCE -MMD -MF crypto/threads_pthread.d.tmp -MT crypto/threads_pthread.o -c -o crypto/threads_pthread.o crypto/threads_pthread.c
crypto/threads_pthread.c: In function 'CRYPTO_THREAD_lock_new':
crypto/threads_pthread.c:48:38: error: 'PTHREAD_MUTEX_RECURSIVE' undeclared (first use in this function); did you mean 'PTHREAD_MUTEX_RECURSIVE_NP'?
   48 |     pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
      |                                      ^~~~~~~~~~~~~~~~~~~~~~~
      |                                      PTHREAD_MUTEX_RECURSIVE_NP
crypto/threads_pthread.c:48:38: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [Makefile:5104: crypto/threads_pthread.o] Error 1
make[1]: Leaving directory '/opt/bw/build/openssl-1.1.1d_debian_sid_i686_5.3.7-genunix.002'
make: *** [Makefile:174: all] Error 2
Command exited with non-zero status 2

titan#

There must be other people that have tried this sort of build I hope?

blastwave · 2019-12-01T20:48:07Z

I needed a small patch inside a questionable bit of code which uses " if (1) " for some reason :

titan# 
titan# diff -u crypto/bio/b_addr.c.orig crypto/bio/b_addr.c     
--- crypto/bio/b_addr.c.orig    2019-09-10 13:13:07.000000000 +0000
+++ crypto/bio/b_addr.c 2019-12-01 07:18:13.859258517 +0000
@@ -195,6 +195,11 @@
     if (1) {
 #ifdef AI_PASSIVE
         int ret = 0;
+        /* given that we are inside an if(1) condition I feel
+         * less horrified at doing a define here. However 
+         * this is disturbed. */
+#define NI_MAXSERV      32
+#define NI_MAXHOST      1025
         char host[NI_MAXHOST] = "", serv[NI_MAXSERV] = "";
         int flags = 0;
 
titan#

For reasons unknown that data is not picked up from within /usr/include/netdb.h and perhaps that is due to using -D_XOPEN_SOURCE=600 regardless the end result passes ALL tests in the testsuite and that is the litmus paper test for me.

So I used these flags :

titan# 
titan# echo $CC
/usr/bin/gcc-9
titan# echo $CFLAGS 
-std=c90 -fno-builtin -g -m32 -march=i686 -mtune=i686 -Wl,-rpath=/opt/bw/lib,--enable-new-dtags -mpc80
titan# echo $CPPFLAGS 
-I/opt/bw/include -D_POSIX_PTHREAD_SEMANTICS -D_TS_ERRNO -D_LARGEFILE64_SOURCE -D_XOPEN_SOURCE=600
titan#

Also I must use no-asm as well as --debug but eventually I get a decent result.

I may patch and clean up that strange " if (1) " in crypto/bio/b_addr.c

--
Dennis Clarke
RISC-V/SPARC/PPC/ARM/CISC
UNIX and Linux spoken
GreyBeard and suspenders optional

blastwave · 2023-09-26T01:14:29Z

I realize that this is the same bug due to some code slippage and here we are
in 2023 with OpenSSL 3.1.3 : 

/usr/pkg/gcc10/bin/gcc  -I. -Iinclude -Iapps/include  -fPIC -pthread -std=iso9899:1990 -pedantic -pedantic-errors 
-m64 -g -O0 -fno-builtin -fno-fast-math -mhard-float -mno-app-regs -mflat -msoft-quad-float 
-mno-unaligned-doubles -mno-faster-structs -mcpu=ultrasparc -mtune=ultrasparc -mno-vis 
-mno-vis2 -mno-vis3 -mno-vis4 -mno-vis4b -mno-cbcond -mno-fmaf -mno-fsmuld -mno-popc 
-mno-subxc -mmemory-model=tso -fPIC -Wl,-rpath=/opt/bw/lib -Wl,-rpath=/usr/pkg/lib
  -DB_ENDIAN -DMD32_REG_T=int -DOPENSSL_PIC -DOPENSSLDIR="\"/opt/bw/ssl\"" 
-DENGINESDIR="\"/opt/bw/lib/engines-3\"" -DMODULESDIR="\"/opt/bw/lib/ossl-modules\"" 
-D_THREAD_SAFE -D_REENTRANT -D_XOPEN_SOURCE=500 
-DOPENSSL_BUILDING_OPENSSL -DZLIB -DNDEBUG  -MMD 
-MF apps/lib/libapps-lib-app_params.d.tmp -MT apps/lib/libapps-lib-app_params.o 
-c -o apps/lib/libapps-lib-app_params.o apps/lib/app_params.c
apps/lib/app_params.c: In function ‘print_param_value’:
apps/lib/app_params.c:106:58: error: ISO C90 does not support ‘long long’ [-Wlong-long]
  106 |             BIO_printf(bio_out, "%llu\n", (unsigned long long int)u);
      |                                                          ^~~~
apps/lib/app_params.c:112:49: error: ISO C90 does not support ‘long long’ [-Wlong-long]
  112 |             BIO_printf(bio_out, "%lld\n", (long long int)i);
      |                                                 ^~~~
*** Error code 1

Stop.
make[1]: stopped in /opt/bw/build/openssl-3.1.3_netbsd_9.3_sparcv9.001
*** Error code 1

Stop.
make: stopped in /opt/bw/build/openssl-3.1.3_netbsd_9.3_sparcv9.001

The -D_XOPEN_SOURCE=500 was just my way of throwing darts at the wall
to see what sticks. 

This likely will not go away unless we pack those 64bits of data into an array
of eight unsigned char or something like two unsigned integers. On the other
hand the OpenSSL Technical Committee could simply update section 14 at
this page : 


         https://www.openssl.org/policies/technical/coding-style.html

         Chapter 14: Portability

         To maximise portability the version of C defined in
         ISO/IEC 9899:1990 should be used. This is more commonly
         referred to as C90. ISO/IEC 9899:1999 (also known as C99) is
         not supported on some platforms that OpenSSL is used on and
         therefore should be avoided.

     There I see the use of the word "should" as opposed to "must".

I know that I can try C99 again but that fails in different ways. 

--
Dennis Clarke
RISC-V/SPARC/PPC/ARM/CISC
UNIX and Linux spoken
GreyBeard and suspenders optional

blastwave · 2023-09-26T01:51:47Z

Going down the rabbit hole here I see the calls to

     int OSSL_PARAM_get_uint64(const OSSL_PARAM *p, uint64_t *val)

actually get handed off to crypto/params.c which use thse two fellows : 

000223  
000224  /* General purpose get unsigned integer parameter call that handles odd sizes */
000225  static int general_get_uint(const OSSL_PARAM *p, void *val, size_t val_size)
000226  {
000227      if (p->data_type == OSSL_PARAM_INTEGER)
000228          return unsigned_from_signed(val, val_size, p->data, p->data_size);
000229      if (p->data_type == OSSL_PARAM_UNSIGNED_INTEGER)
000230          return unsigned_from_unsigned(val, val_size, p->data, p->data_size);
000231      err_not_integer;
000232      return 0;
000233  }
000234  
000235  /* General purpose set unsigned integer parameter call that handles odd sizes */
000236  static int general_set_uint(OSSL_PARAM *p, void *val, size_t val_size)
000237  {
000238      int r = 0;
000239  
000240      p->return_size = val_size; /* Expected size */
000241      if (p->data == NULL)
000242          return 1;
000243      if (p->data_type == OSSL_PARAM_INTEGER)
000244          r = signed_from_unsigned(p->data, p->data_size, val, val_size);
000245      else if (p->data_type == OSSL_PARAM_UNSIGNED_INTEGER)
000246          r = unsigned_from_unsigned(p->data, p->data_size, val, val_size);
000247      else
000248          err_not_integer;
000249      p->return_size = r ? p->data_size : val_size;
000250      return r;
000251  }
000252  

Really the first function to get the integer data. 

I am really wondering how sensitive this all is to the cast that is done in apps/lib/app_params.c at
lines 106 and 112?

blastwave added the issue: bug report The issue was opened to report a bug label Dec 1, 2019

mattcaswell added triaged: bug The issue/pr is/fixes a bug and removed issue: bug report The issue was opened to report a bug labels Dec 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Strict C90 CFLAGS results in sha.h:91 ISO C90 does not support long long #10547

Strict C90 CFLAGS results in sha.h:91 ISO C90 does not support long long #10547

blastwave commented Dec 1, 2019 •

edited by levitte

richsalz commented Dec 1, 2019

blastwave commented Dec 1, 2019 •

edited by levitte

blastwave commented Dec 1, 2019 •

edited by levitte

richsalz commented Dec 1, 2019

levitte commented Dec 1, 2019

levitte commented Dec 1, 2019

blastwave commented Dec 1, 2019 •

edited by levitte

blastwave commented Dec 1, 2019 •

edited by levitte

blastwave commented Dec 1, 2019 •

edited by levitte

blastwave commented Sep 26, 2023

blastwave commented Sep 26, 2023

Strict C90 CFLAGS results in sha.h:91 ISO C90 does not support long long #10547

Strict C90 CFLAGS results in sha.h:91 ISO C90 does not support long long #10547

Comments

blastwave commented Dec 1, 2019 • edited by levitte

richsalz commented Dec 1, 2019

blastwave commented Dec 1, 2019 • edited by levitte

blastwave commented Dec 1, 2019 • edited by levitte

richsalz commented Dec 1, 2019

levitte commented Dec 1, 2019

levitte commented Dec 1, 2019

blastwave commented Dec 1, 2019 • edited by levitte

blastwave commented Dec 1, 2019 • edited by levitte

blastwave commented Dec 1, 2019 • edited by levitte

blastwave commented Sep 26, 2023

blastwave commented Sep 26, 2023

blastwave commented Dec 1, 2019 •

edited by levitte

blastwave commented Dec 1, 2019 •

edited by levitte

blastwave commented Dec 1, 2019 •

edited by levitte

blastwave commented Dec 1, 2019 •

edited by levitte

blastwave commented Dec 1, 2019 •

edited by levitte

blastwave commented Dec 1, 2019 •

edited by levitte