On Sat, 2 Aug 2025, Corinna Vinschen wrote:
> On Aug 1 12:18, Jeremy Drake via Cygwin-patches wrote:
> > This prevents memory corruption if a newer app or dll is used with an
> > older cygwin dll. This is an unsupported scenario, but it's still a
> > good idea to avoid corrupting memory if possible.
> >
> > Fixes: 7d5c55faa1 ("Cygwin: add wrappers for newer new/delete overloads")
> > Co-authored-by: Corinna Vinschen <cori...@vinschen.de>
> > Signed-off-by: Jeremy Drake <cyg...@jdrake.com>
> > ---
> >
> > I left out initializing dll_major/dll_minor in dcrt0.cc as these fields
> > are initialized in globals.cc already.
>
> Oh, yeah, I didn't check that for my POC. But that means we can
> do the same for api_major/api_minor (setting them in globals.cc,
> that is).
OK
> > I also continue to update the
> > __cygwin_cxx_malloc struct even though I don't think anything should be
> > using it (rather the default_cygwin_cxx_malloc via the user_data pointer).
> > It's not static for some reason, so something *could* be accessing it I
> > guess.
>
> I don't see how anything is supposed to access __cygwin_cxx_malloc.
> You'd have to know the symbol name and what it is. Not even gcc
> or libstdc++ use it.
The else case can go away in the CONDITIONALLY_OVERRIDE macro then.
> So I wonder... rather than changing CONDITIONALLY_OVERRIDE, why don't
> we just write to __cygwin_cxx_malloc and then change the pointer?
>
> There's no good reason that user_data->cxx_malloc points to
> default_cygwin_cxx_malloc, other than to prime __cygwin_cxx_malloc.
>
> And then we could just
>
> newu->cxx_malloc = &__cygwin_cxx_malloc;
>
> This survives fork, but not execve. Do you see any reason
> that we shouldn't just overwrite user_data->cxx_malloc as above?
The comments suggest that it used to do that, but a prior bug resulted in
it being changed to what it is now. I'd be concerned if the struct is
expanded again in future that you'd have a hard time telling which version
of the struct the pointer happens to point to when DLLs built with
different versions of startup code are loaded in the same process. I'd
rather keep it pointing tothe default_cygwin_cxx_malloc struct to be safe.
> > diff --git a/winsup/cygwin/lib/_cygwin_crt0_common.cc
> > b/winsup/cygwin/lib/_cygwin_crt0_common.cc
> > index 5900e6315d..87f3e8042b 100644
> > --- a/winsup/cygwin/lib/_cygwin_crt0_common.cc
> > +++ b/winsup/cygwin/lib/_cygwin_crt0_common.cc
> > @@ -124,6 +124,9 @@ _cygwin_crt0_common (MainFunc f, per_process *u)
> > {
> > per_process *newu = (per_process *) cygwin_internal (CW_USER_DATA);
> > bool uwasnull;
> > + bool new_dll_with_additional_operators =
> > + newu ? CYGWIN_VERSION_CHECK_FOR_CXX17_OVERLOADS (newu)
> > + : false;
>
> On second thought, why do we check newu for being not NULL again?
> I added a comment to lib/_cygwin_crt0_common.cc back in 2009:
>
> newu is the Cygwin DLL's internal per_process and never NULL
>
> but never followed up on this. We can be sure that newu is non-NULL
> *and* we can be sure that newu->cxx_malloc is non-NULL. In contrast
> to u and u->cxx_malloc, but those are never referenced.
I'll drop those checks.
