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

Errors when building SDL3.dll with -DSDL_LIBC=OFF #11759

Open
3 tasks done
madebr opened this issue Dec 28, 2024 · 23 comments
Open
3 tasks done

Errors when building SDL3.dll with -DSDL_LIBC=OFF #11759

madebr opened this issue Dec 28, 2024 · 23 comments
Assignees
Milestone

Comments

@madebr
Copy link
Contributor

madebr commented Dec 28, 2024

  • SDL_hashtable uses assert in debug mode:
    SDL_hashtable.c.obj : error LNK2019: unresolved external symbol __imp__wassert referenced in function SDL_IterateHashTableKey

    SDL/src/SDL_hashtable.c

    Lines 24 to 26 in 0cb4a94

    // XXX: We can't use SDL_assert here because it's going to call into hashtable code
    #include <assert.h>
    #define HT_ASSERT(x) assert(x)
  • HRESULT_FROM_WIN32 win32 inlined function causes an issue
    SDL_immdevice.c.obj : error LNK2005: HRESULT_FROM_WIN32 already defined in SDL_malloc.c.obj
    On my system, HRESULT_FROM_WIN32 is a inlined function:
    FORCEINLINE _Translates_Win32_to_HRESULT_(x) HRESULT HRESULT_FROM_WIN32(unsigned long x) { return (HRESULT)(x) <= 0 ? (HRESULT)(x) : (HRESULT) (((x) & 0x0000FFFF) | (FACILITY_WIN32 << 16) | 0x80000000);}
    with #define FORCEINLINE __forceinline
  • SDL_tray uses mbtowc_s and wcslen:
    SDL_tray.c.obj : error LNK2019: unresolved external symbol __imp_wcslen referenced in function SDL_SetTrayEntryLabel_REAL
    SDL_tray.c.obj : error LNK2019: unresolved external symbol __imp_mbstowcs_s referenced in function SDL_CreateTray_REAL
    
    tray tooltip and label encoding, windows implementation #11749 tracks this
@sezero
Copy link
Contributor

sezero commented Dec 28, 2024

HRESULT_FROM_WIN32 win32 inlined function causes an issue
SDL_immdevice.c.obj : error LNK2005: HRESULT_FROM_WIN32 already defined in SDL_malloc.c.obj
On my system, HRESULT_FROM_WIN32 is a inlined function:

FORCEINLINE _Translates_Win32_to_HRESULT_(x) HRESULT HRESULT_FROM_WIN32(unsigned long x) { return (HRESULT)(x) <= 0 ? (HRESULT)(x) : (HRESULT) (((x) & 0x0000FFFF) | (FACILITY_WIN32 << 16) | 0x80000000);}

with #define FORCEINLINE __forceinline

For SDL_immdevice.c, I guess we hit it for referencing E_NOTFOUND,
for SDL_malloc.c I don't know. Is there not a way to make the compiler
to use the macro version __HRESULT_FROM_WIN32 in those sources?

And, how come we don't hit this issue in SDL2?

@madebr
Copy link
Contributor Author

madebr commented Dec 28, 2024

For SDL_immdevice.c, I guess we hit it for referencing E_NOTFOUND, for SDL_malloc.c I don't know. Is there not a way to make the compiler to use the macro version __HRESULT_FROM_WIN32 in those sources?

And, how come we don't hit this issue in SDL2?

SDL3 does not use HRESULT_FROM_WIN32 directly.
I just found out SDL_immdevice.c uses it through E_NOTFOUND, which is defined as #define E_NOTFOUND HRESULT_FROM_WIN32(ERROR_NOT_FOUND).
SDL_malloc.c does #define FORCEINLINE which undoes the inlining

Removing the #define FORCEINLINE from SDL_malloc.c` makes the link error go away.

@sezero
Copy link
Contributor

sezero commented Dec 28, 2024

SDL_malloc.c does #define FORCEINLINE which undoes the inlining

That's the wrong thing to do: I guess that sneaked in with the dlmalloc upgrade to 2.8.6

