Fix safestack

[mattcaswell]

We fix 3 problems with safestack:

• Including an openssl header file without linking against libcrypto
  can cause compilation failures (even if the app does not otherwise need
  to link against libcrypto). See issue The story of safestack.h and lhash.h #8102
• Recent changes means that applications in no-deprecated builds will need
  to include additional macro calls in the source code for all stacks that
  they need to use - which is an API break. This changes avoids that
  necessity.
• It is not possible to write code using stacks that works in both a
  no-deprecated and a normal build of OpenSSL. See issue sk_GENERAL_NAME_num not defined if compiled with no-deprecated #12707.

Fixes #12707
Contains a partial fix for #8102. A similar PR will be needed for hash to
fully fix.

[mattcaswell]

This is marked as WIP because make update gets confused by the header files that are now generated. Not sure what I need to do to fix that at the moment.

[richsalz]

Do you need to tweak doc/man3/DEFINE_STACK_OF.pod?

[mattcaswell]

Do you need to tweak doc/man3/DEFINE_STACK_OF.pod?

Yeah, possibly. I'll check.

[rsbeckerca]

I would like to conduct a build test of this on NonStop, when it's available.

[mattcaswell]

I would like to conduct a build test of this on NonStop, when it's available.

Please do!

[richsalz]

Does this address the 'make update' issue?

diff --git a/util/mknum.pl b/util/mknum.pl
index 871c07055b..f8ae786746 100644
--- a/util/mknum.pl
+++ b/util/mknum.pl
@@ -52,7 +52,9 @@ my %orig_names = ();
 # Invalidate all entries, they get revalidated when we re-check below
 $ordinals->invalidate();
 
-foreach my $f (($symhacks_file // (), @ARGV)) {
+foreach my $forig (($symhacks_file // (), @ARGV)) {
+    my $f = $forig;
+    $f = "$f.in" if -f "$f.in";
     print STDERR $f," ","-" x (69 - length($f)),"\n" if $verbose;
     open IN, $f || die "Couldn't open $f: $!\n";
     foreach (parse(<IN>, { filename => $f,

[levitte]

Does this address the 'make update' issue?

Nope. The C parser doesn't understand the perl snippet.
I'm working on a solution where we scan the generated headers rather than their .in template. Nearly there.

[levitte]

I've pushed a few commits (one squash) that should fix the 'make update' issues

[mattcaswell]

I've pushed a few commits (one squash) that should fix the 'make update' issues

Awesome! Thanks.

[mattcaswell]

Do you need to tweak doc/man3/DEFINE_STACK_OF.pod?

I re-read it, and I think its still accurate. The original macros still remain, and should still be used for generating custom stacks. Its only for our own stacks that appear in public headers where take a slightly different approach. So from an API perspective, nothing has changed.

[mattcaswell]

I've rebased this and taken it out of WIP now that the make update issues are resolved. Let's see what the CIs make of it.

I've fixed the nit @slontis noted above. I also pushed one fixup on top of @levitte's commits to resolve a minor error there.

Please take a look.

[rsbeckerca]

I'm going to try cloning your branch, adding my port changes, and see what happens.

[levitte]

Oops... good catch

[rsbeckerca]

The safestack compile issue went away on NonStop, however, the argument list too long problem is back. This was fixed at 1.1.1 but has returned. Is it possible that when this PR is merged that the build changes will return?

make[1]: execvp: ar: Argument list too long
Makefile:9327: recipe for target 'libcrypto.a' failed
make[1]: *** [libcrypto.a] Error 127

[mattcaswell]

The safestack compile issue went away on NonStop, however, the argument list too long problem is back. This was fixed at 1.1.1 but has returned. Is it possible that when this PR is merged that the build changes will return?

I wonder if @levitte can comment on that

[mattcaswell]

Looks like doc-nits is complaining. It doesn't like all the new macros

[rsbeckerca]

The safestack compile issue went away on NonStop, however, the argument list too long problem is back. This was fixed at 1.1.1 but has returned. Is it possible that when this PR is merged that the build changes will return?

I wonder if @levitte can comment on that

I did try a whole bunch of shell tricks to try to put $? into a file, but it's just too large for the platform's environment space to handle. I'm open to trying suggestions.

[mattcaswell]

Looks like doc-nits is complaining. It doesn't like all the new macros

Fixup pushed to ignore them. We document them as sk_TYPE_blah. Possibly we should link all the macros to that one page....but that would be a very long list.

[levitte]

#12706 should help with the ar problem

[rsbeckerca]

#12706 should help with the ar problem

Yes, we are using that in the 1.1.1 port, I believe. Perhaps we should wait for that to be merged in. @mattcaswell do you think this PR is compatible with your changes?

[mattcaswell]

Yes, we are using that in the 1.1.1 port, I believe. Perhaps we should wait for that to be merged in. @mattcaswell do you think this PR is compatible with your changes?

I don't expect there to be any significant conflict

[rsbeckerca]

I am able to build OpenSSL 3.0.0 on HPE NonStop using this fix and #12706 plus a hack to the Makefile for building libcrypto.so (same issue as with ar - the command line is too large). In any event, this fix looks good to me. I have not been able to run the regression suite yet, but I think that is independent of this fix.

[levitte]

One of the Travis jobs complains:

crypto/ex_data.c:448:6: error: expression result unused; should this cast be to 'void'? [-Werror,-Wunused-value]
    ((void *)OPENSSL_sk_set(ossl_check_void_sk_type(ad->sk), (idx), ossl_check_void_type(val)));
     ^     ~
1 error generated.

I believe, but am not entirely sure that the quick fix is this:

diff --git a/crypto/ex_data.c b/crypto/ex_data.c
index 80a136164a..f241b53f4e 100644
--- a/crypto/ex_data.c
+++ b/crypto/ex_data.c
@@ -447,7 +447,7 @@ int CRYPTO_set_ex_data(CRYPTO_EX_DATA *ad, int idx, void *val)
             return 0;
         }
     }
-    sk_void_set(ad->sk, idx, val);
+    (void)sk_void_set(ad->sk, idx, val);
     return 1;
 }
 

[richsalz]

Thank you for finishing this off (er, doing it correctly). I had no idea it would be so much work.