Bug 244106 - net/libfabric: fails to build against libepoll-shim-0.0.20200212
Summary: net/libfabric: fails to build against libepoll-shim-0.0.20200212
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Jan Beich
URL:
Keywords: patch, regression
Depends on:
Blocks: 243649
  Show dependency treegraph
 
Reported: 2020-02-13 20:30 UTC by Jan Beich
Modified: 2020-02-14 12:28 UTC (History)
3 users (show)

See Also:
yuri: maintainer-feedback+


Attachments
disable helpers (2.22 KB, patch)
2020-02-13 20:31 UTC, Jan Beich
no flags Details | Diff
undef conflicts (2.89 KB, patch)
2020-02-13 20:32 UTC, Jan Beich
no flags Details | Diff
ignore (fix for bug 244087) (1.74 KB, patch)
2020-02-14 02:13 UTC, Jan Beich
no flags Details | Diff
undef conflicts (2.91 KB, patch)
2020-02-14 02:20 UTC, Jan Beich
yuri: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer freebsd_triage 2020-02-13 20:30:33 UTC
Regressed by https://github.com/jiixyj/epoll-shim/commit/71af8c103bb4. Not sure which fix is better.

src/fabric.c:683:7: error: no member named 'epoll_shim_close' in 'fi_ops'
                    FI_CHECK_OP(info->nic->fid.ops, struct fi_ops, close)) {
                    ^                                              ~~~~~
./include/rdma/fabric.h:555:23: note: expanded from macro 'FI_CHECK_OP'
        (ops && (ops->size > offsetof(opstype, op)) && ops->op)
                             ^                 ~~
/usr/include/stddef.h:73:31: note: expanded from macro 'offsetof'
#define offsetof(type, field)   __offsetof(type, field)
                                ^                ~~~~~
/usr/include/sys/cdefs.h:487:34: note: expanded from macro '__offsetof'
#define __offsetof(type, field)  __builtin_offsetof(type, field)
                                 ^                        ~~~~~
src/fabric.c:683:54: error: no member named 'epoll_shim_close' in 'struct fi_ops'
                    FI_CHECK_OP(info->nic->fid.ops, struct fi_ops, close)) {
                                ~~~~~~~~~~~~~~~~~~                 ^
/usr/local/include/libepoll-shim/sys/epoll.h:74:15: note: expanded from macro 'close'
#define close epoll_shim_close
              ^
./include/rdma/fabric.h:555:54: note: expanded from macro 'FI_CHECK_OP'
        (ops && (ops->size > offsetof(opstype, op)) && ops->op)
                                                       ~~~  ^

http://beefy9.nyi.freebsd.org/data/113amd64-default/526001/logs/errors/libfabric-1.8.1.log
Comment 1 Jan Beich freebsd_committer freebsd_triage 2020-02-13 20:31:47 UTC
Created attachment 211623 [details]
disable helpers
Comment 2 Jan Beich freebsd_committer freebsd_triage 2020-02-13 20:32:05 UTC
Created attachment 211624 [details]
undef conflicts
Comment 3 Jan Kokemüller 2020-02-13 21:30:24 UTC
Sorry for the breakage. In the latest epoll-shim code, "epoll_create" returns a handle to an internal context object which manages some state, and therefore needs to be closed with "epoll_shim_close". But the worst that can happen with a normal "close" is a memory leak. This may or may not be tolerable, depending on the use case.

I guess the most robust workaround is something like this:


+#ifdef SHIM_SYS_SHIM_HELPERS
+#undef close
+#endif
        return fid->ops->close(fid);
+#ifdef SHIM_SYS_SHIM_HELPERS
+#define close epoll_shim_close
+#endif

... i.e. redefining "close" after a problematic line back to "epoll_shim_close".

Is it possible to script something like this in the post-patch section with REINPLACE_CMD maybe?
Comment 4 Jan Beich freebsd_committer freebsd_triage 2020-02-14 02:13:07 UTC
Created attachment 211633 [details]
ignore (fix for bug 244087)

(In reply to Jan Kokemüller from comment #3)
> Is it possible to script something like this?

I did so in attachment 211624 [details] but accidentally applied to non-C files as well.
Comment 5 Jan Beich freebsd_committer freebsd_triage 2020-02-14 02:16:01 UTC
Comment on attachment 211623 [details]
disable helpers

As described in comment 3 this version may cause a memory leak.
Comment 6 Jan Beich freebsd_committer freebsd_triage 2020-02-14 02:20:44 UTC
Created attachment 211634 [details]
undef conflicts

Oops, previous version was for a different bug.
Comment 7 Jan Kokemüller 2020-02-14 05:23:03 UTC
When I apply the patch, I get results like:


@@ -495,6 +495,9 @@

 struct fi_ops {
        size_t  size;
+#ifdef SHIM_SYS_SHIM_HELPERS
+#undef close
+#endif
        int     (*close)(struct fid *fid);
        int     (*bind)(struct fid *fid, struct fid *bfid, uint64_t flags);
        int     (*control)(struct fid *fid, int command, void *arg);


...but the redefinition is missing. Something like:


@@ -495,6 +495,9 @@

 struct fi_ops {
        size_t  size;
+#ifdef SHIM_SYS_SHIM_HELPERS
+#undef close
+#endif
        int     (*close)(struct fid *fid);
+#ifdef SHIM_SYS_SHIM_HELPERS
+#define close epoll_shim_close
+#endif
        int     (*bind)(struct fid *fid, struct fid *bfid, uint64_t flags);
        int     (*control)(struct fid *fid, int command, void *arg);


So that each problematic close gets both the undef and the redefine.
Comment 8 Jan Beich freebsd_committer freebsd_triage 2020-02-14 12:23:13 UTC
Landed in ports r526106. I've mistyped bug number.