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
