Re: [PATCH 1/4] powerpc: Replace brk.S with a C implementation

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 18/11/2020 11:47, Matheus Castanho via Libc-alpha wrote:
> From: Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com>
> 
> There is no need to maintain a separate assembly implementation of brk for
> powerpc.  It basically does the same thing as the generic C-based version, so we
> can reuse that instead.

LGTM.  You also might want to check my brk consolidation [1], it
consolidate not only powerpc, but all implementations.

[1] https://sourceware.org/pipermail/libc-alpha/2020-November/119770.html

> ---
>  .../linux/powerpc/{powerpc64/brk.S => brk.c}  | 41 +++++++--------
>  sysdeps/unix/sysv/linux/powerpc/dl-brk.S      |  1 -
>  .../unix/sysv/linux/powerpc/powerpc32/brk.S   | 52 -------------------
>  3 files changed, 18 insertions(+), 76 deletions(-)
>  rename sysdeps/unix/sysv/linux/powerpc/{powerpc64/brk.S => brk.c} (58%)
>  delete mode 100644 sysdeps/unix/sysv/linux/powerpc/dl-brk.S
>  delete mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc32/brk.S
> 
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/brk.S b/sysdeps/unix/sysv/linux/powerpc/brk.c
> similarity index 58%
> rename from sysdeps/unix/sysv/linux/powerpc/powerpc64/brk.S
> rename to sysdeps/unix/sysv/linux/powerpc/brk.c
> index f206909b72..af7d1d73a2 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/brk.S
> +++ b/sysdeps/unix/sysv/linux/powerpc/brk.c
> @@ -1,6 +1,6 @@
> -/* brk system call for Linux.  PowerPC64 version.
> -   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> +/* Copyright (C) 2020 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
> +   Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.

No new 'Contributed by' in newer code.

>  
>     The GNU C Library is free software; you can redistribute it and/or
>     modify it under the terms of the GNU Lesser General Public
> @@ -13,31 +13,26 @@
>     Lesser General Public License for more details.
>  
>     You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> +   License along with the GNU C Library.  If not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +#include <errno.h>
> +#include <unistd.h>
>  #include <sysdep.h>
> -#define _ERRNO_H	1
> -#include <bits/errno.h>
>  
> -	.comm	__curbrk,8,8
> -	.section	".toc","aw"
> -.LC__curbrk:
> -	.tc __curbrk[TC],__curbrk
> -	.section ".text"
> -ENTRY (__brk)
> -	CALL_MCOUNT 1
> +/* This must be initialized data because commons can't have aliases.  */
> +void *__curbrk = 0;
>  
> -	std	r3,-8(r1)
> -	DO_CALL(SYS_ify(brk))
> -	ld	r6,-8(r1)
> -	ld	r5,.LC__curbrk@toc(r2)
> -	std     r3,0(r5)
> -	cmpld   r6,r3
> -	li	r3,0
> -	blelr+
> -	li      r3,ENOMEM
> -	TAIL_CALL_SYSCALL_ERROR
> -END (__brk)
> +int
> +__brk (void *addr)
> +{
> +  __curbrk = (void *) INTERNAL_SYSCALL_CALL (brk, addr);
> +  if (__curbrk < addr)
> +    {
> +      __set_errno (ENOMEM);
> +      return -1;
> +    }
>  
> +  return 0;
> +}
>  weak_alias (__brk, brk)
> diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-brk.S b/sysdeps/unix/sysv/linux/powerpc/dl-brk.S
> deleted file mode 100644
> index eeb96544e3..0000000000
> --- a/sysdeps/unix/sysv/linux/powerpc/dl-brk.S
> +++ /dev/null
> @@ -1 +0,0 @@
> -#include <brk.S>
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/brk.S b/sysdeps/unix/sysv/linux/powerpc/powerpc32/brk.S
> deleted file mode 100644
> index f3b960795e..0000000000
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/brk.S
> +++ /dev/null
> @@ -1,52 +0,0 @@
> -/* brk system call for Linux/ppc.
> -   Copyright (C) 1995-2020 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#include <sysdep.h>
> -#define _ERRNO_H	1
> -#include <bits/errno.h>
> -
> -	.comm	__curbrk,4,4
> -	.section ".text"
> -ENTRY (__brk)
> -	mflr	r0
> -	stwu    r1,-16(r1)
> -	cfi_adjust_cfa_offset (16)
> -	stw	r3,8(r1)
> -	stw	r0,20(r1)
> -	cfi_offset (lr, 4)
> -	DO_CALL(SYS_ify(brk))
> -	lwz     r6,8(r1)
> -#ifdef PIC
> -	SETUP_GOT_ACCESS(r5,got_label)
> -	addis	r5,r5,__curbrk-got_label@ha
> -	stw	r3,__curbrk-got_label@l(r5)
> -#else
> -	lis     r4,__curbrk@ha
> -	stw     r3,__curbrk@l(r4)
> -#endif
> -	lwz	r0,20(r1)
> -	cmplw   r6,r3
> -	addi    r1,r1,16
> -	mtlr	r0
> -	li	r3,0
> -	blelr+
> -	li      r3,ENOMEM
> -	b	__syscall_error@local
> -END (__brk)
> -
> -weak_alias (__brk, brk)
>