Bug 88800 - [8 Regression] Spurious -Werror=array-bounds for non-taken branch
Summary: [8 Regression] Spurious -Werror=array-bounds for non-taken branch
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 8.2.1
: P2 normal
Target Milestone: 9.0
Assignee: Martin Sebor
URL:
Keywords: diagnostic, patch
Depends on:
Blocks: Warray-bounds
  Show dependency treegraph
 
Reported: 2019-01-11 10:47 UTC by Tomasz Grabiec
Modified: 2021-05-14 11:16 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work: 9.0
Known to fail: 8.2.0
Last reconfirmed: 2019-01-11 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tomasz Grabiec 2019-01-11 10:47:00 UTC
Link to godbolt: https://godbolt.org/z/JXfV2y

gcc flags: -Wall -Werror

Compiles fine with gcc 7.4, errors on 8.2 due to -Warray-bounds.

===== 

#include <variant>


#include <stdint.h>
#include <assert.h>
#include <memory>
#include <unordered_map>
#include <type_traits>
#include <string_view>
#include <cstring>

using bytes_view = std::string_view;

struct blob_storage {
    struct [[gnu::packed]] ref_type {
        blob_storage* ptr;

        ref_type() {}
        ref_type(blob_storage* ptr) : ptr(ptr) {}
        operator blob_storage*() const { return ptr; }
        blob_storage* operator->() const { return ptr; }
        blob_storage& operator*() const { return *ptr; }
    };
    using size_type = uint32_t;
    using char_type = bytes_view::value_type;

    ref_type* backref;
    size_type size;
    size_type frag_size;
    ref_type next;
    char_type data[];

    blob_storage(ref_type* backref, size_type size, size_type frag_size) noexcept
        : backref(backref)
        , size(size)
        , frag_size(frag_size)
        , next(nullptr)
    {
        *backref = this;
    }

    blob_storage(blob_storage&& o) noexcept
        : backref(o.backref)
        , size(o.size)
        , frag_size(o.frag_size)
        , next(o.next)
    {
        *backref = this;
        o.next = nullptr;
        if (next) {
            next->backref = &next;
        }
        memcpy(data, o.data, frag_size);
    }
} __attribute__((packed));

class [[gnu::packed]] managed_bytes {
    static constexpr size_t max_inline_size = 15;
    struct small_blob {
        bytes_view::value_type data[max_inline_size];
    };
    union [[gnu::packed]] u {
        u() {}
        ~u() {}
        blob_storage::ref_type ptr;
        small_blob small;
    } _u;
    int8_t _size; // -1 -> use blob_storage
private:
    bool external() const {
        return _size < 0;
    }
public:
    using size_type = blob_storage::size_type;
    struct initialized_later {};

    managed_bytes(initialized_later, size_type size) {
        if (size <= max_inline_size) {
            _size = size;
        } else {
            _size = -1;
            auto now = size;
            void* p = malloc(sizeof(blob_storage) + now);
            new (p) blob_storage(&_u.ptr, size, now);
        }
    }

    managed_bytes(bytes_view v) : managed_bytes(initialized_later(), v.size()) {
        if (!external()) {
            std::copy(v.begin(), v.end(), _u.small.data);
          
  // ^^^^^^^^^^^^^^^ HERE ^^^^^^^^^^^^^^^^^^^

            return;
        }
        auto p = v.data();
        auto s = v.size();
        auto b = _u.ptr;
        while (s) {
            memcpy(b->data, p, b->frag_size);
            p += b->frag_size;
            s -= b->frag_size;
            b = b->next;
        }
        assert(!b);
    }
};

static_assert(sizeof(managed_bytes) == 16, "too large");

int main() {
    char c[16] = { 0, };
    bytes_view v(c, 16);

    managed_bytes b(v);
}


===== 

GCC output:

In static member function ‘static _Tp* std::__copy_move<_IsMove, true, std::random_access_iterator_tag>::__copy_m(const _Tp*, const _Tp*, _Tp*) [with _Tp = signed char; bool _IsMove = false]’,
    inlined from ‘_OI std::__copy_move_a(_II, _II, _OI) [with bool _IsMove = false; _II = const signed char*; _OI = signed char*]’ at /usr/include/c++/8/bits/stl_algobase.h:386:30,
    inlined from ‘_OI std::__copy_move_a2(_II, _II, _OI) [with bool _IsMove = false; _II = const signed char*; _OI = signed char*]’ at /usr/include/c++/8/bits/stl_algobase.h:422:45,
    inlined from ‘_OI std::copy(_II, _II, _OI) [with _II = const signed char*; _OI = signed char*]’ at /usr/include/c++/8/bits/stl_algobase.h:455:8,
    inlined from ‘managed_bytes::managed_bytes(bytes_view)’ at ./utils/managed_bytes.hh:195:22,
    inlined from ‘managed_bytes::managed_bytes(const bytes&)’ at ./utils/managed_bytes.hh:162:77,
    inlined from ‘dht::token dht::bytes_to_token(bytes)’ at dht/random_partitioner.cc:68:57,
    inlined from ‘dht::token dht::random_partitioner::get_token(bytes)’ at dht/random_partitioner.cc:85:39:
