search

openssl / openssl

TLS/SSL and crypto library
https://www.openssl.org
Apache License 2.0
27.35k stars 10.54k forks source link

mingw after refactor-build: misc issues #625

Closed vszakats closed 9 years ago

vszakats commented 9 years ago

Testing mingw after build refactor (https://github.com/levitte/openssl/tree/refactor-build), using configuration that worked for 1.1.0-pre1/2 and 1.0.2, an early abort is the result:

Can't locate Text/Template.pm in @INC (you may need to install the Text::Template module) (@INC contains: util util/../external/perl . /usr/lib/perl5/site_perl /usr/share/perl5/site_perl /usr/lib/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib/perl5/core_perl /usr/share/perl5/core_perl) at /usr/share/perl5/core_perl/parent.pm line 20.
BEGIN failed--compilation aborted at util/dofile.pl line 45.

Ref: https://ci.appveyor.com/project/vsz/harbour-deps/build/1.0.380#L797

What needs to be done with Perl to get the required extra packages, under an AppVeyor environment?

/cc @levitte

vszakats commented 9 years ago

[ If it's not too late to suggest such, I'd argue it'd be best if no extra Perl packages would be required to configure a build. It could save from a lot of potential headaches. ]

levitte commented 9 years ago

I have bundled Text::Template in external/perl for exactly that reason. However, you should have hit this problem with master since Text::Template is already there, so this means that util/dofile.pl is messed up in refactor-build...

... and yes, it is! Seems to be a rebase goof up. Thanks for notifying me on this, I'll fix.

Cheers, Richard

vszakats commented 9 years ago

It should be fine if bundled, thanks for clarifying, and also for your response!

I'd appreciate a nudge here if the fix has landed — in order to rerun the test build.

levitte commented 9 years ago

Yup, try now

vszakats commented 9 years ago

Thank you! Build relaunched, it went past this point fine.

Next issue is when building the .dlls:

if [ -n "libcrypto.dll libssl.dll" ]; then \
    (cd ..; mingw32-make libcrypto.dll); \
fi
mingw32-make[2]: Entering directory 'C:/w/openssl'
mingw32-make[3]: Entering directory 'C:/w/openssl'
/usr/bin/sh: -c: line 23: syntax error near unexpected token `fi'
/usr/bin/sh: -c: line 23: ` fi; \'
Makefile:326: recipe for target 'do_cygwin-shared' failed
mingw32-make[3]: *** [do_cygwin-shared] Error 1

Ref: https://ci.appveyor.com/project/vsz/harbour-deps/build/1.0.381#L1978

(Let me know if you'd like this to be opened as a separate Issue)

levitte commented 9 years ago

Oh what a silly little typo! Fixed.

levitte commented 9 years ago

I retitled the issue to be more accurate. Totally fine with me to use it as a discussion forum around this.

vszakats commented 9 years ago

With the typo fixed, the build proceeds fine in both 32-bit and 64-bit modes, thank you very much for the fixes!

Even after the build refactor, one must patch the build scripts to make it work with certain mingw distros, because the CPU target is not specified when calling the windres tool.

Would you mind taking a look at this Issue, that has all the details?: https://github.com/openssl/openssl/issues/585

There are some new, Windows-specific warnings in code committed since 1.1.0-pre2, which is unrelated to the build refactoring, perhaps they are interesting:

https://ci.appveyor.com/project/vsz/harbour-deps/build/1.0.382

[00:03:29] b_sock2.c:135:56: warning: passing argument 4 of 'setsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:03:29] b_sock2.c:143:56: warning: passing argument 4 of 'setsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:03:29] b_sock2.c:207:47: warning: passing argument 4 of 'getsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:03:29] b_sock2.c:230:56: warning: passing argument 4 of 'setsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:03:29] b_sock2.c:238:56: warning: passing argument 4 of 'setsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:03:29] b_sock2.c:247:57: warning: passing argument 4 of 'setsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:05:39] s3_lib.c:162:33: warning: 'ssl3_ciphers' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
[00:05:47] ssl_lib.c:1411:12: warning: return makes integer from pointer without a cast [-Wint-conversion]
[00:08:17] formdata.c:390:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:10:54] ../include/internal/md32_common.h:262:75: warning: right-hand operand of comma expression has no effect [-Wunused-value]
[00:11:17] bn_lcl.h:185:27: warning: unknown conversion type character 'l' in format [-Wformat=]
[00:11:17] bn_lcl.h:185:27: warning: too many arguments for format [-Wformat-extra-args]
[00:11:17] bn_lcl.h:186:27: warning: unknown conversion type character 'l' in format [-Wformat=]
[00:11:17] bn_lcl.h:186:27: warning: too many arguments for format [-Wformat-extra-args]
[00:12:20] b_sock.c:296:13: warning: overflow in implicit constant conversion [-Woverflow]
[00:12:20] b_sock.c:301:16: warning: overflow in implicit constant conversion [-Woverflow]
[00:12:20] b_sock.c:304:16: warning: overflow in implicit constant conversion [-Woverflow]
[00:12:20] b_sock.c:311:13: warning: overflow in implicit constant conversion [-Woverflow]
[00:12:20] b_sock.c:318:13: warning: overflow in implicit constant conversion [-Woverflow]
[00:12:21] b_sock2.c:90:16: warning: overflow in implicit constant conversion [-Woverflow]
[00:12:21] b_sock2.c:96:16: warning: overflow in implicit constant conversion [-Woverflow]
[00:12:21] b_sock2.c:135:56: warning: passing argument 4 of 'setsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:12:21] b_sock2.c:143:56: warning: passing argument 4 of 'setsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:12:21] b_sock2.c:207:47: warning: passing argument 4 of 'getsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:12:21] b_sock2.c:230:56: warning: passing argument 4 of 'setsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:12:21] b_sock2.c:238:56: warning: passing argument 4 of 'setsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:12:21] b_sock2.c:247:57: warning: passing argument 4 of 'setsockopt' from incompatible pointer type [-Wincompatible-pointer-types]
[00:12:21] b_sock2.c:288:16: warning: overflow in implicit constant conversion [-Woverflow]
[00:12:21] b_sock2.c:292:16: warning: overflow in implicit constant conversion [-Woverflow]
[00:15:34] s3_lib.c:162:33: warning: 'ssl3_ciphers' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
[00:15:44] ssl_lib.c:1411:12: warning: return makes integer from pointer without a cast [-Wint-conversion]
[00:17:18] s_socket.c:192:21: warning: overflow in implicit constant conversion [-Woverflow]
levitte commented 9 years ago

