libucontext icon indicating copy to clipboard operation
libucontext copied to clipboard

make check segfault with -Os on 32 bit x86

Open ncopa opened this issue 1 year ago • 3 comments

ncopa-edge-x86:~/aports/main/libucontext/src/libucontext-1.2$ make check
env LD_LIBRARY_PATH=/home/ncopa/aports/main/libucontext/src/libucontext-1.2 ./test_libucontext_posix
setting up context 1
setting up context 2
doing initial swapcontext
start f2
swap to f1
start f1
checking provided arguments to function f1
looks like all arguments are passed correctly
swap back to f2
finish f2, should swap to f1
make: *** [Makefile:216: check_libucontext_posix] Segmentation fault
ncopa-edge-x86:~/aports/main/libucontext/src/libucontext-1.2$ 

gcc version:

ncopa-edge-x86:~/aports/main/libucontext/src/libucontext-1.2$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/i586-alpine-linux-musl/12.2.1/lto-wrapper
Target: i586-alpine-linux-musl
Configured with: /home/buildozer/aports/main/gcc/src/gcc-12-20220924/configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --build=i586-alpine-linux-musl --host=i586-alpine-linux-musl --target=i586-alpine-linux-musl --enable-checking=release --disable-fixed-point --disable-libstdcxx-pch --disable-multilib --disable-nls --disable-werror --disable-symvers --enable-__cxa_atexit --enable-default-pie --enable-default-ssp --enable-languages=c,c++,objc,go,fortran,ada --with-arch=i586 --with-tune=generic --enable-cld --disable-libssp --disable-libsanitizer --enable-shared --enable-threads --enable-tls --with-bugurl=https://gitlab.alpinelinux.org/alpine/aports/-/issues --with-system-zlib --with-linker-hash-style=gnu --with-pkgversion='Alpine 12.2.1_git20220924-r10'
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.2.1 20220924 (Alpine 12.2.1_git20220924-r10) 

Not sure if the backtrace is any useful.

Core was generated by `./test_libucontext_posix'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0xf7eb7244 in libucontext_trampoline ()
    at arch/common/common-trampoline.c:23
23		FETCH_LINKPTR(uc_link);
(gdb) bt
#0  0xf7eb7244 in libucontext_trampoline ()
    at arch/common/common-trampoline.c:23
#1  0x565e919c in ctx ()
#2  0x42424242 in ?? ()
#3  0x42424242 in ?? ()
#4  0x42424242 in ?? ()
#5  0x00fa6fb4 in ?? ()
#6  0x00000000 in ?? ()

With -O2 it does not happen.

ncopa avatar Apr 19 '23 12:04 ncopa

something weird- 3.17 release cycle built the same revision of libucontext, with the same gcc snapshot (12.2 for both). this means it's not a "new gcc update" thing in itself

nekopsykose avatar Apr 22 '23 01:04 nekopsykose

this seems to now be fixed with the newest gcc version (?)

nekopsykose avatar Jul 26 '23 02:07 nekopsykose

The problem is that with some compilers (e.g. the GCC 13 that we are currently using in Alpine), -Os does not imply -fomit-frame-pointer (we might want to look into enabling -fomit-frame-pointer in abuild again). Unfortunately, the trampoline function does not work correctly without -fomit-frame-pointer (see #60).

Compare the libucontext_trampoline machine code with -Os:

00000000 <libucontext_trampoline>:
   0:   55                      push   %ebp
   1:   89 e5                   mov    %esp,%ebp
   3:   53                      push   %ebx
   4:   e8 fc ff ff ff          call   5 <libucontext_trampoline+0x5>
   9:   81 c3 02 00 00 00       add    $0x2,%ebx
   f:   50                      push   %eax
  10:   8b 04 9c                mov    (%esp,%ebx,4),%eax
  13:   85 c0                   test   %eax,%eax
  15:   75 0a                   jne    21 <libucontext_trampoline+0x21>
  17:   83 ec 0c                sub    $0xc,%esp
  1a:   6a 00                   push   $0x0
  1c:   e8 fc ff ff ff          call   1d <libucontext_trampoline+0x1d>
  21:   83 ec 0c                sub    $0xc,%esp
  24:   50                      push   %eax
  25:   e8 fc ff ff ff          call   26 <libucontext_trampoline+0x26>
  2a:   8b 5d fc                mov    -0x4(%ebp),%ebx
  2d:   83 c4 10                add    $0x10,%esp
  30:   c9                      leave
  31:   c3                      ret

to that with -O2:

00000000 <libucontext_trampoline>:
   0:   53                      push   %ebx
   1:   8b 04 9c                mov    (%esp,%ebx,4),%eax
   4:   e8 fc ff ff ff          call   5 <libucontext_trampoline+0x5>
   9:   81 c3 02 00 00 00       add    $0x2,%ebx
   f:   83 ec 08                sub    $0x8,%esp
  12:   85 c0                   test   %eax,%eax
  14:   74 12                   je     28 <libucontext_trampoline+0x28>
  16:   83 ec 0c                sub    $0xc,%esp
  19:   50                      push   %eax
  1a:   e8 fc ff ff ff          call   1b <libucontext_trampoline+0x1b>
  1f:   83 c4 18                add    $0x18,%esp
  22:   5b                      pop    %ebx
  23:   c3                      ret
  24:   8d 74 26 00             lea    0x0(%esi,%eiz,1),%esi
  28:   83 ec 0c                sub    $0xc,%esp
  2b:   6a 00                   push   $0x0
  2d:   e8 fc ff ff ff          call   2e <libucontext_trampoline+0x2e>

The former pushes %ebp to the stack, therefore changes the stack pointer and causes the FETCH_LINKPTR to no longer work correctly.

nmeum avatar Feb 09 '24 23:02 nmeum

does that attribute work for clang? it generates a warning that it's ignored:

/tmp/mytemp.02x48x33.xpYaw/arch/common/common-trampoline.c:29:17: warning: unknown attribute 'optimize' ignored [-Wunknown-attributes]
   29 | __attribute__ ((optimize ("omit-frame-pointer")))
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

nekopsykose avatar Aug 08 '24 00:08 nekopsykose

Meh, I haven't tested clang. Is there any attribute for passing per-function optimization flags that is support by both clang and gcc?

nmeum avatar Aug 08 '24 06:08 nmeum

from a quick look i don't think there is one :/ just optnone which disables opts / '#pragma optimize on /off' as well. but no per-cflag kinda thing like gcc has

i guess there's not much that can be done then except passing it as a cflag for the whole file via meson, or just not caring

nekopsykose avatar Aug 08 '24 16:08 nekopsykose