[0/1] tools/nolibc/string: export strlen()

  Hi, while using nolibc on debian testing, I found that compilation fails when using strlcat().

The compilation fails with:

            cc -fno-asynchronous-unwind-tables -fno-ident -s -Os -nostdlib -lgcc  -static -o test test.c
            /usr/bin/ld: /tmp/cccIasKL.o: in function `main':
            test.c:(.text.startup+0x1e): undefined reference to `strlen'
            collect2: error: ld returned 1 exit status

This is using debian testing, with gcc 13.2.0.

A small repro case that fails with this error on debian is:

	int main(void) {
		char dst[6] = "12";
		char *src = "abc";
		strlcat(dst, src, 6);
	
		printf("dst is: %s\n", dst);
	
		return 0;
	}

Please note that this code is not using strlen() and strlcat() doesn't seems to use it either.

First I noted that removing the attribute unused in strlen(), the compilation worked fine. And then
I noticied that other functions had the attribute weak, a custom section and export the function.

In particular, what happens here seems to be the same as in commit "tools/nolibc/string: export memset() and
memmove()" (8d304a374023), as removing the -Os or adding the -ffreestanding seem to fix the issue.
So, I did the same as that commit, for strlen().

However, I'm not 100% confident on how to check that this is done by the compiler to later replace
it and provide a builtin. I'm not sure how that was verified for commit 8d304a374023, but if you let
me know, I can verify it too.

What do you think?


As a side note, it seems strlcat()/strlcpy() fail to set the terminating null byte on some cases,
and the return code is not always the same as when using libbsd. It seems to be only on "error"
cases, and not sure if it's worth fixing all/some of those cases.

Let me know if you think it is worth adding some _simple_ patches (I don't think it is worth fixing
all the cases, the code is to fix all of the cases is probably not nice and not worth it).


Best,
Rodrigo

---


Rodrigo Campos (1):
  tools/nolibc/string: export strlen()

 tools/include/nolibc/string.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Hi!

On 2024-01-26 15:24:10+0100, Rodrigo Campos wrote:
> Hi, while using nolibc on debian testing, I found that compilation fails when using strlcat().
> 
> The compilation fails with:
> 
>             cc -fno-asynchronous-unwind-tables -fno-ident -s -Os -nostdlib -lgcc  -static -o test test.c
>             /usr/bin/ld: /tmp/cccIasKL.o: in function `main':
>             test.c:(.text.startup+0x1e): undefined reference to `strlen'
>             collect2: error: ld returned 1 exit status
> 
> This is using debian testing, with gcc 13.2.0.

I can reproduce the issue with gcc 13.2.1 on Arch.

> A small repro case that fails with this error on debian is:
> 
> 	int main(void) {
> 		char dst[6] = "12";
> 		char *src = "abc";
> 		strlcat(dst, src, 6);
> 	
> 		printf("dst is: %s\n", dst);
> 	
> 		return 0;
> 	}
> 
> Please note that this code is not using strlen() and strlcat() doesn't seems to use it either.

I think the comment in strlen() explains it:

    Note that gcc12 recognizes an strlen() pattern and replaces it with a
    jump to strlen().

strlcat() indeed contains this pattern.

I was able to fix the issue by replacing the open-coded strlen() in
strlcat() with a call to the function and that also fixed the issue.

It seems nicer to me as a fix, on the other hand the change to a weak
definition will also catch other instances of the issue.
Maybe we do both.

> First I noted that removing the attribute unused in strlen(), the compilation worked fine. And then
> I noticied that other functions had the attribute weak, a custom section and export the function.
> 
> In particular, what happens here seems to be the same as in commit "tools/nolibc/string: export memset() and
> memmove()" (8d304a374023), as removing the -Os or adding the -ffreestanding seem to fix the issue.
> So, I did the same as that commit, for strlen().
> 
> However, I'm not 100% confident on how to check that this is done by the compiler to later replace
> it and provide a builtin. I'm not sure how that was verified for commit 8d304a374023, but if you let
> me know, I can verify it too.
> 
> What do you think?

Personally I don't know how it was verified, we'll have to wait for
Willy on that.

> As a side note, it seems strlcat()/strlcpy() fail to set the terminating null byte on some cases,
> and the return code is not always the same as when using libbsd. It seems to be only on "error"
> cases, and not sure if it's worth fixing all/some of those cases.
> 
> Let me know if you think it is worth adding some _simple_ patches (I don't think it is worth fixing
> all the cases, the code is to fix all of the cases is probably not nice and not worth it).

