Bug 29454 - dl-cache.c:513: undefined reference to `strcpy' with -Og or -O1 (doesn't happen with -O2)
Summary: dl-cache.c:513: undefined reference to `strcpy' with -Og or -O1 (doesn't happ...
Status: UNCONFIRMED
Alias: None
Product: glibc
Classification: Unclassified
Component: build (show other bugs)
Version: 2.36
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-08-07 13:50 UTC by Martin Jansa
Modified: 2022-08-30 07:25 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
Fix strcpy ifndef guard (383 bytes, patch)
2022-08-07 14:43 UTC, Noah Goldstein
Details | Diff
config.status (11.89 KB, text/plain)
2022-08-07 15:12 UTC, Martin Jansa
Details
build.sh (917 bytes, text/plain)
2022-08-07 15:38 UTC, Martin Jansa
Details
RTLD strcpy hook (766 bytes, patch)
2022-08-07 15:56 UTC, Noah Goldstein
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Jansa 2022-08-07 13:50:31 UTC
When building glibc with -Og I've started to see "dl-cache.c:513: undefined reference to `strcpy'" introduced in:

6b9006bfb0 x86: Move strcpy SSE2 implementation to multiarch/strcpy-sse2.S

x86_64-oe-linux-gcc (GCC) 12.1.0

x86_64-oe-linux-gcc  -m64 -march=core2 -mtune=core2 -msse3 -mfpmath=sse  --sysroot=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/recipe-sysroot -Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed -fmacro-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0=/usr/src/debug/glibc/2.36-r0                      -fdebug-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0=/usr/src/debug/glibc/2.36-r0                      -fdebug-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/recipe-sysroot=                      -fdebug-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/recipe-sysroot-native=  -Wl,-z,relro,-z,now -fuse-ld=bfd  -nostdlib -nostartfiles -r -o /OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/build-x86_64-oe-linux/elf/librtld.os '-Wl,-(' /OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/build-x86_64-oe-linux/elf/dl-allobjs.os /OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/build-x86_64-oe-linux/elf/rtld-libc.a -lgcc '-Wl,-)' \
          -Wl,-Map,/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/build-x86_64-oe-linux/elf/librtld.os.map
x86_64-oe-linux-gcc  -m64 -march=core2 -mtune=core2 -msse3 -mfpmath=sse  --sysroot=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/recipe-sysroot -Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed -fmacro-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0=/usr/src/debug/glibc/2.36-r0                      -fdebug-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0=/usr/src/debug/glibc/2.36-r0                      -fdebug-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/recipe-sysroot=                      -fdebug-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/recipe-sysroot-native=  -Wl,-z,relro,-z,now -fuse-ld=bfd  -nostdlib -nostartfiles -shared -o /OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/build-x86_64-oe-linux/elf/ld.so.new              \
          -Wl,-z,relro -Wl,-z,defs -Wl,-z,now   \
          -Wl,-z,pack-relative-relocs \
          /OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/build-x86_64-oe-linux/elf/librtld.os -Wl,--version-script=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/build-x86_64-oe-linux/ld.map              \
          -Wl,-soname=ld-linux-x86-64.so.2
/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/recipe-sysroot-native/usr/bin/x86_64-oe-linux/../../libexec/x86_64-oe-linux/gcc/x86_64-oe-linux/12.1.0/ld.bfd: /OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/glibc/2.36-r0/build-x86_64-oe-linux/elf/librtld.os: in function `_dl_load_cache_lookup':
/usr/src/debug/glibc/2.36-r0/git/elf/dl-cache.c:513: undefined reference to `strcpy'
collect2: error: ld returned 1 exit status

There was another build issue when building with -Og or -O1 in bug 29249.

As work around I'm reverting these 3 commits (the middle one is just to avoid conflicts with the top one, but 6b9006bfb0 is where the strcpy was first reproducible:

04538ceb0d Revert "x86: Move strcpy SSE2 implementation to multiarch/strcpy-sse2.S"
3e31c3f7c6 Revert "x86: Add support to build st{p|r}{n}{cpy|cat} with explicit ISA level"
e5ec49260b Revert "Linux: Implement a useful version of _startup_fatal"

I don't understand this code at all, but from poking what changed with objdump I've noticed that the symbol in rtld-strcpy-sse2.os is now uppercase:

