add LOCAL_GROUP_STATIC_LIBRARIES for circular dependencies

[chri7325]

Description

Currently using the ndk-build system, there is no way to wrap libraries with -Wl,--start-group and -Wl,--end-group in order to resolve circular dependencies. We unfortunately, have now introduced a circular dependency into our runtime that cannot be resolved any time soon. We have gotten around this by using the start-group and end-group to allow the linker to do two passes in order to fully resolve all symbols using the stand-alone toolchains. However, we need to support ndk build files when our developers use android studio to debug into our core C++ runtime, and now we cannot link. I have tried the suggestion from https://stackoverflow.com/questions/22060102/resolving-circular-dependency-in-android-makefile but that does not work for us as well because we only build portions of some of our 3rdparty libraries and a whole archive forces every symbol to be resolved instead of just the tiny piece that we need. It appears that this used to be built into the ndk but I'm not sure why it would be removed. Unfortunately I don't have a test case for this but all I would need is the ability for the link line to allow me to put -Wl,--start-group before my static runtime libraries and -Wl,--end-group right after. I have not been able to insert these options into any other section to try to get it to work.

Environment Details

Not all of these will be relevant to every bug, but please provide as much
information as you can.

• NDK Version: r10e -> r16beta1 (that I know of)
• Build sytem: ndk-build
• Host OS: All
• Compiler: GCC and Clang
• ABI: arm, arm64 and x86
• STL: gnustl and c++
• NDK API level: 14, 16, and 21
• Device API level: N/A

[alexcohn]

FWIW, you don't need start-group/end-group to resolve circular references. Please check Android NDK: how to link multiple 3rd party libraries.

You can even write

LOCAL_LDLIBS += -Wl,--start-group -L $(TARGET_OUT) -lB -lC -Wl,--end-group

Maybe it will help you with some weird cases where duplication of libraries is not enough.

[DanAlbert]

Whoops, never clicked send on my response.