Souns reasonable to me to fix the return values.
And get some tests for it.

> Best,
> Rodrigo
> 
> ---
> 
> 
> Rodrigo Campos (1):
>   tools/nolibc/string: export strlen()
  

Hi!

On Sat, Jan 27, 2024 at 03:53:32PM +0100, Thomas Weißschuh wrote:
> Hi!
> 
> On 2024-01-26 15:24:10+0100, Rodrigo Campos wrote:
> > Hi, while using nolibc on debian testing, I found that compilation fails when using strlcat().
> > 
> > The compilation fails with:
> > 
> >             cc -fno-asynchronous-unwind-tables -fno-ident -s -Os -nostdlib -lgcc  -static -o test test.c
> >             /usr/bin/ld: /tmp/cccIasKL.o: in function `main':
> >             test.c:(.text.startup+0x1e): undefined reference to `strlen'
> >             collect2: error: ld returned 1 exit status
> > 
> > This is using debian testing, with gcc 13.2.0.
> 
> I can reproduce the issue with gcc 13.2.1 on Arch.
> 
> > A small repro case that fails with this error on debian is:
> > 
> > 	int main(void) {
> > 		char dst[6] = "12";
> > 		char *src = "abc";
> > 		strlcat(dst, src, 6);
> > 	
> > 		printf("dst is: %s\n", dst);
> > 	
> > 		return 0;
> > 	}
> > 
> > Please note that this code is not using strlen() and strlcat() doesn't seems to use it either.
> 
> I think the comment in strlen() explains it:
> 
>     Note that gcc12 recognizes an strlen() pattern and replaces it with a
>     jump to strlen().
> 
> strlcat() indeed contains this pattern.
> 
> I was able to fix the issue by replacing the open-coded strlen() in
> strlcat() with a call to the function and that also fixed the issue.
> 
> It seems nicer to me as a fix, on the other hand the change to a weak
> definition will also catch other instances of the issue.
> Maybe we do both.

Yes, once we have the proof that the compiler may produce such a call, it
can also happen in whatever user code so we need to export the function,
there's no other solution.

> > First I noted that removing the attribute unused in strlen(), the compilation worked fine. And then
> > I noticied that other functions had the attribute weak, a custom section and export the function.
> > 
> > In particular, what happens here seems to be the same as in commit "tools/nolibc/string: export memset() and
> > memmove()" (8d304a374023), as removing the -Os or adding the -ffreestanding seem to fix the issue.
> > So, I did the same as that commit, for strlen().
> > 
> > However, I'm not 100% confident on how to check that this is done by the compiler to later replace
> > it and provide a builtin. I'm not sure how that was verified for commit 8d304a374023, but if you let
> > me know, I can verify it too.
> > 
> > What do you think?
> 
> Personally I don't know how it was verified, we'll have to wait for
> Willy on that.

Oh it's very simple, just build a small code that doesn't contain any
such explicit nor implicit call and check that it doesn't contain the
function.

E.g.:

  $ printf "int main(void) { }\n" | gcc -nostdlib -static -Isysroot/x86/include -include nolibc.h -Os -Wl,--gc-sections -xc -
  $ nm --size a.out 
  0000000000000003 T main
  0000000000000004 V errno
  0000000000000008 V _auxv
  0000000000000008 V environ
  000000000000000f W _start
  0000000000000042 W _start_c

and:

  $ printf "int main(void) { return (long)&strlen;}\n" | gcc -nostdlib -static -Isysroot/x86/include -include nolibc.h -Os -Wl,--gc-sections -xc -
  $ nm --size a.out 
  0000000000000004 V errno
  0000000000000006 T main
  0000000000000008 V _auxv
  0000000000000008 V environ
  000000000000000e t strlen
  000000000000000f W _start
  0000000000000042 W _start_c

> > As a side note, it seems strlcat()/strlcpy() fail to set the terminating null byte on some cases,

Indeed I've just checked and you're right, that defeats their purpose!

> > and the return code is not always the same as when using libbsd. It seems to be only on "error"
> > cases, and not sure if it's worth fixing all/some of those cases.

OK.