Cool! But, do you know that you haven't tested the new build system yet? Did you notice the nudge that Configure gives you, basically saying this?

Please consider configuring with the flag --unified .
It's to test out a new "unified" building system.

C'mon, I dare ya! ;-)

vszakats commented 9 years ago

Ah, okay, I didn't :) Let me try that!

vszakats commented 9 years ago

With --unified added to the ./Configure command-line, this is the result, right after make depend:

+ mingw32-make depend
Makefile:579: target 'libeay32.dll' given more than once in the same rule
Makefile:2948: target 'ssleay32.dll' given more than once in the same rule
mingw32-make: *** No rule to make target '/c/w/openssl/Configurations/unix-Makefile.tmpl', needed by 'Makefile'.  Stop.
+ mingw32-make
Makefile:579: target 'libeay32.dll' given more than once in the same rule
Makefile:2948: target 'ssleay32.dll' given more than once in the same rule
mingw32-make: *** No rule to make target '/c/w/openssl/Configurations/unix-Makefile.tmpl', needed by 'Makefile'.  Stop.

Notice that I'm making some minor and necessary patches to the build system, though they seem to be harmless/unrelated: https://github.com/vszakats/harbour-deps/blob/test/openssl.diff

levitte commented 9 years ago

Would you mind placing the resulting Makefile in gist? and linking to it? I'm quite curious at the result

sav-ix commented 9 years ago

Hi. @vszakats , if you don't mind, few questions concerning your patch:

Thanks.

levitte commented 9 years ago

