Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run tests in memory sanitizer #17460

Closed
maflcko opened this issue Nov 13, 2019 · 5 comments · Fixed by #17725
Closed

Run tests in memory sanitizer #17460

maflcko opened this issue Nov 13, 2019 · 5 comments · Fixed by #17725

Comments

@maflcko
Copy link
Member

maflcko commented Nov 13, 2019

We run the tests in the thread (and other sanitizers), see git grep sanitizer ./ci, but not in the memory sanitizer. This seems like it could be improved. See e.g. #17449

@maflcko
Copy link
Member Author

maflcko commented Nov 14, 2019

On a second thought, it might be better to just run with valgrind: bitcoin-core/secp256k1#687 (comment)

@practicalswift
Copy link
Contributor

practicalswift commented Nov 14, 2019

@MarcoFalke I've tried to add Valgrind to our Travis build before: Valgrind is very good but also very very very slow. Running the unit tests and the functional tests under Valgrind takes hours(!) :(

Perhaps DynamoRIO's Dr. Memory could be a faster option. I haven't investigated that option yet.

Another alternative could be to run a subset of the tests under Valgrind and then use MSAN as a complement. With MSAN running all the tests including those excluded from the Valgrind run due to being too slow.

@maflcko
Copy link
Member Author

maflcko commented Nov 14, 2019

Maybe running a single functional test (and/or unit test) in valgrind would be a good start. #17449 would have been caught when Bitcoin Core just boots up on mainnet.

@practicalswift
Copy link
Contributor

Maybe running a single functional test (and/or unit test) in valgrind would be a good start.

Agreed: to start with something under valgrind would be better nothing :)

#17449 would have been caught when Bitcoin Core just boots up on mainnet.

Do you mean "boots up on mainnet" as in when IBD is finished on mainnet?

FWIW, I've only gotten valgrind to warn about the #17449 case when WarningBitsConditionChecker::Condition(…) is being run. That happens in UpdateTip given that we're done with IBD.

Perhaps one would expect valgrind to warn earlier in the execution, such as when {CreateChain,Select}Params(…) is run. But that is not the case :)

@elichai
Copy link
Contributor

elichai commented Nov 25, 2019

#17568 (comment)

sidhujag pushed a commit to syscoin/syscoin that referenced this issue Dec 17, 2019
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
sidhujag pushed a commit to syscoin-core/syscoin that referenced this issue Nov 10, 2020
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
knst pushed a commit to knst/dash that referenced this issue Apr 18, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
knst pushed a commit to knst/dash that referenced this issue Apr 24, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
knst pushed a commit to knst/dash that referenced this issue Apr 24, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
knst pushed a commit to knst/dash that referenced this issue Apr 25, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
knst pushed a commit to knst/dash that referenced this issue May 15, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
knst pushed a commit to knst/dash that referenced this issue May 15, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
knst pushed a commit to knst/dash that referenced this issue May 16, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
knst pushed a commit to knst/dash that referenced this issue May 16, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
knst pushed a commit to knst/dash that referenced this issue May 18, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
knst pushed a commit to knst/dash that referenced this issue May 25, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
knst pushed a commit to knst/dash that referenced this issue May 30, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
knst pushed a commit to knst/dash that referenced this issue Jun 1, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
PastaPastaPasta pushed a commit to knst/dash that referenced this issue Jun 7, 2023
facb416 ci: Add valgrind run (MarcoFalke)

Pull request description:

  Fixes bitcoin#17460

ACKs for top commit:
  practicalswift:
    ACK facb416

Tree-SHA512: 55396e548a76f976d7b7170b68bc5f93cfd44656162267172f66db7eb549699a2a22d3b1bb0d5f180fe0697931939e652c8cdb86b435e81e7ce572485798009d
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants