[PATCH, x86, libgcc] PR target/83917 Correct debug for -mcall-ms2sysv-xlogues stubs

[PATCH, x86, libgcc] PR target/83917 Correct debug for -mcall-ms2sysv-xlogues stubs

[Daniel Santos]

When stepping through tail-call restore stubs the debugger has to assume
that rsp - 8 is the CFA, although it is not.  This is because I did not
explicitly add any .cfi directives.  This patch adds them to the
tail-call restore stubs, but this is new territory for me, so I would
appreciate feedback.

I've reg-tested on x86_64, but I still need to test on Solaris and
Darwin.  OK to commit after those tests?

Thanks,
Daniel

Signed-off-by: Daniel Santos <daniel.santos@pobox.com>
---
 libgcc/config/i386/resms64fx.h | 19 +++++++++++++++++++
 libgcc/config/i386/resms64x.h  | 22 ++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/libgcc/config/i386/resms64fx.h b/libgcc/config/i386/resms64fx.h
index c5f63d879fe..7dc8c7d89ed 100644
--- a/libgcc/config/i386/resms64fx.h
+++ b/libgcc/config/i386/resms64fx.h
@@ -34,21 +34,40 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 
 	.text
 MS2SYSV_STUB_BEGIN(resms64fx_17)
+.cfi_startproc
+.cfi_def_cfa %rbp, 16
 	mov	-0x68(%rsi),%r15
+.cfi_endproc
 MS2SYSV_STUB_BEGIN(resms64fx_16)
+.cfi_startproc
+.cfi_def_cfa %rbp, 16
 	mov	-0x60(%rsi),%r14
+.cfi_endproc
 MS2SYSV_STUB_BEGIN(resms64fx_15)
+.cfi_startproc
+.cfi_def_cfa %rbp, 16
 	mov	-0x58(%rsi),%r13
+.cfi_endproc
 MS2SYSV_STUB_BEGIN(resms64fx_14)
+.cfi_startproc
+.cfi_def_cfa %rbp, 16
 	mov	-0x50(%rsi),%r12
+.cfi_endproc
 MS2SYSV_STUB_BEGIN(resms64fx_13)
+.cfi_startproc
+.cfi_def_cfa %rbp, 16
 	mov	-0x48(%rsi),%rbx
+.cfi_endproc
 MS2SYSV_STUB_BEGIN(resms64fx_12)
+.cfi_startproc
+.cfi_def_cfa %rbp, 16
 	mov	-0x40(%rsi),%rdi
 	SSE_RESTORE
 	mov	-0x38(%rsi),%rsi
 	leaveq
+.cfi_def_cfa %rsp, 8
 	ret
+.cfi_endproc
 MS2SYSV_STUB_END(resms64fx_12)
 MS2SYSV_STUB_END(resms64fx_13)
 MS2SYSV_STUB_END(resms64fx_14)
diff --git a/libgcc/config/i386/resms64x.h b/libgcc/config/i386/resms64x.h
index 1b44938ae7c..753be1f4c52 100644
--- a/libgcc/config/i386/resms64x.h
+++ b/libgcc/config/i386/resms64x.h
@@ -33,23 +33,45 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 
 	.text
 MS2SYSV_STUB_BEGIN(resms64x_18)
+.cfi_startproc
+.cfi_def_cfa %r10, 8
 	mov	-0x70(%rsi),%r15
+.cfi_endproc
 MS2SYSV_STUB_BEGIN(resms64x_17)
+.cfi_startproc
+.cfi_def_cfa %r10, 8
 	mov	-0x68(%rsi),%r14
+.cfi_endproc
 MS2SYSV_STUB_BEGIN(resms64x_16)
+.cfi_startproc
+.cfi_def_cfa %r10, 8
 	mov	-0x60(%rsi),%r13
+.cfi_endproc
 MS2SYSV_STUB_BEGIN(resms64x_15)
+.cfi_startproc
+.cfi_def_cfa %r10, 8
 	mov	-0x58(%rsi),%r12
+.cfi_endproc
 MS2SYSV_STUB_BEGIN(resms64x_14)
+.cfi_startproc
+.cfi_def_cfa %r10, 8
 	mov	-0x50(%rsi),%rbp
+.cfi_endproc
 MS2SYSV_STUB_BEGIN(resms64x_13)
+.cfi_startproc
+.cfi_def_cfa %r10, 8
 	mov	-0x48(%rsi),%rbx
+.cfi_endproc
 MS2SYSV_STUB_BEGIN(resms64x_12)
+.cfi_startproc
+.cfi_def_cfa %r10, 8
 	mov	-0x40(%rsi),%rdi
 	SSE_RESTORE
 	mov	-0x38(%rsi),%rsi
 	mov	%r10,%rsp
+.cfi_def_cfa_register %rsp
 	ret
+.cfi_endproc
 MS2SYSV_STUB_END(resms64x_12)
 MS2SYSV_STUB_END(resms64x_13)
 MS2SYSV_STUB_END(resms64x_14)
-- 
2.15.0

Re: [PATCH, x86, libgcc] PR target/83917 Correct debug for -mcall-ms2sysv-xlogues stubs

[Jakub Jelinek]

On Fri, Jan 19, 2018 at 05:33:10PM -0600, Daniel Santos wrote:
> When stepping through tail-call restore stubs the debugger has to assume
> that rsp - 8 is the CFA, although it is not.  This is because I did not
> explicitly add any .cfi directives.  This patch adds them to the
> tail-call restore stubs, but this is new territory for me, so I would
> appreciate feedback.
> 
> I've reg-tested on x86_64, but I still need to test on Solaris and
> Darwin.  OK to commit after those tests?

I think you can't assume that the assembler supports .cfi_* directives.
While e.g. libgcc/config/i386/morestack.S uses them unconditionally,
it is guarded with:
        if test "$libgcc_cv_cfi" = "yes"; then
                tmake_file="${tmake_file} t-stack i386/t-stack-i386"
        fi
in config.host.  E.g. cygwin.S has:
#ifdef HAVE_GAS_CFI_SECTIONS_DIRECTIVE
        .cfi_sections   .debug_frame
# define cfi_startproc()                .cfi_startproc
# define cfi_endproc()                  .cfi_endproc
# define cfi_adjust_cfa_offset(X)       .cfi_adjust_cfa_offset X
# define cfi_def_cfa_register(X)        .cfi_def_cfa_register X
# define cfi_register(D,S)              .cfi_register D, S
# ifdef __x86_64__
#  define cfi_push(X)           .cfi_adjust_cfa_offset 8; .cfi_rel_offset X, 0
#  define cfi_pop(X)            .cfi_adjust_cfa_offset -8; .cfi_restore X
# else
#  define cfi_push(X)           .cfi_adjust_cfa_offset 4; .cfi_rel_offset X, 0
#  define cfi_pop(X)            .cfi_adjust_cfa_offset -4; .cfi_restore X
# endif
#else
# define cfi_startproc()
# define cfi_endproc()
# define cfi_adjust_cfa_offset(X)
# define cfi_def_cfa_register(X)
# define cfi_register(D,S)
# define cfi_push(X)
# define cfi_pop(X)
#endif /* HAVE_GAS_CFI_SECTIONS_DIRECTIVE */
perhaps you need something similar or commonize that (though, without
.cfi_sections, you want the default).

	Jakub

Re: [PATCH, x86, libgcc] PR target/83917 Correct debug for -mcall-ms2sysv-xlogues stubs

[Daniel Santos]

On 01/19/2018 05:35 PM, Jakub Jelinek wrote:
> On Fri, Jan 19, 2018 at 05:33:10PM -0600, Daniel Santos wrote:
>> When stepping through tail-call restore stubs the debugger has to assume
>> that rsp - 8 is the CFA, although it is not.  This is because I did not
>> explicitly add any .cfi directives.  This patch adds them to the
>> tail-call restore stubs, but this is new territory for me, so I would
>> appreciate feedback.
>>
>> I've reg-tested on x86_64, but I still need to test on Solaris and
>> Darwin.  OK to commit after those tests?
> I think you can't assume that the assembler supports .cfi_* directives.
> While e.g. libgcc/config/i386/morestack.S uses them unconditionally,
> it is guarded with:
>         if test "$libgcc_cv_cfi" = "yes"; then
>                 tmake_file="${tmake_file} t-stack i386/t-stack-i386"
>         fi

Ah hah! That explains a lot.  Yeah, I wasn't thinking all assemblers
would support it but I saw them in the Solaris assembler manual and
figured that they were maybe more widely supported than I had thought.

> in config.host.  E.g. cygwin.S has:
> #ifdef HAVE_GAS_CFI_SECTIONS_DIRECTIVE
>         .cfi_sections   .debug_frame
> # define cfi_startproc()                .cfi_startproc
> # define cfi_endproc()                  .cfi_endproc
> # define cfi_adjust_cfa_offset(X)       .cfi_adjust_cfa_offset X
> # define cfi_def_cfa_register(X)        .cfi_def_cfa_register X
> # define cfi_register(D,S)              .cfi_register D, S
> # ifdef __x86_64__
> #  define cfi_push(X)           .cfi_adjust_cfa_offset 8; .cfi_rel_offset X, 0
> #  define cfi_pop(X)            .cfi_adjust_cfa_offset -8; .cfi_restore X
> # else
> #  define cfi_push(X)           .cfi_adjust_cfa_offset 4; .cfi_rel_offset X, 0
> #  define cfi_pop(X)            .cfi_adjust_cfa_offset -4; .cfi_restore X
> # endif
> #else
> # define cfi_startproc()
> # define cfi_endproc()
> # define cfi_adjust_cfa_offset(X)
> # define cfi_def_cfa_register(X)
> # define cfi_register(D,S)
> # define cfi_push(X)
> # define cfi_pop(X)
> #endif /* HAVE_GAS_CFI_SECTIONS_DIRECTIVE */
> perhaps you need something similar or commonize that (though, without
> .cfi_sections, you want the default).
>
> 	Jakub

Thanks.  I like the idea of commonizing the macros for consistency.

As far as adding tests, I guess I would need to dig into
lib/gcc-gdb-test.exp to figure out how to do that.

Daniel

Re: [PATCH, x86, libgcc] PR target/83917 Correct debug for -mcall-ms2sysv-xlogues stubs

[Jakub Jelinek]

On Sat, Jan 20, 2018 at 06:01:16PM -0600, Daniel Santos wrote:
> As far as adding tests, I guess I would need to dig into
> lib/gcc-gdb-test.exp to figure out how to do that.

The gdb-test infrastructure allows essentially
b linenumber
p something
p somethingother

Not sure if it would work if instead of using a number of linenumber
you'd use the symbol for these snippets or symbol+number,
plus you are interested not in printing values of variables there, but
about sensible backtraces. 

Probably gdb's testsuite is better suited for that.

	Jakub

[PATCH] Correct debug for -mcall-ms2sysv-xlogues stubs (PR target/83917, take 2)

[Jakub Jelinek]

Hi!

On Sat, Jan 20, 2018 at 06:01:16PM -0600, Daniel Santos wrote:
> Thanks.  I like the idea of commonizing the macros for consistency.

Didn't see a progress on this P3 for a while, so I've written this
version of the patch; no tests though, what I've been using in testing was:
/* { dg-do compile { target lp64 } } */
/* { dg-options "-mno-avx -msse2 -mcall-ms2sysv-xlogues -O2" } */

void __attribute__((sysv_abi, noipa))
foo (void)
{
}

static void __attribute__((sysv_abi)) (*volatile foop) () = foo;

void __attribute__((ms_abi, noipa))
bar (void)
{
  foop ();
}

int
main ()
{
  bar ();
  return 0;
}

with/without -fno-omit-frame-pointer, disas bar; b on the tail
call in there, stepi; bt (which before the patch failed, now works),
also up; p $rbp to see if %rbp has been properly declared to be saved.
There is no need to cfi_startproc/cfi_endproc for every single entrypoint in
there, it is enough if the whole range is covered.  On the other side
we need the cfi_offset for the frame pointer case, otherwise up; p/x $rbp
doesn't work properly.

Ok for trunk if it passes bootstrap/regtest on x86_64-linux and i686-linux?

2018-02-22  Jakub Jelinek  <jakub@redhat.com>

	PR debug/83917
	* config/i386/i386-asm.h (PACKAGE_VERSION, PACKAGE_NAME,
	PACKAGE_STRING, PACKAGE_TARNAME, PACKAGE_URL): Undefine between
	inclusion of auto-target.h and auto-host.h.
	(USE_GAS_CFI_DIRECTIVES): Define if not defined already based on
	__GCC_HAVE_DWARF2_CFI_ASM.
	(cfi_startproc, cfi_endproc, cfi_adjust_cfa_offset,
	cfi_def_cfa_register, cfi_def_cfa, cfi_register, cfi_offset, cfi_push,
	cfi_pop): Define.
	* config/i386/cygwin.S: Don't include auto-host.h here, just
	define USE_GAS_CFI_DIRECTIVES to 1 or 0 and include i386-asm.h.
	(cfi_startproc, cfi_endproc, cfi_adjust_cfa_offset,
	cfi_def_cfa_register, cfi_register, cfi_push, cfi_pop): Remove.
	* config/i386/resms64fx.h: Add cfi_* directives.
	* config/i386/resms64x.h: Likewise.

--- libgcc/config/i386/i386-asm.h.jj	2018-01-03 10:42:56.317763517 +0100
+++ libgcc/config/i386/i386-asm.h	2018-02-22 15:33:43.812922298 +0100
@@ -27,8 +27,47 @@ see the files COPYING3 and COPYING.RUNTI
 #define I386_ASM_H
 
 #include "auto-target.h"
+#undef PACKAGE_VERSION
+#undef PACKAGE_NAME
+#undef PACKAGE_STRING
+#undef PACKAGE_TARNAME
+#undef PACKAGE_URL
 #include "auto-host.h"
 
+#ifndef USE_GAS_CFI_DIRECTIVES
+# ifdef __GCC_HAVE_DWARF2_CFI_ASM
+#  define USE_GAS_CFI_DIRECTIVES 1
+# else
+#  define USE_GAS_CFI_DIRECTIVES 0
+# endif
+#endif
+#if USE_GAS_CFI_DIRECTIVES
+# define cfi_startproc()		.cfi_startproc
+# define cfi_endproc()			.cfi_endproc
+# define cfi_adjust_cfa_offset(X) 	.cfi_adjust_cfa_offset X
+# define cfi_def_cfa_register(X)	.cfi_def_cfa_register X
+# define cfi_def_cfa(R,O)		.cfi_def_cfa R, O
+# define cfi_register(D,S)		.cfi_register D, S
+# define cfi_offset(R,O)		.cfi_offset R, O
+# ifdef __x86_64__
+#  define cfi_push(X)		.cfi_adjust_cfa_offset 8; .cfi_rel_offset X, 0
+#  define cfi_pop(X)		.cfi_adjust_cfa_offset -8; .cfi_restore X
+# else
+#  define cfi_push(X)		.cfi_adjust_cfa_offset 4; .cfi_rel_offset X, 0
+#  define cfi_pop(X)		.cfi_adjust_cfa_offset -4; .cfi_restore X
+# endif
+#else
+# define cfi_startproc()
+# define cfi_endproc()
+# define cfi_adjust_cfa_offset(X)
+# define cfi_def_cfa_register(X)
+# define cfi_def_cfa(R,O)
+# define cfi_register(D,S)
+# define cfi_offset(R,O)
+# define cfi_push(X)
+# define cfi_pop(X)
+#endif
+
 #define PASTE2(a, b) PASTE2a(a, b)
 #define PASTE2a(a, b) a ## b
 
--- libgcc/config/i386/cygwin.S.jj	2018-01-03 10:42:56.309763515 +0100
+++ libgcc/config/i386/cygwin.S	2018-02-22 15:30:34.597925496 +0100
@@ -23,31 +23,13 @@
  * <http://www.gnu.org/licenses/>.
  */
 
-#include "auto-host.h"
-
 #ifdef HAVE_GAS_CFI_SECTIONS_DIRECTIVE
+# define USE_GAS_CFI_DIRECTIVES 1
 	.cfi_sections	.debug_frame
-# define cfi_startproc()		.cfi_startproc
-# define cfi_endproc()			.cfi_endproc
-# define cfi_adjust_cfa_offset(X) 	.cfi_adjust_cfa_offset X
-# define cfi_def_cfa_register(X)	.cfi_def_cfa_register X
-# define cfi_register(D,S)		.cfi_register D, S
-# ifdef __x86_64__
-#  define cfi_push(X)		.cfi_adjust_cfa_offset 8; .cfi_rel_offset X, 0
-#  define cfi_pop(X)		.cfi_adjust_cfa_offset -8; .cfi_restore X
-# else
-#  define cfi_push(X)		.cfi_adjust_cfa_offset 4; .cfi_rel_offset X, 0
-#  define cfi_pop(X)		.cfi_adjust_cfa_offset -4; .cfi_restore X
-# endif
 #else
-# define cfi_startproc()
-# define cfi_endproc()
-# define cfi_adjust_cfa_offset(X)
-# define cfi_def_cfa_register(X)
-# define cfi_register(D,S)
-# define cfi_push(X)
-# define cfi_pop(X)
-#endif /* HAVE_GAS_CFI_SECTIONS_DIRECTIVE */
+# define USE_GAS_CFI_DIRECTIVES 0
+#endif
+#include "i386-asm.h"
 
 #ifdef L_chkstk
 /* Function prologue calls __chkstk to probe the stack when allocating more
--- libgcc/config/i386/resms64fx.h.jj	2018-01-03 10:42:56.246763504 +0100
+++ libgcc/config/i386/resms64fx.h	2018-02-22 15:14:53.391623798 +0100
@@ -33,6 +33,9 @@ see the files COPYING3 and COPYING.RUNTI
  * from the function.  */
 
 	.text
+	cfi_startproc()
+	cfi_offset(%rbp, -16)
+	cfi_def_cfa(%rbp, 16)
 MS2SYSV_STUB_BEGIN(resms64fx_17)
 	mov	-0x68(%rsi),%r15
 MS2SYSV_STUB_BEGIN(resms64fx_16)
@@ -48,7 +51,9 @@ MS2SYSV_STUB_BEGIN(resms64fx_12)
 	SSE_RESTORE
 	mov	-0x38(%rsi),%rsi
 	leaveq
+	cfi_def_cfa(%rsp, 8)
 	ret
+	cfi_endproc()
 MS2SYSV_STUB_END(resms64fx_12)
 MS2SYSV_STUB_END(resms64fx_13)
 MS2SYSV_STUB_END(resms64fx_14)
--- libgcc/config/i386/resms64x.h.jj	2018-01-03 10:42:56.308763515 +0100
+++ libgcc/config/i386/resms64x.h	2018-02-22 15:02:00.400106064 +0100
@@ -32,6 +32,8 @@ see the files COPYING3 and COPYING.RUNTI
  * function.  */
 
 	.text
+	cfi_startproc()
+	cfi_def_cfa(%r10, 8)
 MS2SYSV_STUB_BEGIN(resms64x_18)
 	mov	-0x70(%rsi),%r15
 MS2SYSV_STUB_BEGIN(resms64x_17)
@@ -49,7 +51,9 @@ MS2SYSV_STUB_BEGIN(resms64x_12)
 	SSE_RESTORE
 	mov	-0x38(%rsi),%rsi
 	mov	%r10,%rsp
+	cfi_def_cfa_register(%rsp)
 	ret
+	cfi_endproc()
 MS2SYSV_STUB_END(resms64x_12)
 MS2SYSV_STUB_END(resms64x_13)
 MS2SYSV_STUB_END(resms64x_14)


	Jakub

Re: [PATCH] Correct debug for -mcall-ms2sysv-xlogues stubs (PR target/83917, take 2)

[Jakub Jelinek]

On Thu, Feb 22, 2018 at 03:56:15PM +0100, Jakub Jelinek wrote:
> Ok for trunk if it passes bootstrap/regtest on x86_64-linux and i686-linux?

Now successfully bootstrapped/regtested on these targets.

> 2018-02-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR debug/83917
> 	* config/i386/i386-asm.h (PACKAGE_VERSION, PACKAGE_NAME,
> 	PACKAGE_STRING, PACKAGE_TARNAME, PACKAGE_URL): Undefine between
> 	inclusion of auto-target.h and auto-host.h.
> 	(USE_GAS_CFI_DIRECTIVES): Define if not defined already based on
> 	__GCC_HAVE_DWARF2_CFI_ASM.
> 	(cfi_startproc, cfi_endproc, cfi_adjust_cfa_offset,
> 	cfi_def_cfa_register, cfi_def_cfa, cfi_register, cfi_offset, cfi_push,
> 	cfi_pop): Define.
> 	* config/i386/cygwin.S: Don't include auto-host.h here, just
> 	define USE_GAS_CFI_DIRECTIVES to 1 or 0 and include i386-asm.h.
> 	(cfi_startproc, cfi_endproc, cfi_adjust_cfa_offset,
> 	cfi_def_cfa_register, cfi_register, cfi_push, cfi_pop): Remove.
> 	* config/i386/resms64fx.h: Add cfi_* directives.
> 	* config/i386/resms64x.h: Likewise.

	Jakub

Re: [PATCH] Correct debug for -mcall-ms2sysv-xlogues stubs (PR target/83917, take 2)

[Daniel Santos]

Sorry for the dropping the ball on this and thank you Jakub for stepping in!

I've had a patch set sort-of rotting in my local repo, but I like yours
better.  I think I had gotten hung up on trying to figure out how to
write a test for this, and like you I just tested mine manually in gdb. 
I do have one correction though.


On 02/22/2018 08:56 AM, Jakub Jelinek wrote:
> Hi!
>
> On Sat, Jan 20, 2018 at 06:01:16PM -0600, Daniel Santos wrote:
>> Thanks.  I like the idea of commonizing the macros for consistency.
> Didn't see a progress on this P3 for a while, so I've written this
> version of the patch; no tests though, what I've been using in testing was:
> /* { dg-do compile { target lp64 } } */
> /* { dg-options "-mno-avx -msse2 -mcall-ms2sysv-xlogues -O2" } */
>
> void __attribute__((sysv_abi, noipa))
> foo (void)
> {
> }
>
> static void __attribute__((sysv_abi)) (*volatile foop) () = foo;
>
> void __attribute__((ms_abi, noipa))
> bar (void)
> {
>   foop ();
> }
>
> int
> main ()
> {
>   bar ();
>   return 0;
> }
>
> with/without -fno-omit-frame-pointer, disas bar; b on the tail
> call in there, stepi; bt (which before the patch failed, now works),
> also up; p $rbp to see if %rbp has been properly declared to be saved.
> There is no need to cfi_startproc/cfi_endproc for every single entrypoint in
> there, it is enough if the whole range is covered.  On the other side
> we need the cfi_offset for the frame pointer case, otherwise up; p/x $rbp
> doesn't work properly.
>
> Ok for trunk if it passes bootstrap/regtest on x86_64-linux and i686-linux?
>
> 2018-02-22  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR debug/83917
> 	* config/i386/i386-asm.h (PACKAGE_VERSION, PACKAGE_NAME,
> 	PACKAGE_STRING, PACKAGE_TARNAME, PACKAGE_URL): Undefine between
> 	inclusion of auto-target.h and auto-host.h.
> 	(USE_GAS_CFI_DIRECTIVES): Define if not defined already based on
> 	__GCC_HAVE_DWARF2_CFI_ASM.
> 	(cfi_startproc, cfi_endproc, cfi_adjust_cfa_offset,
> 	cfi_def_cfa_register, cfi_def_cfa, cfi_register, cfi_offset, cfi_push,
> 	cfi_pop): Define.
> 	* config/i386/cygwin.S: Don't include auto-host.h here, just
> 	define USE_GAS_CFI_DIRECTIVES to 1 or 0 and include i386-asm.h.
> 	(cfi_startproc, cfi_endproc, cfi_adjust_cfa_offset,
> 	cfi_def_cfa_register, cfi_register, cfi_push, cfi_pop): Remove.
> 	* config/i386/resms64fx.h: Add cfi_* directives.
> 	* config/i386/resms64x.h: Likewise.
>
> --- libgcc/config/i386/i386-asm.h.jj	2018-01-03 10:42:56.317763517 +0100
> +++ libgcc/config/i386/i386-asm.h	2018-02-22 15:33:43.812922298 +0100
> @@ -27,8 +27,47 @@ see the files COPYING3 and COPYING.RUNTI
>  #define I386_ASM_H
>  
>  #include "auto-target.h"
> +#undef PACKAGE_VERSION
> +#undef PACKAGE_NAME
> +#undef PACKAGE_STRING
> +#undef PACKAGE_TARNAME
> +#undef PACKAGE_URL

This is a beautiful, temporary(?) fix to an ugly problem!

>  #include "auto-host.h"
>  
> +#ifndef USE_GAS_CFI_DIRECTIVES
> +# ifdef __GCC_HAVE_DWARF2_CFI_ASM
> +#  define USE_GAS_CFI_DIRECTIVES 1
> +# else
> +#  define USE_GAS_CFI_DIRECTIVES 0
> +# endif
> +#endif
> +#if USE_GAS_CFI_DIRECTIVES
> +# define cfi_startproc()		.cfi_startproc
> +# define cfi_endproc()			.cfi_endproc
> +# define cfi_adjust_cfa_offset(X) 	.cfi_adjust_cfa_offset X
> +# define cfi_def_cfa_register(X)	.cfi_def_cfa_register X
> +# define cfi_def_cfa(R,O)		.cfi_def_cfa R, O
> +# define cfi_register(D,S)		.cfi_register D, S
> +# define cfi_offset(R,O)		.cfi_offset R, O
> +# ifdef __x86_64__
> +#  define cfi_push(X)		.cfi_adjust_cfa_offset 8; .cfi_rel_offset X, 0
> +#  define cfi_pop(X)		.cfi_adjust_cfa_offset -8; .cfi_restore X
> +# else
> +#  define cfi_push(X)		.cfi_adjust_cfa_offset 4; .cfi_rel_offset X, 0
> +#  define cfi_pop(X)		.cfi_adjust_cfa_offset -4; .cfi_restore X
> +# endif
> +#else
> +# define cfi_startproc()
> +# define cfi_endproc()
> +# define cfi_adjust_cfa_offset(X)
> +# define cfi_def_cfa_register(X)
> +# define cfi_def_cfa(R,O)
> +# define cfi_register(D,S)
> +# define cfi_offset(R,O)
> +# define cfi_push(X)
> +# define cfi_pop(X)
> +#endif
> +
>  #define PASTE2(a, b) PASTE2a(a, b)
>  #define PASTE2a(a, b) a ## b
>  
> --- libgcc/config/i386/cygwin.S.jj	2018-01-03 10:42:56.309763515 +0100
> +++ libgcc/config/i386/cygwin.S	2018-02-22 15:30:34.597925496 +0100
> @@ -23,31 +23,13 @@
>   * <http://www.gnu.org/licenses/>.
>   */
>  
> -#include "auto-host.h"

The following include should be here.

+#include "i386-asm.h"


> -
>  #ifdef HAVE_GAS_CFI_SECTIONS_DIRECTIVE
> +# define USE_GAS_CFI_DIRECTIVES 1
>  	.cfi_sections	.debug_frame
> -# define cfi_startproc()		.cfi_startproc
> -# define cfi_endproc()			.cfi_endproc
> -# define cfi_adjust_cfa_offset(X) 	.cfi_adjust_cfa_offset X
> -# define cfi_def_cfa_register(X)	.cfi_def_cfa_register X
> -# define cfi_register(D,S)		.cfi_register D, S
> -# ifdef __x86_64__
> -#  define cfi_push(X)		.cfi_adjust_cfa_offset 8; .cfi_rel_offset X, 0
> -#  define cfi_pop(X)		.cfi_adjust_cfa_offset -8; .cfi_restore X
> -# else
> -#  define cfi_push(X)		.cfi_adjust_cfa_offset 4; .cfi_rel_offset X, 0
> -#  define cfi_pop(X)		.cfi_adjust_cfa_offset -4; .cfi_restore X
> -# endif
>  #else
> -# define cfi_startproc()
> -# define cfi_endproc()
> -# define cfi_adjust_cfa_offset(X)
> -# define cfi_def_cfa_register(X)
> -# define cfi_register(D,S)
> -# define cfi_push(X)
> -# define cfi_pop(X)
> -#endif /* HAVE_GAS_CFI_SECTIONS_DIRECTIVE */
> +# define USE_GAS_CFI_DIRECTIVES 0
> +#endif
> +#include "i386-asm.h"

...instead of where it is above. ^^

>  
>  #ifdef L_chkstk
>  /* Function prologue calls __chkstk to probe the stack when allocating more
> --- libgcc/config/i386/resms64fx.h.jj	2018-01-03 10:42:56.246763504 +0100
> +++ libgcc/config/i386/resms64fx.h	2018-02-22 15:14:53.391623798 +0100
> @@ -33,6 +33,9 @@ see the files COPYING3 and COPYING.RUNTI
>   * from the function.  */
>  
>  	.text
> +	cfi_startproc()
> +	cfi_offset(%rbp, -16)
> +	cfi_def_cfa(%rbp, 16)
>  MS2SYSV_STUB_BEGIN(resms64fx_17)
>  	mov	-0x68(%rsi),%r15
>  MS2SYSV_STUB_BEGIN(resms64fx_16)
> @@ -48,7 +51,9 @@ MS2SYSV_STUB_BEGIN(resms64fx_12)
>  	SSE_RESTORE
>  	mov	-0x38(%rsi),%rsi
>  	leaveq
> +	cfi_def_cfa(%rsp, 8)
>  	ret
> +	cfi_endproc()
>  MS2SYSV_STUB_END(resms64fx_12)
>  MS2SYSV_STUB_END(resms64fx_13)
>  MS2SYSV_STUB_END(resms64fx_14)
> --- libgcc/config/i386/resms64x.h.jj	2018-01-03 10:42:56.308763515 +0100
> +++ libgcc/config/i386/resms64x.h	2018-02-22 15:02:00.400106064 +0100
> @@ -32,6 +32,8 @@ see the files COPYING3 and COPYING.RUNTI
>   * function.  */
>  
>  	.text
> +	cfi_startproc()
> +	cfi_def_cfa(%r10, 8)
>  MS2SYSV_STUB_BEGIN(resms64x_18)
>  	mov	-0x70(%rsi),%r15
>  MS2SYSV_STUB_BEGIN(resms64x_17)
> @@ -49,7 +51,9 @@ MS2SYSV_STUB_BEGIN(resms64x_12)
>  	SSE_RESTORE
>  	mov	-0x38(%rsi),%rsi
>  	mov	%r10,%rsp
> +	cfi_def_cfa_register(%rsp)
>  	ret
> +	cfi_endproc()
>  MS2SYSV_STUB_END(resms64x_12)
>  MS2SYSV_STUB_END(resms64x_13)
>  MS2SYSV_STUB_END(resms64x_14)
>
>
> 	Jakub
>

Thanks Jakub!
Daniel

Re: [PATCH] Correct debug for -mcall-ms2sysv-xlogues stubs (PR target/83917, take 2)

[Jakub Jelinek]

On Sun, Feb 25, 2018 at 05:56:28PM -0600, Daniel Santos wrote:
> > --- libgcc/config/i386/i386-asm.h.jj	2018-01-03 10:42:56.317763517 +0100
> > +++ libgcc/config/i386/i386-asm.h	2018-02-22 15:33:43.812922298 +0100
> > @@ -27,8 +27,47 @@ see the files COPYING3 and COPYING.RUNTI
> >  #define I386_ASM_H
> >  
> >  #include "auto-target.h"
> > +#undef PACKAGE_VERSION
> > +#undef PACKAGE_NAME
> > +#undef PACKAGE_STRING
> > +#undef PACKAGE_TARNAME
> > +#undef PACKAGE_URL
> 
> This is a beautiful, temporary(?) fix to an ugly problem!
> 
> >  #include "auto-host.h"

> > --- libgcc/config/i386/cygwin.S.jj	2018-01-03 10:42:56.309763515 +0100
> > +++ libgcc/config/i386/cygwin.S	2018-02-22 15:30:34.597925496 +0100
> > @@ -23,31 +23,13 @@
> >   * <http://www.gnu.org/licenses/>.
> >   */
> >  
> > -#include "auto-host.h"
> 
> The following include should be here.
> 
> +#include "i386-asm.h"

I don't understand this.  i386-asm.h needs (both before my patch and after
it) both auto-host.h and auto-target.h, as it tests
HAVE_GAS_SECTIONS_DIRECTIVE (this one newly, comes from cygwin.S)
HAVE_GAS_HIDDEN
macros defined in auto-host.h
and
HAVE_AS_AVX
macro defined in auto-target.h.
Including auto-host.h when i386-asm.h will include it again just doesn't
work, these headers don't have multiple inclusion guards.  And only including
auto-target.h will work only if the
.hidden
and
.cfi_sections .debug_frame
tests are duplicated from gcc/configure.ac to libgcc/configure.ac, then we
could include just auto-target.h in i386-asm.h.
I've just followed what i386-asm.h has been doing.

	Jakub

Re: [PATCH] Correct debug for -mcall-ms2sysv-xlogues stubs (PR target/83917, take 2)

[Jeff Law]

On 02/22/2018 07:56 AM, Jakub Jelinek wrote:
> Hi!
> 
> On Sat, Jan 20, 2018 at 06:01:16PM -0600, Daniel Santos wrote:
>> Thanks.  I like the idea of commonizing the macros for consistency.
> 
> Didn't see a progress on this P3 for a while, so I've written this
> version of the patch; no tests though, what I've been using in testing was:
> /* { dg-do compile { target lp64 } } */
> /* { dg-options "-mno-avx -msse2 -mcall-ms2sysv-xlogues -O2" } */
> 
> void __attribute__((sysv_abi, noipa))
> foo (void)
> {
> }
> 
> static void __attribute__((sysv_abi)) (*volatile foop) () = foo;
> 
> void __attribute__((ms_abi, noipa))
> bar (void)
> {
>   foop ();
> }
> 
> int
> main ()
> {
>   bar ();
>   return 0;
> }
> 
> with/without -fno-omit-frame-pointer, disas bar; b on the tail
> call in there, stepi; bt (which before the patch failed, now works),
> also up; p $rbp to see if %rbp has been properly declared to be saved.
> There is no need to cfi_startproc/cfi_endproc for every single entrypoint in
> there, it is enough if the whole range is covered.  On the other side
> we need the cfi_offset for the frame pointer case, otherwise up; p/x $rbp
> doesn't work properly.
> 
> Ok for trunk if it passes bootstrap/regtest on x86_64-linux and i686-linux?
> 
> 2018-02-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR debug/83917
> 	* config/i386/i386-asm.h (PACKAGE_VERSION, PACKAGE_NAME,
> 	PACKAGE_STRING, PACKAGE_TARNAME, PACKAGE_URL): Undefine between
> 	inclusion of auto-target.h and auto-host.h.
> 	(USE_GAS_CFI_DIRECTIVES): Define if not defined already based on
> 	__GCC_HAVE_DWARF2_CFI_ASM.
> 	(cfi_startproc, cfi_endproc, cfi_adjust_cfa_offset,
> 	cfi_def_cfa_register, cfi_def_cfa, cfi_register, cfi_offset, cfi_push,
> 	cfi_pop): Define.
> 	* config/i386/cygwin.S: Don't include auto-host.h here, just
> 	define USE_GAS_CFI_DIRECTIVES to 1 or 0 and include i386-asm.h.
> 	(cfi_startproc, cfi_endproc, cfi_adjust_cfa_offset,
> 	cfi_def_cfa_register, cfi_register, cfi_push, cfi_pop): Remove.
> 	* config/i386/resms64fx.h: Add cfi_* directives.
> 	* config/i386/resms64x.h: Likewise.
It's a bit ugly.  But OK.  We can refine further if needed.

jeff

Re: [PATCH] Correct debug for -mcall-ms2sysv-xlogues stubs (PR target/83917, take 2)

[Daniel Santos]

On 02/26/2018 02:20 AM, Jakub Jelinek wrote:
> On Sun, Feb 25, 2018 at 05:56:28PM -0600, Daniel Santos wrote:
>>> --- libgcc/config/i386/i386-asm.h.jj	2018-01-03 10:42:56.317763517 +0100
>>> +++ libgcc/config/i386/i386-asm.h	2018-02-22 15:33:43.812922298 +0100
>>> @@ -27,8 +27,47 @@ see the files COPYING3 and COPYING.RUNTI
>>>  #define I386_ASM_H
>>>  
>>>  #include "auto-target.h"
>>> +#undef PACKAGE_VERSION
>>> +#undef PACKAGE_NAME
>>> +#undef PACKAGE_STRING
>>> +#undef PACKAGE_TARNAME
>>> +#undef PACKAGE_URL
>> This is a beautiful, temporary(?) fix to an ugly problem!
>>
>>>  #include "auto-host.h"
>>> --- libgcc/config/i386/cygwin.S.jj	2018-01-03 10:42:56.309763515 +0100
>>> +++ libgcc/config/i386/cygwin.S	2018-02-22 15:30:34.597925496 +0100
>>> @@ -23,31 +23,13 @@
>>>   * <http://www.gnu.org/licenses/>.
>>>   */
>>>  
>>> -#include "auto-host.h"
>> The following include should be here.
>>
>> +#include "i386-asm.h"
> I don't understand this.  i386-asm.h needs (both before my patch and after
> it) both auto-host.h and auto-target.h, as it tests
> HAVE_GAS_SECTIONS_DIRECTIVE (this one newly, comes from cygwin.S)

The problem is that HAVE_GAS_SECTIONS_DIRECTIVE gets defined (or not) in
../../gcc/auto-host.h, but you are testing it before including
auto-host.h, either directly or via i386-asm.h.  So if i386-asm.h
depends upon HAVE_GAS_SECTIONS_DIRECTIVE first being defined then it is
a circular dependency.

In its current form, cygwin.S would never define USE_GAS_CFI_DIRECTIVES
prior to including i386-asm.h and also never emit
        .cfi_sections    .debug_frame
and rather or not USE_GAS_CFI_DIRECTIVES ends up being defined to 1 or 0
depends upon the test of __GCC_HAVE_DWARF2_CFI_ASM in i386-asm.h.

So this area is new for me, but I don't understand why we're testing
HAVE_GAS_SECTIONS_DIRECTIVE in cygwin.S and __GCC_HAVE_DWARF2_CFI_ASM
when included from one of the stubs.  Is this an error, or a lack of my
understanding or both? :)

> HAVE_GAS_HIDDEN
> macros defined in auto-host.h
> and
> HAVE_AS_AVX
> macro defined in auto-target.h.
> Including auto-host.h when i386-asm.h will include it again just doesn't
> work, these headers don't have multiple inclusion guards.  And only including
> auto-target.h will work only if the
> .hidden
> and
> .cfi_sections .debug_frame
> tests are duplicated from gcc/configure.ac to libgcc/configure.ac, then we
> could include just auto-target.h in i386-asm.h.
> I've just followed what i386-asm.h has been doing.

And it's possible that I failed to test something correctly before
presuming it to be available, although I *think* the test for .hidden is
good.

>
> 	Jakub
>

Thanks for your work on this.  If we need to test for CFI directives
differently when being included from cygwin.S, maybe we can just define
a simple cpp macro to indicate this and let i386-asm.h encapsulate the
implementation of it (e.g., testing HAVE_GAS_SECTIONS_DIRECTIVE or
__GCC_HAVE_DWARF2_CFI_ASM as appropriate).

Ultimately, the proper cleanup will be moving these tests out of
{gcc,libgcc}/configure.ac and into .m4 files in the root config
directory so that we don't uglify them with massive copy & pastes. 
These tests are also fairly complex as there are a lot of dependencies. 
m4 isn't my strong suite, but I can look at this after we're out of code
freeze.

Daniel

[PATCH] Fix debug for -mcall-ms2sysv-xlogues stubs fallout (PR target/83917)

[Jakub Jelinek]

On Mon, Feb 26, 2018 at 08:05:56PM -0600, Daniel Santos wrote:
> >>> --- libgcc/config/i386/cygwin.S.jj	2018-01-03 10:42:56.309763515 +0100
> >>> +++ libgcc/config/i386/cygwin.S	2018-02-22 15:30:34.597925496 +0100
> >>> @@ -23,31 +23,13 @@
> >>>   * <http://www.gnu.org/licenses/>.
> >>>   */
> >>>  
> >>> -#include "auto-host.h"
> >> The following include should be here.
> >>
> >> +#include "i386-asm.h"
> > I don't understand this.  i386-asm.h needs (both before my patch and after
> > it) both auto-host.h and auto-target.h, as it tests
> > HAVE_GAS_SECTIONS_DIRECTIVE (this one newly, comes from cygwin.S)
> 
> The problem is that HAVE_GAS_SECTIONS_DIRECTIVE gets defined (or not) in
> ../../gcc/auto-host.h, but you are testing it before including
> auto-host.h, either directly or via i386-asm.h.  So if i386-asm.h
> depends upon HAVE_GAS_SECTIONS_DIRECTIVE first being defined then it is
> a circular dependency.
> 
> In its current form, cygwin.S would never define USE_GAS_CFI_DIRECTIVES
> prior to including i386-asm.h and also never emit
>         .cfi_sections    .debug_frame
> and rather or not USE_GAS_CFI_DIRECTIVES ends up being defined to 1 or 0
> depends upon the test of __GCC_HAVE_DWARF2_CFI_ASM in i386-asm.h.

Ugh, you're right.  I was trying to preserve existing behavior for cygwin.S,
but failed to do so.  Unfortunately the patch which added this stuff from
Kai T. and Richard H. from 2010 is not in gcc-patches archives; in any case,
I think nothing seriously bad happens if with older gas versions which do
support .cfi_* directives but not .cfi_sections .debug_frame we emit the CFI
into .eh_frame section rather than .debug_frame.

So this patch simplifies it, with only one guard for the non-trivial
vs. trivial cfi_* definitions (based on whether GCC itself would use it)
and only guard the .cfi_sections directive on whether it is really
available.

The __GCC_HAVE_DWARF2_CFI_ASM definition actually sometimes depends on the
.cfi_sections presence too:
  /* If we can't get the assembler to emit only .debug_frame, and we don't need
     dwarf2 unwind info for exceptions, then emit .debug_frame by hand.  */
  if (!HAVE_GAS_CFI_SECTIONS_DIRECTIVE && !dwarf2out_do_eh_frame ())
    return false;
but doesn't actually guarantee it always, as when doing .eh_frame it will
not require .cfi_sections.

This spot brings in another, preexisting bug in cygwin.S though - 
the HAVE_GAS_CFI_SECTIONS_DIRECTIVE macro is always defined, to 0 or 1,
rather than sometimes #define and sometines #undef.

> Ultimately, the proper cleanup will be moving these tests out of
> {gcc,libgcc}/configure.ac and into .m4 files in the root config
> directory so that we don't uglify them with massive copy & pastes. 
> These tests are also fairly complex as there are a lot of dependencies. 
> m4 isn't my strong suite, but I can look at this after we're out of code
> freeze.

Not really sure about that, because we really want to do a different thing
in gcc/configure.ac (need to test the assembler directly, use
GCC_TARGET_TEMPLATE) while in libgcc it does usually something different.

The libgcc configure already has all the code for the .hidden directive,
as it uses it too, just it is only a pair of AC_SUBSTs rather than
AC_DEFINE_UNQUOTED.
The test for HAVE_GAS_CFI_SECTIONS_DIRECTIVE alternative can be compile
int foo (int, char *);
int bar (int x) { char *y = __builtin_alloca (x); return foo (x + 1, y) + 1; }
with -g -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-exceptions
and scan for .cfi_sections .debug_frame.

So here is a new (I've committed the previous patch since then), only lightly
tested (only on x86_64-linux and don't have too old binutils around), patch:

2018-02-27  Jakub Jelinek  <jakub@redhat.com>

	PR debug/83917
	* configure.ac (AS_HIDDEN_DIRECTIVE): AC_DEFINE_UNQUOTED this to
	$asm_hidden_op if visibility ("hidden") attribute works.
	(HAVE_AS_CFI_SECTIONS): New AC_DEFINE.
	* config/i386/i386-asm.h: Don't include auto-host.h.
	(PACKAGE_VERSION, PACKAGE_NAME, PACKAGE_STRING, PACKAGE_TARNAME,
	PACKAGE_URL): Don't undefine.
	(USE_GAS_CFI_DIRECTIVES): Don't use nor define this macro, instead
	guard cfi_startproc only on ifdef __GCC_HAVE_DWARF2_CFI_ASM.
	(FN_HIDDEN): Change guard from #ifdef HAVE_GAS_HIDDEN to
	#ifdef AS_HIDDEN_DIRECTIVE, use AS_HIDDEN_DIRECTIVE macro in the
	definition instead of hardcoded .hidden.
	* config/i386/cygwin.S: Include i386-asm.h first before .cfi_sections
	directive.  Use #ifdef HAVE_AS_CFI_SECTIONS rather than
	#ifdef HAVE_GAS_CFI_SECTIONS_DIRECTIVE to guard .cfi_sections.
	(USE_GAS_CFI_DIRECTIVES): Don't define.
	* configure: Regenerated.
	* config.in: Likewise.

--- libgcc/configure.ac.jj	2017-11-20 11:02:56.877786488 +0100
+++ libgcc/configure.ac	2018-02-27 09:20:45.939385138 +0100
@@ -486,11 +486,29 @@ AC_CACHE_CHECK([for __attribute__((visib
 
 if test $libgcc_cv_hidden_visibility_attribute = yes; then
     vis_hide='-fvisibility=hidden -DHIDE_EXPORTS'
+    AC_DEFINE_UNQUOTED(AS_HIDDEN_DIRECTIVE, $asm_hidden_op, [Define to the .hidden-like directive if it exists.])
 else
     vis_hide=
 fi
 AC_SUBST(vis_hide)
 
+# Check for .cfi_sections .debug_frame support.
+AC_CACHE_CHECK([for .cfi_sections .debug_frame],
+    libgcc_cv_cfi_sections_directive, [
+	echo 'int foo (int, char *);' > conftest.c
+	echo 'int bar (int x) { char *y = __builtin_alloca (x); return foo (x + 1, y) + 1; }' >> conftest.c
+	libgcc_cv_cfi_sections_directive=no
+	if AC_TRY_COMMAND(${CC-cc} -Werror -g -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-exceptions -S conftest.c -o conftest.s 1>&AS_MESSAGE_LOG_FD); then
+	    if grep "\\.cfi_sections.*\\.debug_frame" conftest.s >/dev/null; then
+		libgcc_cv_cfi_sections_directive=yes
+	    fi
+	fi
+	rm -f conftest.*
+    ])
+if test $libgcc_cv_cfi_sections_directive = yes; then
+    AC_DEFINE(HAVE_AS_CFI_SECTIONS, 1, [Define to 1 if the assembler supports .cfi_sections .debug_frame directive.])
+fi
+
 # See if we have thread-local storage.  We can only test assembler
 # since link-time and run-time tests require the newly built
 # gcc, which can't be used to build executable due to that libgcc
--- libgcc/config/i386/i386-asm.h.jj	2018-02-26 20:46:08.493354695 +0100
+++ libgcc/config/i386/i386-asm.h	2018-02-27 09:15:23.533496520 +0100
@@ -27,21 +27,8 @@ see the files COPYING3 and COPYING.RUNTI
 #define I386_ASM_H
 
 #include "auto-target.h"
-#undef PACKAGE_VERSION
-#undef PACKAGE_NAME
-#undef PACKAGE_STRING
-#undef PACKAGE_TARNAME
-#undef PACKAGE_URL
-#include "auto-host.h"
 
-#ifndef USE_GAS_CFI_DIRECTIVES
-# ifdef __GCC_HAVE_DWARF2_CFI_ASM
-#  define USE_GAS_CFI_DIRECTIVES 1
-# else
-#  define USE_GAS_CFI_DIRECTIVES 0
-# endif
-#endif
-#if USE_GAS_CFI_DIRECTIVES
+#ifdef __GCC_HAVE_DWARF2_CFI_ASM
 # define cfi_startproc()		.cfi_startproc
 # define cfi_endproc()			.cfi_endproc
 # define cfi_adjust_cfa_offset(X) 	.cfi_adjust_cfa_offset X
@@ -76,8 +63,8 @@ see the files COPYING3 and COPYING.RUNTI
 #ifdef __ELF__
 # define FN_TYPE(fn) .type fn,@function
 # define FN_SIZE(fn) .size fn,.-fn
-# ifdef HAVE_GAS_HIDDEN
-#  define FN_HIDDEN(fn) .hidden fn
+# ifdef AS_HIDDEN_DIRECTIVE
+#  define FN_HIDDEN(fn) AS_HIDDEN_DIRECTIVE fn
 # endif
 #else
 # define FN_TYPE(fn)
--- libgcc/config/i386/cygwin.S.jj	2018-02-26 20:46:08.494354695 +0100
+++ libgcc/config/i386/cygwin.S	2018-02-27 09:16:32.077472841 +0100
@@ -23,13 +23,11 @@
  * <http://www.gnu.org/licenses/>.
  */
 
-#ifdef HAVE_GAS_CFI_SECTIONS_DIRECTIVE
-# define USE_GAS_CFI_DIRECTIVES 1
+#include "i386-asm.h"
+
+#ifdef HAVE_AS_CFI_SECTIONS
 	.cfi_sections	.debug_frame
-#else
-# define USE_GAS_CFI_DIRECTIVES 0
 #endif
-#include "i386-asm.h"
 
 #ifdef L_chkstk
 /* Function prologue calls __chkstk to probe the stack when allocating more
--- libgcc/configure.jj	2018-02-19 19:57:05.758280231 +0100
+++ libgcc/configure	2018-02-27 09:20:57.346381203 +0100
@@ -5214,11 +5214,47 @@ $as_echo "$libgcc_cv_hidden_visibility_a
 
 if test $libgcc_cv_hidden_visibility_attribute = yes; then
     vis_hide='-fvisibility=hidden -DHIDE_EXPORTS'
+
+cat >>confdefs.h <<_ACEOF
+#define AS_HIDDEN_DIRECTIVE $asm_hidden_op
+_ACEOF
+
 else
     vis_hide=
 fi
 
 
+# Check for .cfi_sections .debug_frame support.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for .cfi_sections .debug_frame" >&5
+$as_echo_n "checking for .cfi_sections .debug_frame... " >&6; }
+if test "${libgcc_cv_cfi_sections_directive+set}" = set; then :
+  $as_echo_n "(cached) " >&6
+else
+
+	echo 'int foo (int, char *);' > conftest.c
+	echo 'int bar (int x) { char *y = __builtin_alloca (x); return foo (x + 1, y) + 1; }' >> conftest.c
+	libgcc_cv_cfi_sections_directive=no
+	if { ac_try='${CC-cc} -Werror -g -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-exceptions -S conftest.c -o conftest.s 1>&5'
+  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; }; then
+	    if grep "\\.cfi_sections.*\\.debug_frame" conftest.s >/dev/null; then
+		libgcc_cv_cfi_sections_directive=yes
+	    fi
+	fi
+	rm -f conftest.*
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libgcc_cv_cfi_sections_directive" >&5
+$as_echo "$libgcc_cv_cfi_sections_directive" >&6; }
+if test $libgcc_cv_cfi_sections_directive = yes; then
+
+$as_echo "#define HAVE_AS_CFI_SECTIONS 1" >>confdefs.h
+
+fi
+
 # See if we have thread-local storage.  We can only test assembler
 # since link-time and run-time tests require the newly built
 # gcc, which can't be used to build executable due to that libgcc
--- libgcc/config.in.jj	2017-09-25 10:38:45.182468927 +0200
+++ libgcc/config.in	2018-02-27 09:12:08.000000000 +0100
@@ -1,8 +1,15 @@
 /* config.in.  Generated from configure.ac by autoheader.  */
 
+/* Define to the .hidden-like directive if it exists. */
+#undef AS_HIDDEN_DIRECTIVE
+
 /* Define to 1 if the assembler supports AVX. */
 #undef HAVE_AS_AVX
 
+/* Define to 1 if the assembler supports .cfi_sections .debug_frame directive.
+   */
+#undef HAVE_AS_CFI_SECTIONS
+
 /* Define to 1 if the target assembler supports thread-local storage. */
 #undef HAVE_CC_TLS
 


	Jakub

Re: [PATCH] Fix debug for -mcall-ms2sysv-xlogues stubs fallout (PR target/83917)

[Jakub Jelinek]

On Tue, Feb 27, 2018 at 09:29:36AM +0100, Jakub Jelinek wrote:
> So here is a new (I've committed the previous patch since then), only lightly
> tested (only on x86_64-linux and don't have too old binutils around), patch:
> 
> 2018-02-27  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR debug/83917
> 	* configure.ac (AS_HIDDEN_DIRECTIVE): AC_DEFINE_UNQUOTED this to
> 	$asm_hidden_op if visibility ("hidden") attribute works.
> 	(HAVE_AS_CFI_SECTIONS): New AC_DEFINE.
> 	* config/i386/i386-asm.h: Don't include auto-host.h.
> 	(PACKAGE_VERSION, PACKAGE_NAME, PACKAGE_STRING, PACKAGE_TARNAME,
> 	PACKAGE_URL): Don't undefine.
> 	(USE_GAS_CFI_DIRECTIVES): Don't use nor define this macro, instead
> 	guard cfi_startproc only on ifdef __GCC_HAVE_DWARF2_CFI_ASM.
> 	(FN_HIDDEN): Change guard from #ifdef HAVE_GAS_HIDDEN to
> 	#ifdef AS_HIDDEN_DIRECTIVE, use AS_HIDDEN_DIRECTIVE macro in the
> 	definition instead of hardcoded .hidden.
> 	* config/i386/cygwin.S: Include i386-asm.h first before .cfi_sections
> 	directive.  Use #ifdef HAVE_AS_CFI_SECTIONS rather than
> 	#ifdef HAVE_GAS_CFI_SECTIONS_DIRECTIVE to guard .cfi_sections.
> 	(USE_GAS_CFI_DIRECTIVES): Don't define.
> 	* configure: Regenerated.
> 	* config.in: Likewise.

Now successfully bootstrapped/regtested on x86_64-linux and i686-linux.

	Jakub

Re: [PATCH] Fix debug for -mcall-ms2sysv-xlogues stubs fallout (PR target/83917)

[Jeff Law]

On 02/27/2018 01:29 AM, Jakub Jelinek wrote:
> On Mon, Feb 26, 2018 at 08:05:56PM -0600, Daniel Santos wrote:
>>>>> --- libgcc/config/i386/cygwin.S.jj	2018-01-03 10:42:56.309763515 +0100
>>>>> +++ libgcc/config/i386/cygwin.S	2018-02-22 15:30:34.597925496 +0100
>>>>> @@ -23,31 +23,13 @@
>>>>>   * <http://www.gnu.org/licenses/>.
>>>>>   */
>>>>>  
>>>>> -#include "auto-host.h"
>>>> The following include should be here.
>>>>
>>>> +#include "i386-asm.h"
>>> I don't understand this.  i386-asm.h needs (both before my patch and after
>>> it) both auto-host.h and auto-target.h, as it tests
>>> HAVE_GAS_SECTIONS_DIRECTIVE (this one newly, comes from cygwin.S)
>>
>> The problem is that HAVE_GAS_SECTIONS_DIRECTIVE gets defined (or not) in
>> ../../gcc/auto-host.h, but you are testing it before including
>> auto-host.h, either directly or via i386-asm.h.  So if i386-asm.h
>> depends upon HAVE_GAS_SECTIONS_DIRECTIVE first being defined then it is
>> a circular dependency.
>>
>> In its current form, cygwin.S would never define USE_GAS_CFI_DIRECTIVES
>> prior to including i386-asm.h and also never emit
>>         .cfi_sections    .debug_frame
>> and rather or not USE_GAS_CFI_DIRECTIVES ends up being defined to 1 or 0
>> depends upon the test of __GCC_HAVE_DWARF2_CFI_ASM in i386-asm.h.
> 
> Ugh, you're right.  I was trying to preserve existing behavior for cygwin.S,
> but failed to do so.  Unfortunately the patch which added this stuff from
> Kai T. and Richard H. from 2010 is not in gcc-patches archives; in any case,
> I think nothing seriously bad happens if with older gas versions which do
> support .cfi_* directives but not .cfi_sections .debug_frame we emit the CFI
> into .eh_frame section rather than .debug_frame.
> 
> So this patch simplifies it, with only one guard for the non-trivial
> vs. trivial cfi_* definitions (based on whether GCC itself would use it)
> and only guard the .cfi_sections directive on whether it is really
> available.
> 
> The __GCC_HAVE_DWARF2_CFI_ASM definition actually sometimes depends on the
> .cfi_sections presence too:
>   /* If we can't get the assembler to emit only .debug_frame, and we don't need
>      dwarf2 unwind info for exceptions, then emit .debug_frame by hand.  */
>   if (!HAVE_GAS_CFI_SECTIONS_DIRECTIVE && !dwarf2out_do_eh_frame ())
>     return false;
> but doesn't actually guarantee it always, as when doing .eh_frame it will
> not require .cfi_sections.
> 
> This spot brings in another, preexisting bug in cygwin.S though - 
> the HAVE_GAS_CFI_SECTIONS_DIRECTIVE macro is always defined, to 0 or 1,
> rather than sometimes #define and sometines #undef.
> 
>> Ultimately, the proper cleanup will be moving these tests out of
>> {gcc,libgcc}/configure.ac and into .m4 files in the root config
>> directory so that we don't uglify them with massive copy & pastes. 
>> These tests are also fairly complex as there are a lot of dependencies. 
>> m4 isn't my strong suite, but I can look at this after we're out of code
>> freeze.
> 
> Not really sure about that, because we really want to do a different thing
> in gcc/configure.ac (need to test the assembler directly, use
> GCC_TARGET_TEMPLATE) while in libgcc it does usually something different.
> 
> The libgcc configure already has all the code for the .hidden directive,
> as it uses it too, just it is only a pair of AC_SUBSTs rather than
> AC_DEFINE_UNQUOTED.
> The test for HAVE_GAS_CFI_SECTIONS_DIRECTIVE alternative can be compile
> int foo (int, char *);
> int bar (int x) { char *y = __builtin_alloca (x); return foo (x + 1, y) + 1; }
> with -g -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-exceptions
> and scan for .cfi_sections .debug_frame.
> 
> So here is a new (I've committed the previous patch since then), only lightly
> tested (only on x86_64-linux and don't have too old binutils around), patch:
> 
> 2018-02-27  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR debug/83917
> 	* configure.ac (AS_HIDDEN_DIRECTIVE): AC_DEFINE_UNQUOTED this to
> 	$asm_hidden_op if visibility ("hidden") attribute works.
> 	(HAVE_AS_CFI_SECTIONS): New AC_DEFINE.
> 	* config/i386/i386-asm.h: Don't include auto-host.h.
> 	(PACKAGE_VERSION, PACKAGE_NAME, PACKAGE_STRING, PACKAGE_TARNAME,
> 	PACKAGE_URL): Don't undefine.
> 	(USE_GAS_CFI_DIRECTIVES): Don't use nor define this macro, instead
> 	guard cfi_startproc only on ifdef __GCC_HAVE_DWARF2_CFI_ASM.
> 	(FN_HIDDEN): Change guard from #ifdef HAVE_GAS_HIDDEN to
> 	#ifdef AS_HIDDEN_DIRECTIVE, use AS_HIDDEN_DIRECTIVE macro in the
> 	definition instead of hardcoded .hidden.
> 	* config/i386/cygwin.S: Include i386-asm.h first before .cfi_sections
> 	directive.  Use #ifdef HAVE_AS_CFI_SECTIONS rather than
> 	#ifdef HAVE_GAS_CFI_SECTIONS_DIRECTIVE to guard .cfi_sections.
> 	(USE_GAS_CFI_DIRECTIVES): Don't define.
> 	* configure: Regenerated.
> 	* config.in: Likewise.
OK.
jeff