weidai11 / cryptopp

free C++ class library of cryptographic schemes
https://cryptopp.com
Other
4.67k stars 1.47k forks source link

nbtheory.cpp: warning: possibly dangling reference to a temporary #1210

Closed vgfl closed 1 year ago

vgfl commented 1 year ago

Hello there,

A small warning with that file with g++-13.1. Just in the unlikely case it points at something serious:

/usr/local/gcc-13.1/bin/g++-13.1 -g2 -pipe -std=c++20 -pedantic -Wall -DNDEBUG -O3 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -pthread -c nbtheory.cpp
nbtheory.cpp: In function ‘const CryptoPP::word16* CryptoPP::GetPrimeTable(unsigned int&)’:
nbtheory.cpp:55:36: warning: possibly dangling reference to a temporary [-Wdangling-reference]
   55 |         const std::vector<word16> &primeTable = Singleton<std::vector<word16>, NewPrimeTable>().Ref();
      |                                    ^~~~~~~~~~
nbtheory.cpp:55:100: note: the temporary was destroyed at the end of the full expression ‘CryptoPP::Singleton<std::vector<short unsigned int>, CryptoPP::NewPrimeTable>((CryptoPP::NewPrimeTable(), CryptoPP::NewPrimeTable())).CryptoPP::Singleton<std::vector<short unsigned int>, CryptoPP::NewPrimeTable>::Ref()’
   55 |         const std::vector<word16> &primeTable = Singleton<std::vector<word16>, NewPrimeTable>().Ref();
      |                                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~

Thanks!

noloader commented 1 year ago

Thanks @vgfl,

Yeah, I am seeing that too with -Wdangling-reference. I'll take a look at it soon.

At first glance, I think the prime table is being built on the fly. I think we should just precompute it, and put it in a rodata section. The OS can load it into memory if needed, and discard it when not needed.

Once the table is precomputed, we can do away with the Singleton to build it on the fly.

noloader commented 1 year ago

Ok, this should be cleared at Commit 6ecc789df1ce.