Add guideline to not invoke undefined behavior #2298
Conversation
At my CppCon 2022 talk [ Purging Undefined Behavior & Intel Assumptions in a Legacy C++ Codebase](https://www.youtube.com/watch?v=vEtGtphI3lc) I spoke about a guideline we added to our fork of the Core Guidelines to not invoke undefined behavior. I got a question after the talk: "why is this not part of the core guidelines?" Now that I am giving a new version of this talk at Meeting C++ I was inspired by this question to propose this as an upstream guideline. The current wording is our internal wording that might need some upgrades but first I wanted to know if there was interest in accepting this guideline before doing further refinement of the text.
|
|
||
| Static analyzer tools can also detect some undefined behavior, and we should consider using them | ||
| in our CI. Visual Studio's Analyze, Xcode's analyze, and clang-tidy all include some support | ||
| for this. |
There was a problem hiding this comment.
Question: How to you manage things like POSIX standard functions that have undefined behavior but must be used in the code for one reason or another?
Granted this might effect C code more than C++; but most code bases are probably more mixed between the two.
There was a problem hiding this comment.
Oh good question I hadn't thought about this. Can you provide some examples for me to think about? The only one that comes to mind off the top of my head is casting the void* from dlsym to a function pointer.
There was a problem hiding this comment.
@rothmichaels ones that easily come to mind are the POSIX printf family of functions since I've managed them across platforms before. For example, snprintf has some undefined behavior around the buffer provided, the output buffer, and the return values for being able to dynamically determine the size of the buffer needed to hold the output contents. Windows vs Linux vs Mac vs any Unix may operate differently on these methods. Granted, C++ advises using streams instead; but you there may still be requirements for projects to interface with these or similar and they're required to be available for POSIX compliant systems.
Some references, though they more reference C99 and C89 than POSIX; but the methods are POSIX required:
https://linux.die.net/man/3/snprintf
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/snprintf-snprintf-snprintf-l-snwprintf-snwprintf-l?view=msvc-170
For Microsoft, see snprintf vs _snprintf - both are POSIX compliant; one is C99 compliant which does help, but it comes down to what does your platform support since POSIX defines some things as UNDEFINED as a result of the history of UNIX and building a standard.
There was a problem hiding this comment.
Eh? I don't think the suggested guideline means "don't use functions which can ever have UB under some set of conditions". So it's not saying "never use snprintf".
It's saying don't use snprintf in such a way that invokes undefined behaviour.
So using snprintf is ok, using it incorrectly is not.
|
My initial reaction was that surely this goes without saying, does it really need to be a guideline? But after reading it, I think it does add value. It might be better to add some examples of the kind of thing that some developers might do, thinking it's ok to get away with it. For example, aliasing violations or signed integer overflow. The text is written in the first person but the rest of the guidelines use second person. |
4 participants
At my CppCon 2022 talk
Purging Undefined Behavior & Intel Assumptions in a Legacy C++ Codebase I spoke about a guideline we added to our fork of the Core Guidelines to not invoke undefined behavior. I got a question after the talk: "why is this not part of the core guidelines?"
Now that I am giving a new version of this talk at Meeting C++ I was inspired by this question to propose this as an upstream guideline.
The current wording is our internal wording that might need some upgrades but first I wanted to know if there was interest in accepting this guideline before doing further refinement of the text.