mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] g++ fpermissive compilation error for strdupa
@ 2022-06-22  8:05 He X
  2022-06-22 13:19 ` Rich Felker
  0 siblings, 1 reply; 7+ messages in thread
From: He X @ 2022-06-22  8:05 UTC (permalink / raw)
  To: musl

[-- 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 --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [musl] g++ fpermissive compilation error for strdupa
  2022-06-22  8:05 [musl] g++ fpermissive compilation error for strdupa He X
@ 2022-06-22 13:19 ` Rich Felker
  2022-06-23  2:59   ` He X
  2022-06-23  7:40   ` Bartosz Brachaczek
  0 siblings, 2 replies; 7+ messages in thread
From: Rich Felker @ 2022-06-22 13:19 UTC (permalink / raw)
  To: He X; +Cc: musl

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [musl] g++ fpermissive compilation error for strdupa
  2022-06-22 13:19 ` Rich Felker
@ 2022-06-23  2:59   ` He X
  2022-06-23  3:07     ` Rich Felker
  2022-06-23 19:25     ` Wolf
  2022-06-23  7:40   ` Bartosz Brachaczek
  1 sibling, 2 replies; 7+ messages in thread
From: He X @ 2022-06-23  2:59 UTC (permalink / raw)
  To: Rich Felker; +Cc: musl

[-- 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 --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [musl] g++ fpermissive compilation error for strdupa
  2022-06-23  2:59   ` He X
@ 2022-06-23  3:07     ` Rich Felker
  2022-06-23 19:25     ` Wolf
  1 sibling, 0 replies; 7+ messages in thread
From: Rich Felker @ 2022-06-23  3:07 UTC (permalink / raw)
  To: He X; +Cc: musl

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [musl] g++ fpermissive compilation error for strdupa
  2022-06-22 13:19 ` Rich Felker
  2022-06-23  2:59   ` He X
@ 2022-06-23  7:40   ` Bartosz Brachaczek
  1 sibling, 0 replies; 7+ messages in thread
From: Bartosz Brachaczek @ 2022-06-23  7:40 UTC (permalink / raw)
  To: musl; +Cc: He X

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).

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [musl] g++ fpermissive compilation error for strdupa
  2022-06-23  2:59   ` He X
  2022-06-23  3:07     ` Rich Felker
@ 2022-06-23 19:25     ` Wolf
  2022-06-24  5:24       ` He X
  1 sibling, 1 reply; 7+ messages in thread
From: Wolf @ 2022-06-23 19:25 UTC (permalink / raw)
  To: musl; +Cc: Rich Felker

[-- 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 --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [musl] g++ fpermissive compilation error for strdupa
  2022-06-23 19:25     ` Wolf
@ 2022-06-24  5:24       ` He X
  0 siblings, 0 replies; 7+ messages in thread
From: He X @ 2022-06-24  5:24 UTC (permalink / raw)
  To: musl, Rich Felker; +Cc: wolf

> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-06-24  5:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-22  8:05 [musl] g++ fpermissive compilation error for strdupa He X
2022-06-22 13:19 ` Rich Felker
2022-06-23  2:59   ` He X
2022-06-23  3:07     ` Rich Felker
2022-06-23 19:25     ` Wolf
2022-06-24  5:24       ` He X
2022-06-23  7:40   ` Bartosz Brachaczek

Code repositories for project(s) associated with this inbox:

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).