Bug 265021

Summary:	sysutils/rshim-user-space: fails to build with libepoll-shim 0.0.20220703
Product:	Ports & Packages	Reporter:	Jan Beich <jbeich>
Component:	Individual Port(s)	Assignee:	Hans Petter Selasky <hselasky>
Status:	Closed FIXED
Severity:	Affects Only Me	CC:	jan.kokemueller
Priority:	---	Keywords:	needs-patch
Version:	Latest	Flags:	bugzilla: maintainer-feedback? (hselasky)
Hardware:	Any
OS:	Any
See Also:	https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=265024
Bug Depends on:
Bug Blocks:	265017

Description Jan Beich freebsd_committer

2022-07-03 22:14:34 UTC

Apply bug 265017 then try to build.

  $ make
  [...]
  rshim.c:1208:15: error: no member named 'epoll_shim_read' in 'struct rshim_backend'
      len = bd->read(bd, RSH_DEV_TYPE_TMFIFO, (char *)bd->read_buf,
	    ~~  ^
  /usr/local/include/libepoll-shim/epoll-shim/detail/read.h:10:19: note: expanded from macro 'read'
  #define read(...) epoll_shim_read(__VA_ARGS__)
		    ^
  1 error generated.

Workaround (untested):

  #ifdef read
  #define old_read read
  #undef read
  #endif
      len = bd->read(bd, RSH_DEV_TYPE_TMFIFO, (char *)bd->read_buf,
		     READ_BUF_SIZE);
  #ifdef old_read
  #define read old_read
  #undef old_read
  #endif

Comment 1 Jan Kokemüller 2022-07-04 04:54:21 UTC

The wrapper macros are now defined with parens, which leads to this issue. See also <https://github.com/jiixyj/epoll-shim/issues/30> for some discussion.

I'm not sure if "#define old_read read"/"#define read old_read" will actually restore the needed "epoll_shim_read" macro, so if you do "#undef read" you _must_ make sure that no "read()" call below the "#undef read" reads from an epoll/timerfd/signalfd-like fd.

An alternative to the macros could be the new symbol interposition support of epoll-shim. With this, read/write/close etc. are defined as ELF symbols, which should work great (in theory!) for rshim:


diff --git a/configure.ac b/configure.ac
index d50bcfa..7e29adc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -91,10 +91,7 @@ fi
 ])
 
 if test $backend = freebsd; then
-AC_SUBST(CPPFLAGS, "$CPPFLAGS -I${prefix}/include/libepoll-shim")
-AC_SUBST(LDFLAGS, "$LDFLAGS -L${prefix}/lib")
-AC_CHECK_HEADERS([sys/epoll.h],[],[AC_MSG_ERROR([Missing libepoll-shim])])
-AC_CHECK_LIB(epoll-shim, epoll_create1)
+PKG_CHECK_MODULES(epollshim, epoll-shim-interpose, [], [AC_MSG_ERROR([Can't find epoll-shim-interpose])])
 AC_SUBST(CPPFLAGS, "$CPPFLAGS -DDEFAULT_RSHIM_CONFIG_FILE='\"${prefix}/etc/rshim.conf\"'")
 else
 AC_SUBST(CPPFLAGS, "$CPPFLAGS -DDEFAULT_RSHIM_CONFIG_FILE='\"/etc/rshim.conf\"'")
diff --git a/src/Makefile.am b/src/Makefile.am
index 7d470a8..59b11f9 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -26,3 +26,8 @@ rshim_SOURCES += rshim_fuse.c
 rshim_CPPFLAGS += $(fuse_CFLAGS) -DHAVE_RSHIM_FUSE
 LIBS += $(fuse_LIBS)
 endif
+
+if OS_FREEBSD
+rshim_CPPFLAGS += $(epollshim_CFLAGS)
+LIBS += $(epollshim_LIBS)
+endif


With the above, rshim builds and starts, but I cannot test if it works correctly.

Comment 2 Jan Kokemüller 2022-07-04 16:43:30 UTC

The following patch will also work. it replaces "bd->read(bd, ..." with "(bd->read)(bd, ..." preventing the expansion of the function like macro.



diff --git a/sysutils/rshim-user-space/Makefile b/sysutils/rshim-user-space/Makefile
index 0942d10c113..e77835d8892 100644
--- a/sysutils/rshim-user-space/Makefile
+++ b/sysutils/rshim-user-space/Makefile
@@ -29,4 +29,7 @@ GNU_CONFIGURE=        yes
 IGNORE=                not supported on this operating system combination
 .endif
 
+post-patch:
+       @${REINPLACE_CMD} -e 's|\([^[:space:]]*\->read\)(|(\1)(|g' ${WRKSRC}/src/rshim.c
+
 .include <bsd.port.mk>

Comment 3 Hans Petter Selasky freebsd_committer

2022-07-05 08:54:35 UTC

Hi Jan,

This is technically OK, but is not the right way to solve this. I have a better way and will provide a patch. Have a look at:

https://github.com/freebsd/freebsd-src/blob/main/contrib/ofed/librdmacm/preload.c

There may be several other libraries playing games with open/close and #define won't handle this, simply.

--HPS

Comment 4 Hans Petter Selasky freebsd_committer

2022-07-05 08:56:01 UTC

This will be fixed in libepoll-shim. Working on upstream patch!

Comment 5 Hans Petter Selasky freebsd_committer

2022-07-05 09:23:37 UTC

I see epoll-shim already has interpose support. So I guess that's what we need to use. Let me just set a build environment to test.