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

Add MSVC solution/projects, fix many warnings, and small enhancements. #695

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

axelriet
Copy link

@axelriet axelriet commented Jan 13, 2024

  • Added Visual Studio solutions to msvc/ - Not relying on CMake and VS's less-than-perfect CMake integration. Fuzzer building on Windows for the first time.
  • Modified programs/programs_helper.h (define PRETTY_FUNCTION)
  • Modified programs/test_libmgmt.c (reduced iterations so the test does not hang for 10 minutes)
  • Modified usrsctplib/netinet/sctp_userspace.c (Added support for thread naming on Windows)
  • Modified usrsctplib/user_socket.c (usrsctp_getladdrs()/usrsctp_freeladdrs()/usrsctp_getpaddrs()/usrsctp_freepaddrs() - made sure the output pointer is set to null upon function entry and that the matching free() functions can handle null. Before that change, starting programs/client.c before the server resulted in a heap corruption: the call to usrsctp_getpaddrs() at line 226 returned < 0, leaving addrs untouched. But addrs had been freed at line 224, and was freed again at line 272, corrupting the heap.)
  • Added conditional __cdecl attributes to some functions (notably all main() functions) so the library can be built with different calling conventions. Ideally, all public/exported function should have their calling convention specified explicitely in the public header, via some new SCTP_PUBLIC macro that is defined per-compiler and build option, so people can build their app and the library with whatever calling convention they choose. On Windows, we have __fastcall as well as __vectorcall (that everyone should really use) and this change enables building the apps and the lib with those conventions without breaking the CRT or the fuzzer.
  • Fixed most /W4 warnings (many type casts) instead of globaly suppressing them.
  • Removed all calls to CreateThread() and TerminateThread()
  • Implemented proper thread creation on Windows when the thread routing is using CRT functions and in particular the heap.
  • Implemented proper (graceful) thread termination in sample programs.
  • Simplified loop in read_random() - now calls BCryptGenRandom() (or RtlGenRandom() pre-Vista)

"A thread in an executable that calls the C run-time library (CRT) should use the _beginthreadex and _endthreadex functions for thread management rather than CreateThread and ExitThread; this requires the use of the multithreaded version of the CRT."

https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createthread

"TerminateThread is a dangerous function that should only be used in the most extreme cases. You should call TerminateThread only if you know exactly what the target thread is doing, and you control all of the code that the target thread could possibly be running at the time of the termination. For example, TerminateThread can result in the following problems:

- If the target thread owns a critical section, the critical section will not be released.
- If the target thread is allocating memory from the heap, the heap lock will not be released.
- If the target thread is executing certain kernel32 calls when it is terminated, the kernel32 state for the thread's process could be inconsistent.
- If the target thread is manipulating the global state of a shared DLL, the state of the DLL could be destroyed, affecting other users of the DLL."

https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-terminatethread

…heir _Out_ parameter upon entry, and the matching free functions must be NULL-tolerant.
…ed proper thread creation on Windows when the thread routing is using CRT functions and in particular the heap. Implemented proper (graceful) thread termination.

"A thread in an executable that calls the C run-time library (CRT) should use the _beginthreadex and _endthreadex functions for thread management rather than CreateThread and ExitThread; this requires the use of the multithreaded version of the CRT." https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createthread

"TerminateThread is a dangerous function that should only be used in the most extreme cases. You should call TerminateThread only if you know exactly what the target thread is doing, and you control all of the code that the target thread could possibly be running at the time of the termination. For example, TerminateThread can result in the following problems:

If the target thread owns a critical section, the critical section will not be released.
If the target thread is allocating memory from the heap, the heap lock will not be released.
If the target thread is executing certain kernel32 calls when it is terminated, the kernel32 state for the thread's process could be inconsistent.
If the target thread is manipulating the global state of a shared DLL, the state of the DLL could be destroyed, affecting other users of the DLL." https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-terminatethread
…ther RtlGenRandom() or BCryptGenRandom() depending on build target.
@tuexen
Copy link
Member

tuexen commented Apr 14, 2024

Could you please split this pull request up in separate ones, each one addressing one issue or adding one feature.

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

Successfully merging this pull request may close these issues.

None yet

2 participants