valgrind: remove libsecp256k1 suppression #27173

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:valgrind_supp_prune_secp256k1 changing 1 files +0 −8

fanquake commented at 10:49 AM on February 28, 2023: member

I am no-longer been able to recreate this issue, atleast after the most recent libsecp256k1 changes. Can someone else confirm?
valgrind: remove libsecp256k1 suppression 29b62c01c8
fanquake added the label Tests on Feb 28, 2023
DrahtBot commented at 10:49 AM on February 28, 2023: contributor



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



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK MarcoFalke, sipa

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

Type	Reviewers
ACK	MarcoFalke, sipa

maflcko commented at 12:35 PM on February 28, 2023: member

Which OS did you try on? See also the header:

# Note that suppressions may depend on OS and/or library versions.
# Tested on:
# * aarch64 (Ubuntu 22.04 system libs, clang, without gui)
# * x86_64  (Ubuntu 22.04 system libs, clang, without gui)

fanquake commented at 12:37 PM on February 28, 2023: member

Which OS did you try on?

x86_64 (Ubuntu 22.04
sipa commented at 3:57 PM on February 28, 2023: member

I'm a bit surprised we had this suppression in the first place; it seems to have been added without any discussion in #17455? libsecp256k1 context objects in the Bitcoin Core codebase should always be deleted.

Yet, even if there was a bug around this, or somehow detected incorrectly, it's possible #26691 changed/fixed it (in particular, 4462cb04986d77eddcfc6e8f75e04dc278a8147a, which removed the verification context object).
maflcko commented at 3:59 PM on February 28, 2023: member

lgtm ACK 29b62c01c8d211475ea9dd1a1093820f0a86c06d

should be trivial to re-add in any case, if it happens again
fanquake marked this as ready for review on Feb 28, 2023
sipa commented at 4:28 PM on February 28, 2023: member

utACK 29b62c01c8d211475ea9dd1a1093820f0a86c06d
fanquake merged this on Feb 28, 2023
fanquake closed this on Feb 28, 2023
fanquake deleted the branch on Feb 28, 2023
sidhujag referenced this in commit a74c8ad0c1 on Mar 1, 2023
bitcoin locked this on Feb 28, 2024

Contributors

Labels

valgrind: remove libsecp256k1 suppression #27173

Reviews