[PATCH v9 07/24] linux-user: Do not call get_errno() in do_brk()

[PATCH v9 07/24] linux-user: Do not call get_errno() in do_brk()

[Nathan Egge]

On 2023-08-04 18:00, Richard Henderson wrote:
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>
> Later the returned value is compared with -1, and negated errno is not
> expected.
>
> Fixes: 00faf08c95 ("linux-user: Don't use MAP_FIXED in do_brk()")
> Reviewed-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> Message-Id: <20230802071754.14876-4-akihiko.odaki@daynix.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/syscall.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 95727a816a..b9d2ec02f9 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -862,9 +862,9 @@ abi_long do_brk(abi_ulong brk_val)
>       */
>      if (new_host_brk_page > brk_page) {
>          new_alloc_size = new_host_brk_page - brk_page;
> -        mapped_addr = get_errno(target_mmap(brk_page, new_alloc_size,
> -                                        PROT_READ|PROT_WRITE,
> -                                        MAP_ANON|MAP_PRIVATE, 0, 0));
> +        mapped_addr = target_mmap(brk_page, new_alloc_size,
> +                                  PROT_READ|PROT_WRITE,
> +                                  MAP_ANON|MAP_PRIVATE, 0, 0);
>      } else {
>          new_alloc_size = 0;
>          mapped_addr = brk_page;
> -- 
> 2.34.1

This patch is triggering a gitlab pipeline failure in Richard's tcg-next 
branch:

https://gitlab.com/rth7680/qemu/-/pipelines/956532662

It can be reproduced locally by adding a git remote for rth7680 and then 
running:

$ git checkout rth7680/tcg-next
$ .gitlab-ci.d/check-patch.py

Checking all commits since c26d005e62f4fd177dae0cd70c24cb96761edebc...

f7cca41b17c809958a7f04b7d7f64af40d64e645:31: ERROR: spaces required 
around that '|' (ctx:VxV)
f7cca41b17c809958a7f04b7d7f64af40d64e645:32: ERROR: spaces required 
around that '|' (ctx:VxV)
total: 2 errors, 0 warnings, 12 lines checked
     ❌ FAIL one or more commits failed scripts/checkpatch.pl

The linux-user/syscall.c has many such places where this style check is 
failing. Should these be fixed in a separate patch?

Sincerely,

Nathan

Re: [PATCH v9 07/24] linux-user: Do not call get_errno() in do_brk()

[Richard Henderson]

On 8/4/23 16:40, Nathan Egge wrote:
> The linux-user/syscall.c has many such places where this style check is failing. Should 
> these be fixed in a separate patch?

Yes, eventually.


r~

[PATCH v9 for-8.1 00/24] linux-user + tcg patch queue

[Richard Henderson]

Supercedes: 20230804014517.6361-1-richard.henderson@linaro.org
("[PATCH for-8.1 v8 00/17] linux-user: brk fixes")

Changes for linux-user brk v9:
  Recover some changes that should have been in v8, had I
  generated the patches from the correct tree:
    - bsd-user: Remove last_brk
    - Fix typos in patch 15 ("Define ELF_ET_DYN_BASE...")
    - Disable -Werror=type-limits in patch 13
      ("linux-user: Adjust task_unmapped_base")


r~


Akihiko Odaki (6):
  linux-user: Unset MAP_FIXED_NOREPLACE for host
  linux-user: Fix MAP_FIXED_NOREPLACE on old kernels
  linux-user: Do not call get_errno() in do_brk()
  linux-user: Use MAP_FIXED_NOREPLACE for do_brk()
  linux-user: Do nothing if too small brk is specified
  linux-user: Do not align brk with host page size

Helge Deller (1):
  linux-user: Adjust initial brk when interpreter is close to executable

Matheus Tavares Bernardino (1):
  gdbstub: use 0 ("any process") on packets with no PID

Mikhail Tyutin (1):
  accel/tcg: Call save_iotlb_data from io_readx as well.

Nathan Egge (1):
  linux-user/elfload: Set V in ELF_HWCAP for RISC-V

Richard Henderson (14):
  accel/tcg: Adjust parameters and locking with do_{ld,st}_mmio_*
  accel/tcg: Issue wider aligned i/o in do_{ld,st}_mmio_*
  accel/tcg: Do not issue misaligned i/o
  linux-user: Remove last_brk
  bsd-user: Remove last_brk
  linux-user: Adjust task_unmapped_base for reserved_va
  linux-user: Define TASK_UNMAPPED_BASE in $guest/target_mman.h
  linux-user: Define ELF_ET_DYN_BASE in $guest/target_mman.h
  linux-user: Use MAP_FIXED_NOREPLACE for initial image mmap
  linux-user: Use elf_et_dyn_base for ET_DYN with interpreter
  linux-user: Properly set image_info.brk in flatload
  linux-user: Do not adjust image mapping for host page size
  linux-user: Do not adjust zero_bss for host page size
  linux-user: Use zero_bss for PT_LOAD with no file contents too

 bsd-user/qemu.h                      |   1 -
 linux-user/aarch64/target_mman.h     |  13 ++
 linux-user/alpha/target_mman.h       |  11 +
 linux-user/arm/target_mman.h         |  11 +
 linux-user/cris/target_mman.h        |  12 ++
 linux-user/hexagon/target_mman.h     |  13 ++
 linux-user/hppa/target_mman.h        |   6 +
 linux-user/i386/target_mman.h        |  16 ++
 linux-user/loongarch64/target_mman.h |  11 +
 linux-user/m68k/target_mman.h        |   5 +
 linux-user/microblaze/target_mman.h  |  11 +
 linux-user/mips/target_mman.h        |  10 +
 linux-user/nios2/target_mman.h       |  10 +
 linux-user/openrisc/target_mman.h    |  10 +
 linux-user/ppc/target_mman.h         |  20 ++
 linux-user/qemu.h                    |   2 -
 linux-user/riscv/target_mman.h       |  10 +
 linux-user/s390x/target_mman.h       |  20 ++
 linux-user/sh4/target_mman.h         |   7 +
 linux-user/sparc/target_mman.h       |  25 +++
 linux-user/user-mmap.h               |   6 +-
 linux-user/x86_64/target_mman.h      |  15 ++
 linux-user/xtensa/target_mman.h      |  10 +
 accel/tcg/cputlb.c                   | 289 ++++++++++++++++++---------
 bsd-user/mmap.c                      |   2 -
 gdbstub/gdbstub.c                    |   2 +-
 linux-user/elfload.c                 | 184 ++++++++---------
 linux-user/flatload.c                |   2 +-
 linux-user/main.c                    |  45 ++++-
 linux-user/mmap.c                    |  68 ++++---
 linux-user/syscall.c                 |  69 ++-----
 31 files changed, 622 insertions(+), 294 deletions(-)

-- 
2.34.1

[PATCH v9 07/24] linux-user: Do not call get_errno() in do_brk()

[Richard Henderson]

From: Akihiko Odaki <akihiko.odaki@daynix.com>

Later the returned value is compared with -1, and negated errno is not
expected.

Fixes: 00faf08c95 ("linux-user: Don't use MAP_FIXED in do_brk()")
Reviewed-by: Helge Deller <deller@gmx.de>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Message-Id: <20230802071754.14876-4-akihiko.odaki@daynix.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/syscall.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 95727a816a..b9d2ec02f9 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -862,9 +862,9 @@ abi_long do_brk(abi_ulong brk_val)
      */
     if (new_host_brk_page > brk_page) {
         new_alloc_size = new_host_brk_page - brk_page;
-        mapped_addr = get_errno(target_mmap(brk_page, new_alloc_size,
-                                        PROT_READ|PROT_WRITE,
-                                        MAP_ANON|MAP_PRIVATE, 0, 0));
+        mapped_addr = target_mmap(brk_page, new_alloc_size,
+                                  PROT_READ|PROT_WRITE,
+                                  MAP_ANON|MAP_PRIVATE, 0, 0);
     } else {
         new_alloc_size = 0;
         mapped_addr = brk_page;
-- 
2.34.1