| Message ID | 20220608122953.1051952-1-scgl@linux.ibm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | s390x: Avoid gcc 12 warnings | expand |
On Wed, 8 Jun 2022 14:29:51 +0200 Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote: > gcc 12 warns if a memory operand to inline asm points to memory in the > first 4k bytes. However, in our case, these operands are fine, either > because we actually want to use that memory, or expect and handle the > resulting exception. thanks, queued > > v2 -> v3 > * extend commit msg > * pick up r-b > * use macro instead of pointer to address memory > > v1 -> v2 > * replace mechanism, don't use pragmas, instead use an extern symbol so > gcc cannot conclude that the pointer is <4k > > This new extern symbol refers to the lowcore. As a result, code > generation for lowcore accesses becomes worse. > > Alternatives: > * Don't use extern symbol for lowcore, just for problematic pointers > * Hide value from gcc via inline asm > * Disable the warning globally > * Use memory clobber instead of memory output > Use address in register input instead of memory input > (may require WRITE_ONCE) > * Don't use gcc 12.0, with newer versions --param=min-pagesize=0 might > avoid the problem > > Janis Schoetterl-Glausch (2): > s390x: Introduce symbol for lowcore and use it > s390x: Fix gcc 12 warning about array bounds > > lib/s390x/asm/arch_def.h | 2 ++ > lib/s390x/asm/facility.h | 4 +-- > lib/s390x/asm/mem.h | 4 +++ > lib/s390x/css.h | 2 -- > lib/s390x/css_lib.c | 12 ++++---- > lib/s390x/fault.c | 10 +++---- > lib/s390x/interrupt.c | 61 +++++++++++++++++++------------------- > lib/s390x/mmu.c | 3 +- > s390x/flat.lds | 1 + > s390x/snippets/c/flat.lds | 1 + > s390x/css.c | 4 +-- > s390x/diag288.c | 4 +-- > s390x/edat.c | 5 ++-- > s390x/emulator.c | 15 +++++----- > s390x/mvpg.c | 7 ++--- > s390x/sclp.c | 3 +- > s390x/skey.c | 2 +- > s390x/skrf.c | 11 +++---- > s390x/smp.c | 23 +++++++------- > s390x/snippets/c/spec_ex.c | 5 ++-- > 20 files changed, 83 insertions(+), 96 deletions(-) > > Range-diff against v2: > 1: 412a9962 ! 1: 44b10d41 s390x: Introduce symbol for lowcore and use it > @@ Commit message > The new symbol is not a pointer. While this will lead to worse code > generation (cannot use register 0 for addressing), that should not > matter too much for kvm unit tests. > + Since the lowcore is located per definition at address 0, the symbol is > + defined via the linker scripts. > The symbol also will be used to create pointers that the compiler cannot > warn about as being outside the bounds of an array. > > Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> > + Reviewed-by: Thomas Huth <thuth@redhat.com> > > ## lib/s390x/asm/arch_def.h ## > @@ lib/s390x/asm/arch_def.h: struct lowcore { > 2: 9b2eeee3 ! 2: e9e88996 s390x: Fix gcc 12 warning about array bounds > @@ lib/s390x/asm/mem.h > #define _ASMS390X_MEM_H_ > +#include <asm/arch_def.h> > + > -+/* pointer to 0 used to avoid compiler warnings */ > -+uint8_t *mem_all = (uint8_t *)&lowcore; > ++/* create pointer while avoiding compiler warnings */ > ++#define OPAQUE_PTR(x) ((void *)(((uint64_t)&lowcore) + (x))) > > #define SKEY_ACC 0xf0 > #define SKEY_FP 0x08 > @@ s390x/emulator.c: static __always_inline void __test_cpacf_invalid_parm(unsigned > report_prefix_push("invalid parm address"); > expect_pgm_int(); > - __cpacf_query(opcode, (void *) -1); > -+ __cpacf_query(opcode, (cpacf_mask_t *)&mem_all[-1]); > ++ __cpacf_query(opcode, OPAQUE_PTR(-1)); > check_pgm_int_code(PGM_INT_CODE_ADDRESSING); > report_prefix_pop(); > } > @@ s390x/emulator.c: static __always_inline void __test_cpacf_protected_parm(unsign > expect_pgm_int(); > low_prot_enable(); > - __cpacf_query(opcode, (void *) 8); > -+ __cpacf_query(opcode, (cpacf_mask_t *)&mem_all[8]); > ++ __cpacf_query(opcode, OPAQUE_PTR(8)); > low_prot_disable(); > check_pgm_int_code(PGM_INT_CODE_PROTECTION); > report_prefix_pop(); > @@ s390x/skey.c: static void test_set_prefix(void) > expect_pgm_int(); > install_page(root, virt_to_pte_phys(root, pagebuf), 0); > - set_prefix_key_1((uint32_t *)2048); > -+ set_prefix_key_1((uint32_t *)&mem_all[2048]); > ++ set_prefix_key_1(OPAQUE_PTR(2048)); > install_page(root, 0, 0); > check_pgm_int_code(PGM_INT_CODE_PROTECTION); > report(get_prefix() == old_prefix, "did not set prefix"); > > base-commit: 2eed0bf1096077144cc3a0dd9974689487f9511a
gcc 12 warns if a memory operand to inline asm points to memory in the first 4k bytes. However, in our case, these operands are fine, either because we actually want to use that memory, or expect and handle the resulting exception. v2 -> v3 * extend commit msg * pick up r-b * use macro instead of pointer to address memory v1 -> v2 * replace mechanism, don't use pragmas, instead use an extern symbol so gcc cannot conclude that the pointer is <4k This new extern symbol refers to the lowcore. As a result, code generation for lowcore accesses becomes worse. Alternatives: * Don't use extern symbol for lowcore, just for problematic pointers * Hide value from gcc via inline asm * Disable the warning globally * Use memory clobber instead of memory output Use address in register input instead of memory input (may require WRITE_ONCE) * Don't use gcc 12.0, with newer versions --param=min-pagesize=0 might avoid the problem Janis Schoetterl-Glausch (2): s390x: Introduce symbol for lowcore and use it s390x: Fix gcc 12 warning about array bounds lib/s390x/asm/arch_def.h | 2 ++ lib/s390x/asm/facility.h | 4 +-- lib/s390x/asm/mem.h | 4 +++ lib/s390x/css.h | 2 -- lib/s390x/css_lib.c | 12 ++++---- lib/s390x/fault.c | 10 +++---- lib/s390x/interrupt.c | 61 +++++++++++++++++++------------------- lib/s390x/mmu.c | 3 +- s390x/flat.lds | 1 + s390x/snippets/c/flat.lds | 1 + s390x/css.c | 4 +-- s390x/diag288.c | 4 +-- s390x/edat.c | 5 ++-- s390x/emulator.c | 15 +++++----- s390x/mvpg.c | 7 ++--- s390x/sclp.c | 3 +- s390x/skey.c | 2 +- s390x/skrf.c | 11 +++---- s390x/smp.c | 23 +++++++------- s390x/snippets/c/spec_ex.c | 5 ++-- 20 files changed, 83 insertions(+), 96 deletions(-) Range-diff against v2: 1: 412a9962 ! 1: 44b10d41 s390x: Introduce symbol for lowcore and use it @@ Commit message The new symbol is not a pointer. While this will lead to worse code generation (cannot use register 0 for addressing), that should not matter too much for kvm unit tests. + Since the lowcore is located per definition at address 0, the symbol is + defined via the linker scripts. The symbol also will be used to create pointers that the compiler cannot warn about as being outside the bounds of an array. Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com> + Reviewed-by: Thomas Huth <thuth@redhat.com> ## lib/s390x/asm/arch_def.h ## @@ lib/s390x/asm/arch_def.h: struct lowcore { 2: 9b2eeee3 ! 2: e9e88996 s390x: Fix gcc 12 warning about array bounds @@ lib/s390x/asm/mem.h #define _ASMS390X_MEM_H_ +#include <asm/arch_def.h> + -+/* pointer to 0 used to avoid compiler warnings */ -+uint8_t *mem_all = (uint8_t *)&lowcore; ++/* create pointer while avoiding compiler warnings */ ++#define OPAQUE_PTR(x) ((void *)(((uint64_t)&lowcore) + (x))) #define SKEY_ACC 0xf0 #define SKEY_FP 0x08 @@ s390x/emulator.c: static __always_inline void __test_cpacf_invalid_parm(unsigned report_prefix_push("invalid parm address"); expect_pgm_int(); - __cpacf_query(opcode, (void *) -1); -+ __cpacf_query(opcode, (cpacf_mask_t *)&mem_all[-1]); ++ __cpacf_query(opcode, OPAQUE_PTR(-1)); check_pgm_int_code(PGM_INT_CODE_ADDRESSING); report_prefix_pop(); } @@ s390x/emulator.c: static __always_inline void __test_cpacf_protected_parm(unsign expect_pgm_int(); low_prot_enable(); - __cpacf_query(opcode, (void *) 8); -+ __cpacf_query(opcode, (cpacf_mask_t *)&mem_all[8]); ++ __cpacf_query(opcode, OPAQUE_PTR(8)); low_prot_disable(); check_pgm_int_code(PGM_INT_CODE_PROTECTION); report_prefix_pop(); @@ s390x/skey.c: static void test_set_prefix(void) expect_pgm_int(); install_page(root, virt_to_pte_phys(root, pagebuf), 0); - set_prefix_key_1((uint32_t *)2048); -+ set_prefix_key_1((uint32_t *)&mem_all[2048]); ++ set_prefix_key_1(OPAQUE_PTR(2048)); install_page(root, 0, 0); check_pgm_int_code(PGM_INT_CODE_PROTECTION); report(get_prefix() == old_prefix, "did not set prefix"); base-commit: 2eed0bf1096077144cc3a0dd9974689487f9511a