Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Sections and TLS support for Android executables #784

Merged
merged 1 commit into from May 13, 2014
Merged

Sections and TLS support for Android executables #784

merged 1 commit into from May 13, 2014

Conversation

joakim-noah
Copy link
Contributor

This is the remaining piece to build druntime for Android/x86, along with a corrected function declaration in sys/time.d. rt.sections_android is mostly just a mashup of rt.sections_solaris and rt.sections_osx. All 39 druntime modules' unit tests consistently pass on Android/x86 with this patch, though core.sync.semaphore throws an InvalidMemoryOperation exception even though its tests pass. I uncommented FD_CLR and friends in sys/select.d when running the tests: I'll verify that and add it to this pull later. Finally, I built everything with -fPIC, since that's how Android does it and we'll need it to build shared libraries for Android apps/apks later.

This pull depends on packed TLS for ELF working in dmd, here's that patch. This pull isn't finished yet, just putting it up so others can use it in the meantime. I'll make the tweaks mentioned and it should be ready for merging then.

* Source: $(DRUNTIMESRC src/rt/_sections_android.d)
*/

module rt.sections_android;
Copy link
Member

Choose a reason for hiding this comment

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

Some comments on what TLS scheme, etc. this implementation is targeting would be great. Somebody might work on this in the future without having followed the discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do, once someone with more knowledge of dmd than me signs off on the packed TLS for ELF approach I used.

@MartinNowak
Copy link
Member

The OSX like TLS emulation should work, but it's not compatible yet with shared libraries.
Is there already a Target_Android in DMD?

@joakim-noah
Copy link
Contributor Author

The OSX like TLS emulation should work, but it's not compatible yet with shared libraries.

Hmm, maybe this is why the D apk is segfaulting for me. Is it that you can't build D source even as a single shared library with TLS emulation or that it causes problems with multiple shared libraries?

Is there already a Target_Android in DMD?

No, I suppose I'll have to add one. It'd be nice if we could just build dmd with TARGET_LINUX and add an -android runtime flag to build for Android, like gcc does, but the DSO registry stuff would likely screw that up.

@joakim-noah
Copy link
Contributor Author

Updated this pull with uncommented FD_CLR and friends, as mentioned earlier. I couldn't be bothered to dig down into the assembly versions in bionic to make sure they match the D versions copy-pasted from the linux block in sys.select. The tests pass, so I'm guessing it works fine.

I looked into the InvalidMemoryOperation exception thrown from core.sync.semaphore. It appears to get thrown from the GC after the unit tests are done running and the GC tries to destroy one of the semaphores, the one from testWaitTimeout. sem_destroy fails when it's called and the resulting assert apparently tries to call GC.malloc, which then throws the invalid memory exception because the GC is running during a malloc.

It seems like this would be a problem for any assert thrown while the GC is running, and since it doesn't say which assert, makes it harder to track down where the problem is. I'll look into the semaphore issue further, but someone likely needs to look into the assert issue also.

@dnadlinger
Copy link
Member

Setting a breakpoint on _d_assert might help to track down where the origin of the exception is.

@joakim-noah
Copy link
Contributor Author

The semaphore issue appears to be that sem_destroy in bionic actually checks if the count for the semaphore is negative, while other implementations don't. Shouldn't hit this unless you specifically try to, as we do in the testWaitTimeout unittest, and it just throws an exception, so I think it can be ignored for now.

As for the assert issue, looks like someone filed a bug for it a couple years ago and the underlying issue of allocation from a destructor comes up in the forums every so often. Still around, as I just hit it when debugging this semaphore issue.

@MartinNowak
Copy link
Member

Is it that you can't build D source even as a single shared library with TLS emulation or that it causes problems with multiple shared libraries?

Multiple shared libraries.

WalterBright added a commit that referenced this pull request May 13, 2014
Sections and TLS support for Android executables
@WalterBright WalterBright merged commit dc42812 into dlang:master May 13, 2014
@joakim-noah
Copy link
Contributor Author

Hmm, I wanted to try and get this working with a D shared library, not just executables, before merging. I'll prepare another pull when I get that working, along with the documentation comment David asked for.

Speaking of TLS, @WalterBright, can you take a look at the dmd patch I used and sign off on the modifications I made to backend/el.c? I just copy-pasted the modification to that file from your patch adding packed TLS for OS X years ago, as I don't know dmd that well. It seems to work, but I might be doing something wrong, so I'd appreciate your input.

Would you be interested in a pull for dmd with a cleaned up version of that patch?

@WalterBright
Copy link
Member

Hmm, I wanted to try and get this working with a D shared library, not just executables, before merging.

Since the autotester does not test on Android, I don't see much point in doing a PR for this if it isn't ready to merge. If you don't want things merged, mark it in the subject as "Do not merge".

Anyhow, I think it is fine to merge Android-specific works in progress. I'll look at the dmd one.

@joakim-noah
Copy link
Contributor Author

As I've mentioned earlier, I put the Android/x86 patches up in case anyone else wanted to tinker with them, and I noted that "this pull isn't finished yet" in the original message. Not a big deal, as I looked into the other issues listed, just the two issues left.

Let me know if the packed TLS for ELF approach I used is okay or if you see anything wrong with simply copy-pasting your OS X patch for el_picvar and we'll go from there.

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