glaze icon indicating copy to clipboard operation
glaze copied to clipboard

GCC 14 warning when reading json

Open 20162026 opened this issue 1 year ago • 10 comments

gcc throws maybe-uninitialized warning when read_jmespath is used.

issue only occures when:

  • gcc 14 or up is used
  • optimisation is set to O2
  • glz::opts are set to .error_on_unknown_keys = 0
  • glz::opts are set to ~~.error_on_missing_keys = 1~~ null_terminated=0
  • struct has two or more members that start from the same letter (renaming struct member variables fixes the issue????)
In function 'constexpr void glz::visit(auto:42&&, size_t) [with long unsigned int N = 3; auto:42 = detail::parse_and_invoke<glz::opts{10, '\000', '\000', '\000', '\001', '\001', '\000', '\000', ' ', 3, '\001', '\000', '\000', '\001', '\000', '\000', '\000', '\000', 0, glz::float_precision::full, '\000', '\000', '\000', '\000', '\000', '\000', '\001', '\000', '\001', '\001', 0}, test_t, hash_info<test_t>, test_t&, glz::context&, const char*&, const char*&>(test_t&, glz::context&, const char*&, const char*&)::<lambda()>]',
    inlined from 'constexpr void glz::detail::parse_and_invoke(Value&&, auto:506&&, auto:507&&, auto:508&&, SelectedIndex&& ...) [with glz::opts Opts = glz::opts{10, '\000', '\000', '\000', '\001', '\001', '\000', '\000', ' ', 3, '\001', '\000', '\000', '\001', '\000', '\000', '\000', '\000', 0, glz::float_precision::full, '\000', '\000', '\000', '\000', '\000', '\000', '\001', '\000', '\001', '\001', 0}; T = test_t; auto& HashInfo = hash_info<test_t>; Value = test_t&; SelectedIndex = {}; auto:506 = glz::context&; auto:507 = const char*&; auto:508 = const char*&]' at /opt/compiler-explorer/libs/glaze/trunk/include/glaze/json/read.hpp:277:24:
/opt/compiler-explorer/libs/glaze/trunk/include/glaze/util/for_each.hpp:63:35: error: '<anonymous>' may be used uninitialized [-Werror=maybe-uninitialized]
   63 |          (lambda.*mem_ptrs[index])();
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~^~

https://godbolt.org/z/zacecnY71

from the first glance it looks like a false positive but i'm struggling to understand why this is even happening...

20162026 avatar Jan 13 '25 13:01 20162026

Thanks for bringing this up and sharing the compiler explorer example. I'm looking into it now. ...REMOVED MISTAKEN COMMENT to not confuse others

stephenberry avatar Jan 13 '25 16:01 stephenberry

I added a test case for this in #1562. However, this is passing GCC14, which is confusing to me because it uses -Wall, -Wextra, and -Werror. I would have expected it to fail.

I can understand why GCC might think this line of code is not safe: (lambda.*mem_ptrs[index])();. The index could be out of bounds. But, we always check that the index is less than N, which is the count of function pointers in the array. So, it isn't possible to read out of bounds. And, the '<anonymous>' may be used uninitialized error doesn't make any sense, because the std::array creating function pointers from the <anonymous> lambdas is always instantiated up to N. So, it feels like a GCC bug.

stephenberry avatar Jan 13 '25 16:01 stephenberry

I did not quite understand your remarks regarding the example. You have added test_wrapper_t but didn't use it anywhere and if the structure was wrapped I would not need the read_jmespath in the first place. Or was it only about the ec as return code?

regarding the pipeline, correct me if im wrong but it looks like Werror is only set for Clang and there is no O2 optimization https://github.com/stephenberry/glaze/blob/8c2eb577a2dc8cc245b7a18e672d0ec44b2cc3e6/tests/CMakeLists.txt#L27

20162026 avatar Jan 13 '25 17:01 20162026

regarding the pipeline, correct me if im wrong but it looks like Werror is only set for Clang and there is no O2 optimization

Ah, thanks for pointing that out. I'll add those flags to test GCC.

I did not quite understand your remarks regarding the example...

My bad again. You're right the test_wrapper_t is not needed. I rushed my response. Thanks for the correction.

stephenberry avatar Jan 13 '25 17:01 stephenberry

The fact that this bug doesn't exist with O0, O1 or O3, but occurs with O2 makes it seem very much like a compiler issue. I can keep this alive to report it to GCC, but I'm going to be out of pocket for a few weeks before I'm able to do so.

stephenberry avatar Jan 13 '25 17:01 stephenberry

yeah it is most likely gcc bug. I did tinker around with debuger (although not x86) and it makes no sense

20162026 avatar Jan 13 '25 17:01 20162026

it also appears with Os and Oz

20162026 avatar Jan 13 '25 17:01 20162026

https://godbolt.org/z/6TG8YzqTv warning is also thrown with regular json read of wrapped structure.

20162026 avatar Jan 15 '25 09:01 20162026

looks like the issue was fixed after macro cleanup at 19a189e463dca711ac47fde3f44dc0e5d3234ffc (v4.4.3), still not sure why it was failing in the first place...

P.S. goldbolt links were using trunk, so they no longer show an error.

20162026 avatar Apr 04 '25 14:04 20162026

nevermind, only -O2 was fixed while -Oz and -Os still fail at current trunk fafa42a345752fd54ff33e6b461909d198e20c2d and Im confused even more than before...

https://godbolt.org/z/dM5qdffqK

In file included from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/core/meta.hpp:10,
                 from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/api/hash.hpp:9,
                 from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/api/std/span.hpp:8,
                 from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/core/read.hpp:8,
                 from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/beve/skip.hpp:8,
                 from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/beve/read.hpp:7,
                 from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/beve/ptr.hpp:6,
                 from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/beve.hpp:7,
                 from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/glaze.hpp:35,
                 from <source>:1:
In function 'constexpr void glz::visit(auto:61&&, size_t) [with long unsigned int N = 3; auto:61 = parse_and_invoke<opts{10, false, false, false, true, true, false, false, ' ', 3, true, false, false, false, 0, glz::float_precision::full, false, false, false, false, false, false, false, 0}, inner, hash_info<inner>, inner&, context&, const char*&, const char*&>(inner&, context&, const char*&, const char*&)::<lambda()>]',
    inlined from 'constexpr void glz::parse_and_invoke(Value&&, auto:540&&, auto:541&&, auto:542&&, SelectedIndex&& ...) [with auto Opts = opts{10, false, false, false, true, true, false, false, ' ', 3, true, false, false, false, 0, glz::float_precision::full, false, false, false, false, false, false, false, 0}; T = inner; auto& HashInfo = hash_info<inner>; Value = inner&; SelectedIndex = {}; auto:540 = context&; auto:541 = const char*&; auto:542 = const char*&]' at /opt/compiler-explorer/libs/glaze/trunk/include/glaze/json/read.hpp:314:21:
/opt/compiler-explorer/libs/glaze/trunk/include/glaze/util/for_each.hpp:63:35: error: '<anonymous>' may be used uninitialized [-Werror=maybe-uninitialized]
   63 |          (lambda.*mem_ptrs[index])();
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~^~
In file included from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/json/json_ptr.hpp:11,
                 from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/core/ptr.hpp:9,
                 from /opt/compiler-explorer/libs/glaze/trunk/include/glaze/beve/ptr.hpp:8:
/opt/compiler-explorer/libs/glaze/trunk/include/glaze/json/read.hpp: In function 'constexpr void glz::parse_and_invoke(Value&&, auto:540&&, auto:541&&, auto:542&&, SelectedIndex&& ...) [with auto Opts = opts{10, false, false, false, true, true, false, false, ' ', 3, true, false, false, false, 0, glz::float_precision::full, false, false, false, false, false, false, false, 0}; T = inner; auto& HashInfo = hash_info<inner>; Value = inner&; SelectedIndex = {}; auto:540 = context&; auto:541 = const char*&; auto:542 = const char*&]':
/opt/compiler-explorer/libs/glaze/trunk/include/glaze/json/read.hpp:314:22: note: '<anonymous>' declared here
  314 |             visit<N>([&]<size_t I>() { decode_index<Opts, T, I>(value, ctx, it, end, selected_index...); }, index);
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
Compiler returned: 1

20162026 avatar Apr 04 '25 15:04 20162026

@20162026, this compiler bug should now be avoided with the visiting improvements in: #1771

This new code should be faster as well, and much simpler for the compiler. I ran mini tests building with Os and Oz as well and it looked good. So, I'm closing this issue, but certainly reopen it if the error shows up again.

Thanks for reporting this! It motivated me to make the visiting better.

stephenberry avatar May 30 '25 17:05 stephenberry