> > Let me know if you think it is worth adding some _simple_ patches (I don't think it is worth fixing
> > all the cases, the code is to fix all of the cases is probably not nice and not worth it).
> 
> Souns reasonable to me to fix the return values.
> And get some tests for it.

Seconded!

Thanks!
Willy
  

On 1/27/24 17:24, Willy Tarreau wrote:
> On Sat, Jan 27, 2024 at 03:53:32PM +0100, Thomas Weißschuh wrote:
>> On 2024-01-26 15:24:10+0100, Rodrigo Campos wrote:
>>> Please note that this code is not using strlen() and strlcat() doesn't seems to use it either.
>>
>> I think the comment in strlen() explains it:
>>
>>      Note that gcc12 recognizes an strlen() pattern and replaces it with a
>>      jump to strlen().
>>
>> strlcat() indeed contains this pattern.
>>
>> I was able to fix the issue by replacing the open-coded strlen() in
>> strlcat() with a call to the function and that also fixed the issue.
>>
>> It seems nicer to me as a fix, on the other hand the change to a weak
>> definition will also catch other instances of the issue.
>> Maybe we do both.
> 
> Yes, once we have the proof that the compiler may produce such a call, it
> can also happen in whatever user code so we need to export the function,
> there's no other solution.

Makes sense, thanks!

> 
>>> First I noted that removing the attribute unused in strlen(), the compilation worked fine. And then
>>> I noticied that other functions had the attribute weak, a custom section and export the function.
>>>
>>> In particular, what happens here seems to be the same as in commit "tools/nolibc/string: export memset() and
>>> memmove()" (8d304a374023), as removing the -Os or adding the -ffreestanding seem to fix the issue.
>>> So, I did the same as that commit, for strlen().
>>>
>>> However, I'm not 100% confident on how to check that this is done by the compiler to later replace
>>> it and provide a builtin. I'm not sure how that was verified for commit 8d304a374023, but if you let
>>> me know, I can verify it too.
>>>
>>> What do you think?
>>
>> Personally I don't know how it was verified, we'll have to wait for
>> Willy on that.
> 
> Oh it's very simple, just build a small code that doesn't contain any
> such explicit nor implicit call and check that it doesn't contain the
> function.
> 
> E.g >
>    $ printf "int main(void) { }\n" | gcc -nostdlib -static -Isysroot/x86/include -include nolibc.h -Os -Wl,--gc-sections -xc -
>    $ nm --size a.out

Oh, cool. I can confirm that gcc does indeed add the strlen call
(note I had to remove the "--size" param to nm, as the symbol is 
undefined and not shown otherwise)

I wonder if there is an easy way to check for which functions gcc/clang 
do this...

>>> As a side note, it seems strlcat()/strlcpy() fail to set the terminating null byte on some cases,
> 
> Indeed I've just checked and you're right, that defeats their purpose!

Cool.

>>> Let me know if you think it is worth adding some _simple_ patches (I don't think it is worth fixing
>>> all the cases, the code is to fix all of the cases is probably not nice and not worth it).
>>
>> Souns reasonable to me to fix the return values.
>> And get some tests for it.
> 
> Seconded!

Thanks, I'll see how to improve that too :)



Best,
Rodrigo
  

.
> Yes, once we have the proof that the compiler may produce such a call, it
> can also happen in whatever user code so we need to export the function,
> there's no other solution.

Does that mean that it you try to implement strlen() in C
gcc will generate a recursive call?

I guess an 'asm volatile("");' in the loop fix it.
Although, IIRC, you need a comment in the asm - and there
isn't a portable comment.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
  

On Sat, Jan 27, 2024 at 09:23:01PM +0000, David Laight wrote:
> ..
> > Yes, once we have the proof that the compiler may produce such a call, it
> > can also happen in whatever user code so we need to export the function,
> > there's no other solution.
> 
> Does that mean that it you try to implement strlen() in C
> gcc will generate a recursive call?

Yes, that's exactly what happened the first time with strlen() itself!

> I guess an 'asm volatile("");' in the loop fix it.

That's how we fixed it for strlen(). The problem I'm having with
addressing it this way is that as long as the compiler will decide
to emit calls to strlen() which was not explicitly referenced in
the code, it will still be missing and will continue to fail. Thus
the safest solution is to make sure strlen() remains accessible in
case the compiler decides to make use of it.

Willy