[PATCH] Fixed source and destination overlap in memmove invoked by gfx::CardName

[OMGtechy]

I wanted to contribute to the project, so I thought I good way to start would be to put 0ad through valgrind --track-origin=yes.

When I did so, one of the messages I got was the following:

TIMER| RunHardwareDetection: 3.9549 s
==7265== Source and destination overlap in memcpy_chk(0xffeffd128, 0xffeffd158, 108)
==7265==    at 0x4C352E7: __memcpy_chk (vg_replace_strmem.c:1574)
==7265==    by 0x8E0791: memmove (string3.h:59)
==7265==    by 0x8E0791: gfx::CardName[abi:cxx11]() (gfx.cpp:60)
==7265==    by 0x5E576B: WriteSystemInfo() (Util.cpp:133)
==7265==    by 0x634480: InitGraphics(CmdLineArgs const&, int) (GameSetup.cpp:1029)
==7265==    by 0x42F6BA: RunGameOrAtlas(int, char const**) (main.cpp:528)
==7265==    by 0x4219C6: main (main.cpp:571)

This is almost certainly an issue in valgrind, since memmove exists to allow said overlap, but it's easy to fix and allows people to use the offending valgrind version. I fixed it on that basis.

[Stan]

#1952 and #1575 were fixed by ben. Not sure,but comment should likely be edited.

[OMGtechy]

Replying to stanislas69:

#1952 and #1575 were fixed by ben. Not sure,but comment should likely be edited.

It's not clear what you mean by this; both tickets refer to ​http://trac.wildfiregames.com/changeset/13536 where this was added. I didn't add this. If you're saying that this no longer applies, perhaps this should be in a separate ticket? I don't know the policy around that kind of thing here.

[Stan]

It's just that since you moved refactored that code it might be nice to see if comments are stil' useful.

[elexis]

Just for the record (from 2017-01-02):

16:43 < Philip> Why would Valgrind complain about overlaps in memmove, when the whole point of memmove is that it allows overlaps?
16:45 < OMGtechy> Looks like a bad implementation of memmove
16:45 < OMGtechy> so the bug is probably in valgrind
16:46 < OMGtechy> but the code is hard to read anyway, and the cost is minimal, so I thought it was worth it
16:46 < OMGtechy> plus valgrind is a useful tool and any fix I make to it would have to be backported to whatever people use
16:47 < OMGtechy> (unless they're on arch, in which case they'll have it yesterday ;) )

16:53 < elexis> tried running valgrind too, got incredibly spammed out
16:53 < OMGtechy> yeah, this is the first in a long line of things to sort down that road
16:54 < OMGtechy> many of the things that came up for me are outside of your codebase - turns out my libgl is naughty
16:56 < Philip> Sometimes it's just that Valgrind doesn't understand the syscalls used by drivers, so it thinks memory is uninitialised when actually it's been initialise by some other hardware, etc
16:56 < Philip> although sometimes it's that the drivers are buggy

[echotangoecho]

I will just note here that this function gfx::CardName, and the other functions in this file should probably removed altogether and instead we should just directly use the values of glGetString(GL_VENDOR), glGetString(GL_RENDERER), etc... directly (which we could perhaps cache somewhere at game launch, not sure about the usefulness of that though). Also see #4451.

[Stan]

Fixed in r26072