- Schrdinger's Variable
- Empty Empty Strings
- Coding Conventions
Like this article? We recommend
Like this article? We recommend
Like this article? We recommend
Empty Empty Strings
I came across another C-language issue in some C++ code, when I encountered a strange bug in my Smalltalk compiler. A number of people were complaining that a program would compile when it contained an unreferenced constant string, but when you removed this string the program would crash.
I wasn't able to reproduce this bug, but I did find one in which assignments to variables as the result of arithmetic were sometimes becoming NULL rather than the correct result. Before explaining this issue, I need to sidetrack a little and talk about how arithmetic works in Smalltalk. Since Smalltalk is a pure object-oriented language, everything is an object, including integers. This is fudged slightly, however. Objects are structures containing a pointer to their class and any local state. Allocating a real object and going via the dynamic dispatch mechanism for integer arithmetic would take up a lot of space and be very slow, so there's a means of short-circuiting it. Since objects must be aligned on at least word boundaries, the lowest bit is always 0 in any object pointer. Setting this bit to 1 indicates that the object is a SmallInt—a special kind of object that stores its state in the other 31 or 63 bits.
When you send a + message to a SmallInt, a special lookup mechanism is used. In my implementation, SmallInt methods are implemented as C functions in an Objective-C file, which is compiled to LLVM bitcode with clang and then linked to the Smalltalk code. The LLVM inliner can then inline the implementations.
The issue, I discovered, was in cases where the arithmetic overflowed. In Smalltalk, arithmetic overflow is meant to result in a BigInt, a real object containing an arbitrary-precision integer, and this is exactly what I had implemented. My BigInt object was implemented in Objective-C and constructed in the SmallInt method by sending a message to the BigInt class.
I discovered that the constructor was not being called. I added a debugging line that called printf(), and I got a different result. Now it worked correctly. I added a static variable to which I assigned an intermediate value. This arrangement worked the first time, and then failed.
By this stage, I was quite confused. I added a breakpoint in the Objective-C runtime function for resolving the class name to a class pointer, and discovered that it was being called with "BigInt\129\205..." as the argument, not "BigInt", as I expected.
At this point, a light went on in my head, and I checked the bitcode that clang was generating. Sure enough, the "BigInt" string literal was missing the NULL terminator. Still a compiler bug, but since I also wrote the Objective-C code-generation stuff for the GNU runtime in clang, this one was still my fault.
Only it wasn't. Looking at the code, I saw that it was calling the correct function for emitting a NULL-terminated string. I went to the definition of this function, and here it is:
llvm::Constant *CodeGenModule::GetAddrOfConstantCString(const std::string &str,
const char *GlobalName){
return GetAddrOfConstantString(str + "\0", GlobalName);
}
GetAddrOfConstantString() emits a constant array of bytes, whereas GetAddrOfConstantCString() emits a constant C string—an array of bytes with a NULL terminator. At first glance, it takes a string argument, ensures that it's NULL-terminated, and then emits it as an array of bytes.
This is where I realized why I hate the C++ type system. A good type system reduces the number of valid-but-incorrect programs. In my experience, the C++ type system rejects programs that are correct and completely fails to spot most real errors.
Understanding this error requires you to understand that there are two kinds of string in C++. The first is C strings, which are NULL-terminated arrays of bytes. The second is C++ std::string objects. These are like Pascal strings—an array of bytes and a length—with some operations defined on them. Most of the time, you can pass a C string to something that expects a C++ string, and the compiler will do the correct conversion for you. This allows you to use the double-quote notation for constructing std::string objects—it's not really what you're doing, but you can pretend that it is.
In this case, the distinction is important. "\0" is a C string, being passed to the + operator method on a C++ string. What is "\0" really? It's a two-byte array, where both bytes are NULL. And what is a C string? It's a variable-sized array of bytes from some pointer to the first NULL byte. And herein lies the problem: The string literals "" and "\0" are semantically equivalent when treated as C strings.
This function was appending the empty string to the C++ string and then emitting the result. Appending the empty string is a no-op, so GetAddrOfConstantCString() did exactly the same thing as GetAddrOfConstantString().
Sometimes the string literals would be followed by a variable that happened to be 0. Sometimes they would be followed by nothing, so reading along the string would cause a crash. Sometimes they'd be followed by another string, or something else followed by a 0 byte, so they would just contain the wrong value.
Once this bug was identified, it was easy to fix. Replace the double quotes with single quotes, so you get this:
return GetAddrOfConstantString(str + '\0', GlobalName);
This now appends the 0 character to the string, which is what the original author wanted. Again, this was something that could have been prevented by a more defensive programming style. Style guidelines could have said that single-character strings should not be used. Operator overloading in C++ lets you implement methods that do the same thing on a single character as on a single-character string, and a character literal is always what you mean it to be, while a C string might not be.
More importantly, there's no good reason for emitting array constants that aren't NULL-terminated. If you add a NULL to the end of something that doesn't check beyond a fixed length, you just waste one byte of space in your data segment. If you write the extra byte, you can treat the array as NULL-terminated. The result? For a very low space cost, you get a result that doesn't fail for valid code and doesn't fail for some categories of invalid code.
Technically speaking, doing the right thing on incorrect input is a bug, but it's the kind of bug no one ever complains about, and this is a good rule for avoiding bugs. The general rule is to be strict in what you send and relaxed in what you receive. (Always generate data that's easy to use to do the right thing, but don't expect anyone else to follow that principle.)