Created attachment 40088 [details] unreduced testcase glibc's posix/execvpe.c gets miscompiled with -O3: markus@x4 posix % ./tst-vfork3 script 1 script 1 script 1 script 1 script 1 /bin/sh: : No such file or directory script2.sh failed with status 32512 12912 execve("/tmp/tst-vfork3.Ox2RZ9/script2.sh", ["script2.sh", "2"], [/* 53 vars */]) = -1 ENOEXEC (Exec format error) 12912 execve("/bin/sh", ["/bin/sh", "\1", "2"], [/* 53 vars */] <unfinished ...> Notice the bogus "\1" in the second execve call. 36 /* The file is accessible but it is not an executable file. Invoke 37 the shell to interpret it as a script. */ 38 static void 39 maybe_script_execute (const char *file, char *const argv[], char *const envp[]) 40 { 41 ptrdiff_t argc = 0; 42 while (argv[argc++] != NULL) 43 { 44 if (argc == INT_MAX - 1) 45 { 46 errno = E2BIG; 47 return; 48 } 49 } 50 51 /* Construct an argument list for the shell. */ 52 char *new_argv[argc + 1]; 53 new_argv[0] = (char *) _PATH_BSHELL; 54 new_argv[1] = (char *) file; 55 if (argc > 1) 56 memcpy (new_argv + 2, argv + 1, argc * sizeof(char *)); 57 else 58 new_argv[2] = NULL; 59 60 /* Execute the shell. */ 61 __execve (new_argv[0], new_argv, envp); 62 } 63 64 65 /* Execute FILE, searching in the `PATH' environment variable if it contains 66 no slashes, with arguments ARGV and environment from ENVP. */ 67 int 68 __execvpe (const char *file, char *const argv[], char *const envp[]) 69 { 70 /* We check the simple case first. */ 71 if (*file == '\0') 72 { 73 __set_errno (ENOENT); 74 return -1; 75 } 76 77 /* Don't search when it contains a slash. */ 78 if (strchr (file, '/') != NULL) 79 { 80 __execve (file, argv, envp); 81 82 if (errno == ENOEXEC) 83 maybe_script_execute (file, argv, envp); 84 85 return -1; 86 } 87 I haven't looked deeper yet.
Started with r242590: commit 35d8be37ee5f4bc13bb3ee30b55c0c379e9a89a2 Author: krebbel <krebbel@138bc75d-0d04-0410-961f-82ee72b054a4> Date: Fri Nov 18 14:44:54 2016 +0000 Re-apply: Drop excess size used for run time allocated stack variables. The patch got reverted after hitting PR77359 which turned out to be a rs6000 backend problem. Reapplying after the PR got fixed.
On pcc64le this is the assembly diff: --- good 2016-11-20 12:43:40.318860969 +0000 +++ bad 2016-11-20 12:42:57.277928999 +0000 @@ -178,7 +178,7 @@ sldi 9,9,3 ld 10,0(1) mr 26,1 - addi 9,9,38 + addi 9,9,31 addis 25,2,.LC2@toc@ha rldicr 9,9,0,59 addi 25,25,.LC2@toc@l @@ -286,7 +286,7 @@ sldi 9,9,3 ld 10,0(1) mr 23,1 - addi 9,9,38 + addi 9,9,31 cmpdi 7,5,1 rldicr 9,9,0,59 neg 9,9
This happens on all targets.
On X86_64: --- good 2016-11-20 19:12:27.353412333 +0100 +++ bad 2016-11-20 19:13:37.738531151 +0100 @@ -892,7 +892,7 @@ leaq 1(%rax), %rdx cmpq $0, -8(%r12,%rdx,8) jne .L115 - leaq 38(,%rax,8), %rax + leaq 31(,%rax,8), %rax movq %rsp, %r14 andq $-16, %rax subq %rax, %rsp @@ -1009,7 +1009,7 @@ leaq 1(%rdx), %rax cmpq $0, -8(%r12,%rax,8) jne .L132 - leaq 38(,%rdx,8), %rdx + leaq 31(,%rdx,8), %rdx movq %rsp, -112(%rbp) movq -56(%rbp), %rcx andq $-16, %rdx
Is that with any specific version of Glibc?
(In reply to Dominik Vogt from comment #5) > Is that with any specific version of Glibc? I was using trunk.
To reproduce build glibc with -O3 and then run "make check". Or directly: ~ % ~/glibc_build/elf/ld.so --library-path /home/trippels/glibc_build/ ~/glibc_build/posix/tst-vfork3
This code from maybe_script_execute() writes past the allocated array bounds: /* Construct an argument list for the shell. */ char *new_argv[argc + 1]; ^^^^^^^^ new_argv[0] = (char *) _PATH_BSHELL; new_argv[1] = (char *) file; if (argc > 1) memcpy (new_argv + 2, argv + 1, argc * sizeof(char *)); ^^^^^ ^^^^^^ else new_argv[2] = NULL; ^^^ Before the patch, Gcc often allocated several bytes more than necessary so that many off-by-one bugs went unnoticed.
... and I think the buffer allocated in __execvpe() is also one byte too small: char buffer[path_len + file_len + 1]; ... char *pend = mempcpy (buffer, p, subp - p); <-- path_len *pend = '/'; <-- path_len + 1 memcpy (pend + (p < subp), file, file_len + 1); <-- path_len + file_len + 2
Thanks Dominik. Andreas has reopened the glibc bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20847