build: pass sanitize flags to instrument libsecp256k1 code #27991

1. 
   fanquake commented at 1:48 PM on June 28, 2023: member

   secp256k1 functions are now instrumented:

   # objdump --disassemble-symbols=_secp256k1_xonly_pubkey_serialize src/test/fuzz/fuzz | grep sanitizer_cov
   100eb5244: 73 5d 01 94 	bl	0x100f0c810 <___sanitizer_cov_trace_const_cmp8>
   100eb53f0: cc 5c 01 94 	bl	0x100f0c720 <___sanitizer_cov_trace_pc_indir>
   100eb5418: c2 5c 01 94 	bl	0x100f0c720 <___sanitizer_cov_trace_pc_indir>
   100eb5444: b7 5c 01 94 	bl	0x100f0c720 <___sanitizer_cov_trace_pc_indir>
   

   Not sure if this is the best solution. Would fix #27990.

2. build: pass sanitize flags to instrument libsecp code

   secp256k1 functions are now instrumented:
   ```asm
   objdump --disassemble-symbols=_secp256k1_xonly_pubkey_serialize src/test/fuzz/fuzz | grep sanitizer_cov
   100eb5244: 73 5d 01 94 	bl	0x100f0c810 <___sanitizer_cov_trace_const_cmp8>
   100eb53f0: cc 5c 01 94 	bl	0x100f0c720 <___sanitizer_cov_trace_pc_indir>
   100eb5418: c2 5c 01 94 	bl	0x100f0c720 <___sanitizer_cov_trace_pc_indir>
   100eb5444: b7 5c 01 94 	bl	0x100f0c720 <___sanitizer_cov_trace_pc_indir>
   ```

   5a6e0f7744
3. fanquake requested review from dergoegge on Jun 28, 2023
4. 
   DrahtBot commented at 1:48 PM on June 28, 2023: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	dergoegge

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #28241 (Silent payment index (for light wallets and consistency check) by Sjors)
   • #28202 (Silent Payments: receiving by josibake)
   • #28201 (Silent Payments: sending by josibake)
   • #28122 (Silent Payments: Implement BIP352 by josibake)
   • #27827 (Silent Payments: send and receive by josibake)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

5. DrahtBot added the label Build system on Jun 28, 2023
6. 
   hebasto commented at 1:51 PM on June 28, 2023: member

   Is it guaranteed that sanitizer flags for a C++ compiler are acceptible by a C compiler?

7. 
   dergoegge commented at 2:01 PM on June 28, 2023: member

   Concept ACK

   All of the CI tasks that make use of the integer sanitizer are probably gonna fail.

8. DrahtBot added the label CI failed on Jun 28, 2023
9. in configure.ac:1961 in 5a6e0f7744

   1956 | @@ -1956,7 +1957,11 @@ CPPFLAGS_TEMP="$CPPFLAGS"
   1957 |  unset CPPFLAGS
   1958 |  CPPFLAGS="$CPPFLAGS_TEMP"
   1959 |  
   1960 | +if test "$use_sanitizers" != ""; then
   1961 | +ac_configure_args="${ac_configure_args} --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --disable-module-ecdh CFLAGS=${SANITIZER_CFLAGS}"
   

   ---

   ---

   

   luke-jr commented at 3:07 PM on July 4, 2023:

   Won't this lose the other CFLAGS currently passed by the user?

10. luke-jr changes_requested
11. 
    maflcko commented at 8:43 AM on July 22, 2023: member

    Maybe mark as draft for as long as CI is red?

12. 
    hebasto commented at 1:54 PM on July 23, 2023: member

    FWIW, CMake works out of the box :)

13. 
    emc99 commented at 8:48 PM on July 23, 2023: none

    Maybe mark as draft for as long as CI is red?

    When does CI happen? Why would CI be red in this case?

14. fanquake marked this as a draft on Jul 25, 2023
15. fanquake referenced this in commit fadad10126 on Aug 1, 2023
16. 
    fanquake commented at 11:00 AM on September 7, 2023: member

    Maybe we can followup with this in future (regardless of anything CMake related).

17. fanquake closed this on Sep 7, 2023

    ---

18. fanquake deleted the branch on Sep 11, 2023
19. fanquake referenced this in commit e3b68b3b83 on Jan 26, 2024
20. bitcoin locked this on Sep 10, 2024

Review Requested
dergoegge