Project

General

Profile

Bug #12747

sigsetjmp should allow for 8 byte aligned buffer on amd64

Added by Vitaliy Gusev about 2 months ago. Updated 5 days ago.

Status:
New
Priority:
Normal
Assignee:
Category:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

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

intel-mtune.patch (2.69 KB) intel-mtune.patch Vitaliy Gusev, 2020-07-05 04:43 PM

History

#1

Updated by Vitaliy Gusev about 2 months 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).

#2

Updated by Vitaliy Gusev 25 days 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)
#3

Updated by Joshua M. Clulow 18 days 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.

#4

Updated by Vitaliy Gusev 17 days 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 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.

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.

#5

Updated by Vitaliy Gusev 5 days ago

To reproduce segfault intel-mtune.patch should be applied.

#6

Updated by Vitaliy Gusev 5 days 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>

Also available in: Atom PDF