jens-maus / amissl

:closed_lock_with_key: AmiSSL is the AmigaOS/MorphOS/AROS port of OpenSSL. It wraps the full functionality of OpenSSL into a full-fledged Amiga shared library that makes it possible for Amiga applications to use the full OpenSSL API through a standard Amiga shared library interface (e.g. web browsers wanting to support HTTPS, etc.)...
Apache License 2.0
88 stars 15 forks source link

vbcc proto/inline issues #56

Closed patrikaxelsson closed 2 years ago

patrikaxelsson commented 3 years ago
  1. By just including proto/amissl.h, the resulting binary will increase 40kBytes+, unless at least -O3 is used:

    $ cat EmptyAmiSSLTest.c
    #include <proto/amissl.h>
    
    struct Library *AmiSSLBase;
    
    int EmptyAmiSSLTest(void) {
        return 0;
    }
    $ make EmptyAmiSSLTest EmptyAmiSSLTest_O3                                   
    vc +aos68k -nostdlib -c99 -O2 -lvc -IAmiSSL/Developer/include -o EmptyAmiSSLTest EmptyAmiSSLTest.c
    vc +aos68k -nostdlib -c99 -O3 -lvc -IAmiSSL/Developer/include -o EmptyAmiSSLTest_O3 EmptyAmiSSLTest.c
    $ ls -la EmptyAmiSSLTest EmptyAmiSSLTest_O3                                   
    -rwxr-xr-x  1 patrik  access_bpf  43672  2 Sep 22:12 EmptyAmiSSLTest*
    -rwxr-xr-x  1 patrik  access_bpf     56  2 Sep 22:12 EmptyAmiSSLTest_O3*

    This is not normal when using libraries with vbcc or any compiler on/for the Amiga. Also, apart from building very slowly when proto/amissl.h is included, I noticed that it consumes a lot of memory to build this nop-exe - a 16MB Amiga was not enough.

  2. If a local instead of global AmiSSLBase is used (together with -DNOLIBBASE or not), it will not compile or link:

    $ cat LocalBaseAmiSSLTest.c 
    #include <proto/amissl.h>
    
    int LocalBaseAmiSSLTest(void) {
        struct Library *AmiSSLBase;
        return 0;
    }
    $ make LocalBaseAmiSSLTest
    vc +aos68k -nostdlib -c99 -O2 -lvc -D__NOLIBBASE__ -IAmiSSL/Developer/include -o LocalBaseAmiSSLTest LocalBaseAmiSSLTest.c
    >DEFINE_STACK_OF(SCT)
    error 82 in line 131 of "amissl/inline.h": unknown identifier <AmiSSLBase>
        included from file "AmiSSL/Developer/include/proto/amissl.h":79
        included from file "LocalBaseAmiSSLTest.c":1
    >DEFINE_STACK_OF(SCT)
    error 82 in line 131 of "amissl/inline.h": unknown identifier <AmiSSLBase>
        included from file "AmiSSL/Developer/include/proto/amissl.h":79
        included from file "LocalBaseAmiSSLTest.c":1
    >DEFINE_STACK_OF(SCT)
    error 82 in line 131 of "amissl/inline.h": unknown identifier <AmiSSLBase>
        included from file "AmiSSL/Developer/include/proto/amissl.h":79
        included from file "LocalBaseAmiSSLTest.c":1
    >DEFINE_STACK_OF(SCT)
    error 82 in line 131 of "amissl/inline.h": unknown identifier <AmiSSLBase>
        included from file "AmiSSL/Developer/include/proto/amissl.h":79
        included from file "LocalBaseAmiSSLTest.c":1
    >DEFINE_STACK_OF(SCT)
    error 82 in line 131 of "amissl/inline.h": unknown identifier <AmiSSLBase>
        included from file "AmiSSL/Developer/include/proto/amissl.h":79
        included from file "LocalBaseAmiSSLTest.c":1
    >DEFINE_STACK_OF(SCT)
    error 82 in line 131 of "amissl/inline.h": unknown identifier <AmiSSLBase>
        included from file "AmiSSL/Developer/include/proto/amissl.h":79
        included from file "LocalBaseAmiSSLTest.c":1
    >DEFINE_STACK_OF(SCT)
    error 82 in line 131 of "amissl/inline.h": unknown identifier <AmiSSLBase>
        included from file "AmiSSL/Developer/include/proto/amissl.h":79
        included from file "LocalBaseAmiSSLTest.c":1
    >DEFINE_STACK_OF(SCT)
    error 82 in line 131 of "amissl/inline.h": unknown identifier <AmiSSLBase>
        included from file "AmiSSL/Developer/include/proto/amissl.h":79
        included from file "LocalBaseAmiSSLTest.c":1
    >DEFINE_STACK_OF(SCT)
    error 82 in line 131 of "amissl/inline.h": unknown identifier <AmiSSLBase>
        included from file "AmiSSL/Developer/include/proto/amissl.h":79
        included from file "LocalBaseAmiSSLTest.c":1
    >DEFINE_STACK_OF(SCT)
    error 82 in line 131 of "amissl/inline.h": unknown identifier <AmiSSLBase>
        included from file "AmiSSL/Developer/include/proto/amissl.h":79
        included from file "LocalBaseAmiSSLTest.c":1
    Maximum number of errors reached!
    unexpected end of file
    10 errors found!
    vbccm68k -quiet -hunkdebug "LocalBaseAmiSSLTest.c" -o= "/var/tmp/tmp.0.asm" -c99 -D__NOLIBBASE__ -IAmiSSL/Developer/include  -O=1023 -I$VBCC/targets/m68k-amigaos/include -I/opt/sdk/NDK_3.9/Include/include_h failed
    make: *** [LocalBaseAmiSSLTest] Error 1

    Also not normal and I think this has something to do with the first issue.

