[musl] g++ fpermissive compilation error for strdupa

[musl] g++ fpermissive compilation error for strdupa

[He X]

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

Hi!

Since *alloca* will return *void**, g++ will report error(*-fpermissive,
invalid conversion from void* to char**), if you do *strcpy(alloca(32),
str)*, which is the definition of *strdupa* on musl. I've patched it by
type casting to fix the build of bazel. Could this be merged upstream?

--- a/include/string.h  2022-04-08 01:12:40.000000000 +0800
+++ b/include/string.h  2022-04-08 01:12:40.000000000 +0800
@@ -88,7 +88,7 @@
 #endif

 #ifdef _GNU_SOURCE
-#define        strdupa(x)      strcpy(alloca(strlen(x)+1),x)
+#define        strdupa(x)      strcpy((char*)(alloca(strlen(x)+1)),x)
 int strverscmp (const char *, const char *);
 char *strchrnul(const char *, int);
 char *strcasestr(const char *, const char *);

-- 
Best regards,
xhe

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

Re: [musl] g++ fpermissive compilation error for strdupa

[Rich Felker]

On Wed, Jun 22, 2022 at 04:05:20PM +0800, He X wrote:
> Hi!
> 
> Since *alloca* will return *void**, g++ will report error(*-fpermissive,
> invalid conversion from void* to char**), if you do *strcpy(alloca(32),
> str)*, which is the definition of *strdupa* on musl. I've patched it by
> type casting to fix the build of bazel. Could this be merged upstream?
> 
> --- a/include/string.h  2022-04-08 01:12:40.000000000 +0800
> +++ b/include/string.h  2022-04-08 01:12:40.000000000 +0800
> @@ -88,7 +88,7 @@
>  #endif
> 
>  #ifdef _GNU_SOURCE
> -#define        strdupa(x)      strcpy(alloca(strlen(x)+1),x)
> +#define        strdupa(x)      strcpy((char*)(alloca(strlen(x)+1)),x)
>  int strverscmp (const char *, const char *);
>  char *strchrnul(const char *, int);
>  char *strcasestr(const char *, const char *);

This was raised recently and probably makes sense to do, but it's also
rather bad that we have strdupa at all, since it's a serious code
smell and almost always an exploitable bug (if you already knew the
length of the string and knew it was safe, you wouldn't need strdupa),
so it's been kinda nice that this is catching bugs in C++ programs.

Maybe there's some way we can fix the C++ const issue but make it
produce warnings when strdupa is used in both C and C++...?

Rich

Re: [musl] g++ fpermissive compilation error for strdupa

[He X]

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

> so it's been kinda nice that this is catching bugs in C++ programs.

I agreed. I will see what bazel guys say about the usage of strdupa. Maybe
they are willing to remove strdupa.

> Maybe there's some way we can fix the C++ const issue but make it
produce warnings when strdupa is used in both C and C++...?

I have no idea why '-fno-permissive' does not work for me. But whatever, it
is OK that the patch can not be merged upstream.

xhe

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

Re: [musl] g++ fpermissive compilation error for strdupa

[Rich Felker]

On Thu, Jun 23, 2022 at 10:59:07AM +0800, He X wrote:
> > so it's been kinda nice that this is catching bugs in C++ programs.
> 
> I agreed. I will see what bazel guys say about the usage of strdupa. Maybe
> they are willing to remove strdupa.
> 
> > Maybe there's some way we can fix the C++ const issue but make it
> produce warnings when strdupa is used in both C and C++...?
> 
> I have no idea why '-fno-permissive' does not work for me. But whatever, it
> is OK that the patch can not be merged upstream.

You have it backwards I think. At least as I read the message, you
need -fpermissive to allow this code to compile with the argument type
mismatch.

Rich

Re: [musl] g++ fpermissive compilation error for strdupa

[Wolf]

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

On 2022-06-23 10:59:07 +0800, He X wrote:
> > so it's been kinda nice that this is catching bugs in C++ programs.
> 
> I agreed. I will see what bazel guys say about the usage of strdupa. Maybe
> they are willing to remove strdupa.

As far as I can tell there is exactly one usage of strdupa in bazel [0].
And that usage should be fairly easily replaceable by a dynamic
allocation, or static buffer.

I will explore the possibility of sending a patch to remove this usage
to upstream.

0: https://github.com/bazelbuild/bazel/blob/master/src/main/tools/linux-sandbox-pid1.cc#L149

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [musl] g++ fpermissive compilation error for strdupa

[He X]

> I will explore the possibility of sending a patch to remove this usage
to upstream.

FYI, Wolf, I've opened an issue https://github.com/bazelbuild/bazel/issues/15729

Wolf <wolf@wolfsden.cz> 于2022年6月24日周五 03:25写道：
>
> On 2022-06-23 10:59:07 +0800, He X wrote:
> > > so it's been kinda nice that this is catching bugs in C++ programs.
> >
> > I agreed. I will see what bazel guys say about the usage of strdupa. Maybe
> > they are willing to remove strdupa.
>
> As far as I can tell there is exactly one usage of strdupa in bazel [0].
> And that usage should be fairly easily replaceable by a dynamic
> allocation, or static buffer.
>
> I will explore the possibility of sending a patch to remove this usage
> to upstream.
>
> 0: https://github.com/bazelbuild/bazel/blob/master/src/main/tools/linux-sandbox-pid1.cc#L149
>
> --
> There are only two hard things in Computer Science:
> cache invalidation, naming things and off-by-one errors.



-- 
Best regards,
xhe

Re: [musl] g++ fpermissive compilation error for strdupa

[Bartosz Brachaczek]

On Wed, Jun 22, 2022 at 3:19 PM Rich Felker <dalias@libc.org> wrote:
> Maybe there's some way we can fix the C++ const issue but make it
> produce warnings when strdupa is used in both C and C++...?

How about:
#define strdupa(strdupa_is_unsafe) strcpy((char
*)alloca(strlen(strdupa_is_unsafe)+1),((strdupa_is_unsafe)?0:(long)(strdupa_is_unsafe)<<-1,(strdupa_is_unsafe)))

Or is that too ugly?

BTW, the C++ issue is not about const, but rather that void * is not
convertible to char * (only the other way around).