test: remove file-wide interpreter.cpp ubsan suppression #29541

1. 
   fanquake commented at 10:08 pm on March 2, 2024: member
2. test: remove file-wide interpreter.cpp ubsan suppression 217c0ce552
3. 
   DrahtBot commented at 10:08 pm on March 2, 2024: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	hebasto, Sjors, dergoegge

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #29543 (refactor: Avoid unsigned integer overflow in script/interpreter.cpp by hebasto)

   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.

4. DrahtBot renamed this:
   test: remove file-wide interpreter.cpp ubsan suppression
   test: remove file-wide interpreter.cpp ubsan suppression
   on Mar 2, 2024
5. DrahtBot added the label Tests on Mar 2, 2024
6. hebasto approved
7. 
   hebasto commented at 10:06 am on March 3, 2024: member

   ACK 217c0ce552a5d519b5cc702aba0c82514a1c449e.

   However, I’d prefer an alternative approach.

8. 
   Sjors commented at 9:03 am on March 5, 2024: member

   Is it possible to mark suppressions inside the source code itself? In that case we could specifically suppress the two affected lines. That has the benefit of not having to touch the code itself (like #29543 does), while avoiding future regressions elsewhere in EvalScript.

   utACK 217c0ce552a5d519b5cc702aba0c82514a1c449e

9. in test/sanitizer_suppressions/ubsan:58 in 217c0ce552

   54@@ -55,7 +55,7 @@ unsigned-integer-overflow:MurmurHash3
   55 unsigned-integer-overflow:CBlockPolicyEstimator::processBlockTx
   56 unsigned-integer-overflow:TxConfirmStats::EstimateMedianVal
   57 unsigned-integer-overflow:prevector.h
   58-unsigned-integer-overflow:script/interpreter.cpp
   59+unsigned-integer-overflow:EvalScript
   

   ---

   ---

   

   Sjors commented at 9:20 am on March 5, 2024:

   I assume stacktop / altstacktop don’t work here?

   ---

   

   fanquake commented at 4:38 pm on March 5, 2024:

   Not in this context, because these are runtime suppressions.
10. 
    fanquake commented at 4:40 pm on March 5, 2024: member

    Is it possible to mark suppressions inside the source code itself?

    There are various ways to suppress issues, however, in this PR, I’m not interested in changing the src, just reducing the scope of the currently applied runtime suppression.

11. dergoegge approved
12. 
    dergoegge commented at 4:41 pm on March 5, 2024: member

    ACK 217c0ce552a5d519b5cc702aba0c82514a1c449e
13. 
    fanquake commented at 4:53 pm on March 5, 2024: member

    I’m going to merge this to reduce the scope of the suppression, discussion around refactoring / inlining suppressions could can in #29543.
14. fanquake merged this on Mar 5, 2024
15. fanquake closed this on Mar 5, 2024

    ---

16. fanquake deleted the branch on Mar 5, 2024

Contributors
fanquake DrahtBot hebasto Sjors dergoegge

Labels
Tests