ci: Fuzz with -ftrivial-auto-var-init=pattern #22841

1. 
   MarcoFalke commented at 10:41 AM on August 31, 2021: member

   This makes memory bugs deterministic. -ftrivial-auto-var-init=pattern is incompatible with other memory sanitizers (like valgrind and msan), but that is irrelevant here, because the address sanitizer in this fuzz CI config is already incompatible with them.

   -ftrivial-auto-var-init=pattern goes well with -fsanitize=bool and -fsanitize=enum, but those are already enabled via -fsanitize=undefined. See https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#available-checks

2. fanquake added the label Tests on Aug 31, 2021
3. 
   MarcoFalke commented at 10:44 AM on August 31, 2021: member

   See also https://github.com/google/oss-fuzz/pull/5879

4. 
   fanquake commented at 11:40 AM on August 31, 2021: member

   Concept ACK guessing this needs to use a newer Clang: https://github.com/bitcoin/bitcoin/pull/22841/checks?check_run_id=3471928307

   configure: error: unrecognized option: `-ftrivial-auto-var-init=pattern'
   

5. MarcoFalke force-pushed on Aug 31, 2021
6. ci: Fuzz with -ftrivial-auto-var-init=pattern fa0a5fa744
7. MarcoFalke force-pushed on Aug 31, 2021
8. 
   MarcoFalke commented at 12:51 PM on August 31, 2021: member

   configure: error

   Fixed

9. fanquake requested review from practicalswift on Sep 1, 2021
10. 
    fanquake commented at 12:21 AM on September 1, 2021: member

    cc @guidovranken @agroce

11. 
    MarcoFalke commented at 7:44 AM on September 1, 2021: member

    This would catch the bug fixed in commit 37371268d14ed6d5739af5b65d8bdb38b0e8dda2 . (Memory sanitizers also do that, but there shouldn't be any risk in making our asan config catch it as well.)

12. 
    jonatack commented at 8:28 AM on September 1, 2021: member

    Interesting. I wrote a tiny section about the -ftrivial-auto-var-init=pattern option and listed some resources about default initialization (including a good list of pros/cons in #17627 (comment) by @practicalswift) a couple years ago in the notes of https://bitcoincore.reviews/17639 to try to understand some of its tradeoffs:

    • Pre-initialize variables with dummy values, e.g. compile with Clang -ftrivial-auto-var-init=pattern.
      • Testing sensitivity concerns, e.g. #17627 (comment) by Gregory Maxwell.
      • If a flaw is introduced, it may be undetectable by valgrind.
      • Compilers can warn when they're certain a value will be undefined, and pre-emptive dummy initialization suppresses those warnings.
      • Valgrind has special macros that can be used to mark memory as undefined. It may be best if dummy initialization were to be always done via a macro that would allow disabling it for testing, or valgrind annotating it.

    Concept ACK on running the option on its own in a CI task, for IIUC that would avoid the potential drawbacks.

13. 
    MarcoFalke commented at 2:34 PM on September 1, 2021: member

    I don't think those tradeoffs are relevant to the discussion here. valgrind (and other memory sanitizers) are already incompatible with asan, as mentioned in the OP.

14. 
    jonatack commented at 2:55 PM on September 1, 2021: member

    I don't think those tradeoffs are relevant to the discussion here. valgrind (and other memory sanitizers) are already incompatible with asan, as mentioned in the OP.

    I thought the information could be interesting for reviewers interested to learn more.

15. 
    MarcoFalke commented at 3:01 PM on September 1, 2021: member

    Ah yes, for general background information on the topic that serves as a good starting point.

16. 
    practicalswift commented at 1:34 PM on September 4, 2021: contributor

    cr ACK fa0a5fa744108d81bee9600c80bfda6ca11e5255

17. MarcoFalke merged this on Sep 6, 2021
18. MarcoFalke closed this on Sep 6, 2021

    ---

19. MarcoFalke deleted the branch on Sep 6, 2021
20. sidhujag referenced this in commit 82fcda7541 on Sep 7, 2021
21. DrahtBot locked this on Sep 6, 2022

Review Requested
practicalswift