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

[C++] Reenable Valgrind on Travis-CI #21740

Closed
asfimport opened this issue May 6, 2019 · 10 comments
Closed

[C++] Reenable Valgrind on Travis-CI #21740

asfimport opened this issue May 6, 2019 · 10 comments

Comments

@asfimport
Copy link

asfimport commented May 6, 2019

Running Valgrind on Travis-CI was disabled in ARROW-4611 (apparently because of issues within the re2 library).

We should reenable it at some point in order to exercise the reliability of our C++ code.
(and/or have a build with another piece of instrumentation enabled such as ASAN)

Reporter: Antoine Pitrou / @pitrou

Related issues:

Note: This issue was originally created as ARROW-5270. Please see the migration documentation for further details.

@asfimport
Copy link
Author

Antoine Pitrou / @pitrou:
@xhochy

@asfimport
Copy link
Author

Pindikura Ravindra / @pravindra:
There are two issues :

  1. instructions not recognized by valgrind

    =20276== Your program just tried to execute an instruction that Valgrind ==20276== did not recognise. There are two possible reasons for this. ==20276== 1. Your program has a bug and erroneously jumped to a non-code ==20276== location. If you are running Memcheck and you just saw a ==20276== warning about a bad jump, it's probably your program's fault.

    1. the re2 issues

    I think these are already covered by the suppressions listed in the valgrind.supp but they aren't being recognized due to missing symbols in the stack. 

    When I ran this on my xenial setup without any conda setup, the stacks showed up correctly and got suppressed. so, I suspect this is an issue with conda binaries.

@asfimport
Copy link
Author

@asfimport
Copy link
Author

Antoine Pitrou / @pitrou:
The instructions not recognized seem to be SSE4.2 instructions. We would probably need a more recent Valgrind version on Travis-CI...

@asfimport
Copy link
Author

Antoine Pitrou / @pitrou:
We now test with ASAN and UBSAN on Travis, so perhaps this is not as important as it used to be. @wesm [~emkornfield@gmail.com] opinions?

@asfimport
Copy link
Author

Wes McKinney / @wesm:
I'd be okay relegating valgrind testing to docker-compose

@asfimport
Copy link
Author

Micah Kornfield / @emkornfield:
I think the one thing missing from UBSan/ASAN that I know of that has some utility is making use of uninitialized memory. For that we would need to enable MSan which I don't think is practical. But I agree we should catch a large class of errors with current sanitizers.

@asfimport
Copy link
Author

Antoine Pitrou / @pitrou:
I'm not sure we'll ever re-enable Valgrind. Micah, what do you think? Should we close this issue?

@asfimport
Copy link
Author

Wes McKinney / @wesm:
I'd prefer to run valgrind builds only in Docker (e.g. so they can be run on-demand on Ursabot, much faster)

@asfimport
Copy link
Author

Antoine Pitrou / @pitrou:
Ok, let's close this issue then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant