CRIU gcc9 errors

Submitted by Adrian Reber on Jan. 8, 2019, 4:32 p.m.

Details

Message ID 20190108163245.GE12472@lisas.de
State New
Series "CRIU gcc9 errors"
Headers show

Commit Message

Adrian Reber Jan. 8, 2019, 4:32 p.m.
I just got an email that CRIU will fail building with the upcoming
release of gcc. Fortunately the email also included a patch. I attached
the patch and can submit it properly. Just wanted to hear if anyone has
any feedback/comments to this patch.

I got the following description for the patch



Attached you'll find the fix for criu.  You'll see it's just a matter of
dropping the sp/esp clobber from the relevant asm statements.  THe details:

criu has a macro which defines an asm which appears to want to set a new
stack pointer, then directly issue a sigreturn call to the kernel.  Some
variants clobber sp (aarch64, arm, x86), others do not (ppc, s390)


While the asm does indeed set a new stack pointer, we never return from
a sigreturn syscall -- at least not in the normal way.  We actually
return back to the point where the process was interrupted by the
signal.  So describing the affect of the asm on the stack pointer is
pedantically correct, it actually has no real effect and can just be
dropped to avoid the hard error from gcc-9.

Patch hide | download patch | download mbox

diff --git a/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h b/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h
index 6b511ce..6b9317b 100644
--- a/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h
+++ b/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h
@@ -39,7 +39,7 @@  struct rt_sigframe {
 			"svc #0						\n"	\
 			:							\
 			: "r"(new_sp)						\
-			: "sp", "x8", "memory")
+			: "x8", "memory")
 
 /* cr_sigcontext is copied from arch/arm64/include/uapi/asm/sigcontext.h */
 struct cr_sigcontext {
diff --git a/compel/arch/arm/src/lib/include/uapi/asm/sigframe.h b/compel/arch/arm/src/lib/include/uapi/asm/sigframe.h
index 3e7bc01..b90c0f6 100644
--- a/compel/arch/arm/src/lib/include/uapi/asm/sigframe.h
+++ b/compel/arch/arm/src/lib/include/uapi/asm/sigframe.h
@@ -73,7 +73,7 @@  struct rt_sigframe {
 		     "svc #0					    \n"	\
 		     :							\
 		     : "r"(new_sp)					\
-		     : "sp","memory")
+		     : "memory")
 
 #define RT_SIGFRAME_UC(rt_sigframe)		(&rt_sigframe->sig.uc)
 #define RT_SIGFRAME_REGIP(rt_sigframe)		(rt_sigframe)->sig.uc.uc_mcontext.arm_ip
diff --git a/compel/arch/x86/src/lib/include/uapi/asm/sigframe.h b/compel/arch/x86/src/lib/include/uapi/asm/sigframe.h
index 0ad45a5..c29de3b 100644
--- a/compel/arch/x86/src/lib/include/uapi/asm/sigframe.h
+++ b/compel/arch/x86/src/lib/include/uapi/asm/sigframe.h
@@ -162,7 +162,7 @@  struct rt_sigframe {
 		     "syscall					    \n"	\
 		     :							\
 		     : "r"(new_sp)					\
-		     : "rax","rsp","memory")
+		     : "rax","memory")
 #define ARCH_RT_SIGRETURN_COMPAT(new_sp)				\
 	asm volatile(							\
 		"pushq $"__stringify(USER32_CS)"		\n"	\
@@ -176,7 +176,7 @@  struct rt_sigframe {
 		".code64					\n"	\
 		:							\
 		: "rdi"(new_sp)						\
-		: "eax","esp", "r8", "r9", "r10", "r11", "memory")
+		: "eax", "r8", "r9", "r10", "r11", "memory")
 
 #define ARCH_RT_SIGRETURN(new_sp, rt_sigframe)				\
 do {									\

Comments

Dmitry Safonov Jan. 8, 2019, 5:43 p.m.
On Tue, 8 Jan 2019 at 16:34, Adrian Reber <adrian@lisas.de> wrote:
>
> I just got an email that CRIU will fail building with the upcoming
> release of gcc. Fortunately the email also included a patch. I attached
> the patch and can submit it properly. Just wanted to hear if anyone has
> any feedback/comments to this patch.
>
> I got the following description for the patch
>
>
>
> Attached you'll find the fix for criu.  You'll see it's just a matter of
> dropping the sp/esp clobber from the relevant asm statements.  THe details:
>
> criu has a macro which defines an asm which appears to want to set a new
> stack pointer, then directly issue a sigreturn call to the kernel.  Some
> variants clobber sp (aarch64, arm, x86), others do not (ppc, s390)
>
>
> While the asm does indeed set a new stack pointer, we never return from
> a sigreturn syscall -- at least not in the normal way.  We actually
> return back to the point where the process was interrupted by the
> signal.  So describing the affect of the asm on the stack pointer is
> pedantically correct, it actually has no real effect and can just be
> dropped to avoid the hard error from gcc-9.

Sounds sane for me.

You can add for full submission:
Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com>

Thanks,
             Dmitry