Re: [PATCH 2/2] tools/nolibc: x86-64: Fix startup code bug

[Bedirhan KURT]

(Sending again as previous email had my Ubuntu username as sender and
Thunderbird attached my GPG key on it. Hope I cancelled on time.)

Hi Ammar,

I've tested your patchset on my local clone of Linux kernel with up to
date fetch of master branch and this is the output I've gotten after
executing the test binary compiled;

```
# I have replaced Powerline-specific characters to avoid font rendering
# issue.

 windowzytch@WindowZUbuntu > ~/linux > master > ./test

Dumping argv...
./test

Dumping envp...
SYSTEMD_EXEC_PID=6168
SSH_AUTH_SOCK=/run/user/1001/keyring/ssh
LANGUAGE=en_US:en
LANG=en_US.UTF-8
PAPERSIZE=letter
SESSION_MANAGER=local/WindowZUbuntu:@/tmp/.ICE-unix/3472,unix/WindowZUbuntu:/tmp/.ICE-unix/3472
XDG_CURRENT_DESKTOP=ubuntu:GNOME
LC_IDENTIFICATION=en_US.UTF-8
DEFAULTS_PATH=/usr/share/gconf/ubuntu-xorg.default.path
XDG_SESSION_CLASS=user
_=/home/windowzytch/linux/./test
COLORTERM=truecolor
GPG_AGENT_INFO=/run/user/1001/gnupg/S.gpg-agent:0:1
QT_IM_MODULE=ibus
DESKTOP_SESSION=ubuntu-xorg
USER=windowzytch
XDG_MENU_PREFIX=gnome-
LC_MEASUREMENT=en_US.UTF-8
G_ENABLE_DIAGNOSTIC=0
HOME=/home/windowzytch
GNOME_TERMINAL_SCREEN=/org/gnome/Terminal/screen/f89f0e46_b4c6_4c0a_8434_4fd73845d5aa
JOURNAL_STREAM=8:92304
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1001/bus
XMODIFIERS=@im=ibus
LC_NUMERIC=en_US.UTF-8
SSH_AGENT_LAUNCHER=gnome-keyring
GTK_MODULES=gail:atk-bridge
XDG_DATA_DIRS=/usr/share/ubuntu-xorg:/home/windowzytch/.local/share/flatpak/exports/share:/var/lib/flatpak/exports/share:/usr/local/share/:/usr/share/:/var/lib/snapd/desktop
WINDOWPATH=2
XDG_SESSION_DESKTOP=ubuntu-xorg
XDG_CONFIG_DIRS=/etc/xdg/xdg-ubuntu-xorg:/etc/xdg
QT_ACCESSIBILITY=1
GNOME_DESKTOP_SESSION_ID=this-is-deprecated
MANAGERPID=3189
LC_TIME=en_US.UTF-8
MANDATORY_PATH=/usr/share/gconf/ubuntu-xorg.mandatory.path
LOGNAME=windowzytch
GNOME_TERMINAL_SERVICE=:1.115
LC_PAPER=en_US.UTF-8
GNOME_SHELL_SESSION_MODE=ubuntu
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/snap/bin
XDG_RUNTIME_DIR=/run/user/1001
SHELL=/bin/zsh
XDG_SESSION_TYPE=x11
LC_MONETARY=en_US.UTF-8
LC_TELEPHONE=en_US.UTF-8
USERNAME=windowzytch
VTE_VERSION=6402
INVOCATION_ID=6052906e58884b5487d3c88314961722
PWD=/home/windowzytch/linux
SHLVL=1
XAUTHORITY=/run/user/1001/gdm/Xauthority
LC_NAME=en_US.UTF-8
IM_CONFIG_PHASE=1
GDMSESSION=ubuntu-xorg
LC_ADDRESS=en_US.UTF-8
DISPLAY=:0
TERM=xterm-256color
OLDPWD=/home/windowzytch/twrprebase/recovery_device_vestel_teos
ZSH=/home/windowzytch/.oh-my-zsh
PAGER=less
LESS=-R
LSCOLORS=Gxfxcxdxbxegedabagacad
LS_COLORS=rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=00:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arc=01;31:*.arj=01;31:*.taz=01;31:*.lha=01;31:*.lz4=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.tzo=01;31:*.t7z=01;31:*.zip=01;31:*.z=01;31:*.dz=01;31:*.gz=01;31:*.lrz=01;31:*.lz=01;31:*.lzo=01;31:*.xz=01;31:*.zst=01;31:*.tzst=01;31:*.bz2=01;31:*.bz=01;31:*.tbz=01;31:*.tbz2=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.war=01;31:*.ear=01;31:*.sar=01;31:*.rar=01;31:*.alz=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.cab=01;31:*.wim=01;31:*.swm=01;31:*.dwm=01;31:*.esd=01;31:*.jpg=01;35:*.jpeg=01;35:*.mjpg=01;35:*.mjpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.webm=01;35:*.webp=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=00;36:*.au=00;36:*.flac=00;36:*.m4a=00;36:*.mid=00;36:*.midi=00;36:*.mka=00;36:*.mp3=00;36:*.mpc=00;36:*.ogg=00;36:*.ra=00;36:*.wav=00;36:*.oga=00;36:*.opus=00;36:*.spx=00;36:*.xspf=00;36:
```