Oh good, wasn't sure @sav-ix was present here. Introductions everyone, @sav-ix is the other person who's been talking mingw with me, and who's behind https://gist.github.com/levitte/4ec9a909bbf3a31c77a6

richsalz commented 9 years ago

Any plans to address these tickets as well?

https://rt.openssl.org/Ticket/Display.html?id=2330 https://rt.openssl.org/Ticket/Display.html?id=3579 https://rt.openssl.org/Ticket/Display.html?id=4083 https://rt.openssl.org/Ticket/Display.html?id=3991

☺

levitte commented 9 years ago

Might as well ;-)

vszakats commented 9 years ago

I've started a build, will take the live Makefile and upload it to a Gist.

@sav-ix

Two reasons for not setting options via OpenSSL/Configure:

Let's take them one-by-one:

what sense in string "-march=i686 -mtune=generic" (different options for 'march' and 'mtune'), when "Specifying -march=cpu-type implies -mtune=cpu-type" (https://gcc.gnu.org/onlinedocs/gcc- 5.3.0/gcc/x86-Options.html#x86-Options)?

You're right the second one is unnecessary in this specific case. Current values are a personal choice and they might change as time goes by — that's the only light reason they are both included. In a default configuration however, I think the current i486 setting should not be there.

levitte commented 9 years ago

Two reasons for not setting options via OpenSSL/Configure:

  • I don't know how to (sorry), and was probably too lazy to find out up until this point. If you guys can suggest the best way, I'm all for using it instead.
  • some of these options are required, otherwise the build will be broken or not optimal.

Simple, you add them on the Configure command line. Configure will take anything it doesn't recognise and add them as 1/ LDFLAGS if the start with -Wl,, -L or -l, or 2/ to CFLAGS otherwise. Very simple

vszakats commented 9 years ago

Thank you! I'll update my script to take advantage of that. However, for this to work cleanly and without making the build output unnecessarily verbose, it'd really be good if the CPU customisation option would be stripped from default options.

sav-ix commented 9 years ago

In addition, I'd recommend adding -m32 and -m64 to the mingw, mingw64 targets respectively

It would be nice, as MinGW-W64 starting from version 5.. enabled support for -m32 and -m64 parameters. I didn't added them in patch, as didn't tested them for OpenSSL builds using MinGW-W64 4.. and before.

vszakats commented 9 years ago

@sav-ix mingw-w64 supported the -m64 flag right from its first public release (about 6 years ago). I've made tests with the oldest 32-bit mingw I could find (can't remember the exact version (4.x or 3.x ?), sorry) and -m32 was supported by it. For sure it is not a 5.x feature. I think it's safe to enable these for both targets.

vszakats commented 9 years ago

@levitte Makefile, for mingw64, from this live build: https://ci.appveyor.com/project/vsz/harbour-deps/build/1.0.386

(.txt extension added to let it be attached here) Makefile-mingw64.txt

levitte commented 9 years ago

Cool. I see what's happeniing. But guys, I'm starting to be a bit blurry eyed here, time to call it in, and I'll look things over tomorrow.

Cheers

sav-ix commented 9 years ago

mingw-w64 supported the -m64 flag right from its first public release (about 6 years ago) ... and -m32 was supported by it

Oddly enough. Some time ago I tested cross compiling:

but they didn't work for me (and not only for me). Here another samples from Google:

Thus in some MinGW-W64 4.x.x distributions parameters '-m32' and '-m64' are just a stubs. But things changes with release MinGW-W64 5.x.x (at least for me).

This don't exclude, that distributions, which you've used, supported cross compiling from versions 4.x or 3.x. Unfortunately they did not get my hands on.

UPD.

Simple, you add them on the Configure command line. Configure will take anything it doesn't recognise and add them as 1/ LDFLAGS if the start with -W,l, -L or -l, or 2/ to CFLAGS. Very simple

BTW, adding this info to Wiki and some 'OpenSSL/Install*' files could be helpful for many OpenSSL users.

vszakats commented 9 years ago

@richsalz

vszakats commented 9 years ago

