bugGNU Octave - Bugs: bug #63480, Build error on Cygwin

 
 

bug #63480: Build error on Cygwin

Submitter:  Markus Mützel <mmuetzel>
Submitted:  Wed 07 Dec 2022 09:55:09 AM UTC
   
 
Category:  Configuration and Build System Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Build Failure
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Other
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 12 Dec 2022 02:03:19 PM UTC, comment #10: 

Cygwin has been updated to 3.4.2 over the weekend.
I re-triggered the latest CI run and it is linking correctly now.

Closing as fixed.

Markus Mützel <mmuetzel>
Group administrator
Fri 09 Dec 2022 12:39:27 PM UTC, comment #9: 

I updated to 3.5.0-0.25.g55de3fdd0e85. With it, octave-svgconvert links successfully. 👍

Thank you again for the quick fix. Looking forward for a release of 3.4.1.

Markus Mützel <mmuetzel>
Group administrator
Fri 09 Dec 2022 11:46:13 AM UTC, comment #8: 

Thanks!

https://cygwin.com/git/?p=newlib-cygwin.git;a=commitdiff;h=3a910b624a1b


Corinna

 Anonymous
Fri 09 Dec 2022 11:33:19 AM UTC, comment #7: 

Understood. Maybe something like this?

- Fix a backward incompatibility problem in the definition of the
  base type of the stdio type FILE. This requires that C++ binaries
  compiled with 3.4.0 that have a public facing interface using
  FILE need to be recompiled.
  Addresses: https://savannah.gnu.org/bugs/index.php?63480


 Markus Mützel <mmuetzel>
Group administrator
Fri 09 Dec 2022 09:39:46 AM UTC, comment #6: 

The release messages are supposed to be concise, not supposed to explain
every gory detail.  However, if you think it deserves a better explanation,
please feel free to reply with another text or (better for my lazy ass)
just send a git patch with different wording to the cygwin or cygwin-patches ML.
I'm always open to suggestions.

In terms of the compatibility to 3.4.0, keep in mind that it's a rather
limited incompatibility.  We're basically talking about two problems:

- The name of an underlying internal datatype, indirectly exposed to
  userspace.  This only affects programming languages using name mangling
  and using the FILE type in public facing API.

- Exposure of a difference in structure layout.  Three potential issues:

  - Size.  This is no issue.  First, FILE is used as pointer or ref in APIs,
    not as value.  Second, the size of __sFILE is the same as the size of
    __sFILE64 on LP64 systems.

  - Public macros.  No issue.  There's a small set of macros accessing a
    FILE * in stdio.h, utilizing the knowledge of the internal data layout
    of FILE.  These only access members of FILE which are the same size and
    the same offset in __sFILE as well as in __sFILE64.

  - Applications accessing FILE members.  This should be a no-no, given
    FILE is defined as an opaque type.  But even if, those apps typically
    access the same members as the macros in stdio.h, so they are unaffected.
    If there are still apps left at this point, they are hopelessly
    accessing internal implementation details and deserve a spanking.

That basically leaves the first point, the name mangling problem.

Last but not least, the short shelf life of 3.4.0 limits the number of
affected apps, too.  Hopefully.


Corinna

 Anonymous
Fri 09 Dec 2022 07:29:13 AM UTC, comment #5: 

Thank you for the pointers, I'll try to test that as soon as possible.

Just to check if I understand those changes correctly: From what I can tell, they restore binary compatibility with version of Cygwin prior to 3.4.0 (which is a good thing). But they'll break compatibility for binaries that were built with version 3.4.0.
If that is the case, maybe the note in the release notes about this could be clearer.
https://sourceware.org/git/?p=newlib-cygwin.git;a=commitdiff;h=6429a7a7f6e6881ae7b81b4b566ac6943aa74f2c

 Markus Mützel <mmuetzel>
Group administrator
Thu 08 Dec 2022 04:50:05 PM UTC, comment #4: 


comment #2:

> This has been fixed upstream with this change:
> https://cygwin.com/git/?p=newlib-cygwin.git;a=commit;h=0f376ae22036bac2fbc7863264909b599e42616c
>
> Thank you for the quick reaction!
>
> Afaict, this should be part of Cygwin 3.4.1 when it will be released. Until then, the CI on Cygwin remains broken. So, leaving this open for now.
>


Please note that commit 0f376ae22036 ("Cygwin: rename __sFILE
to __sFILE64 for backward compatibility") has been superseded
by commit 55de3fdd0e85 ("Cygwin: define FILE as struct
__sFILE64, not as __sFILE").

That's the reason I'm asking for another test.


Corinna

 Anonymous
Thu 08 Dec 2022 04:32:42 PM UTC, comment #3: 

Hi,

Cygwin maintainer here.

The problem is that we defined __LARGE64_FILES up to Cygwin 3.3.6 and
with commit 2902b3a09e0a ("Cygwin: drop requirement to build newlib's
stdio64") we stopped defining it, because it was always kind of incorrect
for 64 bit and we didn't want to maintain two sets of stdio functions
doing the same thing, e.g., fopen and fopen64.

Unfortunately that means, the base type of FILE was struct __sFILE64
up to Cygwin 3.3.6, and struct __sFILE starting with 3.4.0, which
results in the above problem due to C++ name mangling.

I just pushed a patch defining FILE as struct __sFILE64 again,
https://sourceware.org/git/?p=newlib-cygwin.git;a=commit;h=55de3fdd0e8

Cygwin release 3.5.0-0.25.g55de3fdd0e85, available via Cygwin's standard
installation procedure as a test release in about an hour, contains that
fix.

For testing, please make sure to install at least the cygwin and matching
cygwin-devel package.  You need at least Windows 8.1 for testing.

If this fixes the issue for you, the patch will go into an upcoming
Cygwin 3.4.1 bugfix release pretty soon.


Corinna

 Anonymous
Thu 08 Dec 2022 01:55:54 PM UTC, comment #2: 

This has been fixed upstream with this change:
https://cygwin.com/git/?p=newlib-cygwin.git;a=commit;h=0f376ae22036bac2fbc7863264909b599e42616c

Thank you for the quick reaction!

Afaict, this should be part of Cygwin 3.4.1 when it will be released. Until then, the CI on Cygwin remains broken. So, leaving this open for now.

Markus Mützel <mmuetzel>
Group administrator
Wed 07 Dec 2022 09:56:47 AM UTC, comment #1: 

Oops. Typo: Cygwin was updated from version 3.3.6 (not 3.3.8) to 3.4.0.

 Markus Mützel <mmuetzel>
Group administrator
Wed 07 Dec 2022 09:55:09 AM UTC, original submission:  

This was first noted in bug #63437#30 and bug #63437#37. But it is probably unrelated to those changes.
IIUC, Cygwin coincidentally updated from 3.3.8 to 3.4.0 when the changes for that report were applied.

Linking octave-svgconvert for Cygwin fails with the following error on the CI:
https://github.com/gnu-octave/octave/actions/runs/3616932076/jobs/6095331859#step:10:29241

/usr/lib/gcc/x86_64-pc-cygwin/11/../../../../x86_64-pc-cygwin/bin/ld: src/octave_svgconvert-octave-svgconvert.o: in function `main':
/cygdrive/d/a/octave/octave/.build/../src/octave-svgconvert.cc:912: undefined reference to `__imp__ZN5QFile4openEP7__sFILE6QFlagsIN9QIODevice12OpenModeFlagEES2_IN11QFileDevice14FileHandleFlagEE'
collect2: error: ld returned 1 exit status


The undefined symbol demangles to
QFile::open(__sFILE*, QFlags<QIODevice::OpenModeFlag>, QFlags<QFileDevice::FileHandleFlag>)

The symbol that is exported by the Qt5Core library is the following:

$  nm -C /lib/libQt5Core.dll.a | grep QFile::open
0000000000000000 T QFile::open(int, QFlags<QIODevice::OpenModeFlag>, QFlags<QFileDevice::FileHandleFlag>)
0000000000000000 T QFile::open(__sFILE64*, QFlags<QIODevice::OpenModeFlag>, QFlags<QFileDevice::FileHandleFlag>)
0000000000000000 T QFile::open(QFlags<QIODevice::OpenModeFlag>)


Note the `__sFILE64*` instead of the `__sFILE*` that we'd like to use.

The following change avoids the link error:

diff -r deb553ac2c54 src/octave-svgconvert.cc
--- a/src/octave-svgconvert.cc        Tue Dec 06 15:45:27 2022 -0500
+++ b/src/octave-svgconvert.cc        Wed Dec 07 10:51:59 2022 +0100
@@ -27,6 +27,9 @@
 #  include "config.h"
 #endif

+#define __LARGE64_FILES
+
+#include <cstdio>
 #include <iostream>

 #if defined (OCTAVE_USE_WINDOWS_API)


I'm not proposing that this should be applied in Octave. Rather, it's just a work-around to illustrate where there might be a regression in Cygwin 3.4.0.

This might be a regression from the following upstream change:
https://github.com/msys2/msys2-runtime/commit/2902b3a09e0a45c4971b59f78ab175b71b3c755e

Markus Mützel <mmuetzel>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Submitted the item)

    •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-12-12 mmuetzel StatusNone Fixed
        Open/ClosedOpen Closed

    Back to the top

    Copyright © 2024 Free Software Foundation, Inc.
    Verbatim copying and distribution of this entire article is permitted in any medium, provided this notice is preserved.
    The Levitating, Meditating, Flute-playing Gnu logo is a GNU GPL'ed image provided by the Nevrax Design Team.
    Page source code

    Powered by Savane 3.13-8a8d.
    Corresponding source code