AmiSSLTest.zip

Futaura commented 3 years ago

You mention "normal" a lot - unfortunately OpenSSL is not normal!

The helper functions are defined that way in OpenSSL, as static inline functions. Macros could in theory be used instead, but then you'd lose all the type checking and tracing, which would make debugging difficult. Unfortunately, VBCC does not have an "inline" keyword for forcing functions to be inlined, unlike GCC (and SAS/C incidentally), so I guess -O3 is your only option there.

As these inline functions simply call OpenSSL functions, AmiSSLBase has to be a global variable - I don't see a way around that.

One alternative to inline functions would be to create a link library instead, but still this approach would require AmiSSLBase to be a global variable too, if those functions are used.

Futaura commented 3 years ago

BTW, I guess you could try the following, if you do not need those helper functions:

#define AMISSL_INLINE_H
#include <proto/amissl.h>

Everything in amissl/inline.h is actually a copy of the corresponding code from openssl/safestack.h and openssl/lhash.h, so that's where those inline functions originate in OpenSSL.

patrikaxelsson commented 3 years ago

Thank you very much - when AMISSL_INLINE_H is defined, none of the issues mentioned above appear when including proto/amissl.h:

$ make EmptyAmiSSLTest EmptyAmiSSLTest_O3 LocalBaseAmiSSLTest
vc +aos68k -nostdlib -c99 -O2 -lvc -DAMISSL_INLINE_H -IAmiSSL/Developer/include -o EmptyAmiSSLTest EmptyAmiSSLTest.c
vc +aos68k -nostdlib -c99 -O3 -lvc -DAMISSL_INLINE_H -IAmiSSL/Developer/include -o EmptyAmiSSLTest_O3 EmptyAmiSSLTest.c
vc +aos68k -nostdlib -c99 -O2 -lvc -DAMISSL_INLINE_H -D__NOLIBBASE__ -IAmiSSL/Developer/include -o LocalBaseAmiSSLTest LocalBaseAmiSSLTest.c
$ ls -la EmptyAmiSSLTest EmptyAmiSSLTest_O3 LocalBaseAmiSSLTest
-rwxr-xr-x  1 patrik  access_bpf  56  4 Sep 00:19 EmptyAmiSSLTest*
-rwxr-xr-x  1 patrik  access_bpf  56  4 Sep 00:19 EmptyAmiSSLTest_O3*
-rwxr-xr-x  1 patrik  access_bpf  40  4 Sep 00:19 LocalBaseAmiSSLTest*

The https.c example included in AmiSSL builds with AMISSL_INLINE_H defined (and works), so I guess it is still functional enough for common use without those helper functions?

Futaura commented 3 years ago

Sure. You'd get a linker error anyway if the code you are compiling actually needs those functions. IBrowse needs them for user certificate handling, for example, but for basic HTTPS they are not required.

patrikaxelsson commented 3 years ago

Would it perhaps be sensible to make amissl/inline.h an opt-in include instead, as it is not needed in the general case and it gives these side-effects?

Apart from the unexpected behavior, the build time and memory consumption makes it impossible or very impractical to build on a real Amiga.

Futaura commented 3 years ago

@patrikaxelsson I don't think so. I think we need to leave the maximum OpenSSL compatibility enabled by default.

I can compile IBrowse perfectly fine for OS3 with SAS/C on my A1200 and for OS4 with GCC on my A1XE, without needing to disable these headers (in fact, IBrowse needs some of those functions). AmiSSL itself is even buildable on my A1XE.

I shall keep this in mind with OpenSSL 3.0 though, which I am currently trying to port - at first glance, it seems they've made the safestack/lhash inline stuff even more complicated, but we shall see.

patrikaxelsson commented 2 years ago

AMISSL_INLINE_H worked great as a workaround for this issue in 4.x, closing.