@sav-ix My impression is that the linked discussions and results are or might well be the result of some misconceptions. In the linked discussions posters expect dual-target functionality with a toolchain that only supports a single target, in such cases it's expected that selecting the other target would result in an error. AFAIR in 2009 there didn't exist a dual-target mingw distro.

To sum up:

-m32 is supposed to work in either dual-target or 32-bit hosted mingw distros (silently ignored in the latter case). -m64 is supposed to work in either dual-target or 64-bit hosted mingw distros (silently ignored in the latter case).

No other combination will work of course, and a compiler error is expected, avoiding a wait for a seemingly correct build only to find out at the end that it was created for the wrong target.

My experience throughout the years was with official 32-bit mingw releases, then mingw-w64 releases (the first ones that came with 64-bit support), later TDM releases (single target), later niXman builds, which are now merged back into mingw-w64 and also come in dual-target flavour.

Maybe useful information that some popular projects like curl have endorsed these options back in 2014, and another project I'm involved in, first in 2010, then finalised it in 2013. I haven't heard of any fallout as a result.

Anyhow I have no issue with supplying these options as custom ones.

[ Apart from this, the windres issue remans to be resolved, as it also needs the CPU target option (in dual-target scenarios), and currently there is no way to configure that cleanly. ]

richsalz commented 9 years ago

Thanks for looking into it. I think Richard will take on the work of fully addressing and closing them. I hope :)

sav-ix commented 9 years ago