objdump -x build-x86_64-oe-linux/string/rtld-strcpy-sse2.os | grep -i strcpy
build-x86_64-oe-linux/string/rtld-strcpy-sse2.os:     file format elf64-x86-64
build-x86_64-oe-linux/string/rtld-strcpy-sse2.os
0000000000000000 g     F .text  00000000000000ec STRCPY

while build from the commit just before 6b9006bfb0 shows:

objdump -x build-x86_64-oe-linux/string/rtld-strcpy-sse2.os | grep -i strcpy
build-x86_64-oe-linux/string/rtld-strcpy-sse2.os:     file format elf64-x86-64
build-x86_64-oe-linux/string/rtld-strcpy-sse2.os
0000000000000000 g     F .text  00000000000000ec strcpy

This might be also interesting, librtld.os.map diff shows only elf/rtld-libc.a(rtld-stpcpy.os) now, while before it was showing elf/rtld-libc.a(rtld-stpcpy-sse2.os) and elf/rtld-libc.a(rtld-strcpy-sse2.os)

--- build-x86_64-oe-linux.O2/elf/librtld.mk     2022-08-07 13:20:11.154223394 +0000
+++ build-x86_64-oe-linux.Og/elf/librtld.mk     2022-08-07 13:18:29.389217048 +0000
@@ -16,6 +16,7 @@
 rtld-string +=stpcpy.os
 rtld-string +=strchr.os
 rtld-string +=strcmp.os
+rtld-string +=strcpy.os
 rtld-string +=strcspn.os
 rtld-string +=strdup.os
 rtld-string +=strerrorname_np.os
@@ -112,6 +113,16 @@
 rtld-string +=strcmp.os
 rtld-string +=strcmp-sse4_2.os
 rtld-string +=strcmp.os
+rtld-string +=strcpy-avx2.os
+rtld-string +=strcpy.os
+rtld-string +=strcpy-avx2-rtm.os
+rtld-string +=strcpy.os
+rtld-string +=strcpy-evex.os
+rtld-string +=strcpy.os
+rtld-string +=strcpy-sse2.os
+rtld-string +=strcpy.os
+rtld-string +=strcpy-sse2-unaligned.os
+rtld-string +=strcpy.os
 rtld-string +=strcspn-sse4.os
 rtld-string +=strcspn.os
 rtld-string +=strlen-avx2.os

