Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build of 0.5.1 broken with MinGW #127

Closed
SpaceIm opened this issue Nov 26, 2023 · 15 comments
Closed

Build of 0.5.1 broken with MinGW #127

SpaceIm opened this issue Nov 26, 2023 · 15 comments
Assignees
Milestone

Comments

@SpaceIm
Copy link

SpaceIm commented Nov 26, 2023

Build of 0.5.1 with MinGW11.0.0/GCC13 fails (it was working fine in 0.5.0):

[7/15] Building C object CMakeFiles/base64-bin.dir/bin/base64.c.obj
FAILED: CMakeFiles/base64-bin.dir/bin/base64.c.obj
D:\Programmes\mingw64\bin\gcc.exe  -IC:/Users/spaceim/.conan2/p/b/base6763a9b6f78112/b/src/include -m64 -O3 -DNDEBUG -MD -MT CMakeFiles/base64-bin.dir/bin/base64.c.obj -MF CMakeFiles\base64-bin.dir\bin\base64.c.obj.d -o CMakeFiles/base64-bin.dir/bin/base64.c.obj -c C:/Users/spaceim/.conan2/p/b/base6763a9b6f78112/b/src/bin/base64.c
C:/Users/spaceim/.conan2/p/b/base6763a9b6f78112/b/src/bin/base64.c:11:10: fatal error: sys/uio.h: No such file or directory
   11 | #include <sys/uio.h>
      |          ^~~~~~~~~~~

sys/uio.h is not provided by MinGW.

@aklomp
Copy link
Owner

aklomp commented Nov 26, 2023

Indeed. The base64 demo utility has been rewritten to use writev(2), which is defined in that header. Does MinGW declare that POSIX function somewhere else?

@SpaceIm
Copy link
Author

SpaceIm commented Nov 26, 2023

AFAIK, there is no writev defined in MinGW.

@BurningEnlightenment
Copy link
Contributor

I'm not surprised that MinGW doesn't provide writev(2) as there is no equivalent WinAPI function, i.e. something that works without requiring the handle to be opened with FILE_FLAG_OVERLAPPED or the like. The best you can do for normal file handles on Windows is repeated write/WriteFile calls.

@aklomp
Copy link
Owner

aklomp commented Nov 27, 2023

So the issue isn't in this project, but with incomplete POSIX emulation in MinGW. I guess there's two things we can do. Either we detect MinGW at compile time and supply a stub writev(2) implementation that loops around write(2), or we can declare that non-POSIX platforms are outside our jurisdiction and don't get support from us.

I'm tempted to do the second, because base64 is just a test binary, but I guess the first isn't that much work and I also know how to detect MinGW reliably from working with/around it at $DAYJOB. It feels slightly servile and will add ugly conditionals to the code, but it brings the code to run somewhere it otherwise wouldn't.

A fix from my side will have to wait a little bit because my availability for the next week or so is still very limited.

@SpaceIm
Copy link
Author

SpaceIm commented Nov 27, 2023

In conan recipe of base64, I've proposed to always disable build of this executable: conan-io/conan-center-index#21386

@sergeevabc
Copy link

Windows 7 x64 with SSE3 Core2Duo user here. I am glad to see that this issue has already been covered. Could someone be so kind to generate a dirty .exe and attach here as zip, while the team is looking for a general solution?

aklomp added a commit that referenced this issue Jan 3, 2024
As of v0.5.1, the `base64' demo program no longer compiled under MinGW because
MinGW does not emulate the `writev(2)' system call.

Fix this by detecting MinGW at compile time and optionally emulating
`writev(2)' as a series of `write(2)' calls.

Resolves #127.
@aklomp
Copy link
Owner

aklomp commented Jan 3, 2024

I pushed a branch called issue127 which emulates writev(2) on MinGW. I'd be happy to hear if someone can test it properly. I did test the emulation on Linux (by manually defining EMULATE_WRITEV) and it seems to work OK.

aklomp added a commit that referenced this issue Jan 3, 2024
As of v0.5.1, the `base64' demo program no longer compiled under MinGW because
MinGW does not emulate the `writev(2)' system call.

Fix this by detecting MinGW at compile time and optionally emulating
`writev(2)' as a series of `write(2)' calls.

Resolves #127.
@sergeevabc
Copy link

@aklomp, thank you for taking a moment to revisit this issue.
Alas, I was not able to compile a binary, at least by means of w64devkit by @skeeto.

Click to see details…
$ w64devkit.exe

~ # cd base64

~/base64 # SSSE3_CFLAGS=-mssse3 make

