Buffer overflows

[SirJonthe]

There seems to be a few Valgrind errors (buffer overflows) in the Linux build on AMD proprietary drivers (although my hunch is that these errors affect all platforms and drivers). Overflows occur after opening an image for the first time.

Invalid write of size 8 in main in /home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352 Address 0x7fe07a1ec500 is not stack'd, malloc'd or (recently) free'd 1: memcpy@@GLIBC_2.14 in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so 2: Papaya::UpdateAndRender(PapayaMemory*, PapayaDebugMemory*) in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/papaya.cpp:1511" >/home/jonathan/Code/contributing/Papaya/repo/src/papaya.cpp:1511</a> 3: main in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/linux_papaya.cpp:352" >/home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352</a>

This error refers to this line:

memcpy(buffer_data, Verts, 6 * sizeof(ImDrawVert)); //TODO: Profile this.

Invalid write of size 8 in main in /home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352 Address 0x7fe07a204d00 is not stack'd, malloc'd or (recently) free'd 1: Papaya::UpdateAndRender(PapayaMemory*, PapayaDebugMemory*) in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/papaya.cpp:1564" >/home/jonathan/Code/contributing/Papaya/repo/src/papaya.cpp:1564</a> 2: main in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/linux_papaya.cpp:352" >/home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352</a>

Invalid write of size 8 in main in /home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352 Address 0x7fe07a204d14 is not stack'd, malloc'd or (recently) free'd 1: Papaya::UpdateAndRender(PapayaMemory*, PapayaDebugMemory*) in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/papaya.cpp:1565" >/home/jonathan/Code/contributing/Papaya/repo/src/papaya.cpp:1565</a> 2: main in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/linux_papaya.cpp:352" >/home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352</a>

Invalid write of size 8 in main in /home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352 Address 0x7fe07a204d28 is not stack'd, malloc'd or (recently) free'd 1: Papaya::UpdateAndRender(PapayaMemory*, PapayaDebugMemory*) in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/papaya.cpp:1566" >/home/jonathan/Code/contributing/Papaya/repo/src/papaya.cpp:1566</a> 2: main in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/linux_papaya.cpp:352" >/home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352</a>

Invalid write of size 8 in main in /home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352 Address 0x7fe07a204d3c is not stack'd, malloc'd or (recently) free'd 1: Papaya::UpdateAndRender(PapayaMemory*, PapayaDebugMemory*) in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/papaya.cpp:1567" >/home/jonathan/Code/contributing/Papaya/repo/src/papaya.cpp:1567</a> 2: main in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/linux_papaya.cpp:352" >/home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352</a>

Invalid write of size 8 in main in /home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352 Address 0x7fe07a204d50 is not stack'd, malloc'd or (recently) free'd 1: Papaya::UpdateAndRender(PapayaMemory*, PapayaDebugMemory*) in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/papaya.cpp:1568" >/home/jonathan/Code/contributing/Papaya/repo/src/papaya.cpp:1568</a> 2: main in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/linux_papaya.cpp:352" >/home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352</a>

Invalid write of size 8 in main in /home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352 Address 0x7fe07a204d64 is not stack'd, malloc'd or (recently) free'd 1: Papaya::UpdateAndRender(PapayaMemory*, PapayaDebugMemory*) in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/papaya.cpp:1569" >/home/jonathan/Code/contributing/Papaya/repo/src/papaya.cpp:1569</a> 2: main in <a href="file:///home/jonathan/Code/contributing/Papaya/debug/../repo/src/linux_papaya.cpp:352" >/home/jonathan/Code/contributing/Papaya/repo/src/linux_papaya.cpp:352</a>

The last 6 errors refer to these consecutive lines:

buffer_data[0].pos = Vec2(Position.x, Position.y);
buffer_data[1].pos = Vec2(Size.x + Position.x, Position.y);
buffer_data[2].pos = Vec2(Size.x + Position.x, Size.y + Position.y);
buffer_data[3].pos = Vec2(Position.x, Position.y);
buffer_data[4].pos = Vec2(Size.x + Position.x, Size.y + Position.y);
buffer_data[5].pos = Vec2(Position.x, Size.y + Position.y);

All of the errors seem to have one thing in common; glMapBuffer. The actual output size of the buffer is not what we are expecting resulting in a buffer overflow.

[ApoorvaJ]

I am actually currently trying to track down the same issue. The program runs without errors on Nvidia cards (which my primary development machine has). On my ATI/Intel laptop, this crash does indeed occur.

An interesting observation is that the crash doesn't seem to occur if we skip the glGetTexImage() in the PushUndo() function in papaya.cpp. I'm currently trying to understand the causality of this.

Could you confirm this by either commenting out the aforementioned line or rolling back to a pre-Undo commit (e.g. this commit)?

Thanks for opening the issue. I'm on it as we speak. :)

[SirJonthe]

No crashes on my computer with or without undo functionality. However, the same buffer overflows seem to exist in both versions at the same lines as before. It could be that these overflows are actually causing trouble down the line, ultimately affecting our call to glGetTexImage.

[ApoorvaJ]

Ah. That's insightful. I'll keep you in the loop.

[ApoorvaJ]

I checked with Valgrind on a Nvidia-Linux machine. No buffer overflows happening.

I tried adding an artificial buffer overflow to ensure that Valgrind caught it. It did.

It seems that the buffer overflow is occurring only on non-Nvidia machines. The plot thickens.

[SirJonthe]

Very odd indeed. I wonder what driver is actually doing what it is supposed to, and which isn't. Maybe Valve's Vogl could be of assistance?

[ApoorvaJ]

Vogl might be overkill. :smile:

I think I'll have to install Linux on my AMD machine to get access to Valgrind and also to cover the full Nvidia/AMD - Linux/Windows spectrum.

I have a feeling that I'll come upon a solution by just putting more debug hours in. Papaya is currently simple enough for manual bug-isolation.

[ApoorvaJ]

I did manage to solve the buffer overflow problem by changing from glMapBuffer to glBufferSubData.

I am still getting data corruption issues on the call to glGetTexImage on non-Nvidia cards, but I suspect that's a different issue, which I am investigating.

Meanwhile, could you confirm that the Valgrind buffer overflows are solved on your end as well?

[SirJonthe]

Buffer overflows seem to be gone. Nice work!

Too bad fixing this issue did not solve the other. What exactly was wrong with the original call to glMapBuffer?

[ApoorvaJ]

(Disclaimer: I am not even remotely an OpenGL expert.)

I am not quite sure what was wrong with glMapBuffer.

Based on what I learnt after Googling, glMapBuffer is a rather complex and often slower operation than glBufferSubData. glMapBuffer may allocate memory or may map across address spaces. glBufferSubData is supposed to be simpler, and closer to a memcpy.

I am keeping this issue open until I resolve the glGetTexImage problem.

[ApoorvaJ]

glGetTexImage causes a crash only when running on ATI Catalyst's low power mode (i.e. the Intel integrated GPU). I'm taking a wild guess here that it's some kind of driver issue. The issue doesn't seem to occur on Linux as far as I can tell, probably because Linux doesn't switch to low-power GPU mode.

Anyway, I'll try and solve it, but no point in keeping this issue open.