Skip to content

Commit

Permalink
fix: avoid calls to isinf or isnan for musl support
Browse files Browse the repository at this point in the history
This patch is a result of rake-compiler-dock using centos
7 (manylinux2014) to cross-compile.

Centos, for reasons I have not been able to discern, implements
`isnan` and `isinf` as a function and not as a macro. Debian knows how
to resolve that function at dynamic-link time (despite using a macro
at compile time), but musl-based systems (like alpine) do not. Running
`nm` on nokogiri.so created on such a centos system shows:

```
                 U __isinf@@GLIBC_2.2.5
                 U __isnan@@GLIBC_2.2.5
```

(see #2142 for more info)

This patch avoids using glibc's `isnan` and `isinf` calls, instead using libxml2's fallback
implementation. There's history here, see libxml2 commit 8813f39:

    commit 8813f39
    Author: Nick Wellnhofer <wellnhofer@aevum.de>
    Date:   2017-09-21 00:11:26 +0200

        Simplify XPath NaN, inf and -0 handling

        Use C99 macros NAN, INFINITY, isnan, isinf. If they're not available:

        - Assume that (0.0 / 0.0) generates a NaN and !(x == x) tests for NaN.
        - Use C89's HUGE_VAL for INFINITY.

        Remove manual handling of NaN, infinity and negative zero in functions
        xmlXPathValueFlipSign and xmlXPathDivValues.

        Remove xmlXPathGetSign. All the tests for negative zero can be replaced
        with a test for negative or positive zero.

        Simplify xmlXPathRoundFunction.

        Remove Trio dependency.

        This should work on IEEE 754 compliant implementations even if the C99
        macros aren't available, but will likely break some ancient platforms.
        If problems arise, my plan is to port the relevant trionan.c solution
        to xpath.c. Note that non-compliant implementations are impossible
        to fully support, anyway, since XPath requires IEEE 754.

This patch would be unnecessary if any of the following was true:

* centos implements these as macros, and doesn't generate an unresolved symbol for either in the shared library
* we had a way to ensure `__isinf` and `__isnan` resolve on musl (e.g., we implement them locally)
  • Loading branch information
flavorjones committed Dec 31, 2020
1 parent 53b333f commit 713e723
Showing 1 changed file with 81 additions and 0 deletions.
81 changes: 81 additions & 0 deletions patches/libxml2/0009-avoid-isnan-isinf.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
This patch is a result of rake-compiler-dock using centos 7 (manylinux2014) to cross-compile.

Centos, for reasons I have not been able to discern, implements `isnan` and `isinf` as a function
and not as a macro. Debian knows how to resolve that function at dynamic-link time (despite using a
macro at compile time), but musl-based systems (like alpine) do not. Running `nm` on nokogiri.so
created on such a centos system shows:

```
U __isinf@@GLIBC_2.2.5
U __isnan@@GLIBC_2.2.5
```

(see https://github.com/sparklemotion/nokogiri/pull/2142 for more info)

This patch avoids using glibc's `isnan` and `isinf` calls, instead using libxml2's fallback
implementation. There's history here, see libxml2 commit 8813f39:

commit 8813f39
Author: Nick Wellnhofer <wellnhofer@aevum.de>
Date: 2017-09-21 00:11:26 +0200

Simplify XPath NaN, inf and -0 handling

Use C99 macros NAN, INFINITY, isnan, isinf. If they're not available:

- Assume that (0.0 / 0.0) generates a NaN and !(x == x) tests for NaN.
- Use C89's HUGE_VAL for INFINITY.

Remove manual handling of NaN, infinity and negative zero in functions
xmlXPathValueFlipSign and xmlXPathDivValues.

Remove xmlXPathGetSign. All the tests for negative zero can be replaced
with a test for negative or positive zero.

Simplify xmlXPathRoundFunction.

Remove Trio dependency.

This should work on IEEE 754 compliant implementations even if the C99
macros aren't available, but will likely break some ancient platforms.
If problems arise, my plan is to port the relevant trionan.c solution
to xpath.c. Note that non-compliant implementations are impossible
to fully support, anyway, since XPath requires IEEE 754.

This patch would be unnecessary if any of the following was true:

* centos implements these as macros, and doesn't generate an unresolved symbol for either in the shared library
* we had a way to ensure `__isinf` and `__isnan` resolve on musl (e.g., we implement them locally)

diff --git a/xpath.c b/xpath.c
index 9f64ab9..5b6d999 100644
--- a/xpath.c
+++ b/xpath.c
@@ -509,11 +509,7 @@ xmlXPathInit(void) {
*/
int
xmlXPathIsNaN(double val) {
-#ifdef isnan
- return isnan(val);
-#else
return !(val == val);
-#endif
}

/**
@@ -524,15 +520,11 @@ xmlXPathIsNaN(double val) {
*/
int
xmlXPathIsInf(double val) {
-#ifdef isinf
- return isinf(val) ? (val > 0 ? 1 : -1) : 0;
-#else
if (val >= INFINITY)
return 1;
if (val <= -INFINITY)
return -1;
return 0;
-#endif
}

#endif /* SCHEMAS or XPATH */

0 comments on commit 713e723

Please sign in to comment.