MSYS2, MinGW64, GCC 10.1: stringop-overflow in UnitTools.cpp

[ligh_de]

Building VVC test software in the media-autobuild suite fails with the following errors:

[3/151] Building CXX object source/Lib/CommonLib/CMakeFiles/CommonLib.dir/UnitTools.cpp.o
FAILED: source/Lib/CommonLib/CMakeFiles/CommonLib.dir/UnitTools.cpp.o 
E:\MABS\msys64\mingw64\bin\ccache.exe  g++  -DENABLE_SPLIT_PARALLELISM=0 -I../source/Lib/CommonLib/. -I../source/Lib/CommonLib/.. -I../source/Lib/CommonLib/./x86 -I../source/Lib/CommonLib/../libmd5 -mthreads -mtune=generic -O2 -pipe -fopenmp -O3 -DNDEBUG   -Wall -fdiagnostics-show-option -Werror -Wno-sign-compare -Wno-class-memaccess -msse4.1 -mthreads -std=gnu++11 -MD -MT source/Lib/CommonLib/CMakeFiles/CommonLib.dir/UnitTools.cpp.o -MF source\Lib\CommonLib\CMakeFiles\CommonLib.dir\UnitTools.cpp.o.d -o source/Lib/CommonLib/CMakeFiles/CommonLib.dir/UnitTools.cpp.o -c ../source/Lib/CommonLib/UnitTools.cpp
../source/Lib/CommonLib/UnitTools.cpp: In function 'void PU::getInterMergeCandidates(const PredictionUnit&, MergeCtx&, int, const int&)':
../source/Lib/CommonLib/UnitTools.cpp:1261:50: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
 1261 |     mrgCtx.interDirNeighbours [uiArrayAddr     ] = 1;
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from ../source/Lib/CommonLib/UnitTools.h:43,
                 from ../source/Lib/CommonLib/UnitTools.cpp:38:
../source/Lib/CommonLib/ContextModelling.h:451:17: note: at offset 6 to object 'MergeCtx::interDirNeighbours' with size 6 declared here
  451 |   unsigned char interDirNeighbours[ MRG_MAX_NUM_CANDS      ];
      |                 ^~~~~~~~~~~~~~~~~~
../source/Lib/CommonLib/UnitTools.cpp:1262:50: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
 1262 |     mrgCtx.BcwIdx             [uiArrayAddr     ] = BCW_DEFAULT;
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
In file included from ../source/Lib/CommonLib/UnitTools.h:43,
                 from ../source/Lib/CommonLib/UnitTools.cpp:38:
../source/Lib/CommonLib/ContextModelling.h:450:17: note: at offset 6 to object 'MergeCtx::BcwIdx' with size 6 declared here
  450 |   uint8_t       BcwIdx            [ MRG_MAX_NUM_CANDS      ];
      |                 ^~~~~~
../source/Lib/CommonLib/UnitTools.cpp:1268:58: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
 1268 |       mrgCtx.interDirNeighbours [ uiArrayAddr          ] = 3;
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from ../source/Lib/CommonLib/UnitTools.h:43,
                 from ../source/Lib/CommonLib/UnitTools.cpp:38:
../source/Lib/CommonLib/ContextModelling.h:451:17: note: at offset 6 to object 'MergeCtx::interDirNeighbours' with size 6 declared here
  451 |   unsigned char interDirNeighbours[ MRG_MAX_NUM_CANDS      ];
      |                 ^~~~~~~~~~~~~~~~~~
cc1plus.exe: all warnings being treated as errors
ninja: build stopped: subcommand failed.

[ligh_de]

archive of log files generated by media-autobuild suite

[ligh_de]

After 2 weeks without any response... maybe "minor" is not the optimal priority for a case that interrupts the compilation?

[ksuehring]

We are currently focused on finalising a video coding standard. People are busy with implementing adopted proposals and checking the specification text for remaining issues. The software compiles fine under a wide range of supported compilers.

Again, this is experimental software used for standardisation purposes. It is an example implementation of VVC, but not intended to be useful for any production use.

If you provide a fix, we are happy to merge it. But I can't spend time for installing an unsupported compiler on an unsupported platform to re-produce the problem.

Maybe explaining why you need to compile the software on your site may help people understand, why this is important for you.

[ligh_de]

The media-autobuild suite helps people who are (like myself) not experienced developers and have only little experience with compilers – to produce software in an automated environment on Windows, one of the most widely used operating systems for users, which keeps itself up to date, based on packages provided by the MSYS2 team.

Then they can use their knowledge and experience in video encoding to test your sample software for possible code mistakes as well as for video encoding efficiency, shared in communities like e.g. the doom9 or VideoHelp forums.

The GNU C++ compiler is not a rarely used niche software, it is certainly among the most widely used C compilers. Version 10 went "stable" on May 7, almost a month ago. The MSYS2 project even updated to this version with a little delay, compared to other Linux distros.

I am no C/C++ developer. I have no clue about compiler interna. I only want to help providing your test software to the experienced video encoding testers in my community, by spending some background computing power for the automated compilation now and then.

I can't promise to know anyone being able to solve this compiler issue for you; but I could imagine that the interest in a new video standard of the future may drop when the availability of current testing software stops. All I can do now is to ask around and mention your restrictions to compilers I am not able to use, not being a full-time developer.

[pierrick.bouvier]

In my company, we ran into this issue as well, since we integrated gcc-10 the week it was released. This is pretty common when new compilers are released, and there is no magic, someone got to clean the thing and check everything builds fine.

The easier solution is simply to add -Wno-stringop-overflow to your build flags (either via CXX_FLAGS, or via CMakeLists directly). Please note that some warnings are already ignored, thus adding one should not be too difficult to accept.

Concerning the warning itself, I looked at it and I think it's a false positive. This is mainly due to a variable with a possible range giving an overflow (thus the warning), but if you look in VLCReader.cpp (or another file, I forgot), the concerned variable is well restricted on size of array (a CHECK macro checks this). Thus, the warning is simply due to compiler not seeing the whole program, but only a single compilation unit.

Feel free to make a Merge Request adding the flag in CMakeLists.

[1480c1]

Feel free to make a Merge Request adding the flag in CMakeLists.

Question, how can someone outside of jvet open a merge request? It seems to be able to fork the repo, you would need to be a jvet member, and to be able to open a Merge Request, you must have a fork.

Would we just have to submit patches through the issue tracker instead?

[pierrick.bouvier]

Note: I am not an official JVET dev, but I submitted a couple of bugfixes.

First, you need to open an account on gitlab there:
​https://vcgit.hhi.fraunhofer.de/jvet/VVCSoftware_VTM

Then, you must ask for your account to be an internal account (for instance asking to M. Suehring), then you fork the official, your push your diff, and you can start offering merge request.

The whole thing is (well explained) here:
​https://vcgit.hhi.fraunhofer.de/jvet/VVCSoftware_VTM/-/wikis/VVC-Software-Development-Workflow

The process is very similar to all forges available on the web (github, other gitlabs, ...), thus it's not a loss of time to learn how to do it.

Concerning posting of patches on bug tracker (here), I can't really speak for official maintainers.

[1480c1]

Ah, I see, so the Ask for internal account section is not limited to jvet members, that did not seem very obvious to me

Is there a list of people I can email to ask, or would it just be M. Suehring?

[pierrick.bouvier]

Patch fixing compilation

[pierrick.bouvier]

I added a patch fixing compilation, you can try it (using git am ~/path/to/patch), it should do it.

I don't know exact list of maintainers but anyone doing merges on master branch of VTM should be fine I guess.

[1480c1]

You might want to add array-bounds to -Wno-error

g++  -DENABLE_SPLIT_PARALLELISM=0 -I../source/Lib/EncoderLib/. -I../source/Lib/CommonLib/. -I../source/Lib/CommonLib/.. -I../source/Lib/CommonLib/./x86 -I../source/Lib/CommonLib/../libmd5 -mthreads -mtune=generic -O2 -pipe -fopenmp -O3 -DNDEBUG   -Wall -fdiagnostics-show-option -Werror -Wno-stringop-overflow -Wno-sign-compare -Wno-class-memaccess -msse4.1 -mthreads -std=gnu++11 -MD -MT source/Lib/EncoderLib/CMakeFiles/EncoderLib.dir/InterSearch.cpp.o -MF source\Lib\EncoderLib\CMakeFiles\EncoderLib.dir\InterSearch.cpp.o.d -o source/Lib/EncoderLib/CMakeFiles/EncoderLib.dir/InterSearch.cpp.o -c ../source/Lib/EncoderLib/InterSearch.cpp
../source/Lib/EncoderLib/InterSearch.cpp: In member function 'void InterSearch::predInterSearch(CodingUnit&, Partitioner&)':
../source/Lib/EncoderLib/InterSearch.cpp:2693:72: error: array subscript -1 is below array bounds of 'AMVPInfo [33]' [-Werror=array-bounds]
 2693 |               cCurMvField.setMvField( aacAMVPInfo[curRefList][refIdxCur].mvCand[i], refIdxCur );
      |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
../source/Lib/EncoderLib/InterSearch.cpp:2693:72: error: array subscript -1 is below array bounds of 'AMVPInfo [33]' [-Werror=array-bounds]
../source/Lib/EncoderLib/InterSearch.cpp:2694:72: error: array subscript -1 is below array bounds of 'AMVPInfo [33]' [-Werror=array-bounds]
 2694 |               cTarMvField.setMvField( aacAMVPInfo[tarRefList][refIdxTar].mvCand[j], refIdxTar );
      |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
../source/Lib/EncoderLib/InterSearch.cpp:2694:72: error: array subscript -1 is below array bounds of 'AMVPInfo [33]' [-Werror=array-bounds]

[ligh_de]

Ignoring the warning may be one solution.

Another would be to be aware that some C compilers are more sensitive regarding a mismatch of signed and unsigned integer types (int and uint32 appear incompatible without explicit cast). So I was told in a forum.

[ksuehring]

!1729 should fix the compile problem