Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FTBFS Debian/Bookworm lua5.4 / lua_getfield(L, tableindex, &key[0]); array ... partly outside array bound #6704

Open
flohoff opened this issue Oct 1, 2023 · 8 comments

Comments

@flohoff
Copy link

flohoff commented Oct 1, 2023

Hi,
while building on Debian 12 aka Bookworm i experience this issue with current git head: v5.25.0-204-g5723eaaa6

mkdir build && cd build && cmake .. && cmake --build .

    inlined from ‘sol::proxy_base< <template-parameter-1-1> >::operator T() const [with T = sol::optional<bool>; typename std::enable_if<sol::meta::all<std::integral_constant<bool, (! std::integral_constant<bool, ((((((is_string_literal_array_of_v<T, char> || is_same_v<T, const char*>) || is_same_v<T, char>) || is_string_of_v<T, char>) || is_same_v<T, std::initializer_list<char> >) || is_string_view_of_v<T, char>) || is_null_pointer_v<T>)>::value)>, sol::is_proxy_primitive<typename std::remove_cv<typename std::remove_reference<_Up>::type>::type> >::value, sol::meta::enable_t>::type <anonymous> = sol::meta::enable_t::_; Super = sol::table_proxy<sol::basic_table_core<false, sol::basic_reference<false> >&, std::tuple<const char (&)[18]> >]’ at /home/flo/projects/osm/osrm/v5.25.0-204-g5723eaaa6/third_party/sol2-3.3.0/include/sol/sol.hpp:16972:32,
    inlined from ‘osrm::extractor::Sol2ScriptingEnvironment::InitContext(osrm::extractor::LuaScriptingContext&)::<lambda()>’ at /home/flo/projects/osm/osrm/v5.25.0-204-g5723eaaa6/src/extractor/scripting_environment_lua.cpp:632:83:
/home/flo/projects/osm/osrm/v5.25.0-204-g5723eaaa6/third_party/sol2-3.3.0/include/sol/sol.hpp:16151:61: error: array subscript ‘const char [30][0]’ is partly outside array bounds of ‘const char [18]’ [-Werror=array-bounds]
16151 |                                                 lua_getfield(L, tableindex, &key[0]);
      |                                                 ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
At global scope:
cc1plus: note: unrecognized command-line option ‘-Wno-shorten-64-to-32’ may have been intended to silence earlier diagnostics
cc1plus: note: unrecognized command-line option ‘-Wno-unused-member-function’ may have been intended to silence earlier diagnostics
cc1plus: note: unrecognized command-line option ‘-Wno-implicit-float-conversion’ may have been intended to silence earlier diagnostics
cc1plus: note: unrecognized command-line option ‘-Wno-implicit-int-conversion’ may have been intended to silence earlier diagnostics
cc1plus: all warnings being treated as errors
gmake[2]: *** [CMakeFiles/EXTRACTOR.dir/build.make:328: CMakeFiles/EXTRACTOR.dir/src/extractor/scripting_environment_lua.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:235: CMakeFiles/EXTRACTOR.dir/all] Error 2
gmake: *** [Makefile:156: all] Error 2

Installed packages:

flo@m3:~/projects/osm/osrm/v5.25.0-204-g5723eaaa6/build$ dpkg -l *tbb* *lua* *gcc* *g++* | egrep ^ii
ii  g++                  4:12.2.0-3   amd64        GNU C++ compiler
ii  g++-12               12.2.0-14    amd64        GNU C++ compiler
ii  gcc                  4:12.2.0-3   amd64        GNU C compiler
ii  gcc-12               12.2.0-14    amd64        GNU C compiler
ii  gcc-12-base:amd64    12.2.0-14    amd64        GCC, the GNU Compiler Collection (base package)
ii  libgcc-12-dev:amd64  12.2.0-14    amd64        GCC support library (development files)
ii  libgcc-s1:amd64      12.2.0-14    amd64        GCC support library
ii  liblua5.1-0:amd64    5.1.5-9      amd64        Shared library for the Lua interpreter version 5.1
ii  liblua5.2-0:amd64    5.2.4-3      amd64        Shared library for the Lua interpreter version 5.2
ii  liblua5.4-0:amd64    5.4.4-3      amd64        Shared library for the Lua interpreter version 5.4
ii  liblua5.4-dev:amd64  5.4.4-3      amd64        Development files for the Lua language version 5.4
ii  libtbb-dev:amd64     2021.8.0-2   amd64        parallelism library for C++ - development files
ii  libtbb12:amd64       2021.8.0-2   amd64        parallelism library for C++ - runtime files
ii  libtbbbind-2-5:amd64 2021.8.0-2   amd64        parallelism library for C++ - runtime files
ii  libtbbmalloc2:amd64  2021.8.0-2   amd64        parallelism helper library for C++ - runtime files
ii  lua5.4               5.4.4-3      amd64        Simple, extensible, embeddable programming language

@nlsvgtr
Copy link

nlsvgtr commented Nov 8, 2023

I can confirm this, on Ubuntu 23.04 as well as on a fresh install of the latest Raspbian, which is Debian bookworm.

I followed the instructions in the README:

sudo apt install build-essential git cmake pkg-config \
libbz2-dev libxml2-dev libzip-dev libboost-all-dev \
lua5.2 liblua5.2-dev libtbb-dev

mkdir -p build
cd build
cmake ..
cmake --build .

I can provide the complete output for both, if that helps.

@flohoff
Copy link
Author

flohoff commented Nov 8, 2023

Interestingly using LLVM it works - So its a gcc incompatibility.

@danpat
Copy link
Member

danpat commented Nov 8, 2023

Looks like newer GCC is catching a bug in the sol2 dependency with -Werror=array-bounds.

The options here are:

  1. Upgrade sol2 and hope that they fixed the code upstream, or
  2. Make sure we're only including sol2 headers via -isystem third_party/sol2-3.3.0/include - doing that will stop the compiler for checking warnings on those external dependencies.

I'm kind of surprised that (2) isn't already working - we use the SYSTEM directive for sol2 already here: https://github.com/Project-OSRM/osrm-backend/blob/master/CMakeLists.txt#L123

As a quick and dirty hack, you can just comment out this line:

https://github.com/Project-OSRM/osrm-backend/blob/master/CMakeLists.txt#L244

which turns off all the -Wall -Werror flags that we have set. Generally not a good idea, but until we get a fixed sol2 in, probably the easiest option.

@nlsvgtr
Copy link

nlsvgtr commented Nov 9, 2023

LLVM works, thanks

@hpkit
Copy link

hpkit commented Nov 25, 2023

I just comment out this line:
https://github.com/Project-OSRM/osrm-backend/blob/master/CMakeLists.txt#L244

and then another two:
https://github.com/Project-OSRM/osrm-backend/blob/master/CMakeLists.txt#L295
https://github.com/Project-OSRM/osrm-backend/blob/master/CMakeLists.txt#L296

and compiled with GCC and debian 12.

@mattwigway
Copy link
Contributor

I think is related to ThePhD/sol2#1535

@mattwigway
Copy link
Contributor

mattwigway commented Dec 12, 2023

I believe this is actually caused by a GCC bug. The key part of the error is this error: array subscript ‘const char [30][0]’ is partly outside array bounds of ‘const char [18]’

What I believe is happening is that we have one place where we use a 30 character key into properties ("continue_straight_at_waypoint" - 29 characters plus a null terminator). It looks like GCC is static-analyzing and assuming that this function will always read 30 characters from the key, and then giving the bounds error any time we try to use a less-than-30-character key.
If you edit the code to have (say) a 36 character key somewhere, you will get the same error, but with [36] instead of [30].

@erlenddahl
Copy link

erlenddahl commented Jun 17, 2024

Looks like newer GCC is catching a bug in the sol2 dependency with -Werror=array-bounds.

The options here are:

1. Upgrade `sol2` and hope that they fixed the code upstream, or

2. Make sure we're only including `sol2` headers via `-isystem third_party/sol2-3.3.0/include` - doing that will stop the compiler for checking warnings on those external dependencies.

I'm kind of surprised that (2) isn't already working - we use the SYSTEM directive for sol2 already here: https://github.com/Project-OSRM/osrm-backend/blob/master/CMakeLists.txt#L123

As a quick and dirty hack, you can just comment out this line:

https://github.com/Project-OSRM/osrm-backend/blob/master/CMakeLists.txt#L244

which turns off all the -Wall -Werror flags that we have set. Generally not a good idea, but until we get a fixed sol2 in, probably the easiest option.

For others like me, who don't know enough about the toolchain to really understand this thread, note that the line number in CMakeLists.txt has changed slightly, it's now line 243.
And in case it changes again, it's this line:

# include(cmake/warnings.cmake)

In addition to this, I also had to comment lines 293 and 294 (like hpkit mentions above, but these have also shifted one line by now):

# target_no_warning(MICROTAR unused-variable)
# target_no_warning(MICROTAR format)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants