`make test` fails due to overflow in conversion

[odiferousmint]

Hello.

When I run make test, I get the following:

make test -f ./Makefile.custom                                                                                               
make[1]: Entering directory '/tmp/libgoldilocks'                                                                             
g++ -fno-strict-aliasing -pedantic -Wall -Wextra -Werror -Wunreachable-code -Wmissing-declarations -Wunused-function -Wno-overlength-strings -Isrc -Isrc/include -Isrc/public_include -Isrc/include/arch_x86_64 -Isrc/arch_x86_64 -Os -march=native -ffunction-sections -fdata-sections -fvisibility=hidden -fomit-frame-pointer -fPIC -c -o build/obj/test_goldilocks.o test/test_goldilocks.cxx                                                                              
In file included from src/public_include/goldilocks/point_448.h:16,                                                          
                 from src/public_include/goldilocks/point_448.hxx:33,                                                        
                 from src/public_include/goldilocks/ed448.hxx:22,                                                            
                 from src/public_include/goldilocks.hxx:16,                                                                  
                 from test/test_goldilocks.cxx:12:                                                                           
src/public_include/goldilocks/common.h: In function ‘goldilocks_bool_t goldilocks_successful(goldilocks_error_t)’:           
src/public_include/goldilocks/common.h:97:55: error: overflow in conversion from ‘goldilocks_word_t’ {aka ‘long unsigned int’} to ‘goldilocks_error_t’ changes value from ‘18446744073709551615’ to ‘GOLDILOCKS_SUCCESS’ [-Werror=overflow]                                                                                                                     
   97 |     goldilocks_dword_t w = ((goldilocks_word_t)e) ^  ((goldilocks_word_t)GOLDILOCKS_SUCCESS);                        
      |                                                      ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                         
cc1plus: all warnings being treated as errors                                                                                
make[1]: *** [Makefile.custom:168: build/obj/test_goldilocks.o] Error 1                                                      
make[1]: Leaving directory '/tmp/libgoldilocks'                                                                              
make: *** [Makefile:864: test] Error 2

diff --git a/src/public_include/goldilocks/common.h b/src/public_include/goldilocks/common.h
index 927369b..2bf3428 100644
--- a/src/public_include/goldilocks/common.h
+++ b/src/public_include/goldilocks/common.h
@@ -94,7 +94,7 @@ goldilocks_succeed_if(goldilocks_bool_t x) {
 /** Return GOLDILOCKS_TRUE iff x == GOLDILOCKS_SUCCESS */
 static GOLDILOCKS_INLINE goldilocks_bool_t
 goldilocks_successful(goldilocks_error_t e) {
-    goldilocks_dword_t w = ((goldilocks_word_t)e) ^  ((goldilocks_word_t)GOLDILOCKS_SUCCESS);
+    goldilocks_dword_t w = ((goldilocks_word_t)e) ^  GOLDILOCKS_SUCCESS;
     return (w-1)>>GOLDILOCKS_WORD_BITS;
 }

seems to suppress the error (warning) and the tests do pass. Thoughts?

Using commit c8d994098ff9094e3021f1a01e43faa3c6cf6fe4.

[claucece]

Hey, @odiferousmint !

Thanks for the comment! Which OS do you use? Which compiler?

Thanks!

[odiferousmint]

OS: Linux arch 5.2.9-arch1-1-ARCH #1 SMP PREEMPT Fri Aug 16 11:29:43 UTC 2019 x86_64 GNU/Linux

It does not work with: gcc version 9.1.0 (GCC). It works with: clang version 8.0.1 (tags/RELEASE_801/final), and gcc version 8.3.0 (GCC).

If you need any more information, please let me know! I suppose this is just a regression in GCC.

---

I would also like to note that make test does not use the CC and CXX that is passed to ./configure, it seems to default to g++. I am not sure if this is intended or not. It is not exactly a problem though, you can just easily do make test CC=gcc-8 CXX=g++-8, for example.

[claucece]

Hi, @odiferousmint !

Indeed. It should be fixed now.

I would also like to note that make test does not use the CC and CXX that is passed to ./configure, it seems to default to g++. I am not sure if this is intended or not. It is not exactly a problem though, you can just easily do make test CC=gcc-8 CXX=g++-8, for example.

Yes, exactly. We need this to pass specific flags, but I'll make a refactor so it always uses the configure one and adds the needed flags ;)

Thanks!