Fix gcc9 build failures

Submitted by Adrian Reber on Jan. 9, 2019, 12:49 p.m.

Details

Message ID 1547038151-6916-1-git-send-email-adrian@lisas.de
State Accepted
Series "Fix gcc9 build failures"
Headers show

Commit Message

Adrian Reber Jan. 9, 2019, 12:49 p.m.
From: Adrian Reber <areber@redhat.com>

I received this patch from Jeff Law as a fix for build failures with the
upcoming GCC 9. The following is Jeff Law's description of 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.

Suggested-by: Jeff Law <law@redhat.com>
Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com>
Signed-off-by: Adrian Reber <areber@redhat.com>
---
 compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h | 2 +-
 compel/arch/arm/src/lib/include/uapi/asm/sigframe.h     | 2 +-
 compel/arch/x86/src/lib/include/uapi/asm/sigframe.h     | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

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

Andrei Vagin Jan. 10, 2019, 5:02 p.m.
On Wed, Jan 09, 2019 at 12:49:11PM +0000, Adrian Reber wrote:
> From: Adrian Reber <areber@redhat.com>

Should it be "From: Jeff Law <law@redhat.com>"?

> 
> I received this patch from Jeff Law as a fix for build failures with the
> upcoming GCC 9. The following is Jeff Law's description of 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.
> 
> Suggested-by: Jeff Law <law@redhat.com>
> Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com>
> Signed-off-by: Adrian Reber <areber@redhat.com>
> ---
>  compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h | 2 +-
>  compel/arch/arm/src/lib/include/uapi/asm/sigframe.h     | 2 +-
>  compel/arch/x86/src/lib/include/uapi/asm/sigframe.h     | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> 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 {									\
> -- 
> 1.8.3.1
>
Adrian Reber Jan. 10, 2019, 5:10 p.m.
On Thu, Jan 10, 2019 at 09:02:19AM -0800, Andrei Vagin wrote:
> On Wed, Jan 09, 2019 at 12:49:11PM +0000, Adrian Reber wrote:
> > From: Adrian Reber <areber@redhat.com>
> 
> Should it be "From: Jeff Law <law@redhat.com>"?

Probably. If you want you can change it. I was not sure how interested
Jeff was in the submission process of the patch, so I just sent it out
with my usual command. If you can fix it up, please do.

> > I received this patch from Jeff Law as a fix for build failures with the
> > upcoming GCC 9. The following is Jeff Law's description of 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.
> > 
> > Suggested-by: Jeff Law <law@redhat.com>
> > Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com>
> > Signed-off-by: Adrian Reber <areber@redhat.com>
> > ---
> >  compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h | 2 +-
> >  compel/arch/arm/src/lib/include/uapi/asm/sigframe.h     | 2 +-
> >  compel/arch/x86/src/lib/include/uapi/asm/sigframe.h     | 4 ++--
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > 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 {									\
> > -- 
> > 1.8.3.1
> >
Andrei Vagin Jan. 13, 2019, 6:23 a.m.
Applied, thanks!

On Wed, Jan 09, 2019 at 12:49:11PM +0000, Adrian Reber wrote:
> From: Adrian Reber <areber@redhat.com>
> 
> I received this patch from Jeff Law as a fix for build failures with the
> upcoming GCC 9. The following is Jeff Law's description of 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.
> 
> Suggested-by: Jeff Law <law@redhat.com>
> Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com>
> Signed-off-by: Adrian Reber <areber@redhat.com>
> ---
>  compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h | 2 +-
>  compel/arch/arm/src/lib/include/uapi/asm/sigframe.h     | 2 +-
>  compel/arch/x86/src/lib/include/uapi/asm/sigframe.h     | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> 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 {									\
> -- 
> 1.8.3.1
>
Salvatore Bonaccorso Dec. 26, 2019, 9:43 p.m.
Hi,

On Wed, Jan 09, 2019 at 12:49:11PM +0000, Adrian Reber wrote:
> From: Adrian Reber <areber@redhat.com>
> 
> I received this patch from Jeff Law as a fix for build failures with the
> upcoming GCC 9. The following is Jeff Law's description of 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.

In Debian, with gcc-9 we are seeing as well build failures on ppc64el
and s390x similarly. See:

https://buildd.debian.org/status/fetch.php?pkg=criu&arch=s390x&ver=3.12-1&stamp=1570688225&raw=0
https://buildd.debian.org/status/fetch.php?pkg=criu&arch=ppc64el&ver=3.12-1&stamp=1570688342&raw=0

Regards,
Salvatore
Adrian Reber Dec. 26, 2019, 11:19 p.m.
On Thu, Dec 26, 2019 at 10:43:05PM +0100, Salvatore Bonaccorso wrote:
> Hi,
> 
> On Wed, Jan 09, 2019 at 12:49:11PM +0000, Adrian Reber wrote:
> > From: Adrian Reber <areber@redhat.com>
> > 
> > I received this patch from Jeff Law as a fix for build failures with the
> > upcoming GCC 9. The following is Jeff Law's description of 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.
> 
> In Debian, with gcc-9 we are seeing as well build failures on ppc64el
> and s390x similarly. See:
> 
> https://buildd.debian.org/status/fetch.php?pkg=criu&arch=s390x&ver=3.12-1&stamp=1570688225&raw=0
> https://buildd.debian.org/status/fetch.php?pkg=criu&arch=ppc64el&ver=3.12-1&stamp=1570688342&raw=0

In Fedora I am building CRIU 3.13 without any errors using GCC 9 on
s390x and ppc64le:

https://koji.fedoraproject.org/koji/buildinfo?buildID=1377921

The error you are seeing is, however, interesting as the file has not
seen any changes for a long time. Curious why it fails on Debian.

		Adrian
Salvatore Bonaccorso Dec. 27, 2019, 7:32 a.m.
Hi Adrian,

On Fri, Dec 27, 2019 at 12:19:26AM +0100, Adrian Reber wrote:
> On Thu, Dec 26, 2019 at 10:43:05PM +0100, Salvatore Bonaccorso wrote:
> > Hi,
> > 
> > On Wed, Jan 09, 2019 at 12:49:11PM +0000, Adrian Reber wrote:
> > > From: Adrian Reber <areber@redhat.com>
> > > 
> > > I received this patch from Jeff Law as a fix for build failures with the
> > > upcoming GCC 9. The following is Jeff Law's description of 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.
> > 
> > In Debian, with gcc-9 we are seeing as well build failures on ppc64el
> > and s390x similarly. See:
> > 
> > https://buildd.debian.org/status/fetch.php?pkg=criu&arch=s390x&ver=3.12-1&stamp=1570688225&raw=0
> > https://buildd.debian.org/status/fetch.php?pkg=criu&arch=ppc64el&ver=3.12-1&stamp=1570688342&raw=0
> 
> In Fedora I am building CRIU 3.13 without any errors using GCC 9 on
> s390x and ppc64le:
> 
> https://koji.fedoraproject.org/koji/buildinfo?buildID=1377921
> 
> The error you are seeing is, however, interesting as the file has not
> seen any changes for a long time. Curious why it fails on Debian.

Ack, that is interesting then, because it fails on both architectures
back from the first build done with gcc-9, so both 3.12 and 3.13.

So needs further investigation.

Regards,
Salvatore
Adrian Reber Jan. 14, 2020, 2:14 p.m.
On Fri, Dec 27, 2019 at 08:32:39AM +0100, Salvatore Bonaccorso wrote:
> On Fri, Dec 27, 2019 at 12:19:26AM +0100, Adrian Reber wrote:
> > On Thu, Dec 26, 2019 at 10:43:05PM +0100, Salvatore Bonaccorso wrote:
> > > On Wed, Jan 09, 2019 at 12:49:11PM +0000, Adrian Reber wrote:
> > > > From: Adrian Reber <areber@redhat.com>
> > > > 
> > > > I received this patch from Jeff Law as a fix for build failures with the
> > > > upcoming GCC 9. The following is Jeff Law's description of 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.
> > > 
> > > In Debian, with gcc-9 we are seeing as well build failures on ppc64el
> > > and s390x similarly. See:
> > > 
> > > https://buildd.debian.org/status/fetch.php?pkg=criu&arch=s390x&ver=3.12-1&stamp=1570688225&raw=0
> > > https://buildd.debian.org/status/fetch.php?pkg=criu&arch=ppc64el&ver=3.12-1&stamp=1570688342&raw=0
> > 
> > In Fedora I am building CRIU 3.13 without any errors using GCC 9 on
> > s390x and ppc64le:
> > 
> > https://koji.fedoraproject.org/koji/buildinfo?buildID=1377921
> > 
> > The error you are seeing is, however, interesting as the file has not
> > seen any changes for a long time. Curious why it fails on Debian.
> 
> Ack, that is interesting then, because it fails on both architectures
> back from the first build done with gcc-9, so both 3.12 and 3.13.
> 
> So needs further investigation.

I was able to get the same error on Fedora 31. It seems this might be
something which was introduced with later releases of gcc 9. Because it
used to compile. I opened a PR to get this fixed:

https://github.com/checkpoint-restore/criu/pull/909

		Adrian
Jeff Law Jan. 14, 2020, 3:33 p.m.
On Tue, 2020-01-14 at 15:14 +0100, Adrian Reber wrote:
> On Fri, Dec 27, 2019 at 08:32:39AM +0100, Salvatore Bonaccorso wrote:
> > On Fri, Dec 27, 2019 at 12:19:26AM +0100, Adrian Reber wrote:
> > > On Thu, Dec 26, 2019 at 10:43:05PM +0100, Salvatore Bonaccorso wrote:
> > > > On Wed, Jan 09, 2019 at 12:49:11PM +0000, Adrian Reber wrote:
> > > > > From: Adrian Reber <areber@redhat.com>
> > > > > 
> > > > > I received this patch from Jeff Law as a fix for build failures with the
> > > > > upcoming GCC 9. The following is Jeff Law's description of 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.
> > > > 
> > > > In Debian, with gcc-9 we are seeing as well build failures on ppc64el
> > > > and s390x similarly. See:
> > > > 
> > > > https://buildd.debian.org/status/fetch.php?pkg=criu&arch=s390x&ver=3.12-1&stamp=1570688225&raw=0
> > > > https://buildd.debian.org/status/fetch.php?pkg=criu&arch=ppc64el&ver=3.12-1&stamp=1570688342&raw=0
> > > 
> > > In Fedora I am building CRIU 3.13 without any errors using GCC 9 on
> > > s390x and ppc64le:
> > > 
> > > https://koji.fedoraproject.org/koji/buildinfo?buildID=1377921
> > > 
> > > The error you are seeing is, however, interesting as the file has not
> > > seen any changes for a long time. Curious why it fails on Debian.
> > 
> > Ack, that is interesting then, because it fails on both architectures
> > back from the first build done with gcc-9, so both 3.12 and 3.13.
> > 
> > So needs further investigation.
> 
> I was able to get the same error on Fedora 31. It seems this might be
> something which was introduced with later releases of gcc 9. Because it
> used to compile. I opened a PR to get this fixed:
> 
> https://github.com/checkpoint-restore/criu/pull/909
It's been a while since I looked at this stuff, but I don't recall it
changing during the gcc-9 cycle.  I do think the diagnostic is
conditional on the warnings enabled/disabled on the command line.  If
those differ, particularly -Wdeprecated, then that would likely explain
the difference in behavior.

Ultimately removing "sp" clobber is still the right thing to do.

jeff