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 coreclr fails with musl 1.2.3 #67763

Closed
ayakael opened this issue Apr 8, 2022 · 20 comments · Fixed by #67772
Closed

Build of coreclr fails with musl 1.2.3 #67763

ayakael opened this issue Apr 8, 2022 · 20 comments · Fixed by #67772

Comments

@ayakael
Copy link
Contributor

ayakael commented Apr 8, 2022

Description

Build of coreclr fails with musl 1.2.3 due to following error:

      [ 14%] Building C object Native.Unix/System.Security.Cryptography.Native/CMakeFiles/objlib.dir/pal_x509_root.c.o
      /home/buildozer/aports/community/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/init/pal.cpp:350:24: error: no matching function for call to 'InterlockedCompareExchangePointerT'
                  if(NULL != InterlockedCompareExchangePointer(&init_critsec, &temp_critsec, NULL))
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      /home/buildozer/aports/community/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/include/pal/palinternal.h:727:43: note: expanded from macro 'InterlockedCompareExchangePointer'
      #define InterlockedCompareExchangePointer InterlockedCompareExchangePointerT
                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      /home/buildozer/aports/community/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/include/pal/palinternal.h:715:11: note: candidate function template not viable: no known conversion from 'nullptr_t' to 'int' for 3rd argument
      inline T* InterlockedCompareExchangePointerT(
                ^
      /home/buildozer/aports/community/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/include/pal/palinternal.h:684:4: note: candidate template ignored: could not match 'T *' against 'nullptr_t'
      T* InterlockedCompareExchangePointerT(
         ^
      /home/buildozer/aports/community/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/include/pal/palinternal.h:705:11: note: candidate template ignored: could not match 'T *' against 'nullptr_t'
      inline T* InterlockedCompareExchangePointerT(
                ^
      1 error generated.

Caused by the following change in musl:
https://git.musl-libc.org/cgit/musl/commit?id=98e688a9da5e7b2925dda17a2d6820dddf1fb287

Reproduction Steps

  1. Setup Alpine Edge environment with musl 1.2.3.
  2. Attempt a build

Expected behavior

Build should pass, as with musl 1.2.2

Actual behavior

Failure, as described in description.

Regression?

No response

Known Workarounds

Removing the problematic commit from musl does the trick.

Configuration

Alpine Edge
Musl 1.2.3
installer version 6.0.102 with runtime 6.0.2 (as problematic files havn't changed between 6.0.2 and 6.0.3, this issue should exist with current version)

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 8, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@nekopsykose
Copy link

could you try replacing if(NULL with if(nullptr in src/init/pal.cpp:350..? it seems like that would fix it

@ayakael
Copy link
Contributor Author

ayakael commented Apr 8, 2022

could you try replacing if(NULL with if(nullptr in src/init/pal.cpp:350..? it seems like that would fix it

On it

@Hello71
Copy link

Hello71 commented Apr 8, 2022

huh? wouldn't you need to change , NULL) to , nullptr)? anyways, it won't work because nullptr doesn't match T*: https://gcc.godbolt.org/z/odvj99rK5. the correct solution is s/int/nullptr_t/ in palinternal.h, and then s/NULL/nullptr/ in pal.cpp.

@Hello71
Copy link

Hello71 commented Apr 8, 2022

@am11
Copy link
Member

am11 commented Apr 8, 2022

JIT coding conventions do not apply in PAL.

@ayakael
Copy link
Contributor Author

ayakael commented Apr 8, 2022

No dice on either the follwing solutions

  1. s/if(NULL/if(nullptr/ pal.cpp
  2. s/NULL/nullptr/g pal.cpp
     [ 18%] Building ASM object vm/wks/CMakeFiles/cee_wks_core.dir/__/amd64/asmhelpers.S.o
      /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/init/pal.cpp:350:27: error: no matching function for call to 'InterlockedCompareExchangePointerT'
                  if(nullptr != InterlockedCompareExchangePointer(&init_critsec, &temp_critsec, nullptr))
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/include/pal/palinternal.h:727:43: note: expanded from macro 'InterlockedCompareExchangePointer'
      #define InterlockedCompareExchangePointer InterlockedCompareExchangePointerT
                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/include/pal/palinternal.h:715:11: note: candidate function template not viable: no known conversion from 'nullptr_t' to 'int' for 3rd argument
      inline T* InterlockedCompareExchangePointerT(
                ^
      /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/include/pal/palinternal.h:684:4: note: candidate template ignored: could not match 'T *' against 'nullptr_t'
      T* InterlockedCompareExchangePointerT(
         ^
      /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/include/pal/palinternal.h:705:11: note: candidate template ignored: could not match 'T *' against 'nullptr_t'
      inline T* InterlockedCompareExchangePointerT(
                ^
  1. s/int/nullptr_t/g palinternal.h s/NULL/nullptr/ pal.cpp
Ton of issues as they are a bunch of int that get changed to nullptr_t. Did you mean to change NULL to nullptr_t on palinternal.h?

(I know very little c++, but a humble packager)

*tried more things, but as I'm inexperienced I'm throwing spaguetti at a wall. I'll let ya'll find a solution :) )

@q66
Copy link

q66 commented Apr 8, 2022

the issue is this:

int exchange, // When NULL is provided as argument.

basically there is multiple overloads of the functions and the code abuses the fact that in older versions of C++, NULL was defined to be the constant 0, so the function overloading rules would pick up the version with int argument first (when passed the NULL constant specifically as an argument)

and now newer version of musl defines NULL to be the constant nullptr (which is not convertible to int by design - it was designed to solve the ambiguity in the first place) which the C++11 standard allows

@ayakael
Copy link
Contributor Author

ayakael commented Apr 8, 2022

Which is why following the solution here: #23650 fixes the issue, that is to say « coreclr didn't build on freebsd (due to dotnet/coreclr@08d39dd) - when I reverted this commit, it builds. cc janvorli for the error below (simple change from NULL to 0 fixed those errors, but it was in many places, so I just reverted whole commit, also I'm not a c++ expert). ci build for freebsd would be nice to have :) » (sec, Oct 4 2017)

@q66
Copy link

q66 commented Apr 8, 2022

the fix is to change the int arguments to std::nullptr_t (or decltype(nullptr)); that way it will resolve the ambiguity and still allow taking both explicit NULL and nullptr constants

@ayakael
Copy link
Contributor Author

ayakael commented Apr 8, 2022

pal.cpp

We're getting somewhere

      [ 28%] Building CXX object pal/src/CMakeFiles/coreclrpal.dir/misc/fmtmessage.cpp.o
      In file included from /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/init/pal.cpp:16:
      In file included from /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/include/pal/dbgmsg.h:155:
      /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/include/pal/palinternal.h:721:70: error: reinterpret_cast from 'std::nullptr_t' (aka 'nullptr_t') to '_CRITICAL_SECTION *' is not allowed
          return InterlockedCompareExchangePointerT(destination, exchange, reinterpret_cast<T*>(comparand));
                                                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/init/pal.cpp:350:27: note: in instantiation of function template specialization 'InterlockedCompareExchangePointerT<_CRITICAL_SECTION>' requested here
                  if(nullptr != InterlockedCompareExchangePointer(&init_critsec, &temp_critsec, nullptr))
                                ^
      /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr/pal/src/include/pal/palinternal.h:727:43: note: expanded from macro 'InterlockedCompareExchangePointer'
      #define InterlockedCompareExchangePointer InterlockedCompareExchangePointerT
                                                ^
      1 error generated.

Now its a matter of adapting reinterpret_cast<T*>(comparand));

@q66
Copy link

q66 commented Apr 8, 2022

change the reinterpret_cast<T*>(comparand) to nullptr in the function where you changed the argument

@am11
Copy link
Member

am11 commented Apr 8, 2022

fcdffe3 fixes the coreclr build for me. @ayakael can you test it?

@ayakael
Copy link
Contributor Author

ayakael commented Apr 8, 2022

reinterpret_cast from 'std::nullptr_t' (aka 'nullptr_t') to '_CRITICAL_SECTION *' is not allowe

Reckoned that would be the next step, thank you :)

@q66
Copy link

q66 commented Apr 8, 2022

@am11 that'll fix it too probably, but it's not a good fix, it's just adapting usage to the bad API

the API itself should be changed like i wrote above

@q66
Copy link

q66 commented Apr 8, 2022

no, the API expects a pointer

the integer type in there is a bad hack to overload the function for explicit NULLs, which now broke, so the API should be fixed

@ayakael
Copy link
Contributor Author

ayakael commented Apr 8, 2022

Build passes with this patch

From 392b4a9263eb39a5ffe6a288324cc59a8587e120 Mon Sep 17 00:00:00 2001
From: "build@apk-groulx" <build@apk-groulx.praxis>
Date: Fri, 8 Apr 2022 18:50:43 +0000
Subject: [PATCH 1/1] fix build on musl 1.2.3

--- 
 .../src/pal/src/include/pal/palinternal.h     | 12 +--
 src/coreclr/src/pal/src/init/pal.cpp          | 96 +++++++++----------
 2 files changed, 54 insertions(+), 54 deletions(-)

diff --git a/src/coreclr/src/pal/src/include/pal/palinternal.h b/src/coreclr/src/pal/src/include/pal/palinternal.h
index c39c65f2455..8603b1289ce 100644
--- a/src/coreclr/src/pal/src/include/pal/palinternal.h
+++ b/src/coreclr/src/pal/src/include/pal/palinternal.h
@@ -694,30 +694,30 @@ T* InterlockedCompareExchangePointerT(
 template <typename T>
 inline T* InterlockedExchangePointerT(
     T* volatile * target,
-    int           value) // When NULL is provided as argument.
+    std::nullptr_t           value) // When NULL is provided as argument.
 {
     //STATIC_ASSERT(value == 0);
-    return InterlockedExchangePointerT(target, reinterpret_cast<T*>(value));
+    return InterlockedExchangePointerT(target, value);
 }
 
 template <typename T>
 inline T* InterlockedCompareExchangePointerT(
     T* volatile * destination,
-    int           exchange,  // When NULL is provided as argument.
+    std::nullptr_t           exchange,  // When NULL is provided as argument.
     T*            comparand)
 {
     //STATIC_ASSERT(exchange == 0);
-    return InterlockedCompareExchangePointerT(destination, reinterpret_cast<T*>(exchange), comparand);
+    return InterlockedCompareExchangePointerT(destination, comparand);
 }
 
 template <typename T>
 inline T* InterlockedCompareExchangePointerT(
     T* volatile * destination,
     T*            exchange,
-    int           comparand) // When NULL is provided as argument.
+    std::nullptr_t           comparand) // When NULL is provided as argument.
 {
     //STATIC_ASSERT(comparand == 0);
-    return InterlockedCompareExchangePointerT(destination, exchange, reinterpret_cast<T*>(comparand));
+    return InterlockedCompareExchangePointerT(destination, exchange, comparand);
 }
 
 #undef InterlockedExchangePointer
diff --git a/src/coreclr/src/pal/src/init/pal.cpp b/src/coreclr/src/pal/src/init/pal.cpp
index b71c537a323..b89fe1674a6 100644
--- a/src/coreclr/src/pal/src/init/pal.cpp
+++ b/src/coreclr/src/pal/src/init/pal.cpp
@@ -127,7 +127,7 @@ BOOL g_useDefaultBaseAddr = FALSE;
 
 /* critical section to protect access to init_count. This is allocated on the
    very first PAL_Initialize call, and is freed afterward. */
-static PCRITICAL_SECTION init_critsec = NULL;
+static PCRITICAL_SECTION init_critsec = nullptr;
 
 static DWORD g_initializeDLLFlags = PAL_INITIALIZE_DLL;
 
@@ -146,7 +146,7 @@ static bool RunningNatively()
 {
     int ret = 0;
     size_t sz = sizeof(ret);
-    if (sysctlbyname("sysctl.proc_native", &ret, &sz, NULL, 0) != 0)
+    if (sysctlbyname("sysctl.proc_native", &ret, &sz, nullptr, 0) != 0)
     {
         // if the sysctl failed, we'll assume this OS does not support
         // binary translation - so we must be running natively.
@@ -219,7 +219,7 @@ int
 PALAPI
 PAL_InitializeDLL()
 {
-    return Initialize(0, NULL, g_initializeDLLFlags);
+    return Initialize(0, nullptr, g_initializeDLLFlags);
 }
 
 /*++
@@ -281,12 +281,12 @@ void
 InitializeDefaultStackSize()
 {
     char* defaultStackSizeStr = getenv("COMPlus_DefaultStackSize");
-    if (defaultStackSizeStr != NULL)
+    if (defaultStackSizeStr != nullptr)
     {
         errno = 0;
         // Like all numeric values specific by the COMPlus_xxx variables, it is a
         // hexadecimal string without any prefix.
-        long int size = strtol(defaultStackSizeStr, NULL, 16);
+        long int size = strtol(defaultStackSizeStr, nullptr, 16);
 
         if (errno == 0)
         {
@@ -323,10 +323,10 @@ Initialize(
     DWORD flags)
 {
     PAL_ERROR palError = ERROR_GEN_FAILURE;
-    CPalThread *pThread = NULL;
-    CSharedMemoryObjectManager *pshmom = NULL;
-    LPWSTR command_line = NULL;
-    LPWSTR exe_path = NULL;
+    CPalThread *pThread = nullptr;
+    CSharedMemoryObjectManager *pshmom = nullptr;
+    LPWSTR command_line = nullptr;
+    LPWSTR exe_path = nullptr;
     int retval = -1;
     bool fFirstTimeInit = false;
 
@@ -348,18 +348,18 @@ Initialize(
 
     CriticalSectionSubSysInitialize();
 
-    if(NULL == init_critsec)
+    if(nullptr == init_critsec)
     {
         pthread_mutex_lock(&init_critsec_mutex); // prevents race condition of two threads
                                                  // initializing the critical section.
-        if(NULL == init_critsec)
+        if(nullptr == init_critsec)
         {
             static CRITICAL_SECTION temp_critsec;
 
             // Want this critical section to NOT be internal to avoid the use of unsafe region markers.
             InternalInitializeCriticalSectionAndSpinCount(&temp_critsec, 0, false);
 
-            if(NULL != InterlockedCompareExchangePointer(&init_critsec, &temp_critsec, NULL))
+            if(nullptr != InterlockedCompareExchangePointer(&init_critsec, &temp_critsec, nullptr))
             {
                 // Another thread got in before us! shouldn't happen, if the PAL
                 // isn't initialized there shouldn't be any other threads
@@ -370,7 +370,7 @@ Initialize(
         pthread_mutex_unlock(&init_critsec_mutex);
     }
 
-    InternalEnterCriticalSection(pThread, init_critsec); // here pThread is always NULL
+    InternalEnterCriticalSection(pThread, init_critsec); // here pThread is always nullptr
 
     if (init_count == 0)
     {
@@ -419,12 +419,12 @@ Initialize(
 
 #ifdef FEATURE_ENABLE_NO_ADDRESS_SPACE_RANDOMIZATION
         char* useDefaultBaseAddr = getenv("COMPlus_UseDefaultBaseAddr");
-        if (useDefaultBaseAddr != NULL)
+        if (useDefaultBaseAddr != nullptr)
         {
             errno = 0;
             // Like all numeric values specific by the COMPlus_xxx variables, it is a
             // hexadecimal string without any prefix.
-            long int flag = strtol(useDefaultBaseAddr, NULL, 16);
+            long int flag = strtol(useDefaultBaseAddr, nullptr, 16);
 
             if (errno == 0)
             {
@@ -520,7 +520,7 @@ Initialize(
         //
 
         pshmom = InternalNew<CSharedMemoryObjectManager>();
-        if (NULL == pshmom)
+        if (nullptr == pshmom)
         {
             ERROR("Unable to allocate new object manager\n");
             palError = ERROR_OUTOFMEMORY;
@@ -543,7 +543,7 @@ Initialize(
         g_pSynchronizationManager =
             CPalSynchMgrController::CreatePalSynchronizationManager();
 
-        if (NULL == g_pSynchronizationManager)
+        if (nullptr == g_pSynchronizationManager)
         {
             palError = ERROR_NOT_ENOUGH_MEMORY;
             ERROR("Failure creating synchronization manager\n");
@@ -557,11 +557,11 @@ Initialize(
 
     palError = ERROR_GEN_FAILURE;
 
-    if (argc > 0 && argv != NULL)
+    if (argc > 0 && argv != nullptr)
     {
         /* build the command line */
         command_line = INIT_FormatCommandLine(argc, argv);
-        if (NULL == command_line)
+        if (nullptr == command_line)
         {
             ERROR("Error building command line\n");
             palError = ERROR_PALINIT_COMMAND_LINE;
@@ -570,7 +570,7 @@ Initialize(
 
         /* find out the application's full path */
         exe_path = INIT_GetCurrentEXEPath();
-        if (NULL == exe_path)
+        if (nullptr == exe_path)
         {
             ERROR("Unable to find exe path\n");
             palError = ERROR_PALINIT_CONVERT_EXE_PATH;
@@ -588,7 +588,7 @@ Initialize(
         }
 
         // InitializeProcessCommandLine took ownership of this memory.
-        command_line = NULL;
+        command_line = nullptr;
 
 #ifdef PAL_PERF
         // Initialize the Profiling structure
@@ -609,7 +609,7 @@ Initialize(
         }
 
         // LOADSetExeName took ownership of this memory.
-        exe_path = NULL;
+        exe_path = nullptr;
     }
 
     if (init_count == 0)
@@ -761,7 +761,7 @@ done:
 
     if (fFirstTimeInit && 0 == retval)
     {
-        _ASSERTE(NULL != pThread);
+        _ASSERTE(nullptr != pThread);
     }
 
     if (retval != 0 && GetLastError() == ERROR_SUCCESS)
@@ -878,7 +878,7 @@ PAL_IsDebuggerPresent()
     struct kinfo_proc info = {};
     size_t size = sizeof(info);
     int mib[4] = { CTL_KERN, KERN_PROC, KERN_PROC_PID, getpid() };
-    int ret = sysctl(mib, sizeof(mib)/sizeof(*mib), &info, &size, NULL, 0);
+    int ret = sysctl(mib, sizeof(mib)/sizeof(*mib), &info, &size, nullptr, 0);
 
     if (ret == 0)
         return ((info.kp_proc.p_flag & P_TRACED) != 0);
@@ -891,12 +891,12 @@ PAL_IsDebuggerPresent()
 
     struct kinfo_proc *info;
 
-    kd = kvm_open(NULL, NULL, NULL, KVM_NO_FILES, "kvm_open");
-    if (kd == NULL)
+    kd = kvm_open(nullptr, nullptr, nullptr, KVM_NO_FILES, "kvm_open");
+    if (kd == nullptr)
         return FALSE;
 
     info = kvm_getprocs(kd, KERN_PROC_PID, getpid(), &cnt);
-    if (info == NULL || cnt < 1)
+    if (info == nullptr || cnt < 1)
     {
         kvm_close(kd);
         return FALSE;
@@ -980,7 +980,7 @@ PAL_TerminateEx(
 {
     ENTRY_EXTERNAL("PAL_TerminateEx()\n");
 
-    if (NULL == init_critsec)
+    if (nullptr == init_critsec)
     {
         /* note that these macros probably won't output anything, since the
         debug channels haven't been initialized yet */
@@ -1077,7 +1077,7 @@ BOOL PALInitLock(void)
     }
 
     CPalThread * pThread =
-        (PALIsThreadDataInitialized() ? InternalGetCurrentThread() : NULL);
+        (PALIsThreadDataInitialized() ? InternalGetCurrentThread() : nullptr);
 
     InternalEnterCriticalSection(pThread, init_critsec);
     return TRUE;
@@ -1099,7 +1099,7 @@ void PALInitUnlock(void)
     }
 
     CPalThread * pThread =
-        (PALIsThreadDataInitialized() ? InternalGetCurrentThread() : NULL);
+        (PALIsThreadDataInitialized() ? InternalGetCurrentThread() : nullptr);
 
     InternalLeaveCriticalSection(pThread, init_critsec);
 }
@@ -1158,7 +1158,7 @@ Abstract:
 
 Parameters :
     int argc : number of arguments in argv
-    char **argv : argument list in an array of NULL-terminated strings
+    char **argv : argument list in an array of nullptr-terminated strings
 
 Return value :
     pointer to Unicode command line. This is a buffer allocated with malloc;
@@ -1181,7 +1181,7 @@ Note : not all peculiarities of Windows command-line processing are supported;
 static LPWSTR INIT_FormatCommandLine (int argc, const char * const *argv)
 {
     LPWSTR retval;
-    LPSTR command_line=NULL, command_ptr;
+    LPSTR command_line=nullptr, command_ptr;
     LPCSTR arg_ptr;
     INT length, i,j;
     BOOL bQuoted = FALSE;
@@ -1206,7 +1206,7 @@ static LPWSTR INIT_FormatCommandLine (int argc, const char * const *argv)
     if(!command_line)
     {
         ERROR("couldn't allocate memory for command line!\n");
-        return NULL;
+        return nullptr;
     }
 
     command_ptr=command_line;
@@ -1240,32 +1240,32 @@ static LPWSTR INIT_FormatCommandLine (int argc, const char * const *argv)
         }
         *command_ptr++=' ';
     }
-    /* replace the last space with a NULL terminator */
+    /* replace the last space with a nullptr terminator */
     command_ptr--;
     *command_ptr='\0';
 
     /* convert to Unicode */
-    i = MultiByteToWideChar(CP_ACP, 0,command_line, -1, NULL, 0);
+    i = MultiByteToWideChar(CP_ACP, 0,command_line, -1, nullptr, 0);
     if (i == 0)
     {
         ASSERT("MultiByteToWideChar failure\n");
         free(command_line);
-        return NULL;
+        return nullptr;
     }
 
     retval = reinterpret_cast<LPWSTR>(InternalMalloc((sizeof(WCHAR)*i)));
-    if(retval == NULL)
+    if(retval == nullptr)
     {
         ERROR("can't allocate memory for Unicode command line!\n");
         free(command_line);
-        return NULL;
+        return nullptr;
     }
 
     if(!MultiByteToWideChar(CP_ACP, 0,command_line, i, retval, i))
     {
         ASSERT("MultiByteToWideChar failure\n");
         free(retval);
-        retval = NULL;
+        retval = nullptr;
     }
     else
         TRACE("Command line is %s\n", command_line);
@@ -1350,7 +1350,7 @@ bool GetEntrypointExecutableAbsolutePath(PathCharString& entrypointExecutable)
     size_t len;
 
     len = sizeof(path);
-    if (sysctl(name, __arraycount(name), path, &len, NULL, 0) != -1)
+    if (sysctl(name, __arraycount(name), path, &len, nullptr, 0) != -1)
     {
         entrypointExecutable.Set(path, len);
         result = true;
@@ -1361,14 +1361,14 @@ bool GetEntrypointExecutableAbsolutePath(PathCharString& entrypointExecutable)
     }
 #elif defined(__sun)
     const char *path;
-    if ((path = getexecname()) == NULL)
+    if ((path = getexecname()) == nullptr)
     {
         result = false;
     }
     else if (*path != '/')
     {
         char *cwd;
-        if ((cwd = getcwd(NULL, PATH_MAX)) == NULL)
+        if ((cwd = getcwd(nullptr, PATH_MAX)) == nullptr)
         {
             result = false;
         }
@@ -1428,21 +1428,21 @@ static LPWSTR INIT_GetCurrentEXEPath()
     if (!GetEntrypointExecutableAbsolutePath(real_path))
     {
         ERROR( "Cannot get current exe path\n" );
-        return NULL;
+        return nullptr;
     }
 
-    return_size = MultiByteToWideChar(CP_ACP, 0, real_path, -1, NULL, 0);
+    return_size = MultiByteToWideChar(CP_ACP, 0, real_path, -1, nullptr, 0);
     if (0 == return_size)
     {
         ASSERT("MultiByteToWideChar failure\n");
-        return NULL;
+        return nullptr;
     }
 
     return_value = reinterpret_cast<LPWSTR>(InternalMalloc((return_size*sizeof(WCHAR))));
-    if (NULL == return_value)
+    if (nullptr == return_value)
     {
         ERROR("Not enough memory to create full path\n");
-        return NULL;
+        return nullptr;
     }
     else
     {
@@ -1451,7 +1451,7 @@ static LPWSTR INIT_GetCurrentEXEPath()
         {
             ASSERT("MultiByteToWideChar failure\n");
             free(return_value);
-            return_value = NULL;
+            return_value = nullptr;
         }
         else
         {
-- 
2.35.1

Seemed odd for reinterpret_cast<T*>(value) to become nullptr rather than value, but feel free to correct me in this matter.

@am11 am11 removed the untriaged New issue has not been triaged by the area owner label Apr 8, 2022
@am11
Copy link
Member

am11 commented Apr 8, 2022

Feel free to put up a PR.

@q66
Copy link

q66 commented Apr 8, 2022

-    return InterlockedCompareExchangePointerT(destination, reinterpret_cast<T*>(exchange), comparand);
+    return InterlockedCompareExchangePointerT(destination, comparand);

you are accidentally dropping an argument here

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 8, 2022
@ayakael
Copy link
Contributor Author

ayakael commented Apr 8, 2022

The build of runtime is stalling here:

    -- Installing: /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/./libsuperpmi-shim-simple.so
    ~/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/src/coreclr
    Repo successfully built.
    Product binaries are available at /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release
    ILCompiler.DependencyAnalysisFramework -> /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/ILCompiler.DependencyAnalysisFramework/x64/Release/ILCompiler.DependencyAnalysisFramework.dll
    ILCompiler.TypeSystem.ReadyToRun -> /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/ILCompiler.TypeSystem.ReadyToRun/x64/Release/ILCompiler.TypeSystem.ReadyToRun.dll
    ILCompiler.Diagnostics -> /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/ILCompiler.Diagnostics/x64/Release/ILCompiler.Diagnostics.dll
    ILCompiler.ReadyToRun -> /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/ILCompiler.ReadyToRun/x64/Release/ILCompiler.ReadyToRun.dll
    crossgen2 -> /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/crossgen2/crossgen2.dll
    crossgen-corelib -> 
    Generating native image of System.Private.CoreLib for Linux.x64.Release. Logging to /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/log/CrossgenCoreLib_Linux__x64__Release.log
    /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/dotnet.sh /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/crossgen2/crossgen2.dll -o:/var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/System.Private.CoreLib.dll -r:/var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/IL/*.dll --targetarch:x64 --perfmap-format-version:1  --embed-pgo-data -O /var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/IL/System.Private.CoreLib.dll --perfmap --perfmap-path:/var/build/dotnet6-newstage0/testing/dotnet6-stage0/src/source-build-tarball-6.0.102/src/runtime.839cdfb0ecca5e0be3dbccd926e7651ef50fdf10/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/

ayakael added a commit to ayakael/runtime that referenced this issue Apr 9, 2022
@ayakael ayakael closed this as completed Apr 9, 2022
ayakael added a commit to ayakael/runtime that referenced this issue Apr 17, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jun 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants