Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NPE when using manually created CcInfo with dynamic_library #11167

Closed
steeve opened this issue Apr 20, 2020 · 9 comments
Closed

NPE when using manually created CcInfo with dynamic_library #11167

steeve opened this issue Apr 20, 2020 · 9 comments
Assignees
@oquenchil
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: bug

Comments

@steeve
Copy link
Contributor

steeve commented Apr 20, 2020 •
edited
Loading

Description of the problem / feature request:

NPE when using a manually created CcInfo and using dynamic_library.

Creating a CcInfo with the following snippet:

def new_cc_import(ctx,
        hdrs = None,
        defines = None,
        local_defines = None,
        dynamic_library = None,
        static_library = None,
        alwayslink = False,
        linkopts = None,
    ):
[find_cpp_toolchain dance here]
    return CcInfo(
        compilation_context = cc_common.create_compilation_context(
            defines = defines or depset([]),
            local_defines = local_defines or depset([]),
            headers = hdrs or depset([]),
            includes = depset([hdr.root.path for hdr in hdrs.to_list()]),
        ),
        linking_context = cc_common.create_linking_context(
            libraries_to_link = [cc_common.create_library_to_link(
                actions = ctx.actions,
                cc_toolchain = cc_toolchain,
                feature_configuration = feature_configuration,
                dynamic_library = dynamic_library,
                static_library = static_library,
                alwayslink = alwayslink,
            )],
            user_link_flags = linkopts,
        ),
    )

Bazel 3.0.0 will NPE when using the shared_library property:

FAILED: Build did NOT complete successfully (9 packages loaded, 6559 targets configured)
Internal error thrown during build. Printing stack trace: java.lang.RuntimeException: Unrecoverable error while evaluating node '//:ccbin BuildConfigurationValue.Key[993a664396fa04af94b42a9bca19b9fad7eb40aeb138000ab6e9ffc0e5219538] false' (requested by nodes )
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:515)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:399)
	at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.exec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
Caused by: java.lang.NullPointerException
	at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:877)
	at com.google.devtools.build.lib.vfs.PathFragment.startsWith(PathFragment.java:299)
	at com.google.devtools.build.lib.rules.cpp.LibrariesToLinkCollector.addLinkerInputs(LibrariesToLinkCollector.java:247)
	at com.google.devtools.build.lib.rules.cpp.LibrariesToLinkCollector.collectLibrariesToLink(LibrariesToLinkCollector.java:205)
	at com.google.devtools.build.lib.rules.cpp.CppLinkActionBuilder.build(CppLinkActionBuilder.java:858)
	at com.google.devtools.build.lib.rules.cpp.CcLinkingHelper.createDynamicLinkAction(CcLinkingHelper.java:759)
	at com.google.devtools.build.lib.rules.cpp.CcLinkingHelper.createCcLinkActions(CcLinkingHelper.java:442)
	at com.google.devtools.build.lib.rules.cpp.CcLinkingHelper.link(CcLinkingHelper.java:356)
	at com.google.devtools.build.lib.rules.cpp.CcBinary.createTransitiveLinkingActions(CcBinary.java:873)
	at com.google.devtools.build.lib.rules.cpp.CcBinary.init(CcBinary.java:529)
	at com.google.devtools.build.lib.rules.cpp.CcBinary.create(CcBinary.java:282)
	at com.google.devtools.build.lib.rules.cpp.CcBinary.create(CcBinary.java:96)
	at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createRule(ConfiguredTargetFactory.java:484)
	at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createConfiguredTarget(ConfiguredTargetFactory.java:190)
	at com.google.devtools.build.lib.skyframe.SkyframeBuildView.createConfiguredTarget(SkyframeBuildView.java:857)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.createConfiguredTarget(ConfiguredTargetFunction.java:898)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:338)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:438)
	... 7 more
java.lang.RuntimeException: Unrecoverable error while evaluating node '//:ccbin BuildConfigurationValue.Key[993a664396fa04af94b42a9bca19b9fad7eb40aeb138000ab6e9ffc0e5219538] false' (requested by nodes )
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:515)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:399)
	at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.exec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
