Add a build option to the assembler to retain local symbols. #1252
Conversation
|
Is it worth adding a test to |
If this will be used by the formal verification tooling then I assume that would provide coverage? Regardless I'm not sure how useful this is to test if its just for visibility to aid in debugging / writing FV tooling. Out of curiosity odes this cause issues if you do this with a FIPS build? ( |
CMakeLists.txt
Outdated
| @@ -633,6 +633,10 @@ if(GCOV) | |||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage") | |||
| endif() | |||
|
|
|||
| if(KEEP_LOCAL_SYMBOLS) | |||
There was a problem hiding this comment.
Since this only affects the assembler should the flag be KEEP_LOCAL_ASM_SYMBOLS?
There was a problem hiding this comment.
I changed it to KEEP_ASM_LOCAL_SYMBOLS.
in a FIPS release build.
I added a test to the FIPS tests and used a side effect of |
The hash with and without the option is the same in Release and Debug FIPS builds. But Release build with gcc7 doesn't finish the break-kat tests. This could be one reason why the test uses a Debug build. Note that higher gcc versions complete the test on a release build. |
### Description of changes: The original intention of #1005 was to ensure break-kat.go always worked with the library. #1252 then used a side effect to get the module hash but didn't actually fail the build if the hash was different. Turn on `pipefail` so the script exits unsuccessfully if any command fails even in a pipe. Previously if test-break-kat.sh failed the script didn't exit, instead it would continue to the grep which would also fail to find any matching string, and then the `|| true` ensured the script always continued on. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
### Description of changes: The original intention of aws#1005 was to ensure break-kat.go always worked with the library. aws#1252 then used a side effect to get the module hash but didn't actually fail the build if the hash was different. Turn on `pipefail` so the script exits unsuccessfully if any command fails even in a pipe. Previously if test-break-kat.sh failed the script didn't exit, instead it would continue to the grep which would also fail to find any matching string, and then the `|| true` ensured the script always continued on. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
Issues:
Resolves CryptoAlg-2142
Description of changes:
Formal verification needed some local symbols to be visible in the symbol table. This build option helps achieve that.
Testing:
Building with
cmake -DKEEP_LOCAL_SYMBOLS=1 -DCMAKE_BUILD_TYPE=Release -GNinja ..K512in the output ofobjdump --syms crypto_test | lessyielded a result.crypto_testyielded results as well.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.