EASTL icon indicating copy to clipboard operation
EASTL copied to clipboard

eastl::fixed_vector<.., .., false> problem when using modern GCC

Open Vuhdo opened this issue 7 years ago • 4 comments

Hi,

We have some issue using eastl::fixed_vector that forbids heap allocations (i.e. has bEnableOverflow = false) when compiling code as simple as that:

eastl::fixed_vector<int, 32, false> v;
...
v.insert(v.begin(), 1);

This compiles fine when using MSVC (v141 toolset, both Windows PC and Xbox One), latest PS4 Clang and WSL Ubuntu's GCC 7.3.0) but fails missurably when I try to compile it on CentOS 7 using GCC 7.4:

../../EASTL/include/EASTL/internal/copy_help.h: In member function 'void eastl::vector<T, Allocator>::DoInsertValue(eastl::vector<T, Allocator>::const_iterator, Args&& ...) [with Args = {const int&}; T = int; Allocator = eastl::fixed_vector_allocator<4, 32, 4, 0, false, eastl::dummy_allocator>]':
../../EASTL/include/EASTL/internal/copy_help.h(116,22): error G67F0193A: argument 1 null where non-null expected [-Werror=nonnull]
       return (T*)memmove(result, first, (size_t)((uintptr_t)last - (uintptr_t)first)) + (last - first);
                  ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from /home/builduser/gnu/gcc/include/c++/7.4.0/cstring:42:0,
                    from ../../code/inc/Core/Core.h:63,
                    from ../../code/src/Game/Precompiled.h:9:
/usr/include/string.h:46:14: note: in a call to function 'void* memmove(void*, const void*, size_t)' declared here
    extern void *memmove (void *__dest, const void *__src, size_t __n)
                 ^~~~~~~

Note that GCC (with -O1) issues -Wnonnull warning (which we treat as error) . I believe it somehow manages to reveal that eastl::fixed_vector_allocator returns nullptr when fixed storage is exhausted even though we want to ignore that kind of checks as we disabled overflow on purpose.

The problem is somewhat subtle and is very hard to reproduce. After all, we observe that only when using CentOS + manually installed compiler toolchain (if I remember correctly, CentOS 7 comes with GCC 4.8 by default which is pretty old by any means). My collegue kind of managed to reproduce this behaviour in Compiler Explorer: https://godbolt.org/z/LF-WLc Here v.DoAllocate(1) is passed as an arguement to memmove() through the call to an intermediate function. Changing anything in the example will likely hide the problem, but there it is.

I'm not sure what to do with the error. For now I plan to disable -Wnonnull, but may be there's a better way?

Vuhdo avatar Jan 14 '19 11:01 Vuhdo

Yeah, this is a legitimate issue as the exhausted fixed_allocator can't allocate an overflow so it has to return nullptr. Are you planning on contributing a PR to fix this?

rparolin avatar Jan 22 '19 20:01 rparolin

Can you check if it is still an issue after merge of #280 ?

eugeneko avatar Feb 05 '20 17:02 eugeneko

I'm not sure if #354 is fully a dupe of this, but seems suspiciously related due to it being in fixed_tuple_vector and the same line... I can currently repro it on Release builds (only).

jwdevel avatar Mar 10 '20 07:03 jwdevel

I found that removing EASTL_UNLIKELY from the check added in #280 fixes the warning for me.

I am a little perplexed by this, since a cursory look at the definition of EASTL_UNLIKELY indicates that it should compile to the same thing in Debug vs. Release builds...

jwdevel avatar Mar 10 '20 07:03 jwdevel