diff mbox series

[7/9] support record failure: allow use from constructor

Message ID 20200326155633.18236-8-mathieu.desnoyers@efficios.com
State New
Headers show
Series Restartable Sequences enablement | expand

Commit Message

Michael Kerrisk \(man-pages\) via Libc-alpha March 26, 2020, 3:56 p.m. UTC
Expose support_record_failure_init () so constructors can explicitly
initialize the record failure API.

This is preferred to lazy initialization at first use, because
lazy initialization does not cover use in constructors within
forked children processes (forked from parent constructor).

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
CC: Joseph Myers <joseph@codesourcery.com>
CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: libc-alpha@sourceware.org
---
 support/check.h                  |  4 ++++
 support/support_record_failure.c | 18 +++++++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

Comments

Florian Weimer April 27, 2020, 4:58 p.m. UTC | #1
* Mathieu Desnoyers via Libc-alpha:

> Expose support_record_failure_init () so constructors can explicitly
> initialize the record failure API.
>
> This is preferred to lazy initialization at first use, because
> lazy initialization does not cover use in constructors within
> forked children processes (forked from parent constructor).

My intent behind lazy initialization was that it avoids perturbing the
address space until the first failure.  This is supposed to make the
test harness more compatible.

A test in the constructor completely breaks major assumptions in the
old test skeleton (and those were carried over into the current test
framework).  In general, if a test involves constructors, we tend to
avoid using the test harness at all.
Mathieu Desnoyers April 27, 2020, 5:49 p.m. UTC | #2
----- On Apr 27, 2020, at 12:58 PM, Florian Weimer fw@deneb.enyo.de wrote:

> * Mathieu Desnoyers via Libc-alpha:
> 
>> Expose support_record_failure_init () so constructors can explicitly
>> initialize the record failure API.
>>
>> This is preferred to lazy initialization at first use, because
>> lazy initialization does not cover use in constructors within
>> forked children processes (forked from parent constructor).
> 
> My intent behind lazy initialization was that it avoids perturbing the
> address space until the first failure.  This is supposed to make the
> test harness more compatible.
> 
> A test in the constructor completely breaks major assumptions in the
> old test skeleton (and those were carried over into the current test
> framework).  In general, if a test involves constructors, we tend to
> avoid using the test harness at all.

OK, now that I moved this to separate patches, we can focus on the
rest and address it later.

Thanks,

Mathieu
diff mbox series

Patch

diff --git a/support/check.h b/support/check.h
index 77d1d1e14d..902cea6878 100644
--- a/support/check.h
+++ b/support/check.h
@@ -88,6 +88,10 @@  void support_test_verify_exit_impl (int status, const char *file, int line,
    does not support reporting failures from a DSO.  */
 void support_record_failure (void);
 
+/* Initialize record failure.  Calling this is only needed when
+   recording failures from constructors.  */
+void support_record_failure_init (void);
+
 /* Static assertion, under a common name for both C++ and C11.  */
 #ifdef __cplusplus
 # define support_static_assert static_assert
diff --git a/support/support_record_failure.c b/support/support_record_failure.c
index f766c06236..957572f487 100644
--- a/support/support_record_failure.c
+++ b/support/support_record_failure.c
@@ -32,8 +32,12 @@ 
    zero, the failure of a test can be detected.
 
    The init constructor function below puts *state on a shared
-   annonymous mapping, so that failure reports from subprocesses
-   propagate to the parent process.  */
+   anonymous mapping, so that failure reports from subprocesses
+   propagate to the parent process.
+
+   support_record_failure_init is exposed so it can be called explicitly
+   in case this API needs to be used from a constructor.  */
+
 struct test_failures
 {
   unsigned int counter;
@@ -41,10 +45,14 @@  struct test_failures
 };
 static struct test_failures *state;
 
-static __attribute__ ((constructor)) void
-init (void)
+__attribute__ ((constructor)) void
+support_record_failure_init (void)
 {
-  void *ptr = mmap (NULL, sizeof (*state), PROT_READ | PROT_WRITE,
+  void *ptr;
+
+  if (state != NULL)
+    return;
+  ptr = mmap (NULL, sizeof (*state), PROT_READ | PROT_WRITE,
                     MAP_ANONYMOUS | MAP_SHARED, -1, 0);
   if (ptr == MAP_FAILED)
     {