Fix building of debug runtime on Windows#820
Conversation
dra27
left a comment
dra27
left a comment
There was a problem hiding this comment.
Various notes on the changes
| include Makefile.common | ||
|
|
||
| CFLAGS=-DOCAML_STDLIB_DIR='"$(LIBDIR)"' $(IFLEXDIR) | ||
| DFLAGS=$(CFLAGS) -DDEBUG |
There was a problem hiding this comment.
Humorously, only the msvc64 Makefile actually defined DEBUG, so the ocamlrund.exe built in the other ports was in fact just a standard runtime!
Defining DFLAGS is vaguely as done in Makefile
|
|
||
| ocamlrund$(EXE): libcamlrund.$(A) prims.$(O) main.$(O) | ||
| $(MKEXE) -o ocamlrund$(EXE) $(BYTECCDBGCOMPOPTS) prims.$(O) \ | ||
| $(MKEXE) -o ocamlrund$(EXE) prims.$(O) \ |
There was a problem hiding this comment.
It makes no sense to include $(BYTECCDBGCOMPOPTS) here (unsurprisingly, flexlink doesn't like the extra arguments).
| >> .depend.win32 | ||
| cat .depend >> .depend.win32 | ||
| sed -e '/\.d\.o/q' -e 's/^\(.*\)\.o:/\1.$$(O) \1.$$(DBGO):/' \ | ||
| sed -ne '/\.pic\.o/q' \ |
There was a problem hiding this comment.
Using -n means that there isn't a stray .pic.o line at the bottom, hence the -e p at the end (or there'd be no lines at all...)
| sed -e '/\.d\.o/q' -e 's/^\(.*\)\.o:/\1.$$(O) \1.$$(DBGO):/' \ | ||
| sed -ne '/\.pic\.o/q' \ | ||
| -e 's/^\(.*\)\.d\.o:/\1.$$(DBGO):/' \ | ||
| -e 's/^\(.*\)\.o:/\1.$$(O):/' \ |
There was a problem hiding this comment.
This never worked - interp.o has additional dependencies when compiled for debug mode.
| BYTECCCOMPOPTS=-O2 -Gy- -MD | ||
|
|
||
| ### Additional compile-time options for $(BYTECC). (For debug version.) | ||
| BYTECCDBGCOMPOPTS=-DDEBUG -Zi -W3 -Wp64 |
There was a problem hiding this comment.
-Wp64 was deprecated a loooong time ago - /W4 replaces it now. -W3 causes a vast number of warnings to be displayed.
damiendoligez
left a comment
damiendoligez
left a comment
There was a problem hiding this comment.
I'm in favor of including this in 4.04, as the risk is null for other ports, and very low for the Windows ports.
| BYTECCDBGCOMPOPTS=-g | ||
|
|
||
| ### Flag to use to rename object files. (for debug version.) | ||
| NAME_OBJ_FLAG=-o |
There was a problem hiding this comment.
This looks wrong. I think you need a space at the end of the variable's value. Here is how to do it:
EMPTY=
SPACE=$(EMPTY) $(EMPTY)
BYTECCDBGCOMPOPTS=-o$(SPACE)You might want to pull the definitions of EMPTY and SPACE to the beginning of the file.
There was a problem hiding this comment.
Hmm, it does work (and is POSIX), but GCC's command line doesn't use the normal GNU long option conventions (with the knock-on effect that you cannot say, for example, gcc -cv to mean gcc -c -v).
If GCC were to add a non-standard long option beginning with o (say, -ofoo) in the future then this would become a problem - however, GCC does add new options using --long-option syntax.
However, would you prefer it changed as you show just for neatness of the command line printed by make? The only reason I didn't do it that way was because it isn't strictly necesssary.
There was a problem hiding this comment.
You're right, gcc accepts -omyprogram.exe, so maybe Damien's space dance is not necessary.
Another way to deal with those space-or-not-space issues is to define macros that take parameters and are to be invoked via $(call...), like MKLIB or MAKE_OCAMLRUN.
There was a problem hiding this comment.
Indeed, I debated using a macro but, in a very uncharacteristic twist for me, kept it simpler 😉
| BYTECCDBGCOMPOPTS=-g | ||
|
|
||
| ### Flag to use to rename object files. (for debug version.) | ||
| NAME_OBJ_FLAG=-o |
There was a problem hiding this comment.
Same remark as above about the space.
config/Makefile.msvc
Outdated
| BYTECCDBGCOMPOPTS=-Zi | ||
|
|
||
| ### Flag to use to rename object files. (for debug version.) | ||
| NAME_OBJ_FLAG=/Fo |
There was a problem hiding this comment.
Why do you use a / for this option when all the others are using - ?
There was a problem hiding this comment.
Because I find the use of - with MS tools looks weird 😄 It's a careless over-sight which I'll correct!
There was a problem hiding this comment.
Actually I vaguely remember some recent Microsoft C compilers warning about the use of "-o" instead of "/Fo"... Or was it for "/Fe" ? (executable name?).
There was a problem hiding this comment.
-o is indeed deprecated, but it's a synonym for /Fe - it was erroneously (well, accidentally is probably fairer) used as though it meant /Fo when building ocamlrund.
config/Makefile.msvc64
Outdated
| BYTECCDBGCOMPOPTS=-Zi | ||
|
|
||
| ### Flag to use to rename object files. (for debug version.) | ||
| NAME_OBJ_FLAG=/Fo |
|
On Oct 1, 2016 2:16 PM, "Xavier Leroy" [email protected] wrote:
No, as I recall you are correct and /Fe is where the executable name is |
f22564 (0f1669 for 4.02) changed the way debugging objects are compiled to allow parallelisation. Unfortunately, this was introduced in a way which doesn't work for the Microsoft C Compiler where -o is deprecated alias of /Fe and doesn't ever mean /Fo.
Fixes various problems with compiling ocamlrund on Windows - most notably the fact that only msvc64 actually activated debug mode!
dra27
force-pushed
the
windows-ocamlrund
branch
from
9d135fe to
d59cace
Compare
|
Rebased and |
* Fix compilation of debug objects on MSVC f22564 (0f1669 for 4.02) changed the way debugging objects are compiled to allow parallelisation. Unfortunately, this was introduced in a way which doesn't work for the Microsoft C Compiler where -o is deprecated alias of /Fe and doesn't ever mean /Fo. * Fix Windows compilation of ocamlrund/libcamlrund Fixes various problems with compiling ocamlrund on Windows - most notably the fact that only msvc64 actually activated debug mode!
* Fix compilation of debug objects on MSVC f22564 (0f1669 for 4.02) changed the way debugging objects are compiled to allow parallelisation. Unfortunately, this was introduced in a way which doesn't work for the Microsoft C Compiler where -o is deprecated alias of /Fe and doesn't ever mean /Fo. * Fix Windows compilation of ocamlrund/libcamlrund Fixes various problems with compiling ocamlrund on Windows - most notably the fact that only msvc64 actually activated debug mode!
* Fix compilation of debug objects on MSVC f22564 (0f1669 for 4.02) changed the way debugging objects are compiled to allow parallelisation. Unfortunately, this was introduced in a way which doesn't work for the Microsoft C Compiler where -o is deprecated alias of /Fe and doesn't ever mean /Fo. * Fix Windows compilation of ocamlrund/libcamlrund Fixes various problems with compiling ocamlrund on Windows - most notably the fact that only msvc64 actually activated debug mode!
* Fix compilation of debug objects on MSVC f22564 (0f1669 for 4.02) changed the way debugging objects are compiled to allow parallelisation. Unfortunately, this was introduced in a way which doesn't work for the Microsoft C Compiler where -o is deprecated alias of /Fe and doesn't ever mean /Fo. * Fix Windows compilation of ocamlrund/libcamlrund Fixes various problems with compiling ocamlrund on Windows - most notably the fact that only msvc64 actually activated debug mode!
* Fix compilation of debug objects on MSVC f22564 (0f1669 for 4.02) changed the way debugging objects are compiled to allow parallelisation. Unfortunately, this was introduced in a way which doesn't work for the Microsoft C Compiler where -o is deprecated alias of /Fe and doesn't ever mean /Fo. * Fix Windows compilation of ocamlrund/libcamlrund Fixes various problems with compiling ocamlrund on Windows - most notably the fact that only msvc64 actually activated debug mode!
* Fix compilation of debug objects on MSVC f22564 (0f1669 for 4.02) changed the way debugging objects are compiled to allow parallelisation. Unfortunately, this was introduced in a way which doesn't work for the Microsoft C Compiler where -o is deprecated alias of /Fe and doesn't ever mean /Fo. * Fix Windows compilation of ocamlrund/libcamlrund Fixes various problems with compiling ocamlrund on Windows - most notably the fact that only msvc64 actually activated debug mode!
* Fix compilation of debug objects on MSVC f22564 (0f1669 for 4.02) changed the way debugging objects are compiled to allow parallelisation. Unfortunately, this was introduced in a way which doesn't work for the Microsoft C Compiler where -o is deprecated alias of /Fe and doesn't ever mean /Fo. * Fix Windows compilation of ocamlrund/libcamlrund Fixes various problems with compiling ocamlrund on Windows - most notably the fact that only msvc64 actually activated debug mode!
* Fix compilation of debug objects on MSVC f22564 (0f1669 for 4.02) changed the way debugging objects are compiled to allow parallelisation. Unfortunately, this was introduced in a way which doesn't work for the Microsoft C Compiler where -o is deprecated alias of /Fe and doesn't ever mean /Fo. * Fix Windows compilation of ocamlrund/libcamlrund Fixes various problems with compiling ocamlrund on Windows - most notably the fact that only msvc64 actually activated debug mode!
Minor improvements
I confirmed this test fails on a version of the compiler with the bug.
…g) (ocaml#841) I confirmed this test fails on a version of the compiler with the bug.
25188da flambda-backend: Missed comment from PR802 (ocaml#887) 9469765 flambda-backend: Improve the semantics of asynchronous exceptions (new simpler version) (ocaml#802) d9e4dd0 flambda-backend: Fix `make runtest` on NixOS (ocaml#874) 4bbde7a flambda-backend: Simpler symbols (ocaml#753) ef37262 flambda-backend: Add opaqueness to Obj.magic under Flambda 2 (ocaml#862) a9616e9 flambda-backend: Add build system hooks for ocaml-jst (ocaml#869) 045ef67 flambda-backend: Allow the compiler to build with stock Dune (ocaml#868) 3cac5be flambda-backend: Simplify Makefile logic for natdynlinkops (ocaml#866) c5b12bf flambda-backend: Remove unnecessary install lines (ocaml#860) ff12bbe flambda-backend: Fix unused variable warning in st_stubs.c (ocaml#861) c84976c flambda-backend: Static check for noalloc: attributes (ocaml#825) ca56052 flambda-backend: Build system refactoring for ocaml-jst (ocaml#857) 39eb7f9 flambda-backend: Remove integer comparison involving nonconstant polymorphic variants (ocaml#854) c102688 flambda-backend: Fix soundness bug by using currying information from typing (ocaml#850) 6a96b61 flambda-backend: Add a primitive to enable/disable the tick thread (ocaml#852) f64370b flambda-backend: Make Obj.dup use a new primitive, %obj_dup (ocaml#843) 9b78eb2 flambda-backend: Add test for ocaml#820 (include functor soundness bug) (ocaml#841) 8f24346 flambda-backend: Add `-dtimings-precision` flag (ocaml#833) 65c2f22 flambda-backend: Add test for ocaml#829 (ocaml#831) 7b27a49 flambda-backend: Follow-up PR#829 (comballoc fixes for locals) (ocaml#830) ad7ec10 flambda-backend: Use a custom condition variable implementation (ocaml#787) 3ee650c flambda-backend: Fix soundness bug in include functor (ocaml#820) 2f57378 flambda-backend: Static check noalloc (ocaml#778) aaad625 flambda-backend: Emit begin/end region only when stack allocation is enabled (ocaml#812) 17c7173 flambda-backend: Fix .cmt for included signatures (ocaml#803) e119669 flambda-backend: Increase delays in tests/lib-threads/beat.ml (ocaml#800) ccc356d flambda-backend: Prevent dynamic loading of the same .cmxs twice in private mode, etc. (ocaml#784) 14eb572 flambda-backend: Make local extension point equivalent to local_ expression (ocaml#790) 487d11b flambda-backend: Fix tast_iterator and tast_mapper for include functor. (ocaml#795) a50a818 flambda-backend: Reduce closure allocation in List (ocaml#792) 96c9c60 flambda-backend: Merge ocaml-jst a775b88 flambda-backend: Fix ocaml/otherlibs/unix 32-bit build (ocaml#767) f7c2679 flambda-backend: Create object files internally to avoid invoking GAS (ocaml#757) c7a46bb flambda-backend: Bugfix for Cmmgen.expr_size with locals (ocaml#756) b337cb6 flambda-backend: Fix build_upstream for PR749 (ocaml#750) 8e7e81c flambda-backend: Differentiate is_int primitive between generic and variant-only versions (ocaml#749) git-subtree-dir: ocaml git-subtree-split: 25188da
…#841) I confirmed this test fails on a version of the compiler with the bug.
…#841) I confirmed this test fails on a version of the compiler with the bug.
…#841) I confirmed this test fails on a version of the compiler with the bug.
4 participants
This fixes the building of
ocamlrund.exeandlibcamlrun.a/libcamlrun.libon Windows. To me, each argument for putting this in 4.04 is as strong an argument for putting in trunk instead:byterun/.depend.ntcompilation is pretty trivial)./Fdoption to cl to ensure that it has a sensible name).FWIW, I'd fix the build system for 4.04 and then aim to improve the debugging support on Windows for 4.05.