2

I would like advice how to proceed in such situation. Imagine I have large C++ project which works well.

I have suspicion there might be some UB in this code (because in different project written by same author I found UB).

Now, say I need to add new features to this project. I am afraid because:

  • if I recompile with new compiler this can increase risk of UB happening if in the code is UB already. (e.g. new compiler might not be OK with UB which the old compiler was fine with).

Is it realistic to eliminate all UB in this large project by eye inspection (before I move to adding new feature)??

If not, then I should at least compile with same version of compiler right? (to decrease chance of problems if there is UB).

Project is done in Visual Studio so I don't know if there are object files, in which case, I could leave object files same and only modify parts in files where I need to add something - thus again minimizing risk of UB.

What is the course of action in such situation? I think this could be pretty common scenario.

I like suggestion that I test the project using new compiler before adding new code, but even then - we know testing might not reveal UB, isn't it?

  • 2
    Don't be affraid to run into bugs. It's better to find them than having them in the code without your knowledge. – SlySherZ Jan 29 '16 at 06:40
  • 4
    Almost any large project is bound to contain some UB, and when switching to a new compiler it is normal to expect some latent bugs to surface. I don't think there is any magic you can do to avoid it. Just write good tests and hire good testers. – StilesCrisis Jan 29 '16 at 06:49
  • static analyzers. I think VS has one built-in. Use the latest VS. – bolov Jan 29 '16 at 07:19
  • @bolov Using latest compiler is not a good advice IMO - because it can surface UB in code which old compiler was fine with –  Jan 29 '16 at 07:25
  • 1
    @user200312 As other answer said, latest compiler is a very good advice. Moreover, the latest VS has some very innovative features in the analyzer. – bolov Jan 29 '16 at 07:31
  • 1
    Compile with the new compiler and run some tests *before making any changes to the code.* If it works, you can proceed, assured that the new compiler will not cause disaster; if it goes haywire, *it wasn't your fault,* and you have proof that the old code has a bug. – Beta Jan 29 '16 at 07:35
  • @Beta That is not a bad idea, but you know that testing might not be able to reveal undefined behaviour right??? –  Jan 29 '16 at 07:40
  • 1
    @user200312 You know in the real world it's ok if you don't have a 100% bug free program, right? That's not because bugs are ok, but because you can never have a 100% bug free program. All you can do is your best to detect them. That's what testing and analyzers are for. You fix a bug when you detect it. There's no other way around it. Sometimes you detect a bug after you've shipped the product. That's what updates and bug fixes are for. – bolov Jan 29 '16 at 07:49
  • 1
    Yes, I'm aware of that, but what are you afraid of? If you want to avoid the slightest danger of UB, you must give up software engineering. If you want to continue to work on this project, you must someday compile your version of the code with the new compiler-- and you must do so without a guarantee from the gods that it is bug-free. You must pull triggers, but at least you can pull them in the right order. – Beta Jan 29 '16 at 08:18
  • @user200312, the fact that the "compiler is fine with it" does **not** mean that there is no bug in the code. It just means that the compiler did not find it. – user23573 Jan 29 '16 at 08:24
  • @Beta So maybe it is safer to find out which version of VS was used to create that project and stay with that compiler as I said? (because old compiler was fine with that UB) - because code worked fine –  Jan 29 '16 at 08:29
  • @user200312, **NO** au contraire! Get the latest compiler and see whether it still works fine. – user23573 Jan 29 '16 at 08:54
  • @BogdanWilli That doesn't make sense what you say, given new compiler might make a problem of UB which old compiler was fine with, and given testing program and if it works well doesn't mean there is no UB? –  Jan 29 '16 at 09:27
  • @user200312, You said: "because code worked fine" therefore I assume that you can check your object code. Repeat the checks with a new compiler and see if it works fine. Do you assume that UB is acceptable while a compiler that yells is bad? Keeping old object files seems to solve this problem, but the problem are silent bugs from UB. **You should always remove UB**. New compilers help you, but new compilers' objects may not be compatible with old ones. You may need to revert back the whole project to the old compiler. This cuts you off from new features that help you avoid UB. (like C++11) – user23573 Jan 29 '16 at 12:04
  • @BogdanWilli But also new compiler introduces UB which was ignored by old UB. So this seems to be a dilemma? (forget object files) –  Jan 29 '16 at 12:09
  • @user200312, lets move to the chat – user23573 Jan 29 '16 at 12:30
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/102001/discussion-between-bogdan-willi-and-user200312). – user23573 Jan 29 '16 at 12:31

3 Answers3

4

In order, I would:

  1. Compile with -Wall (/W4 for you Windows folk) and fix errors.
  2. Write tests if there aren't any already.
  3. Use tools like valgrind to detect issues and fix them.
  4. Study synchronization primitives if in use, and use modern paradigms where possible.
  5. Document the code and adhere to a style guide.

I would not attempt to avoid problems by keeping object files around. That's a nightmarish maintenance problem.

sholsapp
  • 15,542
  • 10
  • 50
  • 67
3

Undefined Behavior = Bugs

It's impossible to prove that a project is bug-free. Even the best programmers do create bugs. Even the best code-review cannot eliminate all bugs in a project. No, it's not realistic to eliminate all UB in a project of some size by code inspection or by any other means. Your best option is to review the code and eliminate as many as possible.

  • Change your perception of UB (bugs): If you encounter a bug during your re-engineering efforts, it's a good thing! You are in the best position to remove one UB.

  • Don't keep the old compiler just because you are afraid of UB. Recompile the project with the latest and best compiler available. Compilers can also have bugs. Newer compilers will produce better, more robust code. Newer compilers will produce better warnings. Use all warnings possible -Wall.

  • Eliminate all the warnings that the compiler produces. Every single warning is there for a reason, it highlights a problem. The likelihood of a "false positive" is quite dim nowadays. This is even true for MSVC (I'm not talking about real old compilers like before VC 2005)

  • Use a static code checker (Cppcheck). It can point you to common problems with the code.

  • Use a custom rule set for your code checker. It will help you to get the code up to some standard.

  • If possible, compile the project with another compiler (GCC, Clang) just for the sake of getting the warnings of these compilers.

  • Don't link against old object files. This will create more problems than what you think it avoids

Community
  • 1
  • 1
user23573
  • 2,479
  • 1
  • 17
  • 36
  • on VS it's `/W4` not `-Wall` – MikeMB Jan 29 '16 at 07:04
  • @MikeMB, Look [here](https://msdn.microsoft.com/en-us/library/thxezb7y.aspx) It **is** `-Wall` even on Visual Studio. And all options on Visual Studio can be prefixed with either `/` or `-` – user23573 Jan 29 '16 at 07:20
  • Yes, there is Wall, but on codebases, that where not compiled with it from the beginning it is pretty useless, as you are burried under a pile of false positives. it is literally activating any and all warning there are (even more aggressive than -Wextra on gcc/clang) – MikeMB Jan 29 '16 at 07:22
  • @MikeMB: Ignoring warnings is not a good practice. It is not possible to fix hundreds of them in one sitting, but now and then you can compile with Wall, fix one or two soft spots, then go back to the quieter options for the more urgent work. – Beta Jan 29 '16 at 07:32
  • @Beta: I havn't tried it in some time, but as you can see e.g. [here](http://stackoverflow.com/questions/4001736/whats-up-with-the-thousands-of-warnings-in-standard-headers-in-msvc-wall) in 2010 even the standard headerfiles would produce thousands of warnings. – MikeMB Jan 29 '16 at 07:34
  • @MikeMB, Don't call for "false positives" too early. In my experience, each warning pointed me to a real problem in my code. This is true even for MSVC. – user23573 Jan 29 '16 at 07:35
  • @Beta & Bogdan Willi: Have you actually worked with Wall on VS? This is (or was) not the same as `-Wextra` in gcc, which I'd certainly call helpfull. – MikeMB Jan 29 '16 at 07:39
  • @MikeMB: Damn... The link to 4292352 shows a workaround, but are MS products really (still) that bug-ridden? – Beta Jan 29 '16 at 07:40
  • @MikeMB, `-Wall` is my standard setting and I work daily with it. If I get warnings from some standard headers, I disable them **for these headers only**. I normally do **not** get warnings in my projects. – user23573 Jan 29 '16 at 08:20
  • "Just use all warnings" means that you accept the compiler writer's view of good coding style. In the case of Visual Studio 2015 I think this is bad advice. `bool result = DeleteFile(m_tempFile);` generates a warning (because DeleteFile returns a BOOL which is semantically identical to a bool, but actually an int and the compiler warns about converting int to bool.) – Martin Bonner supports Monica Jan 29 '16 at 08:29
  • @MartinBonner, This code actually violates type safety. The compiler correctly points to this issue: a `bool` is not the same thing as a `BOOL`. And because these are "semantically identical" the compiler let's you get away with only a warning. It is a matter of self discipline; if you generally accept code like this you will inevitably make bigger mistakes somewhere else. – user23573 Jan 29 '16 at 08:52
  • @BogdanWilli: We will just have to agree to disagree. (In particular, I don't agree the code violates type safety.) – Martin Bonner supports Monica Jan 29 '16 at 09:16
  • Bug is not same as undefined behaviour –  Jan 29 '16 at 09:28
  • 1
    @MartinBonner, Your word in God's ear: `bool` is a intrinsic C++ type. `BOOL` is a windows specific typedef to `int` (VS 2015). Looking at [this page](https://msdn.microsoft.com/en-us/library/s3f49ktz.aspx?f=255&MSPPError=-2147217396) reveals that `bool` has a size of 1 byte while `BOOL` has a size of 4. Don't be surprised if your assumption fails somewhere in the future. Look at [this article](https://social.msdn.microsoft.com/Forums/vstudio/en-US/1646b1fc-dae3-4818-a971-ba1f7e599eef/difference-between-bool-and-bool?forum=vcgeneral) – user23573 Jan 29 '16 at 09:57
  • What assumption? That `BOOL` of zero is false and non-zero is true? MS have a pretty good record on that sort of backwards compatibility. (Anyway, if I didn't disable the warning, I would silence it with `return = (DeleteFile(...) != 0);` which has the same assumption. – Martin Bonner supports Monica Jan 29 '16 at 10:01
  • @user200312, You might be correct in the linguistic sense. UB is a concept, introduced in the standards of C and C++ (and perhaps a couple of other languages), to help the compilers to better optimize. You, as a programmer, **should avoid UB** because your programs may start to "silently misbehave". [(read here)](http://blog.regehr.org/archives/213) So, in real life you should always treat UB as "bug". – user23573 Jan 29 '16 at 10:06
  • @BogdanWilli Still you didn't convince me why using a new compiler is better, please see my latest comment above to you (in comments under question) –  Jan 29 '16 at 10:42
  • @Beta: I guess it depends on what you call a bug. E.g. a warning about a function not being inlined might be helpfull when tracking down a particular performance issue, but I would not call it a bug. I also don't need to be warned that `catch(...)` doesn't catch structured exceptions, when I don't use them in my code anyway, although that might be interesting, when porting an old application. The number of warnings you get might however indicate that there is (to put it nicely) in deed room for improvement in the headerfiles. – MikeMB Jan 29 '16 at 11:53
2

As others said: First and foremost, try to find the errors, not hide them.

  1. The first and simplest measure is to set the warning level to /W4 (you can try Wall, but due to the large amount of noise this will produce (e.g. from standard headerfiles), it is usually only of help if you know you have an error in a certain part of your code)
  2. Use static analyzers - you can start with the builtin Code Analysis tool and then go for external tools (which are usually much more difficult to set up correctly for a non-trivial project).
  3. Write lots of tests and make sure, you are exercising edge cases - thats where UB usually lurks.
  4. If possible, try to compile the project (or parts of it) under clang and activate the different sanitizers (in particular there is UndefinedBehaviorSanitizer) which will further instrument your code to check for UB (only helpfull if you have tests to exercise that UB though)
  5. Test your code at different optimization levels and combination of flags (in VS, especially _ITERATOR_DEBUG_LEVEL can be helpfull to find out-of-bounds errors)

I'd say any non-trivial code base potentially contains undefined behavior. What is special about that particular Programmer? If he/she is prone to a special kind of UB, then you can focus your efforts on this.

MikeMB
  • 20,029
  • 9
  • 57
  • 102