[lttng-dev] urcu 7ca7fe9c03 + _LGPL_SOURCE regression?

* [lttng-dev] urcu 7ca7fe9c03 + _LGPL_SOURCE regression?
@ 2022-08-09 18:19 Eric Wong via lttng-dev
  2022-08-18 18:55 ` Mathieu Desnoyers via lttng-dev
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Wong via lttng-dev @ 2022-08-09 18:19 UTC (permalink / raw)
  To: lttng-dev; +Cc: Sam Saffron, Simon Marchi

Hello, I've noticed liburcu v0.11.4+ and later fails with the
"mwrap" LD_PRELOAD malloc wrapper which initializes an rculfhash
in a constructor.

The problem happens when using _LGPL_SOURCE + rcu_dereference on
a cds_lfht pointer:

  In file included from /tmp/b/include/urcu/pointer.h:39,
                   from /tmp/b/include/urcu/urcu-bp.h:58,
                   from /tmp/b/include/urcu-bp.h:2,
                   from ../7ca7fe9c_regression.c:10:
  ../7ca7fe9c_regression.c: In function ‘main’:
  /tmp/b/include/urcu/static/pointer.h:101:18: error: invalid use of undefined type ‘struct cds_lfht’
    101 |     __typeof__(p + 0) _________p1;    \
        |                  ^
  /tmp/b/include/urcu/pointer.h:47:26: note: in expansion of macro ‘_rcu_dereference’
     47 | #define rcu_dereference  _rcu_dereference
        |                          ^~~~~~~~~~~~~~~~
  ../7ca7fe9c_regression.c:26:23: note: in expansion of macro ‘rcu_dereference’
     26 |  struct cds_lfht *t = rcu_dereference(totals);
        |                       ^~~~~~~~~~~~~~~

Removing _LGPL_SOURCE avoids the problem, but I'd rather not
introduce performance regressions.

In retrospect, my use of rcu_dereference seems unnecessary since
constructor functions should always fire before any threads are
created.

That means I can safely replace the assignment w/ CMM_STORE_SHARED,
and rcu_dereference with CMM_LOAD_SHARED, correct?

Sample code:
/*
 * shows a regression with _LGPL_SOURCE + urcu v0.11.4, commit:
 * 7ca7fe9c03217813 (Make temporary variable in _rcu_dereference non-const, 2021-07-29)
 * $(CC) -o x -I/path/to/include/ -L/path/to/lib/ __FILE__ -lurcu-bp -lurcu-cds
 *
 * Originally noticed in an LD_PRELOAD malloc wrapper:
 * https://80x24.org/mwrap.git (Ruby 2.6+)
 * https://80x24.org/mwrap-perl.git (Perl5)
 */
#define _LGPL_SOURCE
#include <urcu-bp.h>
#include <urcu/rculfhash.h>
static struct cds_lfht *totals; /* immortal */

__attribute__((constructor)) static void init_lfht(void)
{
	/* XXX replace with CMM_STORE_SHARED? */
	totals = cds_lfht_new(16384, 1, 0, CDS_LFHT_AUTO_RESIZE, 0);
}

int main(void) /* this is meant to be a malloc-family function in mwrap */
{
	/* XXX replace rcu_dereference with CMM_LOAD_SHARED? */
	struct cds_lfht *t = rcu_dereference(totals);
	return t ? 0 : 1;
}
---
Thanks for many years of urcu + cds!
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 3+ messages in thread