@alexcohn's workaround is the best you're going to able to do in the short term. ndk-build doesn't play nicely with circular dependencies and isn't set up such that we can easily add support for LOCAL_GROUP_STATIC_LIBRARIES since ndk-build needs to know what order that group would be linked in (ndk-build topologically sorts your dependencies and links them in that order, and this wouldn't play nicely with that).

One thing that we need to add support for anyway that might fix this is allowing LOCAL_WHOLE_STATIC_LIBRARIES for static library modules (i.e. allow merging static libraries). If your libraries are all static, you could use this to make one big static library and then the linker would still be able to remove the unused bits when it goes to create the shared libs.

Alternatively we could add some sort of BUILD_LIBRARY_GROUP build rule that would allow this sort of thing.

Neither of those is going to be happening for r16 though (and probably not r17 either, but it's a little early to say for sure), so Alex's workaround is probably the best choice for the moment.

[DanAlbert]

It appears that this used to be built into the ndk but I'm not sure why it would be removed.

Why do you say that? It doesn't appear anywhere in the git history.

[alexcohn]

Some while ago, it was possible to simply write

LOCAL_STATIC_LIBRARIES := B C B

and this would resolve circular dependency same way as the 'workaround' works now, but in a slightly more natural way, and without warnings.

I believe that prior to commit Fix dependency graph computation in ndk-build, which made it into r8e, using -Wl,--start-group in LOCAL_STATIC_LIBRARIES also could work.

Actually, there was a complaint which was simply ignored in the discussion of ndk-build dependency computation.

[alexcohn]

BTW, with cmake

target_link_libraries(A -Wl,--start-group -L $(TARGET_OUT) B C -Wl,--end-group)

simply works.

[DanAlbert]

Yeah, CMake doesn't do any of the fancy dependency computation.

Thanks for pointing me at the history for this.

[chri7325]

@alexcohn thanks for the workaround and suggestion! I'll give it a shot and see if it works.

[alexcohn]

CMake doesn't do any of the fancy dependency computation

@DanAlbert, in light of that I doubt that there was real justification in doing dependency computation for ndk-build. I wonder, how many cases were resolved by that vs. how many broke due to circular dependencies and other tricks that relied on that all LOCAL_xxx_LIBRARIES were passed to the linker 'as is'.

I sincerely believe that leaving more power to developers is better in this case.

[alexcohn]

Here is an easy fix that could resolve the problem. Change the line

libs_in_ldflags := $(filter -l% %.so %.a,$(LOCAL_LDLIBS) $(LOCAL_LDFLAGS))

to read

libs_in_ldflags := $(filter -l% %.s,$(LOCAL_LDLIBS) $(LOCAL_LDFLAGS))

Using static libraries here can't lead to broken builds and other nasty surprises, but it is fragile, and should only be recommended when other easier approaches fail.

BTW, it is really easy to work around the current protection. The linker allows space after -l, but build-binary.mk does not check that . So to have a clean build, write

LOCAL_LDLIBS += -Wl,--start-group -L $(TARGET_OUT) -l B -l C -Wl,--end-group

[chri7325]

@alexcohn I'm almost done resolving the linker errors but now I'm completely missing std functions. I'm guessing that since I'm now switching my link to manually do it, they are no longer being included in the object file list. I'm thinking I need to also manually link in libgnustl_static.a? I'm also noticing that using LOCAL_LDLIBS for libraries does not fail when a library isn't found at all. I think this is different behavior than other linkers. For example, I add -ltotally_fake_lib_that_does_not_exist and the linker is perfectly happy with that. In my case, I forgot to add a -L path for the location of some of our 3rdparty libs that we don't build with ndk-build and I just ended up with unresolved symbols instead of failing much sooner when the linker couldn't find the libs at all.

Update: Adding -lgnustl_static to my LOCAL_LDLIBS line worked :D

[DanAlbert]

@DanAlbert, in light of that I doubt that there was real justification in doing dependency computation for ndk-build. I wonder, how many cases were resolved by that vs. how many broke due to circular dependencies and other tricks that relied on that all LOCAL_xxx_LIBRARIES were passed to the linker 'as is'.

I sincerely believe that leaving more power to developers is better in this case.

What's done is done. We'd break a lot of builds by changing this. We break no builds that haven't been broken for 5 years by keeping it the way it is. Besides, anyone that really doesn't want the build system to track this stuff for them can just use CMake. If this isn't a feature you want then there aren't many compelling reasons to use ndk-build.

BTW, it is really easy to work around the current protection. The linker allows space after -l, but build-binary.mk does not check that . So to have a clean build, write

LOCAL_LDLIBS += -Wl,--start-group -L $(TARGET_OUT) -l B -l C -Wl,--end-group

lol. I won't close that gap until we have a real solution in place for this.

For example, I add -ltotally_fake_lib_that_does_not_exist and the linker is perfectly happy with that.

The build system just removed that entirely, didn't it? If you build with V=1, I'm guessing you won't see it linked.

Update: Adding -lgnustl_static to my LOCAL_LDLIBS line worked :D

Yeah, LOCAL_LDLIBS come after static libraries, so you were getting:

... $OBJECTS $STATIC_LIBS libgnustl_static.a $SHARED_LIBS $LDLIBS ...

The linker won't backtrack to find symbols, so unless everything you needed from the STL was in one of your other static libraries or objects, those symbols wouldn't be included.

[alexcohn]

I'm thinking I need to also manually link in libgnustl_static.a?

Or you can put the circular dependencies in LOCAL_LDFLAGS.

[alexcohn]

LOCAL_LDLIBS for libraries does not fail when a library isn't found at all

This is not what I see. For me, both clang and gcc linker fails if an unexisiting library appears in LOCAL_LDLIBS.

[chri7325]

The build system just removed that entirely, didn't it? If you build with V=1, I'm guessing you won't see it linked.

This is actually a non issue, or at least, my failed understanding of the linker stages. So what was happening is I had my libraries but forgot to add the additional -L path. Because of that, I was getting unresolved symbol errors. The linker does indeed complain that a library is missing but this is only after all symbols have already been resolved. I had thought that the linker would make sure all -l dependencies were actually able to be found before starting to resolve symbols. So while my build was still unresolved, due to the gcc_static library, I had inserted the fake lib on the linker line just to experiment with what was going on and noticed the linker didn't complain about it.

[DanAlbert]

aiui the move to LLD made this unnecessary.

[znakeeye]

One thing that we need to add support for anyway that might fix this is allowing LOCAL_WHOLE_STATIC_LIBRARIES for static library modules (i.e. allow merging static libraries). If your libraries are all static, you could use this to make one big static library and then the linker would still be able to remove the unused bits when it goes to create the shared libs.

Did you ever add this feature? My static module certainly gets its LOCAL_WHOLE_STATIC_LIBRARIES stripped 😝

[DanAlbert]

No

[DanAlbert]

(and as I said in the other thread, for your use case it would be actively harmful)