Dillemma with possible undefined behaviour in project and whether to add features

Question

I have such dilemma. Maybe someone will consider this opinion based, but I am sure this is relevant for many projects and also maybe someone will give answer which is not opinion based.

Imagine I was given large C++ project, which works correctly for many years (some may say this doesn't mean much if there is UB but come on, it means something).

Now, imagine two situations:

• there is either (undefined behaviour) UB in code but it still works as I mentioned
• there is no UB

Let's take first scenario.

Now if there is UB (and many instances of it) and project has been working fine I guess it is best to not touch project.

But what if I want to add feature to the project, what to do?

In such case:

• adding even new library to project increases risk of project to stop working well
• Recompiling with new compiler increases risk to make project stop working well

So, if there is UB, in the code, and it has been working fine, what to do if one wants to add new features to it? Without increasing risks that project will stop working correctly?

Is fixing all UB realistic in large C++ project by just code inspection? If no, we go to the point, when adding new features may break working project because it has been working well so far (please see two bullets points above).

Or maybe one tells the management because of such situation it is safest to not add anything to the project.

I hope I made my question clear.

I am pretty sure this is common situation, how do people proceed in such cases?

Answer 1

I know about this situation. Old codebase, needs new features, and needs them now (add that it runs on a 8051, and it has only some 31 bytes free of 64K if that's not already enough). The problem is that refactoring, building complete unit tests, whatnot is not an option due to the time constraints of the project - so either it is done in whatever manner it is doable, or not done

Of course it all starts by assessing the existing code, acquire documentation and understand it, what does what, what are the interfaces within the program. If it doesn't exist, then analyze the code until you reasonably figure out its modules, their roles and how they interface with each other, and do some documentation on it.

By your question you seem to worry about whether the compiler would produce "correct" result, now by "correct" meaning that it reproduces the same code which worked, and probably tested (at least an integration test) long ago, and which might still be working fine where it is deployed.

Look around in the codebase to see whether you still have the object files of a successful compile verified in some manner. If necessary, do some research (in the company where you work, probably the original developer's computer may still be sitting around somewhere with the artifacts of a correct verified compilation). Get those.

Then create your build script making sure it doesn't touch any of the existing object files unless requested to do so. This way you don't compile them, only link, thus avoid potential miscompilations coming from potential UB in the sources. Add some warning system to this build script which detect whether a source is changed compared to the originals, so you won't end up with an incorrect link.

With these preparations, you can set up a working environment where you can pull your changes off incrementally.

If you need to touch a source file, obviously you will need to assess it entirely, use some static analysis tool if available (and obviously compile with all warnings on, so you can see whatever the compiler itself might catch).

Cancelling the project

You have to consider this, too. I assumed the strict time limits which doesn't permit a "correct" solution. Of course you will have to assess risks, and if those are too high, the correct operation of the software is too critical, as a developer understanding the risks, you have the responsibility of properly explaining them to whoever requires the thing being done. It may be better for safety concerns or the reputation of the company, whatever, to not do the project at all, but the why's must be understood by those people who are in power of making the decision.

Answer 2

In my opinion, removing an UB should have no negative effects. You change your code from having "random effects" to having "well specified effects". Therefore you should remove it on sight or shortly after or at least comment it and leave for next iteration/sprint/etc to fix it.

Of course, that's an opinion, although a string one, still a guess. There is a small change that it's UB for Standard, but that your compiler has a certain defined behavior for it (even if undocumented, etc). In that case, it may happen that this code relies on that behavior or its sideeffects. Removing that UB and replacing it with valid code may remove that sideeffects and introduce obvious or subtle bugs.

So if your code works, I'd say, when you notice that UB and if it looks nontrivial, analyze it well and start writing tests around it to ensure that its effects are preserved after removing that UB nothing actually changes.

Answer 3

If worried, try applying a pattern like the adapter pattern described by GOF for instance... That is to say, add an intermediary layer between your new library calls and the old subsystems, making sure you can always use that as a filter to keep calling the old stuff in the right way, while still using new code as your primary interface.

Answer 4

When maintaining a large project where UB is detected, you have to consider each case of UB on individual basis. It might be harmless, it might be a severe bug.

For example, the standard will label something like left-shifting a negative integer UB, because the standard allows all manner of obscure signedness implementations. In practice though, you will have a two's completement CPU which on machine instruction level performs such a shift in a well-defined, deterministic, well-documented manner. There will be no crash and there will be no surprises. You could even write code which relied on the shift to give a certain outcome without too much guilty conscience.

The standard is similarly paranoid about a lot of things that are in practice always well-defined by the implementation: integer overflows, function pointer casts, (the lack of) trap representations etc etc. These are examples of UB that you don't need to worry about unless portability to exotic systems is a concern.
Though if you chose to rely on such "harmless" UB, you must document the behavior on the specific system, and explicitly write comments showing that you have considered the outcome, preferably with references to compiler documents.

But then of course, most cases of UB are always bugs and always need to be fixed before they cause havoc. For example a function returning a pointer to a local variable. Even if happens to work today, it is a ticking bomb. "It works today" might mean that there is a dormant, severe bug in the program, which you have yet to detect, because the bug only appears in a rare use-case.

Answer 5

You should realize that not all UB's are created equal. Although it is said that UB can result in nasal demons their consequences are often more predictable and benign than that. Often UB will only alter behavior if you change compiler/platform or the level of optimization.

This means that there's normally no reason to track down all UB and replace them before it becomes natural to do so. It's natural to get rid of UB when you maintain the part of the source where the UB resides. And when doing so you should of course have unit tests to make sure that the code under change does not alter it's behavior (in undesired ways).

And in maintaining you may encounter UB's that generates nasal demons from a distant galaxy. These should hopefully be few and would have to be addressed no matter if they seem related to what you're doing at the moment.