Removing the #define FORCEINLINE from SDL_malloc.c` makes the link error go away.

Yes: That's the right thing to do

@madebr
Copy link
Contributor Author

madebr commented Dec 28, 2024

#11761 fixes this. I marked it as draft because it needs #11749 for ci to pass.

@madebr
Copy link
Contributor Author

madebr commented Dec 28, 2024

Correction, SDL_hashtable still needs a fix.

@sezero
Copy link
Contributor

sezero commented Dec 28, 2024

SDL_hashtable uses assert in debug mode:
SDL_hashtable.c.obj : error LNK2019: unresolved external symbol __imp__wassert referenced in function SDL_IterateHashTableKey

SDL/src/SDL_hashtable.c

Lines 24 to 26 in 0cb4a94

// XXX: We can't use SDL_assert here because it's going to call into hashtable code
#include <assert.h>
#define HT_ASSERT(x) assert(x)

Maybe something like this? (inspired from dynapi.c)

diff --git a/src/SDL_hashtable.c b/src/SDL_hashtable.c
index 192121a..5405cbd 100644
--- a/src/SDL_hashtable.c
+++ b/src/SDL_hashtable.c
@@ -22,8 +22,27 @@
 #include "SDL_hashtable.h"
 
 // XXX: We can't use SDL_assert here because it's going to call into hashtable code
