classification
Title: Engineering Process Improvements
Type: enhancement Stage: resolved
Components: Build Versions:
process
Status: closed Resolution: later
Dependencies: Superseder:
Assigned To: Nosy List: Jeffrey.Walton, r.david.murray
Priority: normal Keywords:

Created on 2014-03-16 14:48 by Jeffrey.Walton, last changed 2014-03-16 15:28 by Jeffrey.Walton. This issue is now closed.

Messages (4)
msg213728 - (view) Author: Jeffrey Walton (Jeffrey.Walton) * Date: 2014-03-16 14:48
Python's code is crisp and sharp. From a software design perspective, I don't see a lot of room for improvement. However, looking at some of the issues flagged by Clang sanitizers and existing bug reports, I think the project has a couple of small opportunities for improvement in the engineering process. The small improvements have the potential for large payoffs, and could help take a mature code base to the next level.

The improvements are not sexy, and they are often overlooked. The improvements are: (1) add instrumentation to the code; and (2) add scanning and analysis to the engineering process. To improve the code and engineering process to proactively hunt obscure and hard to find bugs, I would suggest three actionable items:

  * ASSERTions
  * Clang Santizers
  * Coverity Scans

Placing measures to proactively hunt bugs will improve the code quality, and likely allow the project to drop `-fwrapv` and friends from compiler options (http://bugs.python.org/issue1621).

ASSERTions
==========
The code uses Posix's little assert on occasion. Posix assert is useless during development because it raise SIGABRT. Additionally, the code has minimal assertions, so assertions are not being utilized effectively.

I think there is an opportunity to use a big ASSERT. The big ASSERT raises a SIGTRAP, and during development it will ensure the debugger snaps on an unexpected condition. The big ASSERT raises awareness where needed and allows the developers to continue handling on a negative code path.

ASSERTs should be liberally sprinkled. That is, ASSERT parameters, return values and program state. Python is setup correctly with NDEBUG on Release builds, so the ASSERTs will be removed in production.

With a full compliment of ASSERTs in place, the code will essential debug itself under most circumstances. In fact, I often pitch it as "self-debugging code" to management.

 Self debugging code has three benefits. First, self debugging code is effectively a force multiplier, and it allows fewer developers to maintain a larger code base. Second, it allows non-senior members to effectively contribute to the project. Google Summer of Code interns and junior developers could be effective team members since the ASSERTs will often guide them in tracing problems. Third, ASSERTs free up time for other endeavors, like adding features to Python or watching football.

Clang Sanitizers
===============
I think the code base would benefit from regular Clang sanitizer scans. From initial scanning and testing, I believe Clang and its sanitizers has shown its value.

The sanitizers perform dynamic analysis. The issues flagged are almost always real issues, and require almost no filtering due to false positives. The analysis should include the undefined behavior sanitizer (-fsanitize=undefined) and the address sanitizer (-fsanitize=address). There are more sanitizers available, and they are listed at http://clang.llvm.org/docs/UsersManual.html#controlling-code-generation.

From past experience, I've used Asan (address sanitizer) and UBsan (undefined behavior sanitizer) to find tricky bugs in other projects and libraries. For example, Wei Dai's Crypto++ would fail one self test when compiled with Intel's ICC (but pass them all under Clang, Comeau, GCC and MSVC).

The failed Crypto++ self test was due to ICC aggressively removing undefined behavior, and the Intel compiler/optimizer targeted [not readily apprent] undefined behavior in the underlying library code. After fixing the library code identifed by Clang UBsan, Crypto++ tested fine under all compilers.

The real beauty here is: (1) Python already has a mature process, so the addition of the Clang analysis will take minimal effort; and (2) Python has a rich set of self-test, so the Clang sanitizers can *really* be effective.

For areas that don't have complete self test coverage, its another task that can be delegated to non-senior members and contributors. That frees up time for senior members to do other things, like adding features to Python or watching football.

Coverity Scans
==============
I think the project could benefit from Coverity scanning and analysis. The analysis is quite good most of the time, and I believe it will complement Python's exiting engineering process. In addition, it will complement Clang analysis is Clang is adopted.

Coverity is one of the high-end analysis engines available. Though Coverity is a pay-to-play service, Coverity offers a free scanning service for free projects. The URL for the free service is http://scan.coverity.com. It cost nothing to sign up for, and it costs nothing to run a scan. Minimal effort is required to prepare a binary for upload.

Coverity performs static analysis, and it will flag false positives on occasion. However, its a trade-off in the effort to proactively hunt bugs.
msg213731 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-03-16 15:12
We already have Coverty scan in place, and were in fact featured by them for our code quality.  Currently Christian Heimes is the lead on that effort, and is monitoring the Coverty reports.  We've been working on Clang stuff as developers have had interest, and accepted a patch enabling address sanitizer to be used.  So, yes, we are interested in this stuff and working on it, and your contributions will be welcomed.

The bug tracker is a place for dealing with specific issues, not general approaches.  So please by all means open specific issues for specific suggestions :)
msg213733 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-03-16 15:24
Oh, and if you think there there are general issues to be discussed about approach (and I think our use of asserts might be one such), then the appropriate forum is the python-dev mailing list.
msg213734 - (view) Author: Jeffrey Walton (Jeffrey.Walton) * Date: 2014-03-16 15:28
On Sun, Mar 16, 2014 at 11:12 AM, R. David Murray
<report@bugs.python.org> wrote:
>
> R. David Murray added the comment:
>
> We already have Coverty scan in place, and were in fact featured by them
> for our code quality.  Currently Christian Heimes is the lead on that effort,
> and is monitoring the Coverty reports.
Oh, my bad. I searched for the project and got 0 results. Please
accept my apologies.

EDIT: my dislexia got me. I searched for "Pyhton", and not "Python".
Sorry about that.

> We've been working on Clang stuff as developers have had interest, and
> accepted a patch enabling address sanitizer to be used.
That's great.

Jeff
History
Date User Action Args
2014-03-16 15:28:11Jeffrey.Waltonsetmessages: + msg213734
2014-03-16 15:24:16r.david.murraysetmessages: - msg213732
2014-03-16 15:24:06r.david.murraysetmessages: + msg213733
2014-03-16 15:13:43r.david.murraysetmessages: + msg213732
2014-03-16 15:12:01r.david.murraysetstatus: open -> closed

nosy: + r.david.murray
messages: + msg213731

resolution: later
stage: resolved
2014-03-16 14:48:35Jeffrey.Waltoncreate