[musl] Question about calloc, free in CPU_ALLOC and CPU_FREE

[musl] Question about calloc, free in CPU_ALLOC and CPU_FREE

[Thomas Stüfe]

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]

Greetings,

I have a small question about the way muslc implements the CPU_ALLOC and
CPU_FREE macros.

I see them defined in sched.h as:

#define CPU_ALLOC(n) ((cpu_set_t *)calloc(1,CPU_ALLOC_SIZE(n)))
#define CPU_FREE(set) free(set)

whereas the glibc defines them as calls to functions __sched_cpu_alloc()
and __sched_cpufree():

#define __CPU_ALLOC(count) __sched_cpualloc (count)
#define __CPU_FREE(cpuset) __sched_cpufree (cpuset)

in the end both variants allocate from C-heap, but the muslc variant gets
inlined directly into the calling code. If that calling code has a function
"free" or "calloc" (okay, less likely) these get called instead. Could also
be a class local method in C++.

I realize this is not a big issue. But would it not be safer to do as the
glibc does in this case?

Thank you,

Thomas

[-- Attachment #2: Type: text/html, Size: 1131 bytes --]

Re: [musl] Question about calloc, free in CPU_ALLOC and CPU_FREE

[Markus Wichmann]

On Wed, Jun 29, 2022 at 05:49:55PM +0200, Thomas Stüfe wrote:
> Greetings,
>
> I have a small question about the way muslc implements the CPU_ALLOC and
> CPU_FREE macros.
>
> I see them defined in sched.h as:
>
> #define CPU_ALLOC(n) ((cpu_set_t *)calloc(1,CPU_ALLOC_SIZE(n)))
> #define CPU_FREE(set) free(set)
>
> whereas the glibc defines them as calls to functions __sched_cpu_alloc()
> and __sched_cpufree():
>
> #define __CPU_ALLOC(count) __sched_cpualloc (count)
> #define __CPU_FREE(cpuset) __sched_cpufree (cpuset)
>
> in the end both variants allocate from C-heap, but the muslc variant gets
> inlined directly into the calling code. If that calling code has a function
> "free" or "calloc" (okay, less likely) these get called instead. Could also
> be a class local method in C++.
>

That would be invalid. calloc() and free() are names defined in the C
standard, so no user defined macro or function can have those names.

I don't know about you point about C++, though. Could be conceivably
worked around by using the :: operator, but that is only valid in C++,
so we'd have to #ifdef it.

> I realize this is not a big issue. But would it not be safer to do as the
> glibc does in this case?
>

Not really; if someone wants to use reserved names, there is little
reason to presume that "calloc" is any safer than "__sched_cpualloc".

> Thank you,
>
> Thomas

Ciao,
Markus

Re: [musl] Question about calloc, free in CPU_ALLOC and CPU_FREE

[Thomas Stüfe]

[-- Attachment #1: Type: text/plain, Size: 1899 bytes --]

On Wed, Jun 29, 2022 at 6:11 PM Markus Wichmann <nullplan@gmx.net> wrote:

> On Wed, Jun 29, 2022 at 05:49:55PM +0200, Thomas Stüfe wrote:
> > Greetings,
> >
> > I have a small question about the way muslc implements the CPU_ALLOC and
> > CPU_FREE macros.
> >
> > I see them defined in sched.h as:
> >
> > #define CPU_ALLOC(n) ((cpu_set_t *)calloc(1,CPU_ALLOC_SIZE(n)))
> > #define CPU_FREE(set) free(set)
> >
> > whereas the glibc defines them as calls to functions __sched_cpu_alloc()
> > and __sched_cpufree():
> >
> > #define __CPU_ALLOC(count) __sched_cpualloc (count)
> > #define __CPU_FREE(cpuset) __sched_cpufree (cpuset)
> >
> > in the end both variants allocate from C-heap, but the muslc variant gets
> > inlined directly into the calling code. If that calling code has a
> function
> > "free" or "calloc" (okay, less likely) these get called instead. Could
> also
> > be a class local method in C++.
> >
>
> That would be invalid. calloc() and free() are names defined in the C
> standard, so no user defined macro or function can have those names.
>
> I don't know about you point about C++, though. Could be conceivably
> worked around by using the :: operator, but that is only valid in C++,
> so we'd have to #ifdef it.
>
> > I realize this is not a big issue. But would it not be safer to do as the
> > glibc does in this case?
> >
>
> Not really; if someone wants to use reserved names, there is little
> reason to presume that "calloc" is any safer than "__sched_cpualloc".
>
> > Thank you,
> >
> > Thomas
>
> Ciao,
> Markus
>

Thanks a lot for the quick response, Markus.

I'm with the OpenJDK project, and the VM crashes on Alpine because of this
issue (https://bugs.openjdk.org/browse/JDK-8289477). But I don't think
muslc does anything wrong, and the fix is very simple (now that I know what
happens).

Cheers, Thomas

[-- Attachment #2: Type: text/html, Size: 2545 bytes --]

Re: [musl] Question about calloc, free in CPU_ALLOC and CPU_FREE

[Florian Weimer]

* Thomas Stüfe:

> Greetings,
>
> I have a small question about the way muslc implements the CPU_ALLOC and CPU_FREE
> macros. 
>
> I see them defined in sched.h as:
>
> #define CPU_ALLOC(n) ((cpu_set_t *)calloc(1,CPU_ALLOC_SIZE(n)))
> #define CPU_FREE(set) free(set)
>
> whereas the glibc defines them as calls to functions __sched_cpu_alloc() and
> __sched_cpufree():
>
> #define __CPU_ALLOC(count) __sched_cpualloc (count)
> #define __CPU_FREE(cpuset) __sched_cpufree (cpuset)
>
> in the end both variants allocate from C-heap, but the muslc variant
> gets inlined directly into the calling code. If that calling code has
> a function "free" or "calloc" (okay, less likely) these get called
> instead. Could also be a class local method in C++.

calloc and free can be macros, and if an implementation exercises this
possibility, you will have some trouble defining your own functions of
the same name.  We ran into this issue when we added an iszero macro to
glibc as part of support for future C revisions.  In the end, we had to
use an alternative C++ construct in C++ mode instead of the preprocessor
macro we use for C: too many C++ applications broke because they used
iszero as a member function name.

I think I can guess which code base this is about. 8-) You really should
adopt a non-colliding naming scheme for your os:: wrapper functions.
This sidesteps this entire set of issues.

There really isn't a musl bug here, I think (but I'm not a musl
developer).

Thanks,
Florian

Re: [musl] Question about calloc, free in CPU_ALLOC and CPU_FREE

[Thomas Stüfe]

[-- Attachment #1: Type: text/plain, Size: 1975 bytes --]

On Wed, Jun 29, 2022 at 6:15 PM Florian Weimer <fweimer@redhat.com> wrote:

> * Thomas Stüfe:
>
> > Greetings,
> >
> > I have a small question about the way muslc implements the CPU_ALLOC and
> CPU_FREE
> > macros.
> >
> > I see them defined in sched.h as:
> >
> > #define CPU_ALLOC(n) ((cpu_set_t *)calloc(1,CPU_ALLOC_SIZE(n)))
> > #define CPU_FREE(set) free(set)
> >
> > whereas the glibc defines them as calls to functions __sched_cpu_alloc()
> and
> > __sched_cpufree():
> >
> > #define __CPU_ALLOC(count) __sched_cpualloc (count)
> > #define __CPU_FREE(cpuset) __sched_cpufree (cpuset)
> >
> > in the end both variants allocate from C-heap, but the muslc variant
> > gets inlined directly into the calling code. If that calling code has
> > a function "free" or "calloc" (okay, less likely) these get called
> > instead. Could also be a class local method in C++.
>
> calloc and free can be macros, and if an implementation exercises this
> possibility, you will have some trouble defining your own functions of
> the same name.  We ran into this issue when we added an iszero macro to
> glibc as part of support for future C revisions.  In the end, we had to
> use an alternative C++ construct in C++ mode instead of the preprocessor
> macro we use for C: too many C++ applications broke because they used
> iszero as a member function name.
>
> I think I can guess which code base this is about. 8-) You really should
> adopt a non-colliding naming scheme for your os:: wrapper functions.
> This sidesteps this entire set of issues.
>
>
:-) I guess you guess right.

Yes, the JVM crashes on Alpine because of ::malloc() -> os::free()
disbalance: https://bugs.openjdk.org/browse/JDK-8289477

And you are absolutely right. We should change the naming scheme.


> There really isn't a musl bug here, I think (but I'm not a musl
> developer).
>
>
I think so too now.


> Thanks,
> Florian
>
>
Thank you, Thomas

[-- Attachment #2: Type: text/html, Size: 2891 bytes --]