Caused by: java.lang.NullPointerException
	at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:877)
	at com.google.devtools.build.lib.vfs.PathFragment.startsWith(PathFragment.java:299)
	at com.google.devtools.build.lib.rules.cpp.LibrariesToLinkCollector.addLinkerInputs(LibrariesToLinkCollector.java:247)
	at com.google.devtools.build.lib.rules.cpp.LibrariesToLinkCollector.collectLibrariesToLink(LibrariesToLinkCollector.java:205)
	at com.google.devtools.build.lib.rules.cpp.CppLinkActionBuilder.build(CppLinkActionBuilder.java:858)
	at com.google.devtools.build.lib.rules.cpp.CcLinkingHelper.createDynamicLinkAction(CcLinkingHelper.java:759)
	at com.google.devtools.build.lib.rules.cpp.CcLinkingHelper.createCcLinkActions(CcLinkingHelper.java:442)
	at com.google.devtools.build.lib.rules.cpp.CcLinkingHelper.link(CcLinkingHelper.java:356)
	at com.google.devtools.build.lib.rules.cpp.CcBinary.createTransitiveLinkingActions(CcBinary.java:873)
	at com.google.devtools.build.lib.rules.cpp.CcBinary.init(CcBinary.java:529)
	at com.google.devtools.build.lib.rules.cpp.CcBinary.create(CcBinary.java:282)
	at com.google.devtools.build.lib.rules.cpp.CcBinary.create(CcBinary.java:96)
	at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createRule(ConfiguredTargetFactory.java:484)
	at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createConfiguredTarget(ConfiguredTargetFactory.java:190)
	at com.google.devtools.build.lib.skyframe.SkyframeBuildView.createConfiguredTarget(SkyframeBuildView.java:857)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.createConfiguredTarget(ConfiguredTargetFunction.java:898)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:338)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:438)
FAILED: Build did NOT complete successfully (9 packages loaded, 6559 targets configured)

The same happens with trying to merge this CcInfo with other via cc_common.merge_cc_infos:

Internal error thrown during build. Printing stack trace: java.lang.RuntimeException: Unrecoverable error while evaluating node '//tests/core/c_linkmodes:c-archive_sandwich_test BuildConfigurationValue.Key[12e23b9a2b534aaea26acb9672aa9b32eb7214c6c415232ebf2444fbe20597b9] false' (requested by nodes )
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:515)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:399)
	at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.exec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
Caused by: java.lang.NullPointerException
	at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:877)
	at com.google.devtools.build.lib.vfs.PathFragment.startsWith(PathFragment.java:299)
	at com.google.devtools.build.lib.rules.cpp.LibrariesToLinkCollector.addLinkerInputs(LibrariesToLinkCollector.java:247)
	at com.google.devtools.build.lib.rules.cpp.LibrariesToLinkCollector.collectLibrariesToLink(LibrariesToLinkCollector.java:205)
	at com.google.devtools.build.lib.rules.cpp.CppLinkActionBuilder.build(CppLinkActionBuilder.java:858)
	at com.google.devtools.build.lib.rules.cpp.CcLinkingHelper.createDynamicLinkAction(CcLinkingHelper.java:759)
	at com.google.devtools.build.lib.rules.cpp.CcLinkingHelper.createCcLinkActions(CcLinkingHelper.java:442)
	at com.google.devtools.build.lib.rules.cpp.CcLinkingHelper.link(CcLinkingHelper.java:356)
	at com.google.devtools.build.lib.rules.cpp.CcBinary.createTransitiveLinkingActions(CcBinary.java:873)
	at com.google.devtools.build.lib.rules.cpp.CcBinary.init(CcBinary.java:529)
	at com.google.devtools.build.lib.rules.cpp.CcTest.create(CcTest.java:38)
	at com.google.devtools.build.lib.rules.cpp.CcTest.create(CcTest.java:26)
	at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createRule(ConfiguredTargetFactory.java:484)
	at com.google.devtools.build.lib.analysis.ConfiguredTargetFactory.createConfiguredTarget(ConfiguredTargetFactory.java:190)
	at com.google.devtools.build.lib.skyframe.SkyframeBuildView.createConfiguredTarget(SkyframeBuildView.java:857)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.createConfiguredTarget(ConfiguredTargetFunction.java:898)
	at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:338)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:438)
	... 7 more

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Checkout bazelbuild/rules_go#2445, and run:

$ bazel test //tests/core/c_linkmodes:c-shared_test

What operating system are you running Bazel on?

macOS

What's the output of bazel info release?

release 3.0.0

Have you found anything relevant by searching the web?

No

Any other information, logs, or outputs that you want to share?

See bazelbuild/rules_go#2445
@steeve
Copy link
Contributor Author

steeve commented Apr 20, 2020 •
edited
Loading

cc @katre per Jay's remark

@katre katre self-assigned this Apr 20, 2020
@katre katre added P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug and removed P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: bug labels Apr 20, 2020
@steeve
Copy link
Contributor Author

steeve commented Apr 20, 2020 •
edited
Loading

When using CcInfo from deps target, Bazel will NPE unless linkstatic = True is set.

@steeve steeve changed the title NPE when using manually created CcInfo NPE when using manually created CcInfo with dynamic_library Apr 20, 2020
@jin jin added team-Configurability platforms, toolchains, cquery, select(), config transitions untriaged labels Apr 21, 2020
@katre
Copy link
Member

katre commented Apr 21, 2020

Okay, took a look. The problem here (as the stack trace shows) is in LibrariesToLinkCollector, line 247:

          Preconditions.checkState(
              libDir.startsWith(solibDir) || libDir.startsWith(toolchainLibrariesSolibDir),
              "Artifact '%s' is not under directory expected '%s',"
                  + " neither it is in directory for toolchain libraries '%'.",

In your case, the libDir is this:

bazel-out/k8-fastbuild-ST-ea49dc5320a93d6661c9b487dfc9f290d4998382bc7437b16e78a6a1394b0b3a/bin/_solib_k8/_U_S_Stests_Score_Sc_Ulinkmodes_Cadder_Ushared___Utests_Score_Sc_Ulinkmodes_Slinux_Uamd64_Uc-shared

And the expected soLibDir is:

bazel-out/k8-fastbuild/bin/_solib_k8

So, short answer, this fails to consider libraries in output dirs based on transitions to non-target configurations. The check for a valid soDir needs to be updated.

@katre
Copy link
Member

katre commented Apr 21, 2020

I don't work in this level of the CC code, and @hlopko is no longer part of the Bazel team, so re-assigning to @oquenchil to triage and fix.

@katre katre added team-Rules-CPP Issues for C++ rules type: bug and removed team-Configurability platforms, toolchains, cquery, select(), config transitions labels Apr 21, 2020
@katre katre assigned oquenchil and unassigned katre Apr 21, 2020
@steeve
Copy link
Contributor Author

steeve commented Apr 21, 2020

Thank you for the quick response @katre! Sad this isn't a bug in my code though :(

@oquenchil
Copy link
Contributor

I will update the check to give a proper error instead of a crash.

@oquenchil oquenchil added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels May 11, 2020
@steeve
Copy link
Contributor Author

steeve commented May 13, 2020

I was wondering, could it break cc_import working with a .so coming from a configuration?

@oquenchil oquenchil added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels May 20, 2020
@oquenchil
Copy link
Contributor

I believe you can trigger the code path causing the error with a .so from a cc_import. The problem that wanted to be avoided initially with that check was not to have the same .so be linked more than once if it showed up in the graph having been built in different configurations. Unfortunately, it's more restrictive than it should be because it simply doesn't allow an so from a different config.

I introduced a class during a refactoring that added the label of the target to a wrapper around LibraryToLink:
https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java;drc=61ec8d1bec3c8281a12e17c4498816d945c72058;l=170

That was an incompatible change because people were allowed to create these objects from Starlark. Once everyone is using LinkerInput with a non-null label, I think we can re-write this check and linking a library from a different configuration should work.

Once this flag is flipped:
#10860

We can look at this issue again.

@steeve
Copy link
Contributor Author

steeve commented May 20, 2020 •
edited
Loading

If it does trigger the crash with cc_import too (and I don't see why it wouldn't), then it means the newer version of rules_go might be broken, since one use case is to link to a dynamic object (linkmode = "c-shared"), then use cc_import to reinject in the cc_library graph.

@agluszak agluszak mentioned this issue Jul 13, 2020
Closed
ehkloaj pushed a commit to ehkloaj/bazel that referenced this issue Aug 6, 2020
@agluszak @ehkloaj
Make linking the same shared library twice a rule error
c9b9009 
> As more and more people start using the power of Starlark transitions, we are seeing more people bumping into a precondition baked deep in the C++ rules implementation which checks that a shared library being linked is in the same configuration directory. The check is there to make sure that the same library built in different configurations is not linked more than once.
> Aside from the fact that it should be a proper rule error and not a crash, people are hitting this check when writing cc_import targets that have a transition applied to them. There are other ways to guarantee that the same library is not linked more than once without completely restricting the ability to link a library built in a different configuration.
@oquenchil

This PR makes linking the same shared library twice a rule error instead of a crash and it makes it possible to link a library built in a different configuration. A map from library identifiers to library paths is introduced, making sure that each shared library is linked at most once. Only dynamic linking is taken into account, for static linking see bazelbuild#11727

Fixes bazelbuild#11167 (the check which was causing the NPE was not directly related to transitions, but because it was removed, that NPE should no longer happen)

Closes bazelbuild#11721.

PiperOrigin-RevId: 322563028
@steeve steeve mentioned this issue Aug 21, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@oquenchil oquenchil

Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make linking the same shared library twice a rule error agluszak/bazel
4 participants
@steeve @katre @jin @oquenchil