Bug #12747
closedsigsetjmp should allow for 8 byte aligned buffer on amd64
100%
Description
mdb segfaults if libc compiled with -mtune=intel option on amd64 arch with SSE2 optimisation.
If compile with gcc-7 and '-mtune=intel' option, mdb crashes shortly in __csigsetjmp after starting it:
# pstack /var/cores/core.mdb.1536 core '/var/cores/core.mdb.1536' of 1536: mdb fffffd7fef285484 __csigsetjmp (4fd128, 1, fffffd7fffdfeab0) + 74 fffffd7fef2856f0 sigsetjmp () + 100 0000000000474701 termio_read (4eee50, 527690, 400) + c1 000000000044ef0e iob_read (4e1c50, 4eee50) + 1e 0000000000451e61 mdb_iob_getc (4e1c50) + 51 000000000047a880 input () + 10 000000000047ae9c yylook () + 6c 0000000000479ccf yylex () + f 000000000047b685 yyparse () + 345 000000000043a300 mdb_run () + 300 0000000000457f2d main (1, fffffd7fffdffc78, fffffd7fffdffc88) + d7d 00000000004363b3 _start_crt () + 83 0000000000436318 _start () + 18
Files
Updated by Vitaliy Gusev almost 2 years ago
Segfaults occur when compile libc for AMD64 with -mtune=intel -m64
options.
Disassemble of __csigsetjmp(sigjmp_buf env, int savemask, gregset_t rs):
0000000000135410 <__csigsetjmp>: .... 13547b: 83 c2 01 add $0x1,%edx 13547e: 89 90 f4 00 00 00 mov %edx,0xf4(%rax) 135484: 66 0f 7f 47 10 movdqa %xmm0,0x10(%rdi)
%rdi is exactly address of 'sigjmp_buf env' passed to sigsetjmp() and its value is 0x4fd128, i.e. is not 16-byte aligned.
From movdqa description: When the source or destination operand is a memory operand, the operand must be aligned on a 16-byte boundary or a general-protection exception (#GP) will be generated.
[Intel 64 and IA-32 Architecture Software Developer's Manual Vol. 2B 4-62]
ucontext_t has alignment 16 on AMD64 arch (due to using long double) that causes a compiler to assume that the code can be optimized by using aligned SSE2 instructions: movdqa and movaps, but sigjmp_buf is aligned on 8-byte (by using long).
Updated by Vitaliy Gusev almost 2 years ago
- Subject changed from mdb segfaults if compiled with -mtune=intel option to sigsetjmp should allow for 8 byte aligned buffer on amd64
- Description updated (diff)
Updated by Joshua M. Clulow almost 2 years ago
I was curious about how exactly we end up with this alignment, so I looked into it a little. Note that sigjmp_buf
is defined as an array of long
, with 128 elements and thus has a total size of 1024 bytes as per our ABI.
The AMD64 ABI document has this to say on the subject of alignment and arrays:
An array uses the same alignment as its elements, except that a local or global array variable of length at least 16 bytes or a C99 variable-length array variable always has alignment of at least 16 bytes.[4]
Footnote 4: The alignment requirement allows the use of SSE instructions when operating on the array. The compiler cannot in general calculate the size of a variable-length array (VLA), but it is expected that most VLAs will require at least 16 bytes, so it is logical to mandate that VLAs have at least a 16-byte alignment.
I suspect many uses of sigjmp_buf
are as its own variable, whether a global or local to some function that we want to return to from a subsequent siglongjmp()
call. These will get 16 byte alignment as per the description above.
In the MDB case here, we're instead seeing a sigjmp_buf
embedded in a larger struct, where it appears that the looser language applies and an "array uses the same alignment as its elements"; i.e., 8 bytes here. We can confirm this by looking at the actual call:
static ssize_t termio_read(mdb_io_t *io, void *buf, size_t nbytes) { termio_data_t *td = io->io_data; ... if (sigsetjmp(td->tio_env, 1) != 0) {
And then at the type itself via the CTF:
$ mdb -e '::print -tha termio_data_t' /usr/bin/amd64/mdb 0 termio_data_t { 0 mdb_io_t *tio_io 8 mdb_io_t *tio_out_io ... 10f4 struct termios tio_dtios { 10f4 tcflag_t c_iflag 10f8 tcflag_t c_oflag 10fc tcflag_t c_cflag 1100 tcflag_t c_lflag 1104 cc_t [19] c_cc 1117 uint8_t <<HOLE>> } 1118 sigjmp_buf tio_env <---------------- here ...
If the entire structure is 16 byte-aligned, the sigjmp_buf
will then only be 8-byte aligned as we're seeing with this crash.
Updated by Vitaliy Gusev almost 2 years ago
Joshua M. Clulow wrote:
I was curious about how exactly we end up with this alignment, so I looked into it a little. Note that
sigjmp_buf
is defined as an array oflong
, with 128 elements and thus has a total size of 1024 bytes as per our ABI.The AMD64 ABI document has this to say on the subject of alignment and arrays:
An array uses the same alignment as its elements, except that a local or global array variable of length at least 16 bytes or a C99 variable-length array variable always has alignment of at least 16 bytes.[4]
Footnote 4: The alignment requirement allows the use of SSE instructions when operating on the array. The compiler cannot in general calculate the size of a variable-length array (VLA), but it is expected that most VLAs will require at least 16 bytes, so it is logical to mandate that VLAs have at least a 16-byte alignment.
Thanks for help in deep investigation.
I believe that sigjmp_buf
is not VLA, at least if take a look at the VLA's definition:
In computer programming, a variable-length array (VLA), also called variable-sized, runtime-sized, is an array data structure whose length is determined at run time (instead of at compile time)
But size of sigjmp_buf
is known at compile time.
Updated by Vitaliy Gusev almost 2 years ago
- File intel-mtune.patch intel-mtune.patch added
To reproduce segfault intel-mtune.patch should be applied.
Updated by Vitaliy Gusev almost 2 years ago
Testing¶
Step 1¶
Apply intel-mtune.patch and compile, install. And boot.
vetal@oi-test:~$ sudo mkdir -p /var/cores vetal@oi-test:~$ sudo coreadm -e global -e global-setid -e log \ -g '/var/cores/core.%z.%f.%p' \ -G default vetal@oi-test:~$ mdb > Segmentation Fault vetal@oi-test:~$ ls /var/cores/ core.global.mdb.102210 vetal@oi-test:~$ uname -a SunOS oi-test 5.11 master-1-gb64fb15fed i86pc i386 i86pc vetal@oi-test:~$ vetal@oi-test:~$ vetal@oi-test:~$ pkg info system/library Name: system/library Summary: Core Solaris, (Shared Libs) Description: core shared libraries for a specific instruction-set architecture Category: System/Core State: Installed Publisher: on-nightly Version: 0.5.11 Branch: 2020.0.1.22004 Packaging Date: July 4, 2020 at 10:53:49 PM Last Install Time: May 4, 2020 at 01:27:19 PM Last Update Time: July 5, 2020 at 12:20:48 AM Size: 56.21 MB FMRI: pkg://on-nightly/system/library@0.5.11-2020.0.1.22004:20200704T225349Z
Step 2¶
Apply fix 0001-12747-sigsetjmp-should-allow-for-8-byte-aligned-buff.patch and compile, update, boot. Run mdb, run some workloads like compiling OS. Verify that there is no new core files in /var/cores
vetal@oi-test:~$ mdb > ::version mdb 1.1 (DEBUG) > vetal@oi-test:~$ uname -a SunOS oi-test 5.11 master-2-g9578c11ba4 i86pc i386 i86pc —— vetal@oi-test:~$ pkg info system/library Name: system/library Summary: Core Solaris, (Shared Libs) Description: core shared libraries for a specific instruction-set architecture Category: System/Core State: Installed Publisher: on-nightly Version: 0.5.11 Branch: 2020.0.1.22005 Packaging Date: July 5, 2020 at 03:15:13 PM Last Install Time: May 4, 2020 at 01:27:19 PM Last Update Time: July 5, 2020 at 04:32:48 PM Size: 56.21 MB FMRI: pkg://on-nightly/system/library@0.5.11-2020.0.1.22005:20200705T151513Z
git log¶
commit 9578c11ba439e94e88ffbde9030e9e3556bae8a2 (HEAD) Author: Vitaliy Gusev <gusev.vitaliy@gmail.com> Date: Fri May 22 13:45:36 2020 +0300 12747 sigsetjmp should allow for 8 byte aligned buffer on amd64 Reviewed by: Robert Mustacchi <rm@fingolfin.org> Reviewed by: Joshua M. Clulow <josh@sysmgr.org> commit b64fb15fed65d4ca80153832de94fabc8a530cf8 Author: Aurelien Larcher <aurelien.larcher@gmail.com> Date: Tue Mar 24 01:41:38 2020 +0100 Use -march=x86-64 -mtune=generic commit cf7690ebb38fa81bd6f3904ba5ad4649c0ea3c0b (origin/master, master) Author: Mike Zeller <mike@mikezeller.net> Date: Thu Jun 25 12:49:41 2020 -0700 12897 bhyve mevent can mistakenly handle events twice Reviewed by: Patrick Mooney <pmooney@pfmooney.com> Reviewed by: Andy Fiddaman <andy@omniosce.org> Approved by: Dan McDonald <danmcd@joyent.com>
Updated by Electric Monk almost 2 years ago
- Status changed from New to Closed
- % Done changed from 0 to 100
git commit 6a8fa7ea16d9870b21c82af67a2053cb47ed1fb4
commit 6a8fa7ea16d9870b21c82af67a2053cb47ed1fb4 Author: Vitaliy Gusev <gusev.vitaliy@gmail.com> Date: 2020-07-21T17:44:14.000Z 12747 sigsetjmp should allow for 8 byte aligned buffer on amd64 Reviewed by: Robert Mustacchi <rm@fingolfin.org> Reviewed by: Joshua M. Clulow <josh@sysmgr.org> Approved by: Joshua M. Clulow <josh@sysmgr.org>