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

FYI: Some good news about the health of this code #1086

Closed
krader1961 opened this issue Dec 21, 2018 · 3 comments
Closed

FYI: Some good news about the health of this code #1086

krader1961 opened this issue Dec 21, 2018 · 3 comments
Assignees
Labels

Comments

@krader1961
Copy link
Contributor

krader1961 commented Dec 21, 2018

This is the sort of thing I would normally send to the project's mailing list(s) but since we don't have one at the moment....

I was starting to wonder if there was any point in trying to keep this project alive given the numerous problems with the code. Not least of which is the inscrutability of important portions of the code; e.g., the name/value subsystem.

Issue #1085 (wherein I caused a build failure under ASAN due to missing extern qualifiers) caused me to run the unit tests under ASAN since the last time I did so ~4 months ago. I was surprised that the ASAN test failures had been reduced from ~48 to ~18 in the past four months.

For example, around 2018-08-11 I merged a lot of changes. Nearly all of which were simple cleanups involving elimination of libast code that wasn't being used by ksh. But one of them, commit c52c149, fixed a bug identified by Coverity Scan. That commit was a mere five lines long; two of which were comments. That reduced the ASAN test failures from ~48 to ~30.

Since then other changes by @siteshwar and myself have reduced the ASAN test failures from ~30 to ~18.

It looks like nearly all the failures that happen at random or under the control of tools like ASAN, Valgrind, or a debug malloc subsystem are in the name/value code. Which may be due to bugs in the ksh name/value code or the libast CDT subsystem that ksh uses. However, my hunch is 99% of them are in the ksh, not CDT, code. In no small part because of issue #1038.

Note: The approximation symbol (~) above is because the actual failure count varies slightly from run to run. Why? Because most (all?) of the failures depend on the location of structures in memory and mechanisms like ASLR, or random effects due to timing and such, may cause a given run to pass or fail. All of these numbers involve the same clang compiler, Meson, Ninja, and macOS version.

@krader1961 krader1961 self-assigned this Dec 21, 2018
@krader1961
Copy link
Contributor Author

There is also the reduction in theoretical bugs as shown by the Coverity Scan dashboard. The static analysis defect density is still obscene but it is now much closer to industry norms than it was a year ago. This is also reflected in the problems identified by IWYU, cppcheck, oclint, and gcc/clang (run bin/lint all).

@krader1961
Copy link
Contributor Author

FWIW, We are now down to three distinct heap-use-after-free bugs identified by ASAN. Which means six test failures since there is a shcomp and non-shcomp failure for each unit test. In other words, since Sep 2018 we've gone from 24 bugs identified by ASAN (48 if you ignore the shcomp test variants) to just 3. And at least one of those failures is due to code introduced after the ksh93u release and appears unlikely to affect any real-world ksh script.

@krader1961
Copy link
Contributor Author

With commit fc53766 that I merged yesterday there are zero memory management bugs found by ASAN if you exclude memory leaks; which we've been ignoring until the more serious bugs were fixed.

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

No branches or pull requests

1 participant