@vszakats, since you reviewed tickets concerning OpenSSL builds using MinGW*, could you also reproduce OpenSSL-1.1.0-pre2 build error using MinGW-W64 <i686,x86_64> with -std=c11 compiler parameter (details at https://rt.openssl.org/Ticket/Display.html?id=4255), which not inherent to builds using -std=gnu11? Thanks.

Anyhow I have no issue with supplying these options as custom ones.

With moving to MinGW-W64 5.x.x I have similar experience. The only opened question, what exception handling would be used:

as seems that dwarf2 not supported on Windows x64 and seh not supported on Windows x86.

vszakats commented 9 years ago

@sav-ix dual-target mingw distros I've so far tested (@ https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win[32|64]/Personal%20Builds/mingw-builds/) supported the SJLJ exception handling for both 32-bit and 64-bit modes, so in above two cases the result will be both SJLJ. It also means that if you specifically need DWARF2 or SEH, you need to use the single-target distros instead. (still can supply the -m32/-m64 options for 32/64 bit builds respectively — they will be no-ops in these cases.)

as seems that dwarf2 not supported on Windows x64 and seh not supported on Windows x86.

Indeed. SJLJ is the only one that's supported on both 32 and 64-bit Windows. On the other hand SEH is the ideal solution solution on 64-bit and DWARF2 is the ideal solution on 32-bit. I'm not sure what are reasons that a dual-target mingw build doesn't or can't support SEH for 64-bit and DWARF2 for 32-bit — that would be the most ideal combination. Right now one has to balance between convenience and the better exception handling methods.

For further reading: https://wiki.qt.io/MinGW-64-bit#Exception_handling:_SJLJ.2C_DWARF.2C_and_SEH https://stackoverflow.com/questions/15670169/what-is-difference-between-sjlj-vs-dwarf-vs-seh

As for the -std=c11 setting, like @levitte I'm missing the compiler error message that lead to the failure from the bug report.

After making tests, I got the following error right away:

gcc: error: unrecognized command line option '-std=C11'

After correcting the option to be lower-case, I could reproduce the problem, see the log: https://ci.appveyor.com/project/vsz/harbour-deps/build/1.0.391#L1370

The error details indicate that some source code is not compliant with the C11 standard and therefore fail to compile in this mode. It doesn't seem to be gcc/mingw/Windows/64-bit specific:

In file included from md4_locl.h:87:0,
                 from md4_dgst.c:62:
md4_dgst.c: In function 'md4_block_data_order':
../md32_common.h:167:33: warning: implicit declaration of function 'asm' [-Wimplicit-function-declaration]
                                 asm (                   \
                                 ^
md4_locl.h:105:11: note: in expansion of macro 'ROTATE'

Try with -std=gnu11 instead to leave some optional C extensions enabled, like inline asm support. Current OpenSSL code seems to require it.

vszakats commented 9 years ago

@levitte I've changed my build configuration to pass custom options via ./Configure. It appears to work well, and as a side-effect it seems to have enabled LTO when building the .dlls and openssl.exe. I couldn't find a trivial way to confirm this though, because the .dll and .exe link command-lines are missing from the log output. Do you think it would be possible to echo these to make these steps transparent?

levitte commented 9 years ago

I think this answers your question. It's a commit that's currently undergoing internal review:

https://gist.github.com/levitte/cfbef47bac64f0a30944

sav-ix commented 9 years ago

@vszakats, I can't say that did not seen that articles about MinGW exception handling. But (until your previous post) I have never seen the confirmation that sjlj used as default exception handling for cross-compiling with MinGW-W64, thanks. Although it's obvious that this is the only possible option.

Concerning -std=C11 in upper-case, that was my bad. Unfortunately it's impossible to correct it after publishing on bugtracker. But in my previous post I initially posted it in lower-case.

Try with -std=gnu11 instead to leave some optional C extensions enabled

Currently I bypass this error exactly in this way. Just wondering, why using -std=c11 parameter (which should expand the compiler capabilities) broke OpenSSL build. And whether could this be reported as OpenSSL error.

vszakats commented 9 years ago

@levitte That's perfect, can't wait to try it. Thank you very much!

vszakats commented 9 years ago

@sav-ix I wouldn't call it an error per se. Fully standard compliant modes are rarely used in real-life projects, and I assume this was not a goal when creating this piece of code. Speaking of this specific error, IMO it's also generally cleaner not to mix languages inside the same source file. I can't see the full scope of this in case of OpenSSL, but it'd probably be useful to report this and suggest to move assembly code into separate .s files. Such practice is already used in quite a few places, so this exception may well be an oversight or heritage waiting to be updated.

levitte commented 9 years ago

@vszakats, regarding flags for windres, I'm thinking I'll add another configuration setting for it... shared_rcflag perhaps? What should it be populated with in the mingw and the mingw64 config targets?

levitte commented 9 years ago

Listening to the two of you, this is what I've come up with so far. Those are patches formatted with git format-patch

https://gist.github.com/levitte/b0e531f36ed6beecb38f

Please have a look, and please give me suggestions on what to fill shared_rcflag => "" with.

sav-ix commented 9 years ago

@levitte patch look nice, waiting for opportunity to test it. Could you answer rest of questions, I've asked you in letter OpenSSL builds using MinGW-W64, 27/01/2016? Thanks.

levitte commented 9 years ago

  • https://rt.openssl.org/Ticket/Display.html?id=3579
    My current builds are running under the msys found in Git for Windows (uname -a = MSYS_NT-10.0) and they are apparently working fine, possibly because they explicitly select the targets (?). The patch seems fine, though I had found this shell detection code to work with all Windows shell ports and variants (busybox, cygwin, msys1, msys2, GfW), to differentiate from cygwin, the CYGWIN* match must be made first:

    case "$(uname)" in
CYGWIN*) echo 'Cygwin';;
*_NT*) echo 'POSIX shell under Windows';;
esac

So you're telling me this should be a workable patch?

diff --git a/config b/config
index a70e3a2..0429718 100755
--- a/config
+++ b/config
@@ -320,16 +320,17 @@ case "${SYSTEM}:${RELEASE}:${VERSION}:${MACHINE}" in
    echo "${MACHINE}-v11-${SYSTEM}"; exit 0;
    ;;

-    MINGW*)
-   echo "${MACHINE}-whatever-mingw"; exit 0;
-   ;;
-    CYGWIN*)
-   echo "${MACHINE}-pc-cygwin"; exit 0
-   ;;
-
     vxworks*)
        echo "${MACHINE}-whatever-vxworks"; exit 0;
        ;;
+
+    # CYGWIN* MUST come before *_NT*
+    CYGWIN*)
+        echo "${MACHINE}-pc-cygwin"; exit 0
+        ;;
+    *_NT*) echo 'POSIX shell under Windows';;
+        echo "${MACHINE}-whatever-mingw"; exit 0;
+   ;;
 esac

 #