cc -std=c99 -O3 -Wall -Wextra -pedantic -o bin/base64.o -c bin/base64.c
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib  -o lib/arch/avx512/codec.o -c lib/arch/avx512/codec.c
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib  -o lib/arch/avx2/codec.o -c lib/arch/avx2/codec.c
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib -o lib/arch/generic/codec.o -c lib/arch/generic/codec.c
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib  -o lib/arch/neon32/codec.o -c lib/arch/neon32/codec.c
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib  -o lib/arch/neon64/codec.o -c lib/arch/neon64/codec.c
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib -mssse3 -o lib/arch/ssse3/codec.o -c lib/arch/ssse3/codec.c
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib  -o lib/arch/sse41/codec.o -c lib/arch/sse41/codec.c
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib  -o lib/arch/sse42/codec.o -c lib/arch/sse42/codec.c
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib  -o lib/arch/avx/codec.o -c lib/arch/avx/codec.c
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib -o lib/lib.o -c lib/lib.c
lib/lib.c:18:1: warning: 'base64_stream_encode_init' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
   18 | base64_stream_encode_init (struct base64_state *state, int flags)
      | ^~~~~~~~~~~~~~~~~~~~~~~~~
lib/lib.c:31:1: warning: 'base64_stream_encode' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
   31 | base64_stream_encode
      | ^~~~~~~~~~~~~~~~~~~~
lib/lib.c:43:1: warning: 'base64_stream_encode_final' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
   43 | base64_stream_encode_final
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~
lib/lib.c:68:1: warning: 'base64_stream_decode_init' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
   68 | base64_stream_decode_init (struct base64_state *state, int flags)
      | ^~~~~~~~~~~~~~~~~~~~~~~~~
lib/lib.c:81:1: warning: 'base64_stream_decode' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
   81 | base64_stream_decode
      | ^~~~~~~~~~~~~~~~~~~~
lib/lib.c:103:1: warning: 'base64_encode' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
  103 | base64_encode
      | ^~~~~~~~~~~~~
lib/lib.c:136:1: warning: 'base64_decode' redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
  136 | base64_decode
      | ^~~~~~~~~~~~~
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib -o lib/codec_choose.o -c lib/codec_choose.c
cc -std=c99 -O3 -Wall -Wextra -pedantic -Ilib -o lib/tables/tables.o -c lib/tables/tables.c
ld -r -o lib/libbase64.o lib/arch/avx512/codec.o lib/arch/avx2/codec.o lib/arch/generic/codec.o lib/arch/neon32/codec.o lib/arch/neon64/codec.o lib/arch/ssse3/codec.o lib/arch/sse41/codec.o lib/arch/sse42/codec.o lib/arch/avx/codec.o lib/lib.o lib/codec_choose.o lib/tables/tables.o
objcopy --keep-global-symbols=lib/exports.txt lib/libbase64.o
cc -std=c99 -O3 -Wall -Wextra -pedantic -o bin/base64 bin/base64.o lib/libbase64.o
.../w64devkit/bin/ld.exe: bin/base64.o:base64.c:(.text.startup+0x253): undefined reference to `__imp_base64_stream_decode_init'
.../w64devkit/bin/ld.exe: bin/base64.o:base64.c:(.text.startup+0x28e): undefined reference to `__imp_base64_stream_decode'
.../w64devkit/bin/ld.exe: bin/base64.o:base64.c:(.text.startup+0x347): undefined reference to `__imp_base64_stream_encode_init'
.../w64devkit/bin/ld.exe: bin/base64.o:base64.c:(.text.startup+0x353): undefined reference to `__imp_base64_stream_encode'
.../w64devkit/bin/ld.exe: bin/base64.o:base64.c:(.text.startup+0x3d2): undefined reference to `__imp_base64_stream_encode_final'
collect2.exe: error: ld returned 1 exit status
make: *** [Makefile:65: bin/base64] Error 1

@aklomp
Copy link
Owner

aklomp commented Jan 3, 2024

@sergeevabc Thanks for giving it a try and for posting the full compilation output.

What I think is happening is that the compiler/linker is mangling the names of the functions for some reason. For example, base64_stream_decode_init seems to be renamed internally to __imp_base64_stream_decode_init, and that function is then not found when linking the binary with the library.

My hypothesis: the reason that function is not found is because objcopy is called with this argument: --keep-global-symbols=lib/exports.txt. The file lib/exports.txt contains a hand-curated list of functions that should be exported globally. You can perhaps experiment either by removing that argument to objcopy, or by adding those __imp_* functions to lib/exports.txt by hand.

The real problem is probably something to do with incorrect use of __declspec(dllexport) and __declspec(dllimport). That deserves a closer look, but I think it should be in another ticket.

@skeeto
Copy link

skeeto commented Jan 3, 2024

probably something to do with incorrect use of __declspec(dllexport) and __declspec(dllimport)

Correct, the declaration is marked with dllimport, so it's expecting to import it from a DLL and decorates the name accordingly, but then it's statically linked into the same module. This invocation works:

make SSSE3_CFLAGS=1 CFLAGS='-Ilib -O3 -mssse3 -DBASE64_STATIC_DEFINE'

This builds everything as though it were a static library, which is in essence the intention when building the command line program. However, that build command isn't ideal:

  • The -Ilib is necessary because it's not set properly in the Makefile, nor is there a way to extend it with extras like the -D option. At the top level, Make is a declarative language — resolving all those += before actually building anything — and there isn't a sense that definitions like CFLAGS are scoped to a particular target or recipe. If you want different flags at different times, use different names and a custom recipe. Example:

    lib/arch/ssse3/codec.o:
        $(CC) -c $(CFLAGS) $(SSSE3_CFLAGS) -o $@ $<

    Since -Ilib is so essential, I'd just bake it into the recipe generally, so that CFLAGS can be more freely overridden/extended:

    %.o: %.c
        $(CC) -c -Ilib $(CFLAGS) -o $@ $<
    
    lib/arch/ssse3/codec.o:
        $(CC) -c -Ilib $(CFLAGS) $(SSSE3_CFLAGS) -o $@ $<
  • SSSE3_CFLAGS similarly doesn't work as intended per above. It needs to be set to nonempty for lib/config.h, but otherwise it's not actually used, so I put -mssse3 in CFLAGS as well.

I suspect -DBASE64_STATIC_DEFINE should be in the default CFLAGS for the root Makefile since it exists just to build the command line program.

@sergeevabc
Copy link

@skeeto, thank you for stepping in. The command you provided significantly reduces the compilation log, but it still fails on my end as follows

~/base64 # make SSSE3_CFLAGS=1 CFLAGS='-Ilib -O3 -mssse3 -DBASE64_STATIC_DEFINE'
cc -Ilib -O3 -mssse3 -DBASE64_STATIC_DEFINE -o bin/base64 bin/base64.o lib/libbase64.o
…/w64devkit/bin/ld.exe: bin/base64.o:base64.c:(.text.startup+0x253): undefined reference to `__imp_base64_stream_decode_init'
…/w64devkit/bin/ld.exe: bin/base64.o:base64.c:(.text.startup+0x28e): undefined reference to `__imp_base64_stream_decode'
…/w64devkit/bin/ld.exe: bin/base64.o:base64.c:(.text.startup+0x347): undefined reference to `__imp_base64_stream_encode_init'
…/w64devkit/bin/ld.exe: bin/base64.o:base64.c:(.text.startup+0x353): undefined reference to `__imp_base64_stream_encode'
…/w64devkit/bin/ld.exe: bin/base64.o:base64.c:(.text.startup+0x3d2): undefined reference to `__imp_base64_stream_encode_final'
collect2.exe: error: ld returned 1 exit status
make: *** [Makefile:65: bin/base64] Error 1

@aklomp
Copy link
Owner

aklomp commented Jan 3, 2024

Perhaps it's because the function declaration is marked with dllimport, but the function definition isn't, resulting in a mismatch?

It's getting late, but tomorrow I'll try to add a GitHub Actions CI build to compile the library with w64devkit on Windows, if that's doable. That should make debugging this easier for me, and ensure that this library gets built on that platform by default.

I also propose starting a new issue about compiling the base64 binary under w64devkit, because this issue is really about something else.

@skeeto
Copy link

skeeto commented Jan 3, 2024

@sergeevabc, because the flags changed, be sure to clean out the old build first: make clean.

@sergeevabc
Copy link

@skeeto, make clean helped to resolve my case, indeed. Now I have base64.exe.

@aklomp, alas, it does not work as expected, but it's not related to compilation:

$ echo Z2l0aHVi | base64.exe -d
base64: stdin: decoding error

$ echo Z2l0aHVi | busybox.exe base64 -d
github

@aklomp
Copy link
Owner

aklomp commented Jan 4, 2024

@sergeevabc It does work. The reason why it's throwing an error is because the command adds a newline, which is not part of the base64 alphabet. Try it like this:

$ echo -n Z2l0aHVi | base64.exe -d

The newline thing is a known issue, see #126. I plan to fix it soon by discarding newlines in the input.

aklomp added a commit that referenced this issue Jan 8, 2024
As of v0.5.1, the `base64' demo program no longer compiled under MinGW because
MinGW does not emulate the `writev(2)' system call.

Fix this by detecting MinGW at compile time and optionally emulating
`writev(2)' as a series of `write(2)' calls.

Resolves #127.
mayeut pushed a commit to mayeut/base64 that referenced this issue Jan 8, 2024
As of v0.5.1, the `base64' demo program no longer compiled under MinGW because
MinGW does not emulate the `writev(2)' system call.

Fix this by detecting MinGW at compile time and optionally emulating
`writev(2)' as a series of `write(2)' calls.

Resolves aklomp#127.
aklomp added a commit that referenced this issue Jan 8, 2024
As of v0.5.1, the `base64' demo program no longer compiled under MinGW because
MinGW does not emulate the `writev(2)' system call.

Fix this by detecting MinGW at compile time and optionally emulating
`writev(2)' as a series of `write(2)' calls.

Resolves #127.
@aklomp aklomp self-assigned this Jan 9, 2024
aklomp added a commit that referenced this issue Jan 9, 2024
As of v0.5.1, the `base64' demo program no longer compiled under MinGW
because MinGW does not emulate the `writev(2)' system call.

Fix this by detecting MinGW at compile time and optionally emulating
`writev(2)' as a series of `write(2)' calls.

Resolves #127.
@aklomp aklomp closed this as completed in fff4fb8 Jan 9, 2024
@aklomp aklomp added this to the v0.5.2 milestone Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants