etcimon / botan

Block & stream ciphers, public key crypto, hashing, KDF, MAC, PKCS, TLS, ASN.1, BER/DER, etc.
Other
85 stars 22 forks source link

[WIP] Try to get OPTLINK to work #10

Closed s-ludwig closed 8 years ago

s-ludwig commented 8 years ago

Work in progress! This splits up the library into sub packages, so that each part has a more reasonable size. Unfortunately most modules depend on each other, so that only a few can be split off. It also changes some const constants to enum, because those don't end up as symbols in the final .lib.

The current result is that the tests can run sucessfully on Win32/OPTLINK. Using vibe.d with Botan now results in an unexpected optlink termination instead of an out-of-memory error.

The question is if there are still obvious places to try to reduce the linker pressure, apart from refactoring the code to avoid all those mutual dependencies. Maybe the unexpected termination error should also be reduced and reported on issues.dlang.org, but I don't currently have time for that.

etcimon commented 8 years ago

Would inlining help reduce the number of symbols? I think DMD had pragma(inline) or something, it was the next thing I was going to do for botan.math.* to improve performance issues in public key cryptography, but it could also be used for the template mixins that imitate macro logic in most blocks/hash modules.

s-ludwig commented 8 years ago

I think there was a discussion where it was said that inlined functions currently still result in a full non-inlined copy, but I'm not sure. But for the string mixins it might help to convert from

string AES_ENC_4_ROUNDS(alias K)()
{ 
    const K2 = __traits(identifier, K);
    return `B0 = _mm_aesenc_si128(B0, ` ~ K2 ~ `);
            B1 = _mm_aesenc_si128(B1, ` ~ K2 ~ `);
            B2 = _mm_aesenc_si128(B2, ` ~ K2 ~ `);
            B3 = _mm_aesenc_si128(B3, ` ~ K2 ~ `);`;
}

to

enum AES_ENC_4_ROUNDS(alias K) = q{
    B0 = _mm_aesenc_si128(B0, %1$s);
    B1 = _mm_aesenc_si128(B1, %1$s);
    B2 = _mm_aesenc_si128(B2, %1$s);
    B3 = _mm_aesenc_si128(B3, %1$s);
}.format(__traits(identifier, K));
etcimon commented 8 years ago

That looks much nicer too!

The second biggest problem is with the different Allocators used. For each of them, there's a flavor of every container and every type with every method, using reference counting or not. I think memutils would be where the hunting takes place for stripping out this stuff

s-ludwig commented 8 years ago

Tried this with the aes_ni.d module and it reduced the .lib size by about 3 KB, not much, but it seems to work.

s-ludwig commented 8 years ago

For some reason the binary size went up again as I did this for more modules... Maybe that's not the ideal measure. Do you have something to directly create a dump of the .lib contents?

etcimon commented 8 years ago

My method is a little primitive. I think I was using this:

https://github.com/D-Programming-Language/dmd/blob/master/src/backend/symbol.c#L115

You pipe it to a file and change it a little to get a one-line-per-symbol.

s-ludwig commented 8 years ago

Thanks, I'll try that. BTW, with vibe.d master, applications that use Botan now seem to compile fine. Just the http2-botan-cleanup branch still results in an out-of-memory error.

etcimon commented 8 years ago

That's awesome! The OOM might be because of that "create self signed certificate" code in botan stream, but I'm not sure. Btw, the task message queue was causing segfaults on my end because the Circular Buffer destroys Variant.init resulting in a null pointer access so I did this: https://github.com/etcimon/memutils/commit/d268885d7fb4223948521a2ac6210d5c1d44c0cc

Should I write a pull request for this?

s-ludwig commented 8 years ago

Okay, all mixins are replaced now. A build of h2_server without debug information links successfully, too.

I remember that commit for the ring buffer from somewhere, but something strange must be going on. Calling destroy in T.init should always be legal (or this code would also crash: { Variant v; }). But I've seen at least two other issues in the code: popFront and popBack fail to call destroy, and removeAt for T == struct* destroys the object (which may already be wrong), but doesn't reset the pointer to null.

s-ludwig commented 8 years ago

BTW, I'd replace the destroy in removeAt by a simple T.init assignment. A container type should usually not have to worry about explicit destruction. Do you think you can produce a reproduction case for the Variant issue? Simply adding some Variants and then letting the buffer go out of scope?

etcimon commented 8 years ago

Do you think you can produce a reproduction case for the Variant issue? Simply adding some Variants and then letting the buffer go out of scope?

Hm, I actually fixed it through a stack trace and now I can't reproduce it at all. It was under a very heavy load server and I remember the object pointer in the Variant.~this being null there. After trying to find it through a lot of tests, it's starting to make more sense as a concurrency issue.