levitte commented 9 years ago

@sav-ix, I'm getting to stuff in due time, and for the moment, my time is running short. I need to pause from this, and will get back after the weekend. Sounds like a deal?

sav-ix commented 9 years ago

@levitte, You're the boss :)

levitte commented 9 years ago

Oh, I suspect I won't be the only boss ;-)

vszakats commented 9 years ago

@levitte shared_rcflag is the perfect solution for this. Its value should be:

(long vs. short form is a matter of style, whichever you prefer)

vszakats commented 9 years ago

@levitte Regarding the MSYS detection — my solution is not "battle-tested" (on a wide range of non-Windows platforms), so there is a minor chance for regressions, and it will also catch things like busybox.exe (uname = Windows_NT), which may not be supported by the build system. Having said that, I don't mind giving it a try on the battlefield. For a more conservative approach, the patch found in the bug report is a good alternative.

vszakats commented 9 years ago

Some follow-up on the option standardisation: @levitte's echo modifications helped to reveal that LTO was not a factor in the updated builds, instead the difference was caused by dropping -mtune=generic. It means this options does make a difference (at least when used together with i686), though it's exact effect on generated code has yet to be determined.

vszakats commented 9 years ago

[ Settled to keep -mtune=generic ]

levitte commented 9 years ago

Regarding -mno-cygwin and a bit more, here's another opinion (and I quote):

Well, whatever they say it doesn't feel right. -fomit-frame-pointer for mingw64? More harm than improvement (if any). Remove applink from 32-bit? It means that they prevent users from mixing dlls and applications. Well, they are likely to argue that nobody does it, but doesn't it really only mean that they wouldn't ever do it? For reference, what's -mno-cygwin flag is about? It's about compiling for mingw with cygwin compiler. Mingw compiler did support -mno-cygwin as do-nothing for compatibility, but stopped as far as I understand. But once again, this option is not about mingw compiler, it's about cygwin compiler...

levitte commented 9 years ago

I've added the --target args to the respective shared_rflags

levitte commented 9 years ago

In all your deliberations about -mtune=generic and stuff, it's not entirely clear to me if they should become part of OpenSSL's settings for mingw/minge64 or if you're talking about your own stuff. Could you clarify what should be added to OpenSSL's configurations, if any?

sav-ix commented 9 years ago

For reference, what's -mno-cygwin flag is about? It's about compiling for mingw with cygwin compiler ... this option is not about mingw compiler, it's about cygwin compiler...

as I wrote you in comments to sent patch:

-fomit-frame-pointer for mingw64? More harm than improvement (if any)

Some projects I use (gmp) set it by default for MinGW-W64 x86_64 configuration. I had no troubles using this parameter for x64 platform, but if someone has a different experience, it is wise to not make this change,

Remove applink from 32-bit? It means that they prevent users from mixing dlls and applications

Well, I had negative experience in combining binaries, built using MinGW-W64 and Microsoft\Intel compilers (it's not about OpenSSL). But if applink could count differences with .dll built using MinGW-W64 (exception handling <dwarf2,sjlj> for i686, threading models <posix,Win32>, ability to use own implementation of io, see USE_MINGW_ANSI_STDIO) and applications built using Microsoft/Intel compilers, if that for sure won't be a source of hardly detected errors, then it makes sense to use it (although I'm still going to remove it for my builds and use the same compiler to build OpenSSL .dll and depending apps).

In all your deliberations about -mtune=generic and stuff, it's not entirely clear to me if they should become part of OpenSSL's settings for mingw/minge64 or if you're talking about your own stuff.

I agree with @vszakats , that "it'd really be good if the CPU customisation option would be stripped from default options". Users can add them as 'OpenSSL/Configure' parameters. Although adding autodetection for -march= and -mtune= parameters for GCC and GCC-like compilers could be a significant improvement for OpenSSL build system.

vszakats commented 9 years ago

@levitte -mtune=generic: I was speaking of my own custom settings. No -mtune= and no -march= should go into the default config.