Hi Ilya,

After adding support for riscv (similar to s390x, in that we can
find the total insn length from the first couple of bits, so, easy),
I find that the test case doesn't work without all of the other
changes for PROT_EXEC, including the translator_ld changes.

Other changes from your v5:
  - mprotect invalidates tbs.  The test case is riscv, with a
    4-byte insn at offset 0xffe, which was chained to from the
    insn at offset 0xffa.  The fact that the 0xffe tb was not
    invalidated meant that we chained to it and re-executed
    without revalidating page protections.

  - rewrote the test framework to be agnostic of page size, which
    reduces some of the repetition.  I ran into trouble with the
    riscv linker, which relaxed the segment such that .align+.org
    wasn't actually honored.  This new form doesn't require the
    test bytes to be aligned in the binary.


r~


Ilya Leoshkevich (4):
  linux-user: Clear translations and tb_jmp_cache on mprotect()
  accel/tcg: Introduce is_same_page()
  target/s390x: Make translator stop before the end of a page
  target/i386: Make translator stop before the end of a page

Richard Henderson (17):
  linux-user/arm: Mark the commpage executable
  linux-user/hppa: Allocate page zero as a commpage
  linux-user/x86_64: Allocate vsyscall page as a commpage
  linux-user: Honor PT_GNU_STACK
  tests/tcg/i386: Move smc_code2 to an executable section
  accel/tcg: Properly implement get_page_addr_code for user-only
  accel/tcg: Unlock mmap_lock after longjmp
  accel/tcg: Make tb_htable_lookup static
  accel/tcg: Move qemu_ram_addr_from_host_nofail to physmem.c
  accel/tcg: Use probe_access_internal for softmmu
    get_page_addr_code_hostp
  accel/tcg: Add nofault parameter to get_page_addr_code_hostp
  accel/tcg: Raise PROT_EXEC exception early
  accel/tcg: Remove translator_ldsw
  accel/tcg: Add pc and host_pc params to gen_intermediate_code
  accel/tcg: Add fast path for translator_ld*
  target/riscv: Add MAX_INSN_LEN and insn_len
  target/riscv: Make translator stop before the end of a page

 include/elf.h                     |   1 +
 include/exec/cpu-common.h         |   1 +
 include/exec/exec-all.h           |  87 ++++++------------
 include/exec/translator.h         |  96 +++++++++++++-------
 linux-user/arm/target_cpu.h       |   4 +-
 linux-user/qemu.h                 |   1 +
 accel/tcg/cpu-exec.c              | 134 ++++++++++++++--------------
 accel/tcg/cputlb.c                |  93 ++++++--------------
 accel/tcg/plugin-gen.c            |   4 +-
 accel/tcg/translate-all.c         |  29 +++---
 accel/tcg/translator.c            | 136 +++++++++++++++++++++-------
 accel/tcg/user-exec.c             |  18 +++-
 linux-user/elfload.c              |  82 +++++++++++++++--
 linux-user/mmap.c                 |   8 ++
 softmmu/physmem.c                 |  12 +++
 target/alpha/translate.c          |   5 +-
 target/arm/translate.c            |   5 +-
 target/avr/translate.c            |   5 +-
 target/cris/translate.c           |   5 +-
 target/hexagon/translate.c        |   6 +-
 target/hppa/translate.c           |   5 +-
 target/i386/tcg/translate.c       |  32 ++++++-
 target/loongarch/translate.c      |   6 +-
 target/m68k/translate.c           |   5 +-
 target/microblaze/translate.c     |   5 +-
 target/mips/tcg/translate.c       |   5 +-
 target/nios2/translate.c          |   5 +-
 target/openrisc/translate.c       |   6 +-
 target/ppc/translate.c            |   5 +-
 target/riscv/translate.c          |  32 +++++--
 target/rx/translate.c             |   5 +-
 target/s390x/tcg/translate.c      |  20 +++--
 target/sh4/translate.c            |   5 +-
 target/sparc/translate.c          |   5 +-
 target/tricore/translate.c        |   6 +-
 target/xtensa/translate.c         |   6 +-
 tests/tcg/i386/test-i386.c        |   2 +-
 tests/tcg/riscv64/noexec.c        |  79 +++++++++++++++++
 tests/tcg/s390x/noexec.c          | 106 ++++++++++++++++++++++
 tests/tcg/x86_64/noexec.c         |  75 ++++++++++++++++
 tests/tcg/multiarch/noexec.c.inc  | 141 ++++++++++++++++++++++++++++++
 tests/tcg/riscv64/Makefile.target |   1 +
 tests/tcg/s390x/Makefile.target   |   1 +
 tests/tcg/x86_64/Makefile.target  |   3 +-
 44 files changed, 951 insertions(+), 342 deletions(-)
 create mode 100644 tests/tcg/riscv64/noexec.c
 create mode 100644 tests/tcg/s390x/noexec.c
 create mode 100644 tests/tcg/x86_64/noexec.c
 create mode 100644 tests/tcg/multiarch/noexec.c.inc

-- 
2.34.1

On 8/19/22 11:25, Richard Henderson wrote:
> Hi Ilya,
>
> After adding support for riscv (similar to s390x, in that we can
> find the total insn length from the first couple of bits, so, easy),
> I find that the test case doesn't work without all of the other
> changes for PROT_EXEC, including the translator_ld changes.
>
> Other changes from your v5:
>   - mprotect invalidates tbs.  The test case is riscv, with a
>     4-byte insn at offset 0xffe, which was chained to from the
>     insn at offset 0xffa.  The fact that the 0xffe tb was not
>     invalidated meant that we chained to it and re-executed
>     without revalidating page protections.
>
>   - rewrote the test framework to be agnostic of page size, which
>     reduces some of the repetition.  I ran into trouble with the
>     riscv linker, which relaxed the segment such that .align+.org
>     wasn't actually honored.  This new form doesn't require the
>     test bytes to be aligned in the binary.
>
>
> r~
I've confirmed that this fixes #1155

Tested-by: Vivian Wang <dramforever@live.com>

> Ilya Leoshkevich (4):
>   linux-user: Clear translations and tb_jmp_cache on mprotect()
>   accel/tcg: Introduce is_same_page()
>   target/s390x: Make translator stop before the end of a page
>   target/i386: Make translator stop before the end of a page
>
> Richard Henderson (17):
>   linux-user/arm: Mark the commpage executable
>   linux-user/hppa: Allocate page zero as a commpage
>   linux-user/x86_64: Allocate vsyscall page as a commpage
>   linux-user: Honor PT_GNU_STACK
>   tests/tcg/i386: Move smc_code2 to an executable section
>   accel/tcg: Properly implement get_page_addr_code for user-only
>   accel/tcg: Unlock mmap_lock after longjmp
>   accel/tcg: Make tb_htable_lookup static
>   accel/tcg: Move qemu_ram_addr_from_host_nofail to physmem.c
>   accel/tcg: Use probe_access_internal for softmmu
>     get_page_addr_code_hostp
>   accel/tcg: Add nofault parameter to get_page_addr_code_hostp
>   accel/tcg: Raise PROT_EXEC exception early
>   accel/tcg: Remove translator_ldsw
>   accel/tcg: Add pc and host_pc params to gen_intermediate_code
>   accel/tcg: Add fast path for translator_ld*
>   target/riscv: Add MAX_INSN_LEN and insn_len
>   target/riscv: Make translator stop before the end of a page
>
>  include/elf.h                     |   1 +
>  include/exec/cpu-common.h         |   1 +
>  include/exec/exec-all.h           |  87 ++++++------------
>  include/exec/translator.h         |  96 +++++++++++++-------
>  linux-user/arm/target_cpu.h       |   4 +-
>  linux-user/qemu.h                 |   1 +
>  accel/tcg/cpu-exec.c              | 134 ++++++++++++++--------------
>  accel/tcg/cputlb.c                |  93 ++++++--------------
>  accel/tcg/plugin-gen.c            |   4 +-
>  accel/tcg/translate-all.c         |  29 +++---
>  accel/tcg/translator.c            | 136 +++++++++++++++++++++-------
>  accel/tcg/user-exec.c             |  18 +++-
>  linux-user/elfload.c              |  82 +++++++++++++++--
>  linux-user/mmap.c                 |   8 ++
>  softmmu/physmem.c                 |  12 +++
>  target/alpha/translate.c          |   5 +-
>  target/arm/translate.c            |   5 +-
>  target/avr/translate.c            |   5 +-
>  target/cris/translate.c           |   5 +-
>  target/hexagon/translate.c        |   6 +-
>  target/hppa/translate.c           |   5 +-
>  target/i386/tcg/translate.c       |  32 ++++++-
>  target/loongarch/translate.c      |   6 +-
>  target/m68k/translate.c           |   5 +-
>  target/microblaze/translate.c     |   5 +-
>  target/mips/tcg/translate.c       |   5 +-
>  target/nios2/translate.c          |   5 +-
>  target/openrisc/translate.c       |   6 +-
>  target/ppc/translate.c            |   5 +-
>  target/riscv/translate.c          |  32 +++++--
>  target/rx/translate.c             |   5 +-
>  target/s390x/tcg/translate.c      |  20 +++--
>  target/sh4/translate.c            |   5 +-
>  target/sparc/translate.c          |   5 +-
>  target/tricore/translate.c        |   6 +-
>  target/xtensa/translate.c         |   6 +-
>  tests/tcg/i386/test-i386.c        |   2 +-
>  tests/tcg/riscv64/noexec.c        |  79 +++++++++++++++++
>  tests/tcg/s390x/noexec.c          | 106 ++++++++++++++++++++++
>  tests/tcg/x86_64/noexec.c         |  75 ++++++++++++++++
>  tests/tcg/multiarch/noexec.c.inc  | 141 ++++++++++++++++++++++++++++++
>  tests/tcg/riscv64/Makefile.target |   1 +
>  tests/tcg/s390x/Makefile.target   |   1 +
>  tests/tcg/x86_64/Makefile.target  |   3 +-
>  44 files changed, 951 insertions(+), 342 deletions(-)
>  create mode 100644 tests/tcg/riscv64/noexec.c
>  create mode 100644 tests/tcg/s390x/noexec.c
>  create mode 100644 tests/tcg/x86_64/noexec.c
>  create mode 100644 tests/tcg/multiarch/noexec.c.inc
>