On 7/8/21 12:07 PM, Jonny Grant wrote:
Thank you for your reply.
We can't guarantee safestrlen() won't be called with NULL. So because strlen() itself doesn't check for NULL in C standard we'd need to call the wrapper so that NULL can be checked for.
I'd like to avoid the compiler removing certain execution paths.
I'd rather keep all code paths, even if they are not taken, just in case a NULL pointer creeps in due to an external device that is connected to an embedded system.
Probably this would work:
size_t __attribute__((optimize("O0"))) safestrlen(const char * s)
{
if (NULL == s) return 0;
else return strlen(s);
}
I don't think you don't need that. Unless there's a bug in GCC, it
shouldn't optimize that path unless it is 100% sure that it will never
be called.
Moreover, I recommend you to optimize as much as possible.
Even though NULL is possible in your code, I guess it's unlikely.
Also, calling a function safe is too generic.
I'd call it with the suffix null, as it act different on null.
Also, I recommend avoiding 'size_t' (and any other unsigned types, BTW).
See <https://google.github.io/styleguide/cppguide.html#Integer_Types>.
Use the POSIX type 'ssize_t'.
That also allows differentiating a length of 0 (i.e., "") from an
invalid string (i.e., NULL), by returning -1 for NULL.
Recommended implementation (requires C99 or later due to the usage of
inline):
[
// compiler.h
#define likely(x) __builtin_expect((x), 1)
#define unlikely(x) __builtin_expect((x), 0)
]
[
// strlennull.h
#include <string.h>
#include <sys/types.h>
#include "compiler.h"
[[gnu::pure]]
inline ssize_t strlennull(const char *s);
inline ssize_t strlennull(const char *s)
{
if (unlikely(!s))
return -1;
return strlen(s);
}
]
[
// strlennull.c
#include "strlennull.h"
#include <sys/types.h>
extern inline ssize_t strlennull(const char *s);
]
BTW, if the input is so untrusted that NULL may be possible, I guess it
might as well contain invalid strings (non-NUL-terminated), for which
strlen(3) would behave as bad or worse. So I recommend having a look at
strnlen(3) and maybe implement strnlennull() instead of strlennull().
Unless you _know_ there's a compiler bug that doesn't allow you to
optimize, please optimize. Otherwise, if it's just a bit of paranoia,
you could as well not optimize any code at all. Specifically not
optimizing this code by the use of pragmas or attributes is *wrong*.
Just revise the preprocessor output, the compiler output, and also try
introducing a NULL at run time, to check that everything's fine.
I also use 'volatile' for reads/writes to addresses that I don't want to be optimized out.
Are you sure you don't want to optimize? Are those addresses hardware
I/O or interrupts?
Maybe you just want membarrier(2).
What I recommend you to do from time to time, to make sure you don't miss any warnings, is compile the whole project first with '-O3' and then with '-O0'. If you are a bit paranoic, sporadically you can try all of them : '-Og', '-O0', '-O1', '-Os', '-O2', '-O3', '-Ofast' but I don't think that is necessary. Of course, always use '-fanalyzer' (GCC 10 and above).
Yes, I am looking forward to David Malcom's -fanalyzer when Ubuntu LTS next upgrades, I'm on gcc 9.3 today. But -fanalyzer is only for C anyway.. much of of code base I work with is compiled as C++ so I can't use -fanalyzer yet.
You can install gcc-10 for Bionic: apt-get install gcc-10
I recommend it. It finds bugs very deep in the code.
Cheers,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
