Not an issue here, but a serious question

[bigfatbrowncat]

I wanted to put this one into Avian repo, but it's not strictly connected to Avian, but to our Android classpath.

I have a problem.

I am fixing the https connections. To do so I updated some openssl code and NativeCrypto. And then I've found a strange thing. The headers being used in Avian upstream for Android are pointing to Android version of OpenSSL, that leads to some errors due to slightly different compiler definitions. For example, if I try to execute this:

import java.math.BigInteger;
public class test
{
    public static void main(String... args)
    {
        BigInteger b = new BigInteger("18593836528167530166073666026719346417823020176439626614028413526404999880327491506516272499878595030280607624081438145719716642109530242659974572679669677548979696293574077291727192086389317945295581875611100688257077150290313913955933994627422464025892853888864897311552582747029009460312175941403825512521608234592729773874485778822321589754921410345863648258311304381933433700800812808227503479049701935651549507766659090195540832938734819645038439964149513414251203390539403371826169388236059881359107837063445950264136412956956189058996444497800605449275501685458800039488567279669997212084742578387973641915947");
        System.out.println(b.toString() + "\n" + b.bitLength());
    }
}

I receive an incorrect length value.

I have changed includes to the proper ones - taken from the upstream version and got an ugly problem. Avian compiles fine and starts, but if I try to run any program that uses OpenSSL in any way (like the example above), Avian just crashes. This problem is eproduced only in MinGW (OSX works brilliant).

Did you leave the incorrect includes intentionally for some cause? Or it was just a mistake? Do you have any ideas about how to fix that?

[dicej]

Hi Ilya. What do you mean when you say the includes are incorrect? My understanding is that Android patches OpenSSL before building it (using external/openssl/patches/*.patch), and those patches modify both header files and .c source files. So you can't just use the unpatched upstream header files with a library built from the patched source files -- they won't match up.

If you're modifying the upstream OpenSSL code, you'll still need to apply the Android patches on top of your changes before you build it, otherwise it won't be compatible with the Android .java code.

[bigfatbrowncat]

Of course we used the patched upstream version. And both .cpp and .h files are patched. I'm talking about a bit different thing. Look: https://github.com/ReadyTalk/avian/blob/master/makefile#L247

We are taking the included headers from android openssl while linking with openssl-upstream. That shouldn't (hopefully) had any consequences if the definitions in opensslconf.h were completely the same, but that isn't true cause 64-bit Windows (mingw-w64) differs from 64-bits Android (that the creators of opensslconf-64.h were probably keeping in mind).

Now I changed the includes to the correct ones cause https didn't work otherwise. And I have an ugly corruption - any program using BigInteger or SSL sockets crush with 75-90% probability.

I'm trying to find out what was the cause and want to know - if you left this include devlaration intentionally or by mistake? If intentionally, why did you do that? Maybe there is some incompatibility that I've missed so far...

[dicej]

It's been a long time since I added the Android port, so I don't remember the exact reason for each line in the makefile, but I imagine each -I line was added to ensure the compiler could find all the header files it needed. If we didn't specify -I$(android)/external/openssl/include, how would the compiler find the OpenSSL headers?

[bigfatbrowncat]

-I$(android)/openssl-upstream/include

[dicej]

Hi @bigfatbrowncat, sorry for the delayed response. Yes, that's a good point, it seems like it should be openssl-upstream/include instead of external/openssl/include. I guess it didn't matter which one we used in the past, since both directories contained the same version of OpenSSL. Since you've modified OpenSSL, the versions are no longer the same, hence the issue.

Did changing it to -I$(android)/openssl-upstream/include fix the problem for you?

[bigfatbrowncat]

Unfortunately, no.

After I changed the included header to the correct one, I faced a problem. A very complicated one.

1. If the application doesn't use OpenSSL in any way (i.e. it doesn't use http(s) connections and doesn't use BigInteger which is implemented with OpenSSL backend), it works fine
2. If the application DOES use OpenSSL: a. It may start and work correctly for a long time (my app did dozens of https requests which succeeded) b. but far more probable (about 90%) it crashes during any operation that uses OpenSSL. For example, this code crashes:

import java.math.BigInteger;
public class test
{
    public static void main(String... args)
    {
        BigInteger b = new BigInteger("18593836528167530166073666026719346417823020176439626614028413526404999880327491506516272499878595030280607624081438145719716642109530242659974572679669677548979696293574077291727192086389317945295581875611100688257077150290313913955933994627422464025892853888864897311552582747029009460312175941403825512521608234592729773874485778822321589754921410345863648258311304381933433700800812808227503479049701935651549507766659090195540832938734819645038439964149513414251203390539403371826169388236059881359107837063445950264136412956956189058996444497800605449275501685458800039488567279669997212084742578387973641915947");
        System.out.println(b.toString() + "\n" + b.bitLength());
    }
}

I tried to catch the crash by logging different points in NativeCrypto and in BigInteger (BigInt), but I can't find any special point. It looks like a non-obvious memory corruption.

[JustAMan]

Let me stress that this seems to be a Windows/MinGW-related problem.

Thanks, Vasily 12 янв. 2016 г. 12:02 пользователь "Ilya Mizus" notifications@github.com написал:

Unfortunately, no.

After I changed the included header to the correct one, I faced a problem. A very complicated one.

1. If the application doesn't use OpenSSL in any way (i.e. it doesn't use http(s) connections and doesn't use BigInteger which is implemented with OpenSSL backend), it works fine
2. If the application DOES use OpenSSL: a. It may start and work correctly for long time (my app did dosens of https requests which succeeded) b. but far more probable (about 90%) it crashes during any operation that uses OpenSSL. For example, this code crashes:

import java.math.BigInteger; public class test { public static void main(String... args) { BigInteger b = new BigInteger("18593836528167530166073666026719346417823020176439626614028413526404999880327491506516272499878595030280607624081438145719716642109530242659974572679669677548979696293574077291727192086389317945295581875611100688257077150290313913955933994627422464025892853888864897311552582747029009460312175941403825512521608234592729773874485778822321589754921410345863648258311304381933433700800812808227503479049701935651549507766659090195540832938734819645038439964149513414251203390539403371826169388236059881359107837063445950264136412956956189058996444497800605449275501685458800039488567279669997212084742578387973641915947"); System.out.println(b.toString() + "\n" + b.bitLength()); } }

I tried to catch the crash by logging different points in NativeCrypto and in BigInteger (BigInt), but I can't find any special point. It looks like a non-obvious memory corruption.

— Reply to this email directly or view it on GitHub https://github.com/dicej/android-libcore64/issues/12#issuecomment-170844596 .

[dicej]

Thanks for the clarification. What steps should I take to reproduce this? Just check out avian-pack on Windows, build, and run the BigInteger test you provided? Do you have a branch or patch with the OpenSSL modifications you referred to?

[bigfatbrowncat]

No branches. :) Just checkout the master version and build it. If you encounter any problems, please let us know. Thank you for help!

[dicej]

I was able to reproduce the problem, but I haven't been able to narrow it down yet. It's frustrating because when I run it in GDB, it doesn't crash, but when I run it directly or from Visual Studio's debugger, it does crash. I was able to discover that it was crashing at https://github.com/ReadyTalk/avian/blob/ee7da77c70b9f56cb01a66c972bbd56a2d0ce860/src/compile.cpp#L7402 (actually somewhere in the Windows memory allocator), so it does indeed seem that memory is being corrupted, but I don't know where.

I don't have any more time to look at it now, especially since my Windows VM runs off a USB hard disk and is very slow to rebuild avian-pack. One thing that might be worth trying is using a tool like https://github.com/dynamorio/drmemory to debug it.