Opened 5 years ago

Closed 5 years ago

#115 closed defect (fixed)

Immediate SIGSEGV if not run from source dir or inside gdb

Reported by: D Gowers Owned by: Thomas Bernard
Priority: critical Milestone: 2.7
Component: GrafX2 Version: 2.7
Keywords: crash Cc:

Description

Steps to reproduce:


# make sure there are no stray files lying around..
sudo rm -r /usr/local/grafx2
git clone https://gitlab.com/GrafX2/grafX2.git
cd grafX2
make
cd src
sudo make install
cd ..
which grafx2-sdl
# 'which' should return /usr/local/bin/grafx2-sdl

grafx2-sdl
# should immediately give a message like '“grafx2-sdl” terminated by signal SIGSEGV (Address boundary error)'

gdb $(which grafx2-sdl)
# then type run
# grafx2 runs normally
# quit grafx2
# then quit gdb (enter 'q')

./bin/grafx2-sdl
# as with gdb; everything behaves normally


My suspicion is that this is a memory allocation bug related to the recent MAX_PATH_CHARACTERS changes.

Platform etc details:

Other details:

  • Not sdl specific (I tested with API=x11 as well; same result)
  • The md5sum of the installed binary is the same as the one in the bin/ directory (so the binaries really are identical)
  • Does not depend on working directory... I *guess* this probably means it is dependent mainly on the particular path to the binary..
  • Since gdb hides the problem, I'm not sure what else to try. Valgrind?

A working barebones setup:


# start in the top level directory of the grafx2 git clone, and execute this.
mkdir /tmp/g
cp -a bin/grafx2_sdl /tmp/g
mkdir -p /tmp/share/grafx2/skins
cp -a share/grafx2/skins/*DPaint* /tmp/share/grafx2/skins
/tmp/g/grafx_sdl
# gfx2def.ini is not required, despite the error message. Copying it in to /tmp/share/grafx2 does not effect the result other than removing the error message.
# the same applies to gfx2.png


It's curious that this particular snippet works, but hopefully the fact that it does work is informative somehow.

Change History (13)

comment:1 by D Gowers, 5 years ago

(as a side note -- I guess this is a minor Makefile bug -- gfx2.png is not installed by sudo make install on this platform -- it is only gfx2.gif that gets installed. Hence the 'gdb' invocation of grafx2 eventually complains that gfx2.png is missing. (but the non-gdb invocation does not; it appears to crash before any such message could be printed)

This may be useful in that it implies the crash described in this bug report must be occurring quite early -- before the check for gfx2.png is done.

Line 1184 of the Makefile is roughly where this sub-problem is.
)

Last edited 5 years ago by D Gowers (previous) (diff)

comment:2 by Thomas Bernard, 5 years ago

So it crashes when run directly but not when trying to debug it with GDB ?
running from /tmp/g/ works too ?

comment:3 by Thomas Bernard, 5 years ago

can you run with the "-v" flag ? The debug output will help find when the crash is.

$ /tmp/usr/local/bin/grafx2-sdl -v
Failed to load icon /tmp/usr/local/bin/../share/grafx2/gfx2.png
Load_CFG() trying to load /home/nanard/.config/grafx2/gfx2.cfg
Load_INI() loading /home/nanard/.config/grafx2/gfx2.ini

comment:4 by D Gowers, 5 years ago

To re-summarize, I tried 4 different things:

  • Case 1: grafx2-sdl or /usr/local/bin/grafx2-sdl : Crashes (SIGSEGV)
  • Case 2: ./bin/grafx-sdl, invoked from the root of the git clone : Works
  • Case 3: gdb grafx2-sdl or gdb /usr/local/bin/grafx2-sdl : Works
  • Case 4: /tmp/g/grafx2-sdl, after setting up as described above : Works

Result of invoking with -v option

  • Case 1 : No output with -v. No output without -v.
  • Case 2,3,4 : Some console output, as shown below.

To reiterate that, invoking with the -v option does not give any output in case 1 (debug or otherwise). Presumably this is because the crash occurs before any attempt to output a debug message is made. Notice how in your output sample, the error about the missing PNG is the first message? As I stated in my earlier message, this PNG error does not show for case 1, implying that the crash occurs before this check for the PNG is made.

(I admit that there is a possible alternative explanation here: The crash is interfering with the console output so that the check for the PNG still occurs but the error message does not show. However, I consider this possibility to be unlikely.)

Case 2, 3, 4 give pretty normal looking (and more or less identical) debug output when run with -v.

EDIT: Here is some sample output, from case 3:


Failed to load icon /usr/local/bin/../share/grafx2/gfx2.png
Load_CFG() trying to load /home/kau/.config/grafx2/gfx2.cfg
Load_INI() loading /home/kau/.config/grafx2/gfx2.ini
PNG private chunk 'crNg' :
000000: 00 00 00 02 30 3f 00 00 00 02 40 4f 00 00 00 02 | ....0?....@O....
000010: 50 5f 00 00 00 02 60 6f 00 00 00 02 70 7f 00 00 | P_....`o....p...
000020: 00 02 80 8f 00 00 00 02 90 9f 00 00 00 02 a0 af | ................
000030: 00 00 00 02 b0 bf 00 00 00 02 c0 cf 00 00 00 02 | ................
000040: d0 df 00 00 00 02 e0 ef | ........
PNG type=3 bit_depth=8 : 8bpp
PNG type=3 bit_depth=8 : 8bpp
PNG type=3 bit_depth=8 : 8bpp


(beyond that there is just button/keyboard event reporting obviously generated by me scribbling.)

EDIT: fixed some confusion caused by renumbering.

Last edited 5 years ago by D Gowers (previous) (diff)

comment:5 by Thomas Bernard, 5 years ago

could you try the "crashing setup" with my branch https://gitlab.com/miniupnp/grafX2/commits/issue_115
I added some debug log in the init, to help locate the crash.

By the way, I'm building with CC=clang CFLAGS=-fsanitize=address LDFLAGS=-fsanitize=address OPTIM=0 make I don't know if it can help...

comment:6 by Thomas Bernard, 5 years ago

SIGSEGV (Address boundary error) happens probably because we access memory after the end of a malloc'ed block. GDB change the way memory is managed, that could explain why the bug doesn't happen there.

comment:7 by Thomas Bernard, 5 years ago

comment:8 by Thomas Bernard, 5 years ago

I think the bug is caused by not setting a null terminating character in the string written by readlink()
most of the time there is a 0 that will save us, but if there is none, there will be a stack over-read.

https://gitlab.com/GrafX2/grafX2/blob/master/src/setup.c#L116

cbc6b96639dad37c6b7ef037ae7b16a9bd7af450 should fix it.

@D Gowers : can you verify ?

comment:9 by Thomas Bernard, 5 years ago

the bug was introduced in https://gitlab.com/GrafX2/grafX2/commit/54317d94aef5d2f648eef5915a23b48b9679c229
that's quite a long time ago ;)

comment:10 by D Gowers, 5 years ago

Yes, commit cbc6b96639dad37c6b7ef037ae7b16a9bd7af450 seems to fix it. I've tested all cases -- they all work, no crash.

Last edited 5 years ago by D Gowers (previous) (diff)

comment:12 by Thomas Bernard, 5 years ago

Keywords: crash added
Milestone: 2.7
Owner: changed from pulkomandy to Thomas Bernard
Priority: majorcritical
Status: newaccepted

comment:13 by Thomas Bernard, 5 years ago

Resolution: fixed
Status: acceptedclosed
Note: See TracTickets for help on using tickets.