Welcome to Software Development on Codidact!
Will you help us build our independent community of developers helping developers? We're small and trying to grow. We welcome questions about all aspects of software development, from design to code to QA and more. Got questions? Got answers? Got code you'd like someone to review? Please join us.
Comments on Can you ever assume that casting pointers is safe?
Post
Can you ever assume that casting pointers is safe?
This was inspired by the SO question Can you ever assume typecasting pointers is safe? I started to write an answer there, but I realized I'd rather post it here instead, so here's a self-answered Q&A.
Can you ever assume that casting pointers is safe?
Casting implies the use of the ( ) cast operator. As soon as it is used, C seems to allow all manner of wild and crazy casts and they pass compilation, but are those actually fine? How to know which ones that are and which ones that aren't?
Even when restricting oneself only to casts involving pointers, there are surely a lot of different scenarios here. As seen in the linked question, it is suggested that the size of the pointer and data types matter, which is rather obvious. Alignment might be less obvious, and on top of that C comes with a bunch of specialized rules such as exceptions for character type pointers, the "strict pointer aliasing" rules, function pointers and so on.
What is safe, what is unsafe? Is there an exhaustive list of all safe/dangerous scenarios? What does the standard specify and what does it leave as "poorly-defined behavior" such as undefined behavior?
I use macros to make casts less dangerous. They also improve greppability of casts (since they are known to be extremely dangerous, it's good to be able to find them all easily). Here are a few "casts" (narrow_cast() actually doesn't use any casts) I use:
#define typeas(T) typeof((T){})
#define const_cast(T, p) _Generic(p, const T: (T) (p), default: (p))
#define narrow_cast(T, e) \
({ \
_Pragma("GCC diagnostic push"); \
_Pragma("GCC diagnostic ignored \"-Wconversion\""); \
_Pragma("clang diagnostic ignored \"-Wimplicit-int-conversion\""); \
(typeas(T)){(e)}; \
_Pragma("GCC diagnostic pop"); \
})
alx Coming up with skunky macros is rarely ever the correct solution to any language flaw :) In fact you by doing so, you have instead created a completely broken macro in const_cast:
In C11 it was ambiguous how qualifiers were treated in _Generic, so compilers behaved differently. Some preserved qualifiers, some did not. There was a fix in C17 that removed the confusion - C17 6.5.1.1 "No two generic associations in the same generic selection shall specify compatible types. The type of the controlling expression is the type of the expression as if it had undergone an lvalue conversion". Lvalue conversion meaning that all qualifiers are dropped.
gcc just silently ignores this even with max diagnostics, but will always pick the non-qualified type. Whereas clang gives a proper diagnostic: https://godbolt.org/z/necT34P6G. "warning: due to lvalue conversion of the controlling expression, association of type 'const int' will never be selected because it is qualified"
So essentially the macro always just casts away const silently and a programmer using gcc might not even notice. And if p actually refers to a declared variable/effective type which is const-qualified, the cast could lead to code that explicitly invokes UB.
C23 6.7.4.1: "If an attempt is made to modify an object defined with a const-qualified type through use of an lvalue with non-const-qualified type, the behavior is undefined."
Which brings us back to my often perfectly sensible rule of thumb from the answer: never use the cast operator.
So essentially the macro always just casts away const silently and a programmer using gcc might not even notice. And if p actually refers to a declared variable/effective type which is const-qualified, the cast could lead to code that explicitly invokes UB.
Yes. A programmer should suspect that a macro called xxx_cast() performs a cast, though, but yes, it doesn't remove the dangers of casting const away; it just makes it more readable (and prevents casting volatile away).
Which brings us back to my often perfectly sensible rule of thumb from the answer: never use the cast operator.
Fully agree.
The only reason I have const_cast() is to deal with problems in the standard library (e.g., using strtol(3) with a read-only string). There's a very small number of cases where this is necessary, which is why I have a macro for that: I want to use 0 casts in a program, and if there's an exception to that rule of thumb, I want to be aware of every place where I've used a cast operator.
In fact you by doing so, you have instead created a completely broken macro in const_cast
The use of const_cast() is for dropping it from the pointee in pointer types. That is, for turning a const char * into a char * or a const char ** into a char ** (for use in strtol(3)).
The lvalue conversion only drops qualifiers from the top-level type, and not from pointees, so it does the right thing.
Here's for example one of the very few use cases for const_cast() in my code: ...
#define a2i(T, n, s, endp, base, min, max) \
({ \
T *n_ = n; \
QChar_of(s) **endp_ = endp; \
T min_ = min; \
T max_ = max; \
\
int status_; \
\
*n_ = _Generic((T){}, \
short: strtoi_, \
int: strtoi_, \
long: strtoi_, \
long long: strtoi_, \
unsigned short: strtou_noneg, \
unsigned int: strtou_noneg, \
unsigned long: strtou_noneg, \
unsigned long long: strtou_noneg \
)(s, const_cast(char **, endp_), base, min_, max_, &status_); \
\
if (status_ != 0) \
errno = status_; \
-!!status_; \
})
alx I can't really come up with a valid use-case for dropping const from a const int*, that need is fishy to begin with.
Previously broken versions of the C standard lib forced the implementer to do stuff like that when implementing functions like strstr, since the API was so poorly designed. But then there was never a requirement that the standard lib was written in C, let alone conforming C, and C23 fixed this old language defect anyway.
But if you insist on fishy macros maybe consider that for _Generic((p), ..., we can make a much safer check by not passing along the type, but instead do something like const typeof_unqual(*x)*:. If p isn't a pointer then it will not compile. Otherwise if p is exactly a const something* then it will be found. This rules out the problems with passing plain data types to the macro.
For strtol and friends specifically I think this is even fishier. What happens with restrict after all this casting mess is done with?
I can't really come up with a valid use-case for dropping const from a const int*, that need is fishy to begin with.
Previously broken versions of the C standard lib forced the implementer to do stuff like that when implementing functions like strstr, since the API was so poorly designed. But then there was never a requirement that the standard lib was written in C, let alone conforming C, and C23 fixed this old language defect anyway.
One case where I need to do this is for implementing C23-like string functions myself. For example, I have QChar *strprefix(QChar *s, const char *prefix);, which returns a pointer to the substring after the prefix if found (else, it returns a null pointer). I could implement it twice (const and non-const versions) and switch with _Generic(), which makes it difficult to maintain (and read). Or I could do it as
#define strprefix(s, pfx) const_cast(QChar_of(s) *, strprefix_(s, pfx))
But if you insist on fishy macros maybe consider that for _Generic((p), ..., we can make a much safer check by not passing along the type, but instead do something like const typeof_unqual(x):.
I can't use the typeof operators, because that wouldn't allow using the macro in different levels of const-ness. That is, I can make it work const char ** -> char ** or const char * -> char *, but not both.
However, I'm not very worried about passing a non-pointer to the macro, because it's a no-op. See the macro:
#define const_cast(T, p) _Generic(p, const T: (T) (p), default: (p))
If p is not a pointer, then lvalue conversion will discard const, and thus it won't match const T, and thus it will run the default: branch, which doesn't cast. If I use this incorrectly in a place where I needed to cast, since I haven't casted anything, I'll still get the problems. And if I used this needlessly, I will eventually realize and remove the unnecessary harmless code.
we can make a much safer check by not passing along the type
I think passing the type is a good thing: it improves readability (the explicitness of the code reminds me the type of the thing). Also, because the macro uses the cast operator only if the type matches, I don't foresee any unsafe situations.
This rules out the problems with passing plain data types to the macro.
What kind of problems do you expect? If you pass a wrong type, _Generic() will not match it, and will run default:, which doesn't do any casts.
For strtol and friends specifically I think this is even fishier. What happens with restrict after all this casting mess is done with?
restrict, while it is syntactically a qualifier, it behaves in reality like a function attribute. You can't cast its semantics away. This shouldn't change anything.
IMO, restrict should be removed from the language, and replaced by a function type attribute. I'm working towards a proposal for that.
alx Rather: function attributes should be removed from the language or get specified proper. They are a joke feature currently, not nearly enough thought had been put into them before they were introduced. Other similar experimental/underspecified/joke features like constexpr, new auto and nullptr/nullptr_t also have to be removed, fixed or specified proper, or nobody will ever switch to C23 in actual production. The committee needs to stop adding new useless/broken features and focus on removing useless/broken features instead. C23 is a fiasco currently, cleaning up that mess should be the only thing anyone is focusing on.
As for restrict, the "formal definition of restrict" is apparently containing defects ever since C99, but more importantly nobody sane actually understands/cares to understand what that text says. It needs to be removed. There are just so many use-cases of restrict and it should be easy enough to list them in plain English.
function attributes should be removed from the language or get specified proper.
Agree; I want to specify them properly. I've been considering function qualifiers for replacing some of those.
Other similar experimental/underspecified/joke features like constexpr, new auto and nullptr/nullptr_t also have to be removed, fixed or specified proper
Agree. WG14 is working on fixing auto at the moment. It might be possible to fix nullptr by removing nullptr_t (and thus making nullptr a void*). About constexpr, I can't do much; they're not going to remove it, even though I wish they would.
C23 is a fiasco currently
Agree. It has some good things, such as QChar, but it has several fiasco inventions. To be fair, so was C89, with its broken m/realloc(3) which could return NULL on success; see https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3752.txt. C99 brought <tgmath.h>. C11 brought Annex K. The C Committee inventing broken stuff isn't something new.
alx Yeah well on the positive side, ditching exotic signedness formats and finally fixing enums are milestones. Fixing qualifiers with Qchar/Qvoid, finally dropping K&R style functions, digit separators, memccpy, #warning, typeof and so on. There's a whole lot of good stuff in C23 that I definitely want to use. Anyways this is getting wildly off-topic, like usual :)
Actually, I think memccpy(3) might have been one of the worst mistakes of C23. I've written in private mail discussions about it but never publicly. I'll open a Q&A here. Edit: Even better, as the maintainer of the manual pages, I'll add a CAVEATS section in the memccpy(3) manual page. :)
strncpy(3) wasn't a C89 thing; it was a Seventh Edition Unix thing. And it isn't a bad function; it has just been misused. It is an excellent function for handling fixed-width buffers such as those found in tar(1) or in utmp(5). Here's my code that uses strncpy(3): https://github.com/shadow-maint/shadow/blob/f384b3dfcba36b784ac74e37918b8d47f6d676ef/lib/utmp.c#L286. When it was added in V7 Unix, it was used precisely for this use case.
...
...
The problem comes from programmers that thought this could be used as a function for copying with truncation; that's not what this function is for. Instead of writing their own function for copying with truncation (https://github.com/shadow-maint/shadow/blob/master/lib/string/strcpy/strtcpy.h), programmers misuse this function. That's negligence, IMO, combined with many decades of bad teaching, including bogus "secure" coding guidelines such as SEI CERT https://cmu-sei.github.io/secure-coding-standards/sei-cert-c-coding-standard/rules/characters-and-strings-str/str32-c/#compliant-solution-truncation. Admittedly, the name being of the form str*() doesn't help; this unfortunate name is because back in the times of V7 Unix, the concept of a string was less clear --they considered mem*() to also be string functions, because a byte string was what we now call a byte array; in fact, they called it bcopy(3) instead of memcpy(3)--.
...
...
I suggest considering all strn*() functions as handling nonstrings (see GNU C's [[gnu::nonstring]] attribute https://gcc.gnu.org/onlinedocs/gcc/Common-Attributes.html#index-nonstring). That's actually what they are; all strn*() functions handle nonstrings in one way or another (substrings, pointer+size, and fixed-width buffers are all nonstrings).
About removing strncpy(3) from the standard, that's not possible, because there are legitimate users, including myself. Admittedly, I could live without it, because I'd implement it myself in my projects that use it, though, but I prefer if it stays as part of libc.
So far, I've talked about why strncpy(3) isn't as bad as it seems. But now I'll go into why memccpy(3) is terrible.
It was added as a function for copying a string with truncation. The name of the paper was "Toward more efficient string copying and concatenation"; this already hints that safety was not the goal of the paper. First, let's analyze the efficiency claims:
Because memccpy(3) is a function with a very weird interface, it's almost never been used. Before C23, you could (really!) count the existing uses with the fingers in your hands. As a consequence, implementations haven't made any efforts to optimize it. It is possibly one of the slowest string functions that exists in common libc implementations. Admittedly, implementations could start optimizing it now, but I doubt that'll happen.
Now, about the safety and ergonomics of the function, which is where it is really terrible:
...
...
n2349 https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2349.htm suggests that strcat(strcpy(d, s1), s2) could be written as memccpy(memccpy(d, s1, '\0', SIZE_MAX) - 1, s2, '\0', SIZE_MAX). This is terribly negligent code. That code is prone to off-by-one bugs (anything that requires writing ... - 1 is prone to off-by-one), and many other bugs, just by being completely unreadable. If we were to use a faster replacement of that code, we should look at the POSIX function stpcpy(3). It allows writing it as stpcpy(stpcpy(d, s1), s2). It is even more efficient, because stpcpy(3) is a well-optimized function, and memccpy(3) can't be optimized more than stpcpy(3) because the main difference is that stpcpy(3) has a hard-coded delimiter, '\0', it doesn't need to check the size limit, and it can't return NULL.
...
...
But now about the flagship use case: copying with truncation. It shows that one should do this:
char *p = memccpy (d, s1, '\0', dsize);
dsize -= (p - d - 1);
memccpy (p - 1, s2, '\0', dsize);
This is more prone to off-by-one bugs than the case above, and more than strncpy(3). Anyone suggesting to use this to improve safety compared to strncpy(3), please explain to me how they think this is safer in any way.
In fact, that code is completely bogus, because if the string is truncated, p will be NULL, and it invokes UB in line 2. See how it was predictably prone to bugs? :)
...
...
At the bottom of the paper n2349 there's a more correct example of how memccpy(3) could be used to copy strings. This shows how terrible this function is.
char *p = memccpy (d, s1, '\0', dsize);
if (p) {
--p;
p = memccpy (p, "/", '\0', dsize - (p - d));
if (p) {
--p;
p = memccpy (p, s2, '\0', dsize - (p - d));
}
}
if (!p)
d[dsize - 1] = '\0';
I don't think I need to explain what can go wrong in such unreadable, brittle, and complex code.
I'll show here how it would be written using a better function, similar to POSIX's stpcpy(3):
char *p = d;
char *e = d+dsize;
p = stpecpy(p, e, s1);
p = stpecpy(p, e, "/");
p = stpecpy(p, e, s2);
if (p == NULL)
goto trunc; // The string was truncated
Here's my implementation of stpecpy(): https://github.com/shadow-maint/shadow/blob/f384b3dfcba36b784ac74e37918b8d47f6d676ef/lib/string/strcpy/stpecpy.h#L28. Plan9 has this function too, and they call it strecpy(2), although it has an implementation bug.
If one prefers the simplicity of strcpy(3)/strcat(3), one can also have such functions. They are less efficient, but they can be safer, by being even simpler to use in some cases. I have truncating variants of these functions, which I call strtcpy()/strtcat(). They'd be used as:
if (strtcpy(d, s1, dsize) == -1)
goto trunc;
if (strtcat(d, "/", dsize) == -1)
goto trunc;
if (strtcat(d, s2, dsize) == -1)
goto trunc;
You need to check for truncation after every call, though, so there's a trade-off. Here's an implementation of strtcpy(): https://github.com/shadow-maint/shadow/blob/f384b3dfcba36b784ac74e37918b8d47f6d676ef/lib/string/strcpy/strtcpy.h#L32. (You may notice it's essentially the same as the Linux kernel's strscpy().) And here's strtcat(): https://github.com/shadow-maint/shadow/pull/1334.
This community is part of the non-profit Codidact network. We have other communities too — take a look!
You can also join us in chat!
Want to advertise this community? Use our templates!
Like what we're doing? Support us! Donate

1 comment thread