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.

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

C libraries with deps don't seem to work well with TH / repl #1696

Closed
jcpetruzza opened this issue Feb 15, 2022 · 2 comments
Closed

C libraries with deps don't seem to work well with TH / repl #1696

jcpetruzza opened this issue Feb 15, 2022 · 2 comments

Comments

@jcpetruzza
Copy link
Contributor

Describe the bug

It doesn't seem to be possible to build a package if these conditions are met:

  • the package uses TH, and
  • it depends, directly or indirectly, on a C library, and
  • the C library further depends on other C libraries

Moreover, even if TH is not in use, then the @repl target for a haskell library with such a dependency will fail to load for the same reasons.

To Reproduce

  1. Apply the patch below. It modifies tests/data:ourclibrary so that it know has an additional dep.
  2. Verify that bazel test tests/binary-indirect-cbits:all, which depends on tests/data:ourclibrary still works (no TH).
  3. Run bazel test tests/template-haskell-with-cbits, which also depends on tests/data:ourclibrary but uses TH. It now fails with:
<command line>: user specified .o/.so/.DLL could not be loaded (bazel-out/k8-fastbuild/bin/_solib_k8/libtests_Sdata_Slibourclibrary.so: undefined symbol: c_add)
Whilst trying to load:  (dynamic) bazel-out/k8-fastbuild/bin/_solib_k8/libtests_Sdata_Slibourclibrary.so
Additional directories searched:   bazel-out/k8-fastbuild/bin/_solib_k8
  1. Run bazel run tests/library-with-cbits:library-with-cbits@repl and verify it fails with the same error.

DIFF

diff --git a/tests/data/BUILD.bazel b/tests/data/BUILD.bazel
index f0996aaf..d3b60acb 100644
--- a/tests/data/BUILD.bazel
+++ b/tests/data/BUILD.bazel
@@ -2,9 +2,17 @@
 
 load("@rules_cc//cc:defs.bzl", "cc_library")
 
+cc_library(
+    name = "ourclibrarydep",
+    srcs = [":ourclibrarydep.c"],
+    hdrs = [":ourclibrarydep.h"],
+    visibility = ["//visibility:public"],
+)
+
 cc_library(
     name = "ourclibrary",
     srcs = [":ourclibrary.c"],
+    deps = [":ourclibrarydep"],
     linkstatic = False,
     visibility = ["//visibility:public"],
 )
@@ -12,6 +20,7 @@ cc_library(
 cc_library(
     name = "ourclibrary-static",
     srcs = [":ourclibrary.c"],
+    deps = [":ourclibrarydep"],
     linkstatic = True,
     visibility = ["//visibility:public"],
 )
diff --git a/tests/data/ourclibrary.c b/tests/data/ourclibrary.c
index 587d26ba..17029019 100644
--- a/tests/data/ourclibrary.c
+++ b/tests/data/ourclibrary.c
@@ -1,5 +1,5 @@
-#include <stdint.h>
+#include "ourclibrarydep.h"
 
 int32_t c_add_one(int32_t x) {
-  return 1 + x;
+  return c_add(1, x);
 }
diff --git a/tests/data/ourclibrarydep.c b/tests/data/ourclibrarydep.c
new file mode 100644
index 00000000..55e41b5d
--- /dev/null
+++ b/tests/data/ourclibrarydep.c
@@ -0,0 +1,5 @@
+#include "ourclibrarydep.h"
+
+int32_t c_add(int32_t x, int32_t y) {
+  return x + y;
+}
diff --git a/tests/data/ourclibrarydep.h b/tests/data/ourclibrarydep.h
new file mode 100644
index 00000000..2103bd70
--- /dev/null
+++ b/tests/data/ourclibrarydep.h
@@ -0,0 +1,8 @@
+#ifndef ourclibrarydep_h_INCLUDED
+#define ourclibrarydep_h_INCLUDED
+
+#include <stdint.h>
+
+int32_t c_add(int32_t, int32_t);
+
+#endif

Expected behavior

The test should build and pass.

Environment

  • OS name + version: NixOS 21.05
  • Bazel version: 4.2.1
  • Version of the rules: latest master 9c5d3dc

Additional context

I don't currently have a workaround for this. I initially suspected it could be a problem in the order the libraries are being given with -l, but after reversing the order, the issue persists.

@aherrmann
Copy link
Member

@jcpetruzza Thank you for the detailed repro!

I suspect this is related to how Bazel does dynamic linking and how GHC does dynamic loading.
Specifically, Bazel "under-links" dynamic libraries, i.e. the libraries themselves don't reference their dependencies:

$ bazel build //tests/data:ourclibrary && readelf -d bazel-bin/tests/data/libourclibrary.so | grep NEEDED
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]

That library does not declare its dependency ourclibrarydep as NEEDED.
I'm not sure, but I suspect this interacts badly with GHC's loader loading libraries as RTLD_LOCAL.

I can get your repro to build successfully by adding a NEEDED entry to ourclibrary:

# Awful hack to add NEEDED for ourclibrarydep
$ buildozer "set linkopts -L$(bazel info bazel-bin)/_solib_k8 -ltests_Sdata_Slibourclibrarydep" //tests/data:ourclibrary
$ bazel build //tests/data:ourclibrary && readelf -d bazel-bin/tests/data/libourclibrary.so | grep NEEDED
 0x0000000000000001 (NEEDED)             Shared library: [libtests_Sdata_Slibourclibrarydep.so]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
$ bazel test tests/template-haskell-with-cbits
...
//tests/template-haskell-with-cbits:template-haskell-with-cbits          PASSED in 0.0s
...

Unfortunately, I'm not sure if we can do much about this in rules_haskell. If the above is correct, then this is really a consequence of Bazel doing under-linking and GHC loading in local mode.
As I understand, we cannot change GHC's behavior as it is required for GHCi to override old instances of a library with newly loaded ones.
Bazel does provide some of the pieces to create shared objects that declare their dependencies, but it's not straight-forward. See bazelbuild/bazel#492 for suggestions and some context.

@aherrmann
Copy link
Member

Come to think of it, I think this is a duplicate of #720.

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

No branches or pull requests

2 participants