-#include <assert.h>
-#define HT_ASSERT(x) assert(x)
+#ifdef NDEBUG
+#define HT_ASSERT(x) (void)(0)
+#else
+/* This is not declared in any header, although it is shared between some
+   parts of SDL, because we don't want anything calling it without an
+   extremely good reason. */
+extern SDL_NORETURN void SDL_ExitProcess(int exitcode);
+static void HT_ASSERT_FAIL(const char *msg)
+{
+    const char *caption = "SDL_HashTable Assertion Failure!";
+    (void)caption;
+#if (defined(_WIN32) || defined(SDL_PLATFORM_CYGWIN)) && !defined(SDL_PLATFORM_XBOXONE) && !defined(SDL_PLATFORM_XBOXSERIES)
+    MessageBoxA(NULL, msg, caption, MB_OK | MB_ICONERROR);
+#elif defined(HAVE_STDIO_H)
+    fprintf(stderr, "\n\n%s\n%s\n\n", caption, msg);
+    fflush(stderr);
+#endif
+    SDL_ExitProcess(-1);
+}
+#define HT_ASSERT(x) if (!(x)) HT_ASSERT_FAIL("SDL_HashTable Assertion Failure: " #x)
+#endif
 
 typedef struct SDL_HashItem
 {

@sezero
Copy link
Contributor

sezero commented Dec 28, 2024

P.S.: About assert.h in SDL_HashTable.c: Does Xcode define NDEBUG in release builds by itself? (Because the project file doesn't define it manually.)

@sezero
Copy link
Contributor

sezero commented Dec 28, 2024

P.S.: About assert.h in SDL_HashTable.c: Does Xcode define NDEBUG in release builds by itself? (Because the project file doesn't define it manually.)

(CC @slouken about that.)

@sezero
Copy link
Contributor

sezero commented Dec 28, 2024 via email

@slouken
Copy link
Collaborator

slouken commented Dec 28, 2024

P.S.: About assert.h in SDL_HashTable.c: Does Xcode define NDEBUG in release builds by itself? (Because the project file doesn't define it manually.)

No, it does not.

@slouken
Copy link
Collaborator

slouken commented Dec 28, 2024

I don't believe it does, but you should be able to use assert()
unconditionally.
It will typically disable itself in release builds the same way.
How will assert() disable itself if NDBEUG isn't defined?
Although looking at the diff, that's completely irrelevant.
Um? Which diff?

Message ID: @.***>

Never mind, I was typing ahead of my brain. :)

Your question is answered above.

@sezero
Copy link
Contributor

sezero commented Dec 28, 2024

P.S.: About assert.h in SDL_HashTable.c: Does Xcode define NDEBUG in release builds by itself? (Because the project file doesn't define it manually.)

No, it does not.

It should. Should it not?

@slouken
Copy link
Collaborator

slouken commented Dec 28, 2024

P.S.: About assert.h in SDL_HashTable.c: Does Xcode define NDEBUG in release builds by itself? (Because the project file doesn't define it manually.)

No, it does not.

It should. Should it not?

It's not required, but done on Windows by convention. I added it in ec29d3f

@madebr
Copy link
Contributor Author

madebr commented Dec 28, 2024

Maybe something like this? (inspired from dynapi.c)

That looks good for me. I added it to #11761

@sezero
Copy link
Contributor

sezero commented Dec 28, 2024

Maybe something like this? (inspired from dynapi.c)

That looks good for me. I added it to #11761

I hope I haven't made a mess with msg and caption and stuff in there :)
(I had only build-tested it.)

@madebr
Copy link
Contributor Author

madebr commented Dec 28, 2024

I hope I haven't made a mess with msg and caption and stuff in there :) (I had only build-tested it.)

Perhaps we should add a SDL_TriggerBreakpoint before the SDL_ExitProcess.
It's always nice if your IDE jumps to the problem location when you hit a fatal error instead of closing the application.

@sezero
Copy link
Contributor

sezero commented Dec 28, 2024

I hope I haven't made a mess with msg and caption and stuff in there :) (I had only build-tested it.)

Perhaps we should add a SDL_TriggerBreakpoint before the SDL_ExitProcess. It's always nice if your IDE jumps to the problem location when you hit a fatal error instead of closing the application.

@slouken to decide?

P.S.: SDL_ttf has the same hashtable code duplicated in it. Why are we not making hashtable a public api instead?

@slouken
Copy link
Collaborator

slouken commented Dec 28, 2024

I hope I haven't made a mess with msg and caption and stuff in there :) (I had only build-tested it.)

Perhaps we should add a SDL_TriggerBreakpoint before the SDL_ExitProcess. It's always nice if your IDE jumps to the problem location when you hit a fatal error instead of closing the application.

@slouken to decide?

Sounds good to me. Should we do that for other assert implementations (if we don't already?)

P.S.: SDL_ttf has the same hashtable code duplicated in it. Why are we not making hashtable a public api instead?

Because it's an ugly, unwieldy API and most people use languages with other implementations available. @icculus and I talked about it and agreed to keep it simple so anyone can grab it and drop it in their project if they want it.

@madebr
Copy link
Contributor Author

madebr commented Dec 28, 2024

Sounds good to me. Should we do that for other assert implementations (if we don't already?)

There are no other non-SDL_assert implementations.
The only call to SDL_ExitProcess (besides SDL_asserts) is in dynapi, which is a fatal error that must always be checked (debug and release). We can perhaps add SDL_TriggerBreakpoint there, wrapped in a #ifndef NDEBUG ... #endif

(I've added this to #11761)

@bjadamson
Copy link

bjadamson commented Jan 21, 2025

Was this released? I'm getting errors trying to compile SDL3 with -DSDL_LIB=OFF using the following script:

File: abc.sh
#!/bin/sh

cmake -S . -B build \
    -DCMAKE_BUILD_TYPE=RelWithDebInfo \
    -DSDL_LIBC=OFF \
    -DSDL_STATIC=ON \
    -DSDL_SHARED=OFF \
    -DSDL_INSTALL_DOCS=OFF \
    -DSDL_TEST_LIBRARY=OFF
 30%] Building C object CMakeFiles/SDL3-static.dir/src/hidapi/SDL_hidapi.c.o
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/cpuinfo/SDL_cpuinfo.c: In function ‘SDL_GetCPUCacheLineSize’:
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/cpuinfo/SDL_cpuinfo.c:870:23: error: implicit declaration of function ‘fopen’ [-Wimplicit-function-declaration]
  870 |             FILE *f = fopen("/sys/devices/system/cpu/cpu0/cache/index0/coherency_line_size", "r");
      |                       ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/cpuinfo/SDL_cpuinfo.c:25:1: note: ‘fopen’ is defined in header ‘<stdio.h>; this is probably fixable by adding ‘#include <stdio.h>’
   24 | #include "SDL_cpuinfo_c.h"
  +++ |+#include <stdio.h>
   25 |
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/cpuinfo/SDL_cpuinfo.c:870:23: error: initialization of ‘FILE *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
  870 |             FILE *f = fopen("/sys/devices/system/cpu/cpu0/cache/index0/coherency_line_size", "r");
      |                       ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/cpuinfo/SDL_cpuinfo.c:873:21: error: implicit declaration of function ‘fscanf’ [-Wimplicit-function-declaration]
  873 |                 if (fscanf(f, "%d", &size) == 1) {
      |                     ^~~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/cpuinfo/SDL_cpuinfo.c:873:21: note: include ‘<stdio.h>’ or provide a declaration of ‘fscanf’
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/cpuinfo/SDL_cpuinfo.c:873:21: warning: incompatible implicit declaration of built-in function ‘fscan’ [-Wbuiltin-declaration-mismatch]
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/cpuinfo/SDL_cpuinfo.c:873:21: note: include ‘<stdio.h>’ or provide a declaration of ‘fscanf’
[ 27%] Building C object CMakeFiles/SDL3-static.dir/src/io/SDL_iostream.c.o
[ 30%] Building C object CMakeFiles/SDL3-static.dir/src/main/SDL_main_callbacks.c.o
[ 30%] Building C object CMakeFiles/SDL3-static.dir/src/io/SDL_asyncio.c.o
[ 30%] Building C object CMakeFiles/SDL3-static.dir/src/joystick/SDL_gamepad.c.o
[ 31%] Building C object CMakeFiles/SDL3-static.dir/src/locale/SDL_locale.c.o
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/cpuinfo/SDL_cpuinfo.c:876:17: error: implicit declaration of function ‘fclose’; did you mean ‘false’? [-Wimplicit-function-declaration]
  876 |                 fclose(f);
      |                 ^~~~~~
      |                 false
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/SDL_log.c:427:20: warning: ‘GetLogPriorityPrefix’ defined but not used [-Wunused-function]
  427 | static const char *GetLogPriorityPrefix(SDL_LogPriority priority)
      |                    ^~~~~~~~~~~~~~~~~~~~
[ 31%] Building C object CMakeFiles/SDL3-static.dir/src/misc/SDL_url.c.o
[ 31%] Building C object CMakeFiles/SDL3-static.dir/src/joystick/controller_type.c.o
[ 31%] Building C object CMakeFiles/SDL3-static.dir/src/joystick/SDL_steam_virtual_gamepad.c.o
[ 32%] Building C object CMakeFiles/SDL3-static.dir/src/joystick/SDL_joystick.c.o
[ 32%] Building C object CMakeFiles/SDL3-static.dir/src/power/SDL_power.c.o
[ 34%] Building C object CMakeFiles/SDL3-static.dir/src/render/SDL_d3dmath.c.o
gmake[2]: *** [CMakeFiles/SDL3-static.dir/build.make:445: CMakeFiles/SDL3-static.dir/src/cpuinfo/SDL_cpuinfo.c.o] Error 1
gmake[2]: *** Waiting for unfinished jobs....
[ 35%] Building C object CMakeFiles/SDL3-static.dir/src/render/direct3d/SDL_shaders_d3d.c.o
[ 35%] Building C object CMakeFiles/SDL3-static.dir/src/render/direct3d/SDL_render_d3d.c.o
[ 37%] Building C object CMakeFiles/SDL3-static.dir/src/render/SDL_render_unsupported.c.o
[ 37%] Building C object CMakeFiles/SDL3-static.dir/src/render/SDL_render.c.o
[ 37%] Building C object CMakeFiles/SDL3-static.dir/src/render/SDL_yuv_sw.c.o
[ 37%] Building C object CMakeFiles/SDL3-static.dir/src/main/SDL_runapp.c.o
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c: In function ‘fd_seek’:
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:407:65: error: ‘errno’ undeclared (first use in this function)
  407 |         SDL_SetError("Couldn't get stream offset: %s", strerror(errno));
      |                                                                 ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:43:1: note: ‘errno’ is defined in header ‘<errno.h>; this is probably fixable by adding ‘#include <errno.h>’
   42 | #include "SDL_iostream_c.h"
  +++ |+#include <errno.h>
   43 |
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:407:65: note: each undeclared identifier is reported only once for each function it appears in
  407 |         SDL_SetError("Couldn't get stream offset: %s", strerror(errno));
      |                                                                 ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c: In function ‘fd_read’:
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:418:27: error: ‘errno’ undeclared (first use in this function)
  418 |     } while (bytes < 0 && errno == EINTR);
      |                           ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:418:27: note: ‘errno’ is defined in header ‘<errno.h>; this is probably fixable by adding ‘#include <errno.h>’
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:418:36: error: ‘EINTR’ undeclared (first use in this function)
  418 |     } while (bytes < 0 && errno == EINTR);
      |                                    ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:421:22: error: ‘EAGAIN’ undeclared (first use in this function)
  421 |         if (errno == EAGAIN) {
      |                      ^~~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c: In function ‘fd_write’:
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:437:27: error: ‘errno’ undeclared (first use in this function)
  437 |     } while (bytes < 0 && errno == EINTR);
      |                           ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:437:27: note: ‘errno’ is defined in header ‘<errno.h>; this is probably fixable by adding ‘#include <errno.h>’
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:437:36: error: ‘EINTR’ undeclared (first use in this function)
  437 |     } while (bytes < 0 && errno == EINTR);
      |                                    ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:440:22: error: ‘EAGAIN’ undeclared (first use in this function)
  440 |         if (errno == EAGAIN) {
      |                      ^~~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c: In function ‘fd_flush’:
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:456:28: error: ‘errno’ undeclared (first use in this function)
  456 |     } while (result < 0 && errno == EINTR);
      |                            ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:456:28: note: ‘errno’ is defined in header ‘<errno.h>; this is probably fixable by adding ‘#include <errno.h>’
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:456:37: error: ‘EINTR’ undeclared (first use in this function)
  456 |     } while (result < 0 && errno == EINTR);
      |                                     ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c: In function ‘fd_close’:
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:470:76: error: ‘errno’ undeclared (first use in this function)
  470 |             status = SDL_SetError("Error closing datastream: %s", strerror(errno));
      |                                                                            ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:470:76: note: ‘errno’ is defined in header ‘<errno.h>; this is probably fixable by adding ‘#include <errno.h>’
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c: In function ‘SDL_IOFromFD’:
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:499:17: error: storage size of ‘st’ isn’t known
  499 |     struct stat st;
      |                 ^~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:500:30: error: implicit declaration of function ‘fstat’ [-Wimplicit-function-declaration]
  500 |     iodata->regular_file = ((fstat(fd, &st) == 0) && S_ISREG(st.st_mode));
      |                              ^~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:500:54: error: implicit declaration of function ‘S_ISREG’ [-Wimplicit-function-declaration]
  500 |     iodata->regular_file = ((fstat(fd, &st) == 0) && S_ISREG(st.st_mode));
      |                                                      ^~~~~~~
/home/benjaminadamson/Downloads/SDL-prerelease-3.1.10/src/io/SDL_iostream.c:499:17: warning: unused variable ‘st’ [-Wunused-variable]
  499 |     struct stat st;

@madebr
Copy link
Contributor Author

madebr commented Jan 21, 2025

-DSDL_LIBC=OFF on Linux is not a supported/tested configuration: we need dlopen for a lot of things.

@slouken slouken added this to the 3.2.2 milestone Jan 21, 2025
@slouken
Copy link
Collaborator

slouken commented Jan 21, 2025

Is the DLL use case completed?

@madebr
Copy link
Contributor Author

madebr commented Jan 21, 2025

Is the DLL use case completed?

Yes, I recently fixed this in #11761

Keep this issue open. I'm going to add a SDL-msvc-x64-nolibc job to the ci matrix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants