[Bug middle-end/101134] New: Bogus -Wstringop-overflow warning about non-existent overflow

[dangelog at gmail dot com via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101134

            Bug ID: 101134
           Summary: Bogus -Wstringop-overflow warning about non-existent
                    overflow
           Product: gcc
           Version: 11.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dangelog at gmail dot com
  Target Milestone: ---

Hello,

This reduced testcase from Qt raises a -Wstring-overflow warning on GCC 11.1
when compiling under -O2 -g -Wall -Wextra:

    #include <stdlib.h>
    #include <string.h>

    struct QTestCharBuffer
    {
        enum { InitialSize = 512 };

        inline QTestCharBuffer() : buf(staticBuf)
        {
            staticBuf[0] = '\0';
        }

        QTestCharBuffer(const QTestCharBuffer &) = delete;
        QTestCharBuffer &operator=(const QTestCharBuffer &) = delete;

        inline ~QTestCharBuffer()
        {
            if (buf != staticBuf)
                free(buf);
        }

        inline char *data()
        {
            return buf;
        }

        inline int size() const
        {
            return _size;
        }

        inline bool reset(int newSize)
        {
            char *newBuf = nullptr;
            if (buf == staticBuf) {
                // if we point to our internal buffer, we need to malloc first
                newBuf = reinterpret_cast<char *>(malloc(newSize));
            } else {
                // if we already malloc'ed, just realloc
                newBuf = reinterpret_cast<char *>(realloc(buf, newSize));
            }

            // if the allocation went wrong (newBuf == 0), we leave the object
as is
            if (!newBuf)
                return false;

            _size = newSize;
            buf = newBuf;
            return true;
        }

    private:
        int _size = InitialSize;
        char* buf;
        char staticBuf[InitialSize];
    };


    typedef int (*StringFormatFunction)(QTestCharBuffer*,char const*,size_t);

    /*
        A wrapper for string functions written to work with a fixed size buffer
so they can be called
        with a dynamically allocated buffer.
    */
    int allocateStringFn(QTestCharBuffer* str, char const* src,
StringFormatFunction func)
    {
        static const int MAXSIZE = 1024*1024*2;

        int size = str->size();

        int res = 0;

        for (;;) {
            res = func(str, src, size);
            str->data()[size - 1] = '\0';
            if (res < size) {
                // We succeeded or fatally failed
                break;
            }
            // buffer wasn't big enough, try again
            size *= 2;
            if (size > MAXSIZE) {
                break;
            }
            if (!str->reset(size))
                break; // ran out of memory - bye
        }

        return res;
    }

    int xmlQuote(QTestCharBuffer* destBuf, char const* src, size_t n)
    {
        if (n == 0) return 0;

        char *dest = destBuf->data();
        *dest = 0;
        if (!src) return 0;

        char* begin = dest;
        char* end = dest + n;

        while (dest < end) {
            switch (*src) {

    #define MAP_ENTITY(chr, ent) \
                case chr:                                   \
                    if (dest + sizeof(ent) < end) {         \
                        strcpy(dest, ent);                  \
                        dest += sizeof(ent) - 1;            \
                    }                                       \
                    else {                                  \
                        *dest = 0;                          \
                        return (dest+sizeof(ent)-begin);    \
                    }                                       \
                    ++src;                                  \
                    break;

                MAP_ENTITY('>', "&gt;");
                MAP_ENTITY('<', "&lt;");
                MAP_ENTITY('\'', "&apos;");
                MAP_ENTITY('"', "&quot;");
                MAP_ENTITY('&', "&amp;");

                // not strictly necessary, but allows handling of comments
without
                // having to explicitly look for `--'
                MAP_ENTITY('-', "&#x002D;");

    #undef MAP_ENTITY

                case 0:
                    *dest = 0;
                    return (dest-begin);

                default:
                    *dest = *src;
                    ++dest;
                    ++src;
                    break;
            }
        }

        // If we get here, dest was completely filled (dest == end)
        *(dest-1) = 0;
        return (dest-begin);
    }

    int xmlQuote(QTestCharBuffer* str, char const* src)
    {
        return allocateStringFn(str, src, xmlQuote);
    }

    void enterTestFunction(const char *function)
    {
        QTestCharBuffer quotedFunction;
        xmlQuote(&quotedFunction, function);
    }


Godbolt link: https://gcc.godbolt.org/z/aPMdYjqEa

The warning is

    In function 'int allocateStringFn(QTestCharBuffer*, const char*,
StringFormatFunction)',
        inlined from 'int xmlQuote(QTestCharBuffer*, const char*)' at
<source>:150:28,
        inlined from 'void enterTestFunction(const char*)' at <source>:156:13:
    <source>:75:31: warning: writing 1 byte into a region of size 0
[-Wstringop-overflow=]
       75 |         str->data()[size - 1] = '\0';
          |         ~~~~~~~~~~~~~~~~~~~~~~^~~~~~
    <source>: In function 'void enterTestFunction(const char*)':
    <source>:55:10: note: at offset -1 into destination object
'QTestCharBuffer::staticBuf' of size 512
       55 |     char staticBuf[InitialSize];
          |          ^~~~~~~~~
    Compiler returned: 0



It's wrong because `size` can never be 0: the allocated object is visible to
GCC and has size == 512; size can only get multiplied by 2 and the result of
the multiplication is checked (so it can't overflow). Adding enough
__builtin_unreachable() for that condition removes the warnings, but it should
not be necessary.