GCC warning

neclepsio commented

2022-05-24 10:19:21 +00:00

(Migrated from github.com)

Version/Branch of Dear ImGui:

Version: 1.87
Branch: master

Back-end/Renderer/Compiler/OS

Compiler: gcc version 12.1.0 (Rev1, Built by MSYS2 project)
Operating System: Windows

My Issue/Question:

Compiling gives the following warning (with target Windows 64 bit):

imgui_draw.cpp: In function 'void ImFontAtlasBuildPackCustomRects(ImFontAtlas*, void*)':
imgui_draw.cpp:2630:11: warning: 'void* memset(void*, int, size_t)' specified size between 18446744071562067968 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
 2630 |     memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes());
      |     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

**Version/Branch of Dear ImGui:** Version: 1.87 Branch: master **Back-end/Renderer/Compiler/OS** Compiler: gcc version 12.1.0 (Rev1, Built by MSYS2 project) Operating System: Windows **My Issue/Question:** Compiling gives the following warning (with target Windows 64 bit): imgui_draw.cpp: In function 'void ImFontAtlasBuildPackCustomRects(ImFontAtlas*, void*)': imgui_draw.cpp:2630:11: warning: 'void* memset(void*, int, size_t)' specified size between 18446744071562067968 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=] 2630 | memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes()); | ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

ocornut commented

2022-05-24 10:28:58 +00:00

(Migrated from github.com)

Hello, thanks for reporting.

I am confused how that's possible.
Can you look at the code and investigate ?
Do you have an unmodified version of dear imgui ? What is the exact command line leading to compile imgui_draw.cpp ?

Hello, thanks for reporting. I am confused how that's possible. Can you look at the code and investigate ? Do you have an unmodified version of dear imgui ? What is the exact command line leading to compile imgui_draw.cpp ?

neclepsio commented

2022-05-25 15:45:40 +00:00

(Migrated from github.com)

I'm confused too. gcc -O1 -c imgui_draw.cpp on unmodified sources is enough to trigger the warning. Without -O1 there is no warning.

I'm confused too. `gcc -O1 -c imgui_draw.cpp` on unmodified sources is enough to trigger the warning. Without `-O1` there is no warning.

ocornut commented

2022-05-31 15:04:18 +00:00

(Migrated from github.com)

I'm struggling to understand why it does it on:

    ImVector<stbrp_rect> pack_rects;
    pack_rects.resize(user_rects.Size);
    memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes());

And not

    ImVector<ImFontBuildSrcData> src_tmp_array;
    ImVector<ImFontBuildDstData> dst_tmp_array;
    src_tmp_array.resize(atlas->ConfigData.Size);
    dst_tmp_array.resize(atlas->Fonts.Size);
    memset(src_tmp_array.Data, 0, (size_t)src_tmp_array.size_in_bytes());
    memset(dst_tmp_array.Data, 0, (size_t)dst_tmp_array.size_in_bytes());

    ImVector<stbrp_rect> buf_rects;
    ImVector<stbtt_packedchar> buf_packedchars;
    buf_rects.resize(total_glyphs_count);
    buf_packedchars.resize(total_glyphs_count);
    memset(buf_rects.Data, 0, (size_t)buf_rects.size_in_bytes());
    memset(buf_packedchars.Data, 0, (size_t)buf_packedchars.size_in_bytes());

Which are located earlier the same file.

I'm struggling to understand why it does it on: ``` ImVector<stbrp_rect> pack_rects; pack_rects.resize(user_rects.Size); memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes()); ``` And not ``` ImVector<ImFontBuildSrcData> src_tmp_array; ImVector<ImFontBuildDstData> dst_tmp_array; src_tmp_array.resize(atlas->ConfigData.Size); dst_tmp_array.resize(atlas->Fonts.Size); memset(src_tmp_array.Data, 0, (size_t)src_tmp_array.size_in_bytes()); memset(dst_tmp_array.Data, 0, (size_t)dst_tmp_array.size_in_bytes()); ``` ``` ImVector<stbrp_rect> buf_rects; ImVector<stbtt_packedchar> buf_packedchars; buf_rects.resize(total_glyphs_count); buf_packedchars.resize(total_glyphs_count); memset(buf_rects.Data, 0, (size_t)buf_rects.size_in_bytes()); memset(buf_packedchars.Data, 0, (size_t)buf_packedchars.size_in_bytes()); ``` Which are located earlier the same file.

ocornut commented

2022-05-31 15:14:57 +00:00

(Migrated from github.com)

I am tempted to say this might be a bug in GCC..

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

They seem to be collecting reports for that... If you can report on GCC website it may be helpful.

I am tempted to say this might be a bug in GCC.. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88443 They seem to be collecting reports for that... If you can report on GCC website it may be helpful.

ocornut commented

2022-06-13 09:40:23 +00:00

(Migrated from github.com)

@neclepsio Does changing it to

memset((void*)pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes());

Fixes the warning?

@neclepsio Does changing it to ```cpp memset((void*)pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes()); ``` Fixes the warning?

neclepsio commented

2022-06-13 13:38:24 +00:00

(Migrated from github.com)

No, it doesn't.

👍 1 😕 1

cfillion commented

2023-04-27 15:50:03 +00:00

(Migrated from github.com)

I got this warning with GCC 12.2.1. It was triggered by my IM_ASSERT being defined to call a function that did if(!expr) throw + the compiler deciding to not inline it(?).

It looks like GCC missed the hint from the IM_ASSERT(user_rects.Size >= 1) and thought there's a valid path where the signed size can be negative.

https://godbolt.org/z/ETcsb3n7P

The warning went away after changing my IM_ASSERT to be (_EXPR ? (void)0 : UnconditionallyThrow(#_EXPR)). (And it comes back if I remove that assert in ImFontAtlasBuildPackCustomRects.)

EDIT: Doesn't happen here with gcc -O1 -c imgui_draw.cpp and stock imconfig.h – unless that assertion is commented out.

I got this warning with GCC 12.2.1. It was triggered by my `IM_ASSERT` being defined to call a function that did `if(!expr) throw` + the compiler deciding to not inline it(?). It looks like GCC missed the hint from the `IM_ASSERT(user_rects.Size >= 1)` and thought there's a valid path where the signed size can be negative. https://godbolt.org/z/ETcsb3n7P The warning went away after changing my `IM_ASSERT` to be `(_EXPR ? (void)0 : UnconditionallyThrow(#_EXPR))`. (And it comes back if I remove that assert in `ImFontAtlasBuildPackCustomRects`.) EDIT: Doesn't happen here with `gcc -O1 -c imgui_draw.cpp` and stock imconfig.h – unless that assertion is commented out.

ocornut commented

2023-04-27 16:27:52 +00:00

(Migrated from github.com)

Thanks for posting.
Do you have a suggestion of what could be done on our side?

Thanks for posting. Do you have a suggestion of what could be done on our side?

cfillion commented

2023-04-27 16:47:33 +00:00

(Migrated from github.com)

General fix would probably be using an unsigned type for quantities everywhere. A more reasonable change, to workaround that one specific case (unsure if GCC bug?), could be:

diff --git a/imgui_draw.cpp b/imgui_draw.cpp
index 6082f10..5e74943 100644
--- a/imgui_draw.cpp
+++ b/imgui_draw.cpp
@@ -2630,6 +2630,11 @@ void ImFontAtlasBuildPackCustomRects(ImFontAtlas* atlas, void* stbrp_context_opa
     ImVector<ImFontAtlasCustomRect>& user_rects = atlas->CustomRects;
     IM_ASSERT(user_rects.Size >= 1); // We expect at least the default custom rects to be registered, else something went wrong.
 
+#ifdef __GNUC__
+    if(user_rects.Size < 1)
+        __builtin_unreachable();
+#endif
+
     ImVector<stbrp_rect> pack_rects;
     pack_rects.resize(user_rects.Size);
     memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes());

General fix would probably be using an unsigned type for quantities everywhere. A more reasonable change, to workaround that one specific case (unsure if GCC bug?), could be: ```diff diff --git a/imgui_draw.cpp b/imgui_draw.cpp index 6082f10..5e74943 100644 --- a/imgui_draw.cpp +++ b/imgui_draw.cpp @@ -2630,6 +2630,11 @@ void ImFontAtlasBuildPackCustomRects(ImFontAtlas* atlas, void* stbrp_context_opa ImVector<ImFontAtlasCustomRect>& user_rects = atlas->CustomRects; IM_ASSERT(user_rects.Size >= 1); // We expect at least the default custom rects to be registered, else something went wrong. +#ifdef __GNUC__ + if(user_rects.Size < 1) + __builtin_unreachable(); +#endif + ImVector<stbrp_rect> pack_rects; pack_rects.resize(user_rects.Size); memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes()); ```

ocornut commented

2023-04-27 17:11:55 +00:00

(Migrated from github.com)

I would push this fix but could you provide a repro in the form of an #define IM_ASSERT xxx and gcc compilation line that triggers the issue for you?

I would push this fix but could you provide a repro in the form of an `#define IM_ASSERT xxx` and gcc compilation line that triggers the issue for you?

cfillion commented

2023-04-27 20:48:51 +00:00

(Migrated from github.com)

Must be building with -O3
IMGUI_ENABLE_FREETYPE is defined (or the call to stbrp_pack_rects in ImFontAtlasBuildWIthStbTruetype is commented out)
Conditionally does throw SomethingThatCanThrow(); (eg. throw std::runtime_error { msg }, throw new int;, throw std::string { msg };)

It's weird... (If the assertion function always returns or #define IM_ASSERT(_EXPR) (void)0 then it can be duplicated with only -O1 + IMGUI_ENABLE_FREETYPE or same commented stbrp_pack_rects)

diff --git a/imconfig.h b/imconfig.h
index 876cf32..28a886e 100644
--- a/imconfig.h
+++ b/imconfig.h
@@ -19,6 +19,22 @@
 //#define IM_ASSERT(_EXPR)  MyAssert(_EXPR)
 //#define IM_ASSERT(_EXPR)  ((void)(_EXPR))     // Disable asserts
 
+#ifdef TRIGGER_BUG_5343
+#  define IM_ASSERT(_EXPR) NonHintingAssert(_EXPR)
+extern int runtime_value;
+int MayThrow()
+{
+  if(runtime_value)
+    return 4;
+  throw 2;
+}
+void NonHintingAssert(bool ok)
+{
+  if(!ok)
+    throw MayThrow(); // any function or constructor the may also conditionally throw
+}
+#endif
+
 //---- Define attributes of all API symbols declarations, e.g. for DLL under Windows
 // Using Dear ImGui via a shared library is not recommended, because of function call overhead and because we don't guarantee backward nor forward ABI compatibility.
 // DLL users: heaps and globals are not shared across DLL boundaries! You will need to call SetCurrentContext() + SetAllocatorFunctions()
diff --git a/imgui_draw.cpp b/imgui_draw.cpp
index 6082f10..140e571 100644
--- a/imgui_draw.cpp
+++ b/imgui_draw.cpp
@@ -2512,7 +2512,9 @@ static bool ImFontAtlasBuildWithStbTruetype(ImFontAtlas* atlas)
         if (src_tmp.GlyphsCount == 0)
             continue;
 
+#ifndef NO_PACK_RECTS
         stbrp_pack_rects((stbrp_context*)spc.pack_info, src_tmp.Rects, src_tmp.GlyphsCount);
+#endif
 
         // Extend texture height and mark missing glyphs as non-packed so we won't render them.
         // FIXME: We are not handling packing failure here (would happen if we got off TEX_HEIGHT_MAX or if a single if larger than TexWidth?)
@@ -2630,6 +2632,11 @@ void ImFontAtlasBuildPackCustomRects(ImFontAtlas* atlas, void* stbrp_context_opa
     ImVector<ImFontAtlasCustomRect>& user_rects = atlas->CustomRects;
     IM_ASSERT(user_rects.Size >= 1); // We expect at least the default custom rects to be registered, else something went wrong.
 
+#ifdef WORKAROUND_BUG_5343
+    if (user_rects.Size < 1)
+        __builtin_unreachable();
+#endif
+
     ImVector<stbrp_rect> pack_rects;
     pack_rects.resize(user_rects.Size);
     memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes());

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/12.2.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc/src/gcc/configure --enable-languages=c,c++,ada,fortran,go,lto,objc,obj-c++,d --enable-bootstrap --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --with-build-config=bootstrap-lto --with-linker-hash-style=gnu --with-system-zlib --enable-__cxa_atexit --enable-cet=auto --enable-checking=release --enable-clocale=gnu --enable-default-pie --enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object --enable-libstdcxx-backtrace --enable-link-serialization=1 --enable-linker-build-id --enable-lto --enable-multilib --enable-plugin --enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch --disable-werror
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.2.1 20230201 (GCC)

$ gcc -O3 -I. -DIMGUI_ENABLE_FREETYPE -c imgui_draw.cpp
# (no output)

$ gcc -O3 -I. -DIMGUI_ENABLE_FREETYPE -c imgui_draw.cpp -DTRIGGER_BUG_5343
imgui_draw.cpp: In function ‘void ImFontAtlasBuildPackCustomRects(ImFontAtlas*, void*)’:
imgui_draw.cpp:2642:11: warning: ‘void* memset(void*, int, size_t)’ specified size between 18446744071562067968 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
 2642 |     memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes());
      |     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

$ gcc -O3 -I. -DIMGUI_ENABLE_FREETYPE -c imgui_draw.cpp -DTRIGGER_BUG_5343 -DWORKAROUND_BUG_5343
# (no output)

$ gcc -O3 -DNO_PACK_RECTS -c imgui_draw.cpp
# (no output)

$ gcc -O3 -DNO_PACK_RECTS -c imgui_draw.cpp -DTRIGGER_BUG_5343
imgui_draw.cpp: In function ‘void ImFontAtlasBuildPackCustomRects(ImFontAtlas*, void*)’:
imgui_draw.cpp:2642:11: warning: ‘void* memset(void*, int, size_t)’ specified size between 18446744071562067968 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
 2642 |     memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes());
      |     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

$ gcc -O3 -DNO_PACK_RECTS -c imgui_draw.cpp -DTRIGGER_BUG_5343 -DWORKAROUND_BUG_5343
# (no output)

EDIT: Tried to narrow it down further by removing parts of imgui_draw.cpp until the warning went away (hoping to find a minimal snippet that could be reported). At some point adding or removing one of StyleColorsDark's lines determined whether that warning shows up!

- Must be building with `-O3` - `IMGUI_ENABLE_FREETYPE` is defined (or the call to `stbrp_pack_rects` in `ImFontAtlasBuildWIthStbTruetype` is commented out) - Conditionally does `throw SomethingThatCanThrow();` (eg. `throw std::runtime_error { msg }`, `throw new int;`, `throw std::string { msg };`) It's weird... (If the assertion function always returns or `#define IM_ASSERT(_EXPR) (void)0` then it can be duplicated with only `-O1` + `IMGUI_ENABLE_FREETYPE` or same commented `stbrp_pack_rects`) ```diff diff --git a/imconfig.h b/imconfig.h index 876cf32..28a886e 100644 --- a/imconfig.h +++ b/imconfig.h @@ -19,6 +19,22 @@ //#define IM_ASSERT(_EXPR) MyAssert(_EXPR) //#define IM_ASSERT(_EXPR) ((void)(_EXPR)) // Disable asserts +#ifdef TRIGGER_BUG_5343 +# define IM_ASSERT(_EXPR) NonHintingAssert(_EXPR) +extern int runtime_value; +int MayThrow() +{ + if(runtime_value) + return 4; + throw 2; +} +void NonHintingAssert(bool ok) +{ + if(!ok) + throw MayThrow(); // any function or constructor the may also conditionally throw +} +#endif + //---- Define attributes of all API symbols declarations, e.g. for DLL under Windows // Using Dear ImGui via a shared library is not recommended, because of function call overhead and because we don't guarantee backward nor forward ABI compatibility. // DLL users: heaps and globals are not shared across DLL boundaries! You will need to call SetCurrentContext() + SetAllocatorFunctions() diff --git a/imgui_draw.cpp b/imgui_draw.cpp index 6082f10..140e571 100644 --- a/imgui_draw.cpp +++ b/imgui_draw.cpp @@ -2512,7 +2512,9 @@ static bool ImFontAtlasBuildWithStbTruetype(ImFontAtlas* atlas) if (src_tmp.GlyphsCount == 0) continue; +#ifndef NO_PACK_RECTS stbrp_pack_rects((stbrp_context*)spc.pack_info, src_tmp.Rects, src_tmp.GlyphsCount); +#endif // Extend texture height and mark missing glyphs as non-packed so we won't render them. // FIXME: We are not handling packing failure here (would happen if we got off TEX_HEIGHT_MAX or if a single if larger than TexWidth?) @@ -2630,6 +2632,11 @@ void ImFontAtlasBuildPackCustomRects(ImFontAtlas* atlas, void* stbrp_context_opa ImVector<ImFontAtlasCustomRect>& user_rects = atlas->CustomRects; IM_ASSERT(user_rects.Size >= 1); // We expect at least the default custom rects to be registered, else something went wrong. +#ifdef WORKAROUND_BUG_5343 + if (user_rects.Size < 1) + __builtin_unreachable(); +#endif + ImVector<stbrp_rect> pack_rects; pack_rects.resize(user_rects.Size); memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes()); ``` ```sh $ gcc -v Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/12.2.1/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: /build/gcc/src/gcc/configure --enable-languages=c,c++,ada,fortran,go,lto,objc,obj-c++,d --enable-bootstrap --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --with-build-config=bootstrap-lto --with-linker-hash-style=gnu --with-system-zlib --enable-__cxa_atexit --enable-cet=auto --enable-checking=release --enable-clocale=gnu --enable-default-pie --enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object --enable-libstdcxx-backtrace --enable-link-serialization=1 --enable-linker-build-id --enable-lto --enable-multilib --enable-plugin --enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch --disable-werror Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 12.2.1 20230201 (GCC) ``` ```sh $ gcc -O3 -I. -DIMGUI_ENABLE_FREETYPE -c imgui_draw.cpp # (no output) $ gcc -O3 -I. -DIMGUI_ENABLE_FREETYPE -c imgui_draw.cpp -DTRIGGER_BUG_5343 imgui_draw.cpp: In function ‘void ImFontAtlasBuildPackCustomRects(ImFontAtlas*, void*)’: imgui_draw.cpp:2642:11: warning: ‘void* memset(void*, int, size_t)’ specified size between 18446744071562067968 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=] 2642 | memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes()); | ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ $ gcc -O3 -I. -DIMGUI_ENABLE_FREETYPE -c imgui_draw.cpp -DTRIGGER_BUG_5343 -DWORKAROUND_BUG_5343 # (no output) ``` ```sh $ gcc -O3 -DNO_PACK_RECTS -c imgui_draw.cpp # (no output) $ gcc -O3 -DNO_PACK_RECTS -c imgui_draw.cpp -DTRIGGER_BUG_5343 imgui_draw.cpp: In function ‘void ImFontAtlasBuildPackCustomRects(ImFontAtlas*, void*)’: imgui_draw.cpp:2642:11: warning: ‘void* memset(void*, int, size_t)’ specified size between 18446744071562067968 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=] 2642 | memset(pack_rects.Data, 0, (size_t)pack_rects.size_in_bytes()); | ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ $ gcc -O3 -DNO_PACK_RECTS -c imgui_draw.cpp -DTRIGGER_BUG_5343 -DWORKAROUND_BUG_5343 # (no output) ``` EDIT: Tried to narrow it down further by removing parts of imgui_draw.cpp until the warning went away (hoping to find a minimal snippet that could be reported). At some point adding or removing one of `StyleColorsDark`'s lines determined whether that warning shows up!

Rows
Columns

GCC warning #5343