Replacing -Og with -O2 fixes the issue (but unlike bug 29249 "-O1 -fexpensive-optimizations" instead of "-Og" isn't enough here).
Comment 1 Martin Jansa 2022-08-07 14:36:27 UTC
diffoscope output when comparing elf/dl-cache.os built with -Og and O2.

|  /usr/src/debug/glibc/2.36-r0/git/elf/dl-cache.c:513
│ -     call   881 <_dl_load_cache_lookup+0xa1>
│ - R_X86_64_PLT32      memcpy-0x4
│ -     mov    %rax,%rdi
│ +     mov    %rbx,%rsi
│ +     mov    %r12,%rdi
│ +     call   7c9 <_dl_load_cache_lookup+0xaa>
│ + R_X86_64_PLT32      strcpy-0x4
│  /usr/src/debug/glibc/2.36-r0/git/elf/dl-cache.c:514
│ -     call   889 <_dl_load_cache_lookup+0xa9>
│ +     mov    %r12,%rdi
│ +     call   7d1 <_dl_load_cache_lookup+0xb2>
│   R_X86_64_PLT32      __strdup-0x4

And then it's also reproducible with dl-cache.os rebuilt using "-O2 -fno-builtin-strcpy"
Comment 2 Noah Goldstein 2022-08-07 14:43:46 UTC
Created attachment 14258 [details]
Fix strcpy ifndef guard
Comment 3 Noah Goldstein 2022-08-07 14:44:14 UTC
(In reply to Martin Jansa from comment #1)
> diffoscope output when comparing elf/dl-cache.os built with -Og and O2.
> 
> |  /usr/src/debug/glibc/2.36-r0/git/elf/dl-cache.c:513
> │ -     call   881 <_dl_load_cache_lookup+0xa1>
> │ - R_X86_64_PLT32      memcpy-0x4
> │ -     mov    %rax,%rdi
> │ +     mov    %rbx,%rsi
> │ +     mov    %r12,%rdi
> │ +     call   7c9 <_dl_load_cache_lookup+0xaa>
> │ + R_X86_64_PLT32      strcpy-0x4
> │  /usr/src/debug/glibc/2.36-r0/git/elf/dl-cache.c:514
> │ -     call   889 <_dl_load_cache_lookup+0xa9>
> │ +     mov    %r12,%rdi
> │ +     call   7d1 <_dl_load_cache_lookup+0xb2>
> │   R_X86_64_PLT32      __strdup-0x4
> 
> And then it's also reproducible with dl-cache.os rebuilt using "-O2
> -fno-builtin-strcpy"

Attached a patch can you test if that fixes it?
Comment 4 Martin Jansa 2022-08-07 15:00:26 UTC
Thanks for super fast reply, but the attached patch doesn't seem to fix it.

Also this "ifndef STPCPY" was introduced few commits later in:
49889fb256 x86: Add support to build st{p|r}{n}{cpy|cat} with explicit ISA level

Cheers,
Comment 5 Noah Goldstein 2022-08-07 15:05:04 UTC
(In reply to Martin Jansa from comment #4)
> Thanks for super fast reply, but the attached patch doesn't seem to fix it.
> 
> Also this "ifndef STPCPY" was introduced few commits later in:
> 49889fb256 x86: Add support to build st{p|r}{n}{cpy|cat} with explicit ISA
> level
> 
> Cheers,

What is the exact command you are using?
I tried:

```
rm -rf /home/noah/programs/opensource/glibc-dev/build; mkdir -p /home/noah/programs/opensource/glibc-dev/build/glibc/; (cd /home/noah/programs/opensource/glibc-dev/build/glibc/; unset LD_LIBRARY_PATH; /home/noah/programs/opensource/glibc-dev/src/glibc/configure --prefix=/usr CC="gcc -march=core2 -Og"; CXX="g++ -march=core2 -Og"; make -j3 --silent;);
```

And wasn't able to reproduce this.
Comment 6 Martin Jansa 2022-08-07 15:12:09 UTC
Created attachment 14259 [details]
config.status
Comment 7 Martin Jansa 2022-08-07 15:13:29 UTC
I'm using OpenEmbedded cross-compilation, whole log is here:
http://errors.yoctoproject.org/Errors/Details/663898/

The flags are used from config.status (attached) generated in do_configure, then it runs 'make PARALLELMFLAGS="-j 70 -l 140" SHELL=/bin/bash KSHELL=/bin/sh "$@"' in the build-x86_64-oe-linux created by do_configure.

I'll try to reproduce it in native build on gentoo (which will be closer to what you're using).
Comment 8 Noah Goldstein 2022-08-07 15:26:44 UTC
(In reply to Martin Jansa from comment #6)
> Created attachment 14259 [details]
> config.status

Thanks.

H.J do you know why rtld-strcpy-sse2 is being built at all?

I was under the impression it would only be built with an rtld-... hook. strcpy-sse2.S should not be built for rtld/non-multiarch directly, it should go through
sysdeps/x86_64/strcpy.S
Comment 9 Noah Goldstein 2022-08-07 15:28:14 UTC
(In reply to Martin Jansa from comment #7)
> I'm using OpenEmbedded cross-compilation, whole log is here:
> http://errors.yoctoproject.org/Errors/Details/663898/
> 
> The flags are used from config.status (attached) generated in do_configure,
> then it runs 'make PARALLELMFLAGS="-j 70 -l 140" SHELL=/bin/bash
> KSHELL=/bin/sh "$@"' in the build-x86_64-oe-linux created by do_configure.
> 
> I'll try to reproduce it in native build on gentoo (which will be closer to
> what you're using).

Thanks.

I see whats causing this in the code but am not sure why it's occurring. H.J may have an idea.
Comment 10 Martin Jansa 2022-08-07 15:38:09 UTC
I was able to reproduce it in native build after exporting CFLAGS="-Og"

my build.sh attached.
Comment 11 Martin Jansa 2022-08-07 15:38:29 UTC
Created attachment 14260 [details]
build.sh
Comment 12 Noah Goldstein 2022-08-07 15:56:28 UTC
(In reply to Martin Jansa from comment #11)
> Created attachment 14260 [details]
> build.sh

Thanks was able to reproduce with that.

Attached is another patch that I think fixes it. Now fail at 
```
/usr/bin/ld: /home/noah/programs/opensource/glibc-dev/build/glibc/libc.a(libc-tls.o): in function `__libc_setup_tls':
libc-tls.c:(.text+0x2f2): undefined reference to `_startup_fatal_not_constant'
collect2: error: ld returned 1 exit status
```

which I think is a different issue.

H.J I will post this patch (and the other one for the ifdef tomorrow morning). Do you know why this was necessary for the non-optimized build?
Comment 13 Noah Goldstein 2022-08-07 15:56:49 UTC
Created attachment 14261 [details]
RTLD strcpy hook
Comment 14 Martin Jansa 2022-08-07 16:03:57 UTC
> Attached is another patch that I think fixes it.

Yes, it fixes it for me as well in native as well as OpenEmbedded build, thanks a lot for quick update!

> undefined reference to `_startup_fatal_not_constant'
> which I think is a different issue.

Yes, I'm seeing this one as well, that's bug 29249
Comment 15 Noah Goldstein 2022-08-07 16:07:20 UTC
(In reply to Martin Jansa from comment #14)
> > Attached is another patch that I think fixes it.
> 
> Yes, it fixes it for me as well in native as well as OpenEmbedded build,
> thanks a lot for quick update!

Great. Will keep this open until changes reach master / are potentially backported.

Thanks for the report.
> 
> > undefined reference to `_startup_fatal_not_constant'
> > which I think is a different issue.
> 
> Yes, I'm seeing this one as well, that's bug 29249
Comment 16 H.J. Lu 2022-08-07 19:39:46 UTC
(In reply to Noah Goldstein from comment #12)

> which I think is a different issue.
> 
> H.J I will post this patch (and the other one for the ifdef tomorrow
> morning). Do you know why this was necessary for the non-optimized build?

Some parts of glibc must be compiled with optimization.  If there is foo.[cS]
under sysdeps/x86_64/multiarch, the one in sysdeps/x86_64 will be ignored.
Does GCC inline strcpy with -O2 in dl-cache.c?
Comment 17 Noah Goldstein 2022-08-08 02:52:17 UTC
(In reply to H.J. Lu from comment #16)
> (In reply to Noah Goldstein from comment #12)
> 
> > which I think is a different issue.
> > 
> > H.J I will post this patch (and the other one for the ifdef tomorrow
> > morning). Do you know why this was necessary for the non-optimized build?
> 
> Some parts of glibc must be compiled with optimization.  If there is foo.[cS]
> under sysdeps/x86_64/multiarch, the one in sysdeps/x86_64 will be ignored.
> Does GCC inline strcpy with -O2 in dl-cache.c?

Yeah the usage is:

```
  char *temp;
  temp = alloca (strlen (best) + 1);
  strcpy (temp, best);
  return __strdup (temp);
```

Which gets converted to `memcpy` (because of already computed strlen) in strlen_pass::handle_builtin_strcpy.
Comment 18 Noah Goldstein 2022-08-08 03:27:06 UTC
Patch posted. Ended up just replacing the strcpy call with memcpy directly so we don't need to include strcpy in the rtld build.
Comment 19 Noah Goldstein 2022-08-09 10:16:52 UTC
(In reply to Noah Goldstein from comment #18)
> Patch posted. Ended up just replacing the strcpy call with memcpy directly
> so we don't need to include strcpy in the rtld build.

Pushed patch:
https://sourceware.org/git/?p=glibc.git;a=commit;h=483cfe1a6a33d6335b1901581b41040d2d412511

H.J should we backport this?
Comment 20 H.J. Lu 2022-08-09 14:49:16 UTC
(In reply to Noah Goldstein from comment #19)
> (In reply to Noah Goldstein from comment #18)
> > Patch posted. Ended up just replacing the strcpy call with memcpy directly
> > so we don't need to include strcpy in the rtld build.
> 
> Pushed patch:
> https://sourceware.org/git/?p=glibc.git;a=commit;
> h=483cfe1a6a33d6335b1901581b41040d2d412511
> 
> H.J should we backport this?

It doesn't look like a regression.  Please ask it on the mailing list.
Comment 21 Martin Jansa 2022-08-09 14:51:29 UTC
Please backport it, this part was building with -Og in 2.35 and with 2.36 it doesn't, so I would stay it's an regression, right?
Comment 22 Noah Goldstein 2022-08-11 15:33:34 UTC
(In reply to Martin Jansa from comment #21)
> Please backport it, this part was building with -Og in 2.35 and with 2.36 it
> doesn't, so I would stay it's an regression, right?

backported: https://sourceware.org/git/?p=glibc.git;a=commit;h=302bc33bc53c787da6e74162a7092e9c0fb964a8