Tue 01 Aug 2017 06:32:25 PM UTC, comment #19:
Rik: I'll make this change.
|
Tue 01 Aug 2017 06:21:35 PM UTC, comment #18:
I took a closer look at mach-info.cpp and more than one routine is using the unguaranteed union behavior. So, rather than convert the whole thing to C++ it probably does make sense to compile the file with a straight 'C' compiler. I don't know what that means for the use of namespaces which are in the file. I filed a new bug report to keep track of it.
|
Wed 26 Jul 2017 11:46:02 PM UTC, comment #17:
It seems clumsy to leave C++ and change the build system. Why not change mach-info.cpp to use an accepted method in C++?
|
Mon 24 Jul 2017 05:38:47 PM UTC, comment #16:
Maybe I should just move the is_big_endian and get_float_format functions to a C-language file?
|
Mon 24 Jul 2017 05:23:55 PM UTC, comment #15:
There is a problem with at least some of the code in mach-info.cc.
The code sets one member of a union, and then reads from another. In C, this behavior is guaranteed. In C++, this is not guaranteed. From cppreference.com (http://en.cppreference.com/w/cpp/language/union):
For a much longer article corroborating this, and where I got the idea for using memcpy, see http://dbp-consulting.com/tutorials/StrictAliasing.html.
|
Mon 24 Jul 2017 04:54:51 PM UTC, comment #14:
Yeah, no problem. I checked in a changeset:
http://hg.savannah.gnu.org/hgweb/octave/rev/e263cf468abd
|
Mon 24 Jul 2017 04:42:45 PM UTC, comment #13:
@jwe: I was feeling lazy so I thought I would just ask the question rather than troll through the source. It seems perfectly reasonable to use the code from comment #10 in place of what I wrote.
|
Mon 24 Jul 2017 04:34:06 PM UTC, comment #12:
Rik: It is initialized at run time the first time it is needed. See the get_float_format function in liboctve/system/mach-info.cc and the other functions there that use it.
|
Mon 24 Jul 2017 04:27:14 PM UTC, comment #11:
@jwe: I think your cset from comment #10 will work.
Is octave::mach_info::native_float_format () determined at compile-time or runtime? We need runtime evaluation because when cross-building with MXE the compile-time platform may have different endianness than the target.
|
Mon 24 Jul 2017 03:56:45 PM UTC, comment #10:
If I understand the intent of this code correctly, then I think the change would be as in the attached file. If so, then this seems clearer than using memcpy to set bytes.
(file #41317)
|
Mon 24 Jul 2017 03:43:10 PM UTC, comment #9:
Sorry to arrive a bit late on this one, but if these tests were just about determining endianness, could we use the functions from octave::mach_info instead?
|
Mon 24 Jul 2017 12:24:42 AM UTC, comment #8:
Great. I'm closing this report.
|
Sun 23 Jul 2017 07:55:17 PM UTC, comment #7:
That works too.
Dmitri.
--
|
Sun 23 Jul 2017 05:36:58 PM UTC, comment #6:
Detecting endianness turns out to be a difficult problem in C++. I checked in a different solution which should avoid the align warning from sanitize (http://hg.savannah.gnu.org/hgweb/octave/rev/f864950f837c).
|
Sun 23 Jul 2017 01:50:39 AM UTC, comment #5:
make check runs w/o problem.
Dmitri.
--
|
Sun 23 Jul 2017 01:20:56 AM UTC, comment #4:
Both problems appears fixed:
I am running full make check.
Dmitri.
|
Sun 23 Jul 2017 01:06:22 AM UTC, comment #3:
I can't reproduce the second issue, but it is probably because I have an older gcc. Can you try the attached patch align.diff. I've made a guess about what is wrong and I think this should fix it.
(file #41294)
|
Sun 23 Jul 2017 12:34:20 AM UTC, comment #2:
I fixed the first problem detected in oct-inttypes.cc. See cset http://hg.savannah.gnu.org/hgweb/octave/rev/203d3676730f.
|
Fri 21 Jul 2017 07:41:51 PM UTC, comment #1:
My "verbatim"s got messed up.
Trying again:
HG ID for this build is "cda0614beaec"
running "make check" results in two runtime errors:
Dmitri.
--
|
Fri 21 Jul 2017 07:33:45 PM UTC, original submission:
HG ID for this build is "cda0614beaec"
|