I hope these are helpful and I could help throughout this patchset. I
didn't get any SegFaults compared to my tests with same code on pure
state either so I think everything works just fine. You can append me in 
Tested-by tag if you want.

--
Bedirhan KURT

On 10/15/21 11:57, Ammar Faizi wrote:
> Hi,
>
> This is a code to test.
>
> Compile with:
>    gcc -O3 -ggdb3 -nostdlib -o test test.c
>
> Technical explanation:
> The System V ABI mandates the %rsp must be 16-byte aligned before
> performing a function call, but the current nolibc.h violates it.
>
> This %rsp alignment violation makes the callee can't align its stack
> properly. Note that the callee may have a situation where it requires
> vector aligned move. For example, `movaps` with memory operand w.r.t.
> xmm registers, it requires the src/dst address be 16-byte aligned.
>
> Since the callee can't align its stack properly, it will segfault when
> executing `movaps`. The following C code is the reproducer and test
> to ensure the bug really exists and this patch fixes it.
>
> ```
> /* SPDX-License-Identifier: LGPL-2.1 OR MIT */
>
> /*
>   * Bug reproducer and test for Willy's nolibc (x86-64) by Ammar.
>   *
>   * Compile with:
>   *  gcc -O3 -ggdb3 -nostdlib -o test test.c
>   */
>
> #include "tools/include/nolibc/nolibc.h"
>
> static void dump_argv(int argc, char *argv[])
> {
> 	int i;
> 	const char str[] = "\nDumping argv...\n";
>
> 	write(1, str, strlen(str));
> 	for (i = 0; i < argc; i++) {
> 		write(1, argv[i], strlen(argv[i]));
> 		write(1, "\n", 1);
> 	}
> }
>
> static void dump_envp(char *envp[])
> {
> 	int i = 0;
> 	const char str[] = "\nDumping envp...\n";
>
> 	write(1, str, strlen(str));
> 	while (envp[i] != NULL) {
> 		write(1, envp[i], strlen(envp[i]));
> 		write(1, "\n", 1);
> 		i++;
> 	}
> }
>
> #define read_barrier(PTR) __asm__ volatile(""::"r"(PTR):"memory")
>
> __attribute__((__target__("sse2")))
> static void test_sse_move_aligned(void)
> {
> 	int i;
> 	int arr[32] __attribute__((__aligned__(16)));
>
> 	/*
> 	 * The assignment inside this loop is very likely
> 	 * performed with aligned move, thus if we don't
> 	 * align the %rsp properly, it will fault!
> 	 *
> 	 * If we fault due to misaligned here, then there
> 	 * must be a caller below us that violates SysV
> 	 * ABI w.r.t. to %rsp alignment before func call.
> 	 */
> 	for (i = 0; i < 32; i++)
> 		arr[i] = 1;
>
> 	read_barrier(arr);
> }
>
> int main(int argc, char *argv[], char *envp[])
> {
> 	dump_argv(argc, argv);
> 	dump_envp(envp);
> 	test_sse_move_aligned();
> 	return 0;
> }
> ```