/usr/include/c++/8/bits/stl_algobase.h:368:23: error: ‘void* __builtin_memmove(void*, const void*, long unsigned int)’ offset 16 from the object at ‘<anonymous>’ is out of the bounds of referenced subobject ‘managed_bytes::small_blob::data’ with type ‘signed char [15]’ at offset 0 [-Werror=array-bounds]
      __builtin_memmove(__result, __first, sizeof(_Tp) * _Num);
      ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors

{code}

{code}
Comment 1 Tomasz Grabiec 2019-01-11 11:12:16 UTC
The gcc flags given in the description were incomplete, should be:

gcc flags: -Wall -Werror -std=c++17 -O2
Comment 2 Paweł Dziepak 2019-01-11 14:37:06 UTC
Smaller reproducer: https://godbolt.org/z/M1EFOv
Comment 3 Martin Sebor 2019-01-11 17:47:03 UTC
Confirmed.  The warning is issued by the restrict pass when the memmove call is being folded into MEM_REF, i.e.,

  __builtin_memmove (&b.data, &c, 16);

into:

  _15 = MEM[(char * {ref-all})&c];
  MEM[(char * {ref-all})&b] = _15;

and before b.size's initial negative value has been constant-propagated.  I don't know if teaching the restrict pass about MEM_REF rather than having the folder call into the pass would be viable given the unreliability of the MEM_REF argument.
Comment 4 Martin Sebor 2019-01-15 00:08:51 UTC
Patch: https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00810.html
Comment 5 Martin Sebor 2019-01-17 16:34:27 UTC
Author: msebor
Date: Thu Jan 17 16:33:55 2019
New Revision: 268037

URL: https://gcc.gnu.org/viewcvs?rev=268037&root=gcc&view=rev
Log:
PR tree-optimization/88800 - Spurious -Werror=array-bounds for non-taken branch

gcc/ChangeLog:

	PR tree-optimization/88800
	* gimple-fold.c (gimple_fold_builtin_memory_op): Avoid checking
	NO_WARNING bit here.  Avoid folding out-of-bounds calls.
	* gimple-ssa-warn-restrict.c (maybe_diag_offset_bounds): Remove
	redundant argument.  Add new argument and issue diagnostics under
	its control.  Detect out-of-bounds access even with warnings
	disabled.
	(check_bounds_or_overlap): Change return type.  Add argument.
	(wrestrict_dom_walker::check_call): Adjust.
	* gimple-ssa-warn-restrict.h (check_bounds_or_overlap): Add argument.
	* tree-ssa-strlen.c (handle_builtin_strcpy): Adjust to change in
	check_bounds_or_overlap's return value.
	(handle_builtin_stxncpy): Same.
	(handle_builtin_strcat): Same.

gcc/testsuite/ChangeLog:

	PR tree-optimization/88800
	* c-c++-common/Wrestrict.c: Adjust.
	* gcc.dg/Warray-bounds-37.c: New test.
	* gcc.dg/builtin-memcpy-2.c: New test.
	* gcc.dg/builtin-memcpy.c: New test.


Added:
    trunk/gcc/testsuite/gcc.dg/Warray-bounds-37.c
    trunk/gcc/testsuite/gcc.dg/builtin-memcpy-2.c
    trunk/gcc/testsuite/gcc.dg/builtin-memcpy.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-fold.c
    trunk/gcc/gimple-ssa-warn-restrict.c
    trunk/gcc/gimple-ssa-warn-restrict.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/Wrestrict.c
    trunk/gcc/tree-ssa-strlen.c
Comment 6 Martin Sebor 2019-01-17 16:35:25 UTC
Fixed via r268037 on the trunk (GCC 9).
Comment 7 Jakub Jelinek 2020-03-04 09:45:05 UTC
GCC 8.4.0 has been released, adjusting target milestone.
Comment 8 Jakub Jelinek 2021-05-14 11:16:17 UTC
The GCC 8 branch is being closed, fixed in GCC 9.1.