Hi Barak,
thank you for your two big-endian reports (3 items). All items are dealt
with in *SVN 2157*.
Summary
-------
Fixed: 3 (#1 _Atomic_word, #2 CDR ravel byte order,
#3 packed Boolean ravels)
Since big-endian hardware is not available here, there are no new
testcases; the existing ones (Quad_CR.tc, ZZ17_Regression.tc, AP100.tc,
AP210.tc) cover the problems on big-endian machines. On amd64 all
testcases pass, and B ≡ 12 ⎕CR 11 ⎕CR B as well as Q ≡ 41 ⎕CR QP hold for
nested, Unicode, float, complex, APV and (packed) Boolean values.
Fixed
-----
#1 Fixed differently than in your patch: GNU APL no longer uses
_Atomic_word or ext/atomicity.h at all. The atomic counters and locks
for parallel execution are now C++11 std::atomic<int>, which is the
same on every compiler, C++ library and platform. This also removed
the platform-specific OSX and Solaris variants and their ./configure
checks.
#2 The inline copy of the trunk patch had lost its white space, so I
re-typed it by hand. The result is the same as yours: byte-wise
little-endian helpers (CDR_header::get_le32(), CDR::get_4_le(),
get_8_le(), get_float_le()) for all ravel reads in from_CDR() and
AP210. The encoder and the CDR format are unchanged.
#3 Your patch was applied as is. In addition, two more byte-wise readers
in Value.cc now use Ravel::get_packed_bit(): the Boolean to float and
the Boolean to complex upgrades.
Your 0014 patch for 2.0 was not applied to trunk since trunk does not
need it. A packed variable saved by (patched) 2.0 (flg="0x4400",
bytes="⁹...") can still not be )LOADed by trunk; as you say, an
unpatched 2.0 saved garbage for such values, so I left that alone.
I would be glad if you could run trunk on sparc64 as you offered, in
particular Quad_CR.tc, ZZ17_Regression.tc, AP100.tc, AP210.tc and a full
make test.
Best Regards,
Jürgen
PS: The analysis of your report and the resulting changes were made with
the help of Claude Code (Anthropic). All changes were reviewed by me and
verified against the GNU APL testcases before being committed.
On 11/2/26 09:36, Barak A. Pearlmutter wrote:
Hi Jürgen,
While updating the Debian package of GNU APL 2.0 I tested it on sparc64
(big endian) and ran into two problems that are still present on SVN
trunk. Patches against current trunk are below. A version
with these applied to 2.0 will be uploaded to Debian shortly.
1. Build failure on sparc64: _Atomic_word
When PARALLEL_ENABLED is not defined, src/Parallel.hh does
typedef int _Atomic_word;
but _Atomic_word is a reserved identifier that libstdc++ already
defines (via <bits/atomic_word.h>), and on some architectures it is
not int. On sparc64 the build fails with
Parallel.hh:70:13: error: conflicting declaration 'typedef int
_Atomic_word'
sparc64-linux-gnu/c++/16/bits/atomic_word.h:29:16: note: previous
declaration as 'typedef long int _Atomic_word'
The patch only provides the fallback typedef when not using libstdc++.
2. CDR ravel items decoded in host byte order
As the comment in from_CDR() (Bugs9 #5) says, GNU APL's CDR is
deliberately mixed endian: the header is big endian and CDR::fill()
writes ravel items (4-byte ints, 4-byte chars, nested offsets, 8-byte
floats) little endian, byte by byte. But from_CDR() reads them back
with *reinterpret_cast<const uint32_t *> / <const APL_Float *>, i.e.,
in host byte order. That is little endian on x86, so it works there,
but on a big-endian host every value is byte-swapped. AP210 decodes
the integers of its command variable C210 the same way.
Since shared variables travel as CDR, this breaks a lot on big-endian
machines (s390x, sparc64, ppc64, ...), e.g.:
12 ⎕CR 11 ⎕CR B is not ≡ B (Quad_CR.tc)
0 ⎕SVO 'X' ◊ X←260⍴2 ◊ X gives 33554432 ... (ZZ17_Regression.tc)
every AP210 command is rejected as an invalid subcommand
(AP210.tc)
The casts are also unaligned in places (e.g. an 8-byte float at
16+4×rank), which strict-alignment CPUs may not tolerate.
The patch adds byte-wise little-endian helpers (CDR_header::get_le32(),
CDR::get_4_le(), get_8_le(), get_float_le()) and uses them for all
ravel reads in from_CDR(), including CDR_PROG64, and in AP210. The
encoder is unchanged, so the CDR format stays the same.
Testing: with the 2.0 versions of these patches the Debian package
builds on sparc64 (on the Debian porter box), and Quad_CR.tc,
ZZ17_Regression.tc, AP100.tc and AP210.tc pass there; before, they
failed. I also applied the trunk versions below to current trunk and
checked them on amd64: it builds, those testcases pass, and
B ≡ 12 ⎕CR 11 ⎕CR B holds for nested, Unicode, float and complex
values. I have not run the trunk versions on a big-endian machine.
Thanks for GNU APL!
--Barak.
--- a/src/Parallel.hh
+++ b/src/Parallel.hh
@@ -67,7 +67,13 @@
//
// parallel execution disabled, no need for atomicity
//
+ // _Atomic_word is reserved and already defined by libstdc++, with
+ // an architecture-dependent type (e.g. long on sparc64), so only
+ // define it when not using libstdc++.
+ //
+# ifndef __GLIBCXX__
typedef int _Atomic_word;
+# endif
/// @param counter counter to update atomically
/// @param increment amount to add
--- a/src/APs/AP210.cc
+++ b/src/APs/AP210.cc
@@ -352,11 +352,10 @@
}
else if (cdr.get_type() == CDR_INT32)
{
- const uint32_t * ravel =
- reinterpret_cast<const uint32_t *>(cdr.ravel());
- op = ravel[0];
- if (nelm >= 2) ctx.rec_num = ravel[1];
- if (nelm >= 3) ctx.rec_size = ravel[2];
+ const uint8_t * ravel = cdr.ravel();
+ op = CDR_header::get_le32(ravel);
+ if (nelm >= 2) ctx.rec_num = CDR_header::get_le32(ravel + 4);
+ if (nelm >= 3) ctx.rec_size = CDR_header::get_le32(ravel + 8);
}
else // neither int nor bool:
{
--- a/src/CDR.cc
+++ b/src/CDR.cc
@@ -171,7 +171,7 @@
{
loop(n, nelm)
{
- APL_Integer d = *reinterpret_cast<const uint32_t *>(ravel + 4*n);
+ APL_Integer d = get_4_le(ravel + 4*n);
if (d & 0x80000000) d |= 0xFFFFFFFF00000000ULL;
Z->next_ravel_Int(d);
}
@@ -181,7 +181,7 @@
loop(n, nelm)
{
const APL_Float v =
- *reinterpret_cast<const APL_Float *>(ravel + 8*n);
+ get_float_le(ravel + 8*n);
Z->next_ravel_Float(v);
}
}
@@ -190,9 +190,9 @@
loop(n, nelm)
{
const APL_Float vr =
- *reinterpret_cast<const APL_Float *>(ravel + 16*n);
+ get_float_le(ravel + 16*n);
const APL_Float vi =
- *reinterpret_cast<const APL_Float *>(ravel + 16*n + 8);
+ get_float_le(ravel + 16*n + 8);
Z->next_ravel_Complex(vr, vi);
}
}
@@ -209,15 +209,15 @@
{
loop(n, nelm)
{
- const uint32_t d = *reinterpret_cast<const uint32_t *>(ravel+4*n);
+ const uint32_t d = get_4_le(ravel+4*n);
Z->next_ravel_Char(Unicode(d));
}
}
else if (vtype == CDR_PROG64) // arithmetic progression: first, incr
{
- APL_Integer first = *reinterpret_cast<const uint32_t *>(ravel);
+ APL_Integer first = get_4_le(ravel);
if (first & 0x80000000) first |= 0xFFFFFFFF00000000ULL;
- APL_Integer incr = *reinterpret_cast<const uint32_t *>(ravel + 4);
+ APL_Integer incr = get_4_le(ravel + 4);
if (incr & 0x80000000) incr |= 0xFFFFFFFF00000000ULL;
loop(n, nelm) Z->next_ravel_Int(first + n*incr);
@@ -227,7 +227,7 @@
loop(n, nelm)
{
APL_Integer offset =
- *reinterpret_cast<const uint32_t *>(ravel + 4*n);
+ get_4_le(ravel + 4*n);
if ((size_t)(offset + 14) > cdr.size())
{
MORE_ERROR() << "CDR nested offset " << offset
@@ -275,8 +275,7 @@
if (sub_vtype == 1) // 4 byte int
{
- APL_Integer d = *reinterpret_cast<const uint32_t *>
- (sub_ravel);
+ APL_Integer d = get_4_le(sub_ravel);
if (d & 0x80000000) d |= 0xFFFFFFFF00000000ULL;
Z->next_ravel_Int(d);
continue; // next n
@@ -285,18 +284,15 @@
if (sub_vtype == 2) // 8 byte real
{
Z->next_ravel_Float(
- *reinterpret_cast<const APL_Float *>
- (sub_ravel));
+ get_float_le(sub_ravel));
continue; // next n
}
if (sub_vtype == 3) // 16 byte complex
{
Z->next_ravel_Complex(
- *reinterpret_cast<const APL_Float *>
- (sub_ravel),
- *reinterpret_cast<const APL_Float *>
- (sub_ravel + 8));
+ get_float_le(sub_ravel),
+ get_float_le(sub_ravel + 8));
continue; // next n
}
@@ -317,8 +313,7 @@
// in the wrong order (e.g. U+012C came back as
// 0x2C010000).
Z->next_ravel_Char(Unicode(
- *reinterpret_cast<const uint32_t *>
- (sub_ravel)));
+ get_4_le(sub_ravel)));
continue; // next n
}
@@ -356,10 +351,10 @@
const uint8_t * sub_body = sub_data + 16 + 4*sub_rank;
APL_Integer first =
- *reinterpret_cast<const uint32_t *>(sub_body);
+ get_4_le(sub_body);
if (first & 0x80000000) first |= 0xFFFFFFFF00000000ULL;
APL_Integer incr =
- *reinterpret_cast<const uint32_t *>(sub_body + 4);
+ get_4_le(sub_body + 4);
if (incr & 0x80000000) incr |= 0xFFFFFFFF00000000ULL;
Value_P sub_val(sh, LOC);
--- a/src/CDR.hh
+++ b/src/CDR.hh
@@ -58,6 +58,24 @@
return ret | get_4_be(data + 4);
}
+ /// return 4 bytes starting at \b data in little endian byte order
+ /// (the byte order in which fill() stores ravel items)
+ static uint32_t get_4_le(const uint8_t * data)
+ { return CDR_header::get_le32(data); }
+
+ /// return 8 bytes starting at \b data in little endian byte order
+ static uint64_t get_8_le(const uint8_t * data)
+ { return get_4_le(data) | uint64_t(get_4_le(data + 4)) << 32; }
+
+ /// return the APL_Float stored at \b data in little endian byte order
+ static APL_Float get_float_le(const uint8_t * data)
+ {
+ const uint64_t bits = get_8_le(data);
+ APL_Float ret;
+ memcpy(&ret, &bits, sizeof(ret));
+ return ret;
+ }
+
/// create a value from a CDR_string, throwing
/// DOMAIN ERROR if cdr is ill-formed
/// @param cdr CDR byte string to deserialise
--- a/src/CDR_string.hh
+++ b/src/CDR_string.hh
@@ -60,6 +60,14 @@
ret |= *data++;
return ret;
}
+
+ /// convert little endian 32 bit value to 32 bit host value. CDR
+ /// headers are big endian, but ravel items are stored little endian.
+ static uint32_t get_le32(const uint8_t * data)
+ {
+ return uint32_t(data[0]) | uint32_t(data[1]) << 8
+ | uint32_t(data[2]) << 16 | uint32_t(data[3]) << 24;
+ }
};
/// a string containing a CDR record
