Skip to content

Bugfix: Maintain safe_pointers per-path #2879

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

Merged

Conversation

karkhaz
Copy link
Collaborator

@karkhaz karkhaz commented Sep 1, 2018

The map of safe pointers is now maintained separately for each path,
ensuring that pointer accesses on one path do not influence a (lack of)
pointer accesses on a different path.

This commit fixes #2866, a segfault that would happen when running CBMC
on the included regression test.

@karkhaz karkhaz force-pushed the kk-path-explore-safe-pointer-bugfix branch from 58db2a1 to 0c9f069 Compare September 1, 2018 08:48
@karkhaz karkhaz force-pushed the kk-path-explore-safe-pointer-bugfix branch 3 times, most recently from 1e867b6 to bd42010 Compare September 1, 2018 09:23
@@ -0,0 +1,19 @@
default: tests.log
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add this directory to regression/Makefile and regression/CMakeLists.txt -- although I'm not sure a new directory is strictly required, it might be safe to include that test in the cbmc/ folder?

@karkhaz karkhaz force-pushed the kk-path-explore-safe-pointer-bugfix branch from bd42010 to 122f0a9 Compare September 3, 2018 12:04
The map of safe pointers is now maintained separately for each path,
ensuring that pointer accesses on one path do not influence a (lack of)
pointer accesses on a different path.

This commit fixes diffblue#2866, a segfault that would happen when running CBMC
on the included regression test.
@karkhaz karkhaz force-pushed the kk-path-explore-safe-pointer-bugfix branch from 122f0a9 to 959c7a5 Compare September 3, 2018 12:05
@karkhaz
Copy link
Collaborator Author

karkhaz commented Sep 3, 2018

@tautschnig thanks, I moved the test over to regression/cbmc/

@tautschnig tautschnig merged commit 704ea1a into diffblue:develop Sep 3, 2018
@karkhaz karkhaz deleted the kk-path-explore-safe-pointer-bugfix branch September 4, 2018 05:06
@smowton
Copy link
Contributor

smowton commented Sep 10, 2018

Sorry this comment is late, just getting back from holiday. This doesn't make sense to me currently, as the safe-pointers analysis isn't path sensitive-- this is a very local analysis looking for accesses trivially dominated by checks. The result should therefore always be the same, hence keeping them globally. It should have the same usage pattern as dirtyt in this respect. So what's going on here?

@karkhaz
Copy link
Collaborator Author

karkhaz commented Sep 12, 2018

I'm actually not sure about this. As far as I can see, values are only added to that map in two places in the codebase (symex_dereference.cpp:250 and symex_function_call.cpp:228), and the keys shouldn't be going out of scope or being mysteriously removed or anything. Let's wait for @tautschnig to get back from holiday, he suggested the fix?

However, note that dirtyt is also owned by goto_symex_statet. IIRC, I moved it there for exactly the same reason when I wrote the original path exploration code (it's been a while now, can't remember what the exact problems were, but that's why the class now has the awkward copy constructor).

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

Successfully merging this pull request may close these issues.

path-based symex throws map out of range error
3 participants