Skip to content
This repository has been archived by the owner on Apr 15, 2020. It is now read-only.

add stormlib package + example #877

Merged
merged 1 commit into from
Aug 26, 2017
Merged

add stormlib package + example #877

merged 1 commit into from
Aug 26, 2017

Conversation

wheybags
Copy link
Contributor

Can be moved to a hunter-packages fork

@ruslo
Copy link
Owner

ruslo commented Jul 17, 2017

@wheybags
Copy link
Contributor Author

moved

@ruslo
Copy link
Owner

ruslo commented Jul 18, 2017

/home/travis/build/ingenue/hunter/examples/stormlib/main.cpp:1:22: fatal error: Stormlib.h: No such file or directory

@wheybags
Copy link
Contributor Author

Should be fixed now. Their example code had the wrong case in the header filename. windows...

@ruslo
Copy link
Owner

ruslo commented Jul 21, 2017

Undefined symbols for architecture x86_64:
  "_ltc_mp", referenced from:

@wheybags
Copy link
Contributor Author

wheybags commented Aug 1, 2017

Ok, finally got an osx vm up to debug this, and I don't get what's going on here at all. It works on every other platform, and I used an object inspector to make sure the symbol was there and it is.
nm -gU on the libtomcrypt static lib outputs:

/Users/wheybags/stormlib/build/hunter/_Base/xxxxxxx/f77a954/3b0b532/Install/lib/libtomcryptd.a(crypt_ltc_mp_descriptor.c.o):
0000000000000188 C _ltc_mp

It's just a struct, declared here: https://github.com/hunter-packages/libtomcrypt/blob/1.17-p1/src/misc/crypt/crypt_ltc_mp_descriptor.c

and declared as extern here: https://github.com/hunter-packages/libtomcrypt/blob/1.17-p1/src/headers/tomcrypt_math.h#L416

Is there some weirdness on osx around extern structs? Do I need to do some equivalent of the windows style declspec dllimport crap?

@ruslo any idea what's going on here, because I'm stumped :(

@ruslo
Copy link
Owner

ruslo commented Aug 1, 2017

Library source is C and you're using it in C++ example. I guess we need extern C:

@wheybags
Copy link
Contributor Author

wheybags commented Aug 1, 2017

@ruslo
Copy link
Owner

ruslo commented Aug 1, 2017

But the whole thing is only ever included through tomcrypt.h

Are we talking about tomcrypt or stormlib? :)

In examples/stormlib/main.cpp I see #include <StormLib.h>.

@wheybags
Copy link
Contributor Author

wheybags commented Aug 1, 2017

I'm not sure which package contains the problem :(
Stormlib links to libtomcrypt, and uses that symbol in only two places https://github.com/hunter-packages/StormLib/blob/v9.21_hunter/src/SBaseCommon.cpp#L187 and https://github.com/hunter-packages/StormLib/blob/v9.21_hunter/src/libtomcrypt/src/pk/rsa/rsa_verify_simple.c (disregard the weird path on the second one, it was using a bundled version of the lib with that file added in).

Those files are getting the definition of the ltc_mp symbol from https://github.com/hunter-packages/StormLib/blob/v9.21_hunter/src/StormCommon.h#L49 and https://github.com/hunter-packages/StormLib/blob/v9.21_hunter/src/libtomcrypt/src/pk/rsa/rsa_verify_simple.c#L11 , so it should be picking up on the extern "C" from there, and indeed it works on linux and windows.

@ruslo
Copy link
Owner

ruslo commented Aug 2, 2017

Looks weird indeed. Add usage of ltc_mp to tomcrypt example and you will see the same error so it's not stormlib issue. May be some mess with defines? While building tomcrypt we are using one set and while using tomcrypt another set? And linker think that those structs ltc_math_descriptor don't match?

@wheybags
Copy link
Contributor Author

wheybags commented Aug 3, 2017

oooook, so (hesistant to say this but), this looks like it might a linker bug?

I added an extra symbol into that file, so:

diff --git a/src/headers/tomcrypt_math.h b/src/headers/tomcrypt_math.h
index aee6105..ba60fba 100644
--- a/src/headers/tomcrypt_math.h
+++ b/src/headers/tomcrypt_math.h
@@ -414,6 +414,7 @@ typedef struct {
 } ltc_math_descriptor;
 
 extern ltc_math_descriptor ltc_mp;
+extern int testInt;
 
 int ltc_init_multi(void **a, ...);
 void ltc_deinit_multi(void *a, ...);
diff --git a/src/misc/crypt/crypt_ltc_mp_descriptor.c b/src/misc/crypt/crypt_ltc_mp_descriptor.c
index 0577d1d..f0c781f 100644
--- a/src/misc/crypt/crypt_ltc_mp_descriptor.c
+++ b/src/misc/crypt/crypt_ltc_mp_descriptor.c
@@ -11,3 +11,5 @@
 #include "tomcrypt.h"
 
 ltc_math_descriptor ltc_mp;
+
+int testInt = 500;

^Added testInt to the .c and the header.

Then I made an example program:

#include <tomcrypt.h>

int main(int argc, char** argv)
{
    ltc_mp = ltm_desc;  
    printf("%d\n", testInt);

    printf("%s\n", ltc_mp.name);
    return 0;
}

And this works just fine, links perfectly and prints:

500
LibTomMath

As you might expect.

But, if I comment out the first printf, the one using testInt, it fails to link.
So it seems like the linker is ignoring that file unless there is another symbol in there that's being used besides ltc_mp.

So, yeah, how should I proceed on this? Am I misunderstanding some subtlety of the osx linker?

@ruslo
Copy link
Owner

ruslo commented Aug 3, 2017

oooook, so (hesistant to say this but), this looks like it might a linker bug?

Might be. Try to create SSCCE and if linker will fail in this case you can open bugreport.

Am I misunderstanding some subtlety of the osx linker?

I don't think so. Code looks good, error is weird.

So, yeah, how should I proceed on this?

Up to you. You can exclude OSX from testing or if adding dummy extern int will fix the issue you can add this as a workaround.

@wheybags
Copy link
Contributor Author

wheybags commented Aug 3, 2017

Minimal example here: https://gist.github.com/wheybags/0cf9abe991d1c667e5218bf59ab24649
I'm using yosemite, if you have access to the newest version of osx/xcode, would you mind verifying this?

@ruslo
Copy link
Owner

ruslo commented Aug 3, 2017

Minimal example here

Makefile, really? :)

I'm using yosemite, if you have access to the newest version of osx/xcode, would you mind verifying this?

Latest Xcode 9 available on Travis.

GitHub + Travis:

@wheybags
Copy link
Contributor Author

wheybags commented Aug 3, 2017 via email

wheybags added a commit to hunter-packages/libtomcrypt that referenced this pull request Aug 19, 2017
wheybags added a commit to hunter-packages/libtomcrypt that referenced this pull request Aug 19, 2017
wheybags added a commit to wheybags/hunter that referenced this pull request Aug 19, 2017
@wheybags
Copy link
Contributor Author

Ok, so updated with a workaround someone pointed out to me on IRC (forgot your name workaround guy, sorry :( )

@@ -108,11 +108,12 @@ hunter_config(SDL2 VERSION 2.0.4-p4)
hunter_config(SDL_mixer VERSION 2.0.1-p1)
hunter_config(SQLite3 VERSION autoconf-3080803) #R-Tree enabled
hunter_config(Sober VERSION 0.1.3)
hunter_config(stormlib VERSION 9.21-p1)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please split these changes.

@wheybags
Copy link
Contributor Author

done: #971

@ruslo
Copy link
Owner

ruslo commented Aug 19, 2017

done: #971

Merged, please rebase.

@wheybags
Copy link
Contributor Author

done

@ruslo
Copy link
Owner

ruslo commented Aug 19, 2017

Testing

Some toolchains failed, feel free to exclude them from testing.

@wheybags
Copy link
Contributor Author

disabled analyze: ingenue#113
Also, updated, should fix MSYS2 build

@ruslo
Copy link
Owner

ruslo commented Aug 20, 2017

@wheybags
Copy link
Contributor Author

Ok, updated again, should work now.

@ruslo ruslo merged commit d412918 into ruslo:master Aug 26, 2017
@ruslo
Copy link
Owner

ruslo commented Aug 26, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants