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

[executable semantics] if2.6c example segfaults #311

Closed
dabrahams opened this issue Feb 28, 2021 · 4 comments
Closed

[executable semantics] if2.6c example segfaults #311

dabrahams opened this issue Feb 28, 2021 · 4 comments
Assignees

Comments

@dabrahams
Copy link

dabrahams commented Feb 28, 2021

The if2.6c example segfaults. I'm currently in the process of temporarily disabling it and making our build not hide the problem. #312 will disable the test by commenting it out of the BUILD file.

dabrahams pushed a commit that referenced this issue Feb 28, 2021
* Add assertion to detect UB in interpreter.cpp

This change, applied to 2111b85 ("Adapting jsiek's executable semantics tooling
for commit. (#237)"), causes the if2.6c test to segfault.  Becase the assertion
fires only when `stmt == nullptr` and no code has permission to change `stmt`
(it is `const`) before it is dereferenced in the `switch`, and nothiing in
`PrintStatement` is supposed to exit the program, the assertion is a valid
change that detects a bug.

The crash was originally manifest in 0213c6d ("Executable Semantics: 1st-class
stacks (#296)").

* Temporarily disable the if2 test pending #311

See #311

* [executable semantics] Record exit code on expected error.

This will prevent a final segfault from sneaking by, detected as a passing test.
A more principled follow-up commit would bottleneck detected error exit
reporting and have it write something to std::cerr that can be recognized.
@jsiek
Copy link
Contributor

jsiek commented Feb 28, 2021

This bug should be fixed with the following PR
#315

@dabrahams
Copy link
Author

@jsiek then you should feel free to close it, along with saying so. At least that's how it is on most projects I've worked on. Also see https://github.blog/2013-01-22-closing-issues-via-commit-messages/

@jsiek
Copy link
Contributor

jsiek commented Mar 2, 2021 via email

@dabrahams
Copy link
Author

Oh, in that case, just procedurally, we shouldn't have closed it until #323 landed. And "Fixes #311" in 323's message would've done that automatically.

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

No branches or pull requests

2 participants