Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

32-bit gcc potential segfault whith -malign-double #7681

Closed
vicuna opened this issue Nov 28, 2017 · 6 comments
Closed

32-bit gcc potential segfault whith -malign-double #7681

vicuna opened this issue Nov 28, 2017 · 6 comments

Comments

@vicuna
Copy link

vicuna commented Nov 28, 2017

Original bug ID: 7681
Reporter: @nojb
Status: acknowledged (set by @xavierleroy on 2017-12-20T16:57:36Z)
Resolution: open
Priority: normal
Severity: minor
Category: runtime system and C interface
Monitored by: @gasche @alainfrisch

Bug description

We recently encountered a segfault caused by a 32-bit gcc using -malign-double by default. This option means that all doubles are assumed aligned by gcc, a hypothesis that is typically false for doubles allocated in the OCaml heap.

We saw this problem with i686-w64-mingw32-gcc 6.4.0 under Windows (which activates the -malign-double option by default) with high enough optimisations ("-msse2 -O3").

For correctness it seems it would be best to pass -mno-align-double explicitly to all 32-bit gcc targets (or at the very least for the mingw targets).

Steps to reproduce

(see attached daxpy.c and crash.ml)

$ ocamlc -ccopt -msse2 -ccopt -O3 -ccopt -malign-double -c daxpy.c
$ ocamlopt daxpy.o crash.ml
$ ./a.out
a=0xb7ce0f34
Segmentation fault (core dumped)
$ ocamlc -ccopt -msse2 -ccopt -O3 -mno-align-double -c daxpy.c
$ ocamlopt daxpy.o crash.ml
$ ./a.out
a=0xb7cbdf34
$

File attachments

@vicuna
Copy link
Author

vicuna commented Nov 29, 2017

Comment author: @nojb

Added file daxpy.s.diff showing the diff between the generated assembly with -malign-double and -mno-align-double. You can see there that "aligned" instructions movaps and movapd are replaced by "unaligned" instructions movups and movupd.

@vicuna
Copy link
Author

vicuna commented Nov 29, 2017

Comment author: @nojb

Also, you can find out your default -malign-double gcc setting by inspecting

gcc -Q --help=target

@vicuna
Copy link
Author

vicuna commented Dec 20, 2017

Comment author: @xavierleroy

There is a configure-time test to try and see whether doubles need 8-alignment, see config/auto-aux/dblalign.c. However it is easily fooled by clever compilers, hence the many exceptions in the configure script.

@github-actions
Copy link

github-actions bot commented May 7, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 7, 2020
@stedolan
Copy link
Contributor

This looks like a bug in the attached C stub, which directly casts an OCaml value to double*:

      daxpy(Wosize_val(a), (double *)a);

Accessing floats from C should be instead done using the Double_val macro that the FFI provides, which expands to different code depending on whether ARCH_ALIGN_DOUBLE is set. (This setting is detected more reliably since #8532)

@nojb
Copy link
Contributor

nojb commented May 18, 2020

Thanks @stedolan for the explanation, sounds right!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants