From: Ian Jackson <ian.jackson@eu.citrix.com> To: xen-devel@lists.xenproject.org Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wei.liu2@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Jan Beulich <jbeulich@suse.com> Subject: [PATCH STABLE for-4.8] x86emul/test: don't use *_len symbols Date: Wed, 15 May 2019 12:01:25 +0100 [thread overview] Message-ID: <20190515110125.16882-1-ian.jackson@eu.citrix.com> (raw) From: Jan Beulich <JBeulich@suse.com> ... as they don't work as intended with -fPIC. I did prefer them over *_end ones at the time because older gcc would cause .L* symbols to be public, due to issuing .globl for all referenced externals. And labels at the end of instructions collide with the ones at the start of the next instruction, making disassembly harder to grok. Luckily recent gcc no longer issues those .globl directives, and hence .L* labels, staying local by default, no longer get in the way. Reported-by: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Tested-by: Wei Liu <wei.liu2@citrix.com> (cherry picked from commit 9315fa0ef736d1153c98ce42bff5853da5ec697f) This backport had some conflicts. Notably 5ad98e3c7fa92f46d77a788e1109b7d282bd1256 x86emul: support ADCX/ADOX contains a change to the definition of set_insn. This is not mentioned in the commit message and therefore lacks any kind of justification. I strongly deprecate this; that change ought to have been split out into its own commit, no matter that it's very small. After consultation I have decided to drop rather than backport that change to the definition of set_insn. CC: Jan Beulich <jbeulich@suse.com> CC: Andrew Cooper <andrew.cooper3@citrix.com> CC: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> --- tools/tests/x86_emulator/test_x86_emulator.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c index 7b467fe021..f902d0b99b 100644 --- a/tools/tests/x86_emulator/test_x86_emulator.c +++ b/tools/tests/x86_emulator/test_x86_emulator.c @@ -766,15 +766,15 @@ int main(int argc, char **argv) goto fail; printf("okay\n"); -#define decl_insn(which) extern const unsigned char which[], which##_len[] +#define decl_insn(which) extern const unsigned char which[], \ + which##_end[] asm ( ".L" #which "_end" ) #define put_insn(which, insn) ".pushsection .test, \"ax\", @progbits\n" \ #which ": " insn "\n" \ - ".equ " #which "_len, .-" #which "\n" \ + ".L" #which "_end:\n" \ ".popsection" #define set_insn(which) (regs.eip = (unsigned long)memcpy(instr, which, \ - (unsigned long)which##_len)) -#define check_eip(which) (regs.eip == (unsigned long)instr + \ - (unsigned long)which##_len) + (unsigned long)which##_end - (unsigned long)(which))) +#define check_eip(which) (regs.eip == (unsigned long)which##_end) printf("%-40s", "Testing movq %mm3,(%ecx)..."); if ( stack_exec && cpu_has_mmx ) -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
WARNING: multiple messages have this Message-ID (diff)
From: Ian Jackson <ian.jackson@eu.citrix.com> To: <xen-devel@lists.xenproject.org> Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wei.liu2@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Jan Beulich <jbeulich@suse.com> Subject: [Xen-devel] [PATCH STABLE for-4.8] x86emul/test: don't use *_len symbols Date: Wed, 15 May 2019 12:01:25 +0100 [thread overview] Message-ID: <20190515110125.16882-1-ian.jackson@eu.citrix.com> (raw) Message-ID: <20190515110125.fGcHSMQW1h9mVFEJMrFc2vlHlzr4LNSyh0Qyli83F70@z> (raw) From: Jan Beulich <JBeulich@suse.com> ... as they don't work as intended with -fPIC. I did prefer them over *_end ones at the time because older gcc would cause .L* symbols to be public, due to issuing .globl for all referenced externals. And labels at the end of instructions collide with the ones at the start of the next instruction, making disassembly harder to grok. Luckily recent gcc no longer issues those .globl directives, and hence .L* labels, staying local by default, no longer get in the way. Reported-by: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com> Tested-by: Wei Liu <wei.liu2@citrix.com> (cherry picked from commit 9315fa0ef736d1153c98ce42bff5853da5ec697f) This backport had some conflicts. Notably 5ad98e3c7fa92f46d77a788e1109b7d282bd1256 x86emul: support ADCX/ADOX contains a change to the definition of set_insn. This is not mentioned in the commit message and therefore lacks any kind of justification. I strongly deprecate this; that change ought to have been split out into its own commit, no matter that it's very small. After consultation I have decided to drop rather than backport that change to the definition of set_insn. CC: Jan Beulich <jbeulich@suse.com> CC: Andrew Cooper <andrew.cooper3@citrix.com> CC: Wei Liu <wei.liu2@citrix.com> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> --- tools/tests/x86_emulator/test_x86_emulator.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c index 7b467fe021..f902d0b99b 100644 --- a/tools/tests/x86_emulator/test_x86_emulator.c +++ b/tools/tests/x86_emulator/test_x86_emulator.c @@ -766,15 +766,15 @@ int main(int argc, char **argv) goto fail; printf("okay\n"); -#define decl_insn(which) extern const unsigned char which[], which##_len[] +#define decl_insn(which) extern const unsigned char which[], \ + which##_end[] asm ( ".L" #which "_end" ) #define put_insn(which, insn) ".pushsection .test, \"ax\", @progbits\n" \ #which ": " insn "\n" \ - ".equ " #which "_len, .-" #which "\n" \ + ".L" #which "_end:\n" \ ".popsection" #define set_insn(which) (regs.eip = (unsigned long)memcpy(instr, which, \ - (unsigned long)which##_len)) -#define check_eip(which) (regs.eip == (unsigned long)instr + \ - (unsigned long)which##_len) + (unsigned long)which##_end - (unsigned long)(which))) +#define check_eip(which) (regs.eip == (unsigned long)which##_end) printf("%-40s", "Testing movq %mm3,(%ecx)..."); if ( stack_exec && cpu_has_mmx ) -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
next reply other threads:[~2019-05-15 11:02 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-15 11:01 Ian Jackson [this message] 2019-05-15 11:01 ` [Xen-devel] [PATCH STABLE for-4.8] x86emul/test: don't use *_len symbols Ian Jackson
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190515110125.16882-1-ian.jackson@eu.citrix.com \ --to=ian.jackson@eu.citrix.com \ --cc=andrew.cooper3@citrix.com \ --cc=jbeulich@suse.com \ --cc=wei.liu2@citrix.com \ --cc=xen-devel@lists.xenproject.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).