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
"struct hack" for bigarrays clashes with clang array bounds checks #5516
Comments
Comment author: @xavierleroy Tentative fix in SVN trunk, revision 12188. Use a "flexible" array type if C99 or GCC. Tested using GCC under Linux; let us know if it doesn't fix the issue with Clang. |
Comment author: mww Does not seem to work: OCaml itself builds and installs fine; I: Running test 'test_runner' This is OS X 10.7.3/x86_64/clang-llvm 3.0/ocaml 3.12.1 with the bigarray.h patch |
Comment author: @mmottl please use the latest version of bin-prot (2.0.7), which can be downloaded from here: http://forge.ocamlcore.org/projects/bin-prot The latest version should fix some portability problems, which are the likely reason for the observed errors. The next release of bin-prot will be part of the Jane Street Core library at bitbucket, which should be finalized any day now. Until then, I'm afraid, there will still be some version confusion due to multiple download sites. |
Comment author: mww bin-prot 2.0.7 also has a test failure rate of 50%; the crash-reporter gives me something like this: Exception Type: EXC_BAD_ACCESS (SIGBUS) VM Regions Near 0x10055cff0: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread |
Comment author: @xavierleroy After rereading the change, I still can't see anything wrong. It doesn't mean nothing is wrong, just that I need help finding what's wrong :-) I downloaded bin-prot 2.0.7 and tried to test it on my Linux x86-64 box with the development version of OCaml. I gave up because I'm too lazy to set up ocamlfind and the dependencies of bin-prot with the devel version. To mww: does bin-prot 2.0.7 work on your machine with a stock OCaml 3.12.1 ? |
Comment author: @mmottl I have tried to reproduce the problem on my Mac using clang for compilation (using the -cc flag with the OCaml compiler), but have not managed to trigger any problems. My configuration seems to be the same as yours. The resulting code seems to be running somewhat faster than with gcc, though this library is highly sensitive to code placement, etc. @mww: please send me an email detailing how exactly you are using clang with bin-prot on your Mac. @xLeroy: it's usually easiest to install Godi first (takes a few human minutes only) and just select the bin-prot package. This should build and install everything you need for testing. Then you can toy with the library source as usual. |
Comment author: mww bin-prot works w/o the bigarray patch (though I had to patch bin-prot to silence the array-out-of-bound warnings and to remove all "inline" directives); |
Comment author: @damiendoligez Another data point: bin-prot 2.0.3 works with the current trunk (revision 12189) on Mac OS 10.6. (modulo a trivial change to bin-prot to silence C compiler warnings). |
Comment author: @mmottl I'm pretty certain the observed problem was due to an incorrect patch file applied to the MacPorts OCaml package. It did not contain changes to bigarray_stubs.c, which would make allocated arrays too short with clang. @mww, if updating the patch accordingly solves your problem, please also let us know here so that this issue can be closed. Thanks! |
Comment author: mww ok, I've added also the corresponding changes from bigarray_stubs.c (complete patch is here [1]) and bin-prot seems to compile and work flawless. Regarding the changes: Don't they break for gcc? At least for Apple's gcc 4.2 on 10.6 this seems to break -- the preprocessor macro that checks for clang/llvm also says gcc is always fine: #if (STDC_VERSION >= 199901L) || defined(GNUC) This does not seem to work for Apple's XCode on 10.6: gcc 4.2; I changed the patch so the check only reads: #if (STDC_VERSION >= 199901L) which seems to work for XCode/gcc 4.2 on 10.6 and also for XCode/clang/llvm 4.3 on 10.7. [1] https://trac.macports.org/browser/trunk/dports/lang/ocaml/files/patch-otherlibs-bigarray.diff |
Comment author: @xavierleroy Thanks for the testing. Normally, a compiler that defines GNUC should implement all GNU extensions to the C language, which include flexible arrays at end of structs, predating C99. But, to be on the safe side, I removed the "|| defined(GNUC)". (Commits 12311 and 12312.) |
Original bug ID: 5516
Reporter: @mmottl
Status: closed (set by @xavierleroy on 2015-12-11T18:08:10Z)
Resolution: fixed
Priority: normal
Severity: minor
Platform: Apple Mac
OS: Mac OS X
OS Version: 10.7.3
Version: 3.12.1
Fixed in version: 3.13.0+dev
Category: otherlibs
Related to: #5761
Monitored by: mww
Bug description
The most recent release of clang (part of Xcode) for Mac OS X Lion reportedly fails to compile C-bindings accessing bigarrays due to the "struct hack" used to declare the "dim" field in the caml_ba_array declaration. This hack can trigger bogus array bounds errors in user code.
Would it be possible to make this declaration more general? It seems that using "CAML_BA_MAX_NUM_DIMS" instead of "1" as declared size (+ accordingly adapting caml_ba_alloc) would work. Or using the C-preprocessor to generate different code if C99 mode is detected as described here (would also require adapting caml_ba_alloc):
http://bugs.call-cc.org/attachment/ticket/778/0001-Use-flexible-array-member-in-C99-mode-silences-clang.patch
The text was updated successfully, but these errors were encountered: