public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
@ 2014-01-08 18:00 Andreas Arnez
  2014-01-10 11:29 ` Pedro Alves
  0 siblings, 1 reply; 15+ messages in thread
From: Andreas Arnez @ 2014-01-08 18:00 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand, Andreas Krebbel

Since upstream gcc has recently increased the function alignment on
S390, the dw2-dir-file-name test case fails in the first
gdb_continue_to_breakpoint.  Indeed, the breakpoint is now placed into
the alignment gap *before* the actual function.

This happens because the test case declares the respective "*_start"
symbol as a "loose" label before the function definition, and the
compiler inserts the alignment between that label and the function
itself.  The fix defines the "*_start" symbol as a global alias to the
function instead.

testsuite/
2014-01-08  Andreas Arnez  <arnez@linux.vnet.ibm.com>

	* gdb.dwarf2/dw2-dir-file-name.c (FUNC): Declare "*_start" symbol
	as an alias of the function instead of a label pointing before the
	function, to avoid possible alignment issues.
---
 gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
index 21a4d2a..a0a9731 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
+++ b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
@@ -63,13 +63,13 @@ FUNC (compdir_absolute_ldir_absolute_file_relative_different)	\
 FUNC (compdir_absolute_ldir_absolute_file_absolute_same)	\
 FUNC (compdir_absolute_ldir_absolute_file_absolute_different)
 
-#define FUNC(name)					\
-  asm (#name "_start: .globl " #name "_start\n");	\
-  static void						\
-  name (void)						\
-  {							\
-    v++;						\
-  }							\
+#define FUNC(name)						\
+  static void							\
+  name (void)							\
+  {								\
+    v++;							\
+  }								\
+  void name ## _start (void) __attribute__ ((alias (#name)));	\
   asm (#name "_end: .globl " #name "_end\n");
 FUNCBLOCK
 #undef FUNC
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-08 18:00 [PATCH] Fix possible alignment issue with dw2-dir-file-name test case Andreas Arnez
@ 2014-01-10 11:29 ` Pedro Alves
  2014-01-10 14:30   ` Andreas Arnez
  0 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2014-01-10 11:29 UTC (permalink / raw)
  To: Andreas Arnez; +Cc: gdb-patches, Ulrich Weigand, Andreas Krebbel

On 01/08/2014 06:00 PM, Andreas Arnez wrote:
> Since upstream gcc has recently increased the function alignment on
> S390, the dw2-dir-file-name test case fails in the first
> gdb_continue_to_breakpoint.  Indeed, the breakpoint is now placed into
> the alignment gap *before* the actual function.
> 
> This happens because the test case declares the respective "*_start"
> symbol as a "loose" label before the function definition, and the
> compiler inserts the alignment between that label and the function
> itself.  The fix defines the "*_start" symbol as a global alias to the
> function instead.

It seems the _start symbol only needs to exist because
the functions were declared static:

> -#define FUNC(name)					\
> -  asm (#name "_start: .globl " #name "_start\n");	\
> -  static void						\
> -  name (void)						\

But I see nothing that needs them to be static.  This
seems simpler to me:

---
 gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c   | 3 +--
 gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp | 6 +++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
index 21a4d2a..2b9cc6a 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
+++ b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
@@ -64,8 +64,7 @@ FUNC (compdir_absolute_ldir_absolute_file_absolute_same)	\
 FUNC (compdir_absolute_ldir_absolute_file_absolute_different)

 #define FUNC(name)					\
-  asm (#name "_start: .globl " #name "_start\n");	\
-  static void						\
+  void							\
   name (void)						\
   {							\
     v++;						\
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
index 7f29581..e523e89 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
@@ -54,7 +54,7 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {
 	.ascii	\"GNU C\\0\"			/* DW_AT_producer */
 	.byte	2				/* DW_AT_language (DW_LANG_C) */
 	.4byte	.Lline_${name}_begin		/* DW_AT_stmt_list */
-	.4byte	${name}_start			/* DW_AT_low_pc */
+	.4byte	${name}			/* DW_AT_low_pc */
 	.4byte	${name}_end			/* DW_AT_high_pc */
 "
     if { $cu_dir != "" } {
@@ -65,7 +65,7 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {

 	.uleb128	3			/* Abbrev: DW_TAG_subprogram */
 	.asciz		\"${name}\"		/* DW_AT_name */
-	.4byte		${name}_start		/* DW_AT_low_pc */
+	.4byte		${name}		/* DW_AT_low_pc */
 	.4byte		${name}_end		/* DW_AT_high_pc */

 	.byte		0			/* End of children of CU */
@@ -122,7 +122,7 @@ proc out_line { name cu_dir cu_name line_dir line_name } {
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		${name}_start
+	.4byte		${name}
 	.byte		1	/* DW_LNS_copy */
 	.byte		3	/* DW_LNS_advance_line */
 	.sleb128	1	/* ... to 1000 */
-- 
1.7.11.7


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-10 11:29 ` Pedro Alves
@ 2014-01-10 14:30   ` Andreas Arnez
  2014-01-10 14:53     ` Jan Kratochvil
  2014-01-10 15:32     ` Pedro Alves
  0 siblings, 2 replies; 15+ messages in thread
From: Andreas Arnez @ 2014-01-10 14:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Ulrich Weigand, Andreas Krebbel, jan.kratochvil

On Fri, Jan 10 2014, Pedro Alves wrote:

> On 01/08/2014 06:00 PM, Andreas Arnez wrote:
>> Since upstream gcc has recently increased the function alignment on
>> S390, the dw2-dir-file-name test case fails in the first
>> gdb_continue_to_breakpoint.  Indeed, the breakpoint is now placed into
>> the alignment gap *before* the actual function.
>> 
>> This happens because the test case declares the respective "*_start"
>> symbol as a "loose" label before the function definition, and the
>> compiler inserts the alignment between that label and the function
>> itself.  The fix defines the "*_start" symbol as a global alias to the
>> function instead.
>
> It seems the _start symbol only needs to exist because
> the functions were declared static:
>
>> -#define FUNC(name)					\
>> -  asm (#name "_start: .globl " #name "_start\n");	\
>> -  static void						\
>> -  name (void)						\
>
> But I see nothing that needs them to be static.  This
> seems simpler to me:

It's certainly simpler.  Maybe Jan can explain why the functions had
been declared static?

Your patch fixes the FAILs for me, so if there's no reason for the
static-ness, then I agree we should go with that.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-10 14:30   ` Andreas Arnez
@ 2014-01-10 14:53     ` Jan Kratochvil
  2014-01-10 16:39       ` Pedro Alves
  2014-01-10 15:32     ` Pedro Alves
  1 sibling, 1 reply; 15+ messages in thread
From: Jan Kratochvil @ 2014-01-10 14:53 UTC (permalink / raw)
  To: Andreas Arnez; +Cc: Pedro Alves, gdb-patches, Ulrich Weigand, Andreas Krebbel

On Fri, 10 Jan 2014 15:30:46 +0100, Andreas Arnez wrote:
> It's certainly simpler.  Maybe Jan can explain why the functions had
> been declared static?
> 
> Your patch fixes the FAILs for me, so if there's no reason for the
> static-ness, then I agree we should go with that.

The Pedro's patch should be OK, there is nothing magic there.


Thanks,
Jan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-10 14:30   ` Andreas Arnez
  2014-01-10 14:53     ` Jan Kratochvil
@ 2014-01-10 15:32     ` Pedro Alves
  1 sibling, 0 replies; 15+ messages in thread
From: Pedro Alves @ 2014-01-10 15:32 UTC (permalink / raw)
  To: Andreas Arnez
  Cc: gdb-patches, Ulrich Weigand, Andreas Krebbel, jan.kratochvil

On 01/10/2014 02:30 PM, Andreas Arnez wrote:
> 
> Your patch fixes the FAILs for me, so if there's no reason for the
> static-ness, then I agree we should go with that.

OK, I'll merge it with Andreas' commit log, and push it in in a sec.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-10 14:53     ` Jan Kratochvil
@ 2014-01-10 16:39       ` Pedro Alves
  2014-01-15 21:33         ` Edjunior Barbosa Machado
  0 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2014-01-10 16:39 UTC (permalink / raw)
  To: Jan Kratochvil
  Cc: Andreas Arnez, gdb-patches, Ulrich Weigand, Andreas Krebbel

On 01/10/2014 02:53 PM, Jan Kratochvil wrote:
> On Fri, 10 Jan 2014 15:30:46 +0100, Andreas Arnez wrote:
>> It's certainly simpler.  Maybe Jan can explain why the functions had
>> been declared static?
>>
>> Your patch fixes the FAILs for me, so if there's no reason for the
>> static-ness, then I agree we should go with that.
> 
> The Pedro's patch should be OK, there is nothing magic there.
> 

Thanks.

On 01/10/2014 03:32 PM, Pedro Alves wrote:
> On 01/10/2014 02:30 PM, Andreas Arnez wrote:
>>
>> Your patch fixes the FAILs for me, so if there's no reason for the
>> static-ness, then I agree we should go with that.
>
> OK, I'll merge it with Andreas' commit log, and push it in in a sec.

(Eh, I had meant to send that as reply to Jan, but somehow managed
to botch it.)

Here's what I pushed.

--------
From: Andreas Arnez <arnez@linux.vnet.ibm.com>
Date: Fri, 10 Jan 2014 15:37:36 +0000
Subject: [PATCH] Since upstream gcc has recently increased the function
 alignment on S390, the dw2-dir-file-name test case fails in
 the first gdb_continue_to_breakpoint.  Indeed, the
 breakpoint is now placed into the alignment gap *before*
 the actual function.

This happens because the test case declares the respective "*_start"
symbol as a "loose" label before the function definition, and the
compiler inserts the alignment between that label and the function
itself.

The "*_start" symbols were only necessary because FUNC made the
function static.  The fix makes the functions extern instead, thus
making the "*_start" labels unnecessary.

testsuite/
2014-01-10  Andreas Arnez  <arnez@linux.vnet.ibm.com>
	    Pedro Alves <palves@redhat.com>

	* gdb.dwarf2/dw2-dir-file-name.c (FUNC): Remove "*_start" symbol.
	Make "name" extern.
	* gdb.dwarf2/dw2-dir-file-name.exp (out_cu, out_line): Replace
	references to ${name}_start by references to ${name}.
---
 gdb/testsuite/ChangeLog                        | 8 ++++++++
 gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c   | 3 +--
 gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp | 6 +++---
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 2bfeec1..904cca7 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2014-01-10  Andreas Arnez  <arnez@linux.vnet.ibm.com>
+	    Pedro Alves <palves@redhat.com>
+
+	* gdb.dwarf2/dw2-dir-file-name.c (FUNC): Remove "*_start" symbol.
+	Make "name" extern.
+	* gdb.dwarf2/dw2-dir-file-name.exp (out_cu, out_line): Replace
+	references to ${name}_start by references to ${name}.
+
 2014-01-10  Joel Brobecker  <brobecker@adacore.com>

 	* gdb.ada/pp-rec-component.exp: Remove path from "source" test.
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
index 21a4d2a..2b9cc6a 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
+++ b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
@@ -64,8 +64,7 @@ FUNC (compdir_absolute_ldir_absolute_file_absolute_same)	\
 FUNC (compdir_absolute_ldir_absolute_file_absolute_different)

 #define FUNC(name)					\
-  asm (#name "_start: .globl " #name "_start\n");	\
-  static void						\
+  void							\
   name (void)						\
   {							\
     v++;						\
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
index 7f29581..e523e89 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
@@ -54,7 +54,7 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {
 	.ascii	\"GNU C\\0\"			/* DW_AT_producer */
 	.byte	2				/* DW_AT_language (DW_LANG_C) */
 	.4byte	.Lline_${name}_begin		/* DW_AT_stmt_list */
-	.4byte	${name}_start			/* DW_AT_low_pc */
+	.4byte	${name}			/* DW_AT_low_pc */
 	.4byte	${name}_end			/* DW_AT_high_pc */
 "
     if { $cu_dir != "" } {
@@ -65,7 +65,7 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {

 	.uleb128	3			/* Abbrev: DW_TAG_subprogram */
 	.asciz		\"${name}\"		/* DW_AT_name */
-	.4byte		${name}_start		/* DW_AT_low_pc */
+	.4byte		${name}		/* DW_AT_low_pc */
 	.4byte		${name}_end		/* DW_AT_high_pc */

 	.byte		0			/* End of children of CU */
@@ -122,7 +122,7 @@ proc out_line { name cu_dir cu_name line_dir line_name } {
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		${name}_start
+	.4byte		${name}
 	.byte		1	/* DW_LNS_copy */
 	.byte		3	/* DW_LNS_advance_line */
 	.sleb128	1	/* ... to 1000 */
-- 
1.7.11.7


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-10 16:39       ` Pedro Alves
@ 2014-01-15 21:33         ` Edjunior Barbosa Machado
  2014-01-16 14:37           ` Pedro Alves
  0 siblings, 1 reply; 15+ messages in thread
From: Edjunior Barbosa Machado @ 2014-01-15 21:33 UTC (permalink / raw)
  To: Pedro Alves, Jan Kratochvil
  Cc: Andreas Arnez, gdb-patches, Ulrich Weigand, Andreas Krebbel



On 01/10/2014 01:41 PM, Pedro Alves wrote:
> (Eh, I had meant to send that as reply to Jan, but somehow managed
> to botch it.)
> 
> Here's what I pushed.
> 
> --------
> From: Andreas Arnez <arnez@linux.vnet.ibm.com>
> Date: Fri, 10 Jan 2014 15:37:36 +0000
> Subject: [PATCH] Since upstream gcc has recently increased the function
>  alignment on S390, the dw2-dir-file-name test case fails in
>  the first gdb_continue_to_breakpoint.  Indeed, the
>  breakpoint is now placed into the alignment gap *before*
>  the actual function.
> 
> This happens because the test case declares the respective "*_start"
> symbol as a "loose" label before the function definition, and the
> compiler inserts the alignment between that label and the function
> itself.
> 
> The "*_start" symbols were only necessary because FUNC made the
> function static.  The fix makes the functions extern instead, thus
> making the "*_start" labels unnecessary.
> 
> testsuite/
> 2014-01-10  Andreas Arnez  <arnez@linux.vnet.ibm.com>
> 	    Pedro Alves <palves@redhat.com>
> 
> 	* gdb.dwarf2/dw2-dir-file-name.c (FUNC): Remove "*_start" symbol.
> 	Make "name" extern.
> 	* gdb.dwarf2/dw2-dir-file-name.exp (out_cu, out_line): Replace
> 	references to ${name}_start by references to ${name}.

For some reason, the testcase is no longer successful on ppc64 (although it
still passes on ppc32): it hits the breakpoints, but no longer shows the
filename in none of the tests.

From gdb.log:
...
(gdb) break compdir_missing__ldir_missing__file_basename
Breakpoint 2 at 0x100006c4
(gdb) continue
Continuing.

Breakpoint 2, 0x00000000100006c4 in .compdir_missing.ldir_missing.file_basename ()
(gdb) FAIL: gdb.dwarf2/dw2-dir-file-name.exp: compdir_missing__ldir_missing__file_basename: continue to breakpoint: compdir_missing__ldir_missing__file_basename
...
                === gdb Summary ===

# of expected passes            97
# of unexpected failures        128


--
Edjunior

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-15 21:33         ` Edjunior Barbosa Machado
@ 2014-01-16 14:37           ` Pedro Alves
  2014-01-17 17:58             ` Andreas Arnez
  0 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2014-01-16 14:37 UTC (permalink / raw)
  To: Edjunior Barbosa Machado
  Cc: Jan Kratochvil, Andreas Arnez, gdb-patches, Ulrich Weigand,
	Andreas Krebbel

On 01/15/2014 09:33 PM, Edjunior Barbosa Machado wrote:
> 

>> testsuite/
>> 2014-01-10  Andreas Arnez  <arnez@linux.vnet.ibm.com>
>> 	    Pedro Alves <palves@redhat.com>
>>
>> 	* gdb.dwarf2/dw2-dir-file-name.c (FUNC): Remove "*_start" symbol.
>> 	Make "name" extern.
>> 	* gdb.dwarf2/dw2-dir-file-name.exp (out_cu, out_line): Replace
>> 	references to ${name}_start by references to ${name}.
> 
> For some reason, the testcase is no longer successful on ppc64 (although it
> still passes on ppc32): it hits the breakpoints, but no longer shows the
> filename in none of the tests.
> 
> From gdb.log:
> ...
> (gdb) break compdir_missing__ldir_missing__file_basename
> Breakpoint 2 at 0x100006c4
> (gdb) continue
> Continuing.
> 
> Breakpoint 2, 0x00000000100006c4 in .compdir_missing.ldir_missing.file_basename ()

Bah, looks like the function's low_pc ends up pointing to the function
descriptor (because that's what the "name" symbol resolves to in the
debug info in the .S file)?  Looks like we'll need some other solution.

> (gdb) FAIL: gdb.dwarf2/dw2-dir-file-name.exp: compdir_missing__ldir_missing__file_basename: continue to breakpoint: compdir_missing__ldir_missing__file_basename
> ...

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-16 14:37           ` Pedro Alves
@ 2014-01-17 17:58             ` Andreas Arnez
  2014-01-17 18:09               ` Pedro Alves
                                 ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Andreas Arnez @ 2014-01-17 17:58 UTC (permalink / raw)
  To: Pedro Alves
  Cc: Edjunior Barbosa Machado, Jan Kratochvil, gdb-patches,
	Ulrich Weigand, Andreas Krebbel

On Thu, Jan 16 2014, Pedro Alves wrote:

> On 01/15/2014 09:33 PM, Edjunior Barbosa Machado wrote:
>> 
>
>>> testsuite/
>>> 2014-01-10  Andreas Arnez  <arnez@linux.vnet.ibm.com>
>>> 	    Pedro Alves <palves@redhat.com>
>>>
>>> 	* gdb.dwarf2/dw2-dir-file-name.c (FUNC): Remove "*_start" symbol.
>>> 	Make "name" extern.
>>> 	* gdb.dwarf2/dw2-dir-file-name.exp (out_cu, out_line): Replace
>>> 	references to ${name}_start by references to ${name}.
>> 
>> For some reason, the testcase is no longer successful on ppc64 (although it
>> still passes on ppc32): it hits the breakpoints, but no longer shows the
>> filename in none of the tests.
>> 
>> From gdb.log:
>> ...
>> (gdb) break compdir_missing__ldir_missing__file_basename
>> Breakpoint 2 at 0x100006c4
>> (gdb) continue
>> Continuing.
>> 
>> Breakpoint 2, 0x00000000100006c4 in .compdir_missing.ldir_missing.file_basename ()
>
> Bah, looks like the function's low_pc ends up pointing to the function
> descriptor (because that's what the "name" symbol resolves to in the
> debug info in the .S file)?  Looks like we'll need some other solution.

OK, how about this?

----
Subject: [PATCH] Re-introduce '_start' labels and add alignment in dw2-dir-file-name test case.

On ppc64-linux a function symbol does not point to code, but to the
function descriptor.  Thus the previous change for this test case
broke it:

      https://sourceware.org/ml/gdb-patches/2014-01/msg00275.html

This patch reverts to the original method, re-introducing '_start'
symbols.  In addition, it adds sufficient alignment before the label,
such that the label never points into an alignment gap.

testsuite/
2014-01-17  Andreas Arnez  <arnez@linux.vnet.ibm.com>

	* gdb.dwarf2/dw2-dir-file-name.c (FUNC): Insert alignment and
	define "*_start" label.  Make "name" static.

	* gdb.dwarf2/dw2-dir-file-name.exp: Replace references to
	${name} by references to ${name}_start.
---
 gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c   |   10 +++++++++-
 gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp |    6 +++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
index 2b9cc6a..517df90 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
+++ b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.c
@@ -63,8 +63,16 @@ FUNC (compdir_absolute_ldir_absolute_file_relative_different)	\
 FUNC (compdir_absolute_ldir_absolute_file_absolute_same)	\
 FUNC (compdir_absolute_ldir_absolute_file_absolute_different)
 
+/* Notes: (1) The '*_start' label below is needed because 'name' may
+   point to a function descriptor instead of to the actual code.  (2)
+   The '.balign' should specify the highest possible function
+   alignment across all supported architectures, such that the label
+   never points into the alignment gap.  */
+
 #define FUNC(name)					\
-  void							\
+  asm (".balign 8");					\
+  asm (#name "_start: .globl " #name "_start\n");	\
+  static void						\
   name (void)						\
   {							\
     v++;						\
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
index e523e89..7f29581 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-dir-file-name.exp
@@ -54,7 +54,7 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {
 	.ascii	\"GNU C\\0\"			/* DW_AT_producer */
 	.byte	2				/* DW_AT_language (DW_LANG_C) */
 	.4byte	.Lline_${name}_begin		/* DW_AT_stmt_list */
-	.4byte	${name}			/* DW_AT_low_pc */
+	.4byte	${name}_start			/* DW_AT_low_pc */
 	.4byte	${name}_end			/* DW_AT_high_pc */
 "
     if { $cu_dir != "" } {
@@ -65,7 +65,7 @@ proc out_cu { name cu_dir cu_name line_dir line_name } {
 
 	.uleb128	3			/* Abbrev: DW_TAG_subprogram */
 	.asciz		\"${name}\"		/* DW_AT_name */
-	.4byte		${name}		/* DW_AT_low_pc */
+	.4byte		${name}_start		/* DW_AT_low_pc */
 	.4byte		${name}_end		/* DW_AT_high_pc */
 
 	.byte		0			/* End of children of CU */
@@ -122,7 +122,7 @@ proc out_line { name cu_dir cu_name line_dir line_name } {
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		${name}
+	.4byte		${name}_start
 	.byte		1	/* DW_LNS_copy */
 	.byte		3	/* DW_LNS_advance_line */
 	.sleb128	1	/* ... to 1000 */
-- 
1.7.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-17 17:58             ` Andreas Arnez
@ 2014-01-17 18:09               ` Pedro Alves
  2014-01-17 18:20                 ` Pedro Alves
  2014-01-17 18:21               ` Edjunior Barbosa Machado
  2014-01-22 16:13               ` Andreas Krebbel
  2 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2014-01-17 18:09 UTC (permalink / raw)
  To: Andreas Arnez
  Cc: Edjunior Barbosa Machado, Jan Kratochvil, gdb-patches,
	Ulrich Weigand, Andreas Krebbel

On 01/17/2014 05:58 PM, Andreas Arnez wrote:
> On Thu, Jan 16 2014, Pedro Alves wrote:
> 
>> On 01/15/2014 09:33 PM, Edjunior Barbosa Machado wrote:
>>>
>>
>>>> testsuite/
>>>> 2014-01-10  Andreas Arnez  <arnez@linux.vnet.ibm.com>
>>>> 	    Pedro Alves <palves@redhat.com>
>>>>
>>>> 	* gdb.dwarf2/dw2-dir-file-name.c (FUNC): Remove "*_start" symbol.
>>>> 	Make "name" extern.
>>>> 	* gdb.dwarf2/dw2-dir-file-name.exp (out_cu, out_line): Replace
>>>> 	references to ${name}_start by references to ${name}.
>>>
>>> For some reason, the testcase is no longer successful on ppc64 (although it
>>> still passes on ppc32): it hits the breakpoints, but no longer shows the
>>> filename in none of the tests.
>>>
>>> From gdb.log:
>>> ...
>>> (gdb) break compdir_missing__ldir_missing__file_basename
>>> Breakpoint 2 at 0x100006c4
>>> (gdb) continue
>>> Continuing.
>>>
>>> Breakpoint 2, 0x00000000100006c4 in .compdir_missing.ldir_missing.file_basename ()
>>
>> Bah, looks like the function's low_pc ends up pointing to the function
>> descriptor (because that's what the "name" symbol resolves to in the
>> debug info in the .S file)?  Looks like we'll need some other solution.
> 
> OK, how about this?

Nice and simple.  I like it.

> ----
> Subject: [PATCH] Re-introduce '_start' labels and add alignment in dw2-dir-file-name test case.
> 
> On ppc64-linux a function symbol does not point to code, but to the
> function descriptor.  Thus the previous change for this test case
> broke it:
> 
>       https://sourceware.org/ml/gdb-patches/2014-01/msg00275.html
> 
> This patch reverts to the original method, re-introducing '_start'
> symbols.  In addition, it adds sufficient alignment before the label,
> such that the label never points into an alignment gap.
> 
> testsuite/
> 2014-01-17  Andreas Arnez  <arnez@linux.vnet.ibm.com>
> 
> 	* gdb.dwarf2/dw2-dir-file-name.c (FUNC): Insert alignment and
> 	define "*_start" label.  Make "name" static.
> 

No empty line here please.

> 	* gdb.dwarf2/dw2-dir-file-name.exp: Replace references to
> 	${name} by references to ${name}_start.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-17 18:09               ` Pedro Alves
@ 2014-01-17 18:20                 ` Pedro Alves
  2014-01-20 19:47                   ` Andreas Arnez
  0 siblings, 1 reply; 15+ messages in thread
From: Pedro Alves @ 2014-01-17 18:20 UTC (permalink / raw)
  To: Pedro Alves
  Cc: Andreas Arnez, Edjunior Barbosa Machado, Jan Kratochvil,
	gdb-patches, Ulrich Weigand, Andreas Krebbel, Omair Javaid

BTW,

(adding Omair)

On 01/17/2014 05:58 PM, Andreas Arnez wrote:
> +/* Notes: (1) The '*_start' label below is needed because 'name' may
> +   point to a function descriptor instead of to the actual code.  (2)
> +   The '.balign' should specify the highest possible function
> +   alignment across all supported architectures, such that the label
> +   never points into the alignment gap.  */
> +
>  #define FUNC(name)					\
> -  void							\
> +  asm (".balign 8");					\
> +  asm (#name "_start: .globl " #name "_start\n");	\
> +  static void						\
>    name (void)						\

Not sure you were following the
 "testsuite/gdb.dwarf2: Fix for dw2-ifort-parameter failure on ARM"
thread.  Seems to me this exact same thing should be done to
dw2-ifort-parameter.c.  I assume that test is currently failing
on ppc64 for the exact same reason, and that if it's not failing
on S390 with current gcc, it'll be by lucky alignment.  I believe
this approach should fix Thumb there as well.  Can you guys
coordinate on handling that test?  Thanks.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-17 17:58             ` Andreas Arnez
  2014-01-17 18:09               ` Pedro Alves
@ 2014-01-17 18:21               ` Edjunior Barbosa Machado
  2014-01-22 16:13               ` Andreas Krebbel
  2 siblings, 0 replies; 15+ messages in thread
From: Edjunior Barbosa Machado @ 2014-01-17 18:21 UTC (permalink / raw)
  To: Andreas Arnez, Pedro Alves
  Cc: Jan Kratochvil, gdb-patches, Ulrich Weigand, Andreas Krebbel

On 01/17/2014 03:58 PM, Andreas Arnez wrote:
> OK, how about this?
> 
> ----
> Subject: [PATCH] Re-introduce '_start' labels and add alignment in dw2-dir-file-name test case.
> 
> On ppc64-linux a function symbol does not point to code, but to the
> function descriptor.  Thus the previous change for this test case
> broke it:
> 
>       https://sourceware.org/ml/gdb-patches/2014-01/msg00275.html
> 
> This patch reverts to the original method, re-introducing '_start'
> symbols.  In addition, it adds sufficient alignment before the label,
> such that the label never points into an alignment gap.
> 
> testsuite/
> 2014-01-17  Andreas Arnez  <arnez@linux.vnet.ibm.com>
> 
> 	* gdb.dwarf2/dw2-dir-file-name.c (FUNC): Insert alignment and
> 	define "*_start" label.  Make "name" static.
> 
> 	* gdb.dwarf2/dw2-dir-file-name.exp: Replace references to
> 	${name} by references to ${name}_start.

Great! This patch solves all the failures on ppc64.

Thanks a lot, Andreas.

--
Edjunior

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-17 18:20                 ` Pedro Alves
@ 2014-01-20 19:47                   ` Andreas Arnez
  2014-01-22 13:15                     ` Pedro Alves
  0 siblings, 1 reply; 15+ messages in thread
From: Andreas Arnez @ 2014-01-20 19:47 UTC (permalink / raw)
  To: Pedro Alves
  Cc: Pedro Alves, Edjunior Barbosa Machado, Jan Kratochvil,
	gdb-patches, Ulrich Weigand, Andreas Krebbel, Omair Javaid

On Fri, Jan 17 2014, Pedro Alves wrote:

> BTW,
>
> (adding Omair)
>
> On 01/17/2014 05:58 PM, Andreas Arnez wrote:
>> +/* Notes: (1) The '*_start' label below is needed because 'name' may
>> +   point to a function descriptor instead of to the actual code.  (2)
>> +   The '.balign' should specify the highest possible function
>> +   alignment across all supported architectures, such that the label
>> +   never points into the alignment gap.  */
>> +
>>  #define FUNC(name)					\
>> -  void							\
>> +  asm (".balign 8");					\
>> +  asm (#name "_start: .globl " #name "_start\n");	\
>> +  static void						\
>>    name (void)						\
>
> Not sure you were following the
>  "testsuite/gdb.dwarf2: Fix for dw2-ifort-parameter failure on ARM"
> thread.  Seems to me this exact same thing should be done to
> dw2-ifort-parameter.c.  I assume that test is currently failing
> on ppc64 for the exact same reason, and that if it's not failing
> on S390 with current gcc, it'll be by lucky alignment.  I believe
> this approach should fix Thumb there as well.  Can you guys
> coordinate on handling that test?  Thanks.

I haven't been following this thread, but the test case indeed fails on
s390x (64 bit) -- not just with a new GCC.

* On s390x, the test case fails like this:

        Breakpoint 1, 0x00000000800005d2 in func (param=<error reading
        variable: Cannot access memory at address 0x0>)
        (gdb) p/x param
        Cannot access memory at address 0x0

  Note that the debug info looks correct, and all addresses fit into
  four bytes.  Still, the failure disappears when changing the DWARF
  pointer size to 8.  Thus it seems that the address size mismatch
  causes confusion somewhere in GDB on big-endian systems.

* On PPC64, 'func' and 'main' are function descriptors and don't point
  to the actual code.  Thus the usage of these symbols in
  dw2-ifort-parameter-debug.S is broken (similar to what has been
  discussed in the thread, I think).  But once this is fixed, the issue
  above is surfaced.

For illustration, the patch below "fixes" both issues on PPC64.
However, for a real fix I think we should find the root cause for the
first issue (which I haven't dug much into yet).

-----
	Modified   gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter-debug.S
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter-debug.S b/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter-debug.S
index c7dd9be..7e65c70 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter-debug.S
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter-debug.S
@@ -22,21 +22,21 @@
 .Lcu1_start:
 	.2byte	2				/* DWARF Version */
 	.4byte	.Labbrev1_begin			/* Offset into abbrev section */
-	.byte	4				/* Pointer size */
+	.byte	8				/* Pointer size */
 
 	/* CU die */
 	.uleb128 1				/* Abbrev: DW_TAG_compile_unit */
 	.ascii	"file1.txt\0"			/* DW_AT_name */
 	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
 	.byte	1				/* DW_AT_language (C) */
-	.4byte	func				/* DW_AT_low_pc */
-	.4byte	main				/* DW_AT_high_pc */
+	.8byte	func_start			/* DW_AT_low_pc */
+	.8byte	func_end			/* DW_AT_high_pc */
 
 	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
 	.byte		1			/* DW_AT_external */
 	.ascii		"func\0"		/* DW_AT_name */
-	.4byte		func			/* DW_AT_low_pc */
-	.4byte		main			/* DW_AT_high_pc */
+	.8byte		func_start		/* DW_AT_low_pc */
+	.8byte		func_end		/* DW_AT_high_pc */
 
 	.uleb128	3			/* Abbrev: DW_TAG_formal_parameter */
 	.ascii		"param\0"		/* DW_AT_name */
@@ -44,7 +44,7 @@
 	.4byte		.Ltype_int - .Lcu1_begin	/* DW_AT_type */
 	.byte		2f - 1f			/* DW_AT_location */
 1:	.byte		3			/*   DW_OP_addr */
-	.4byte		ptr			/*   <addr> */
+	.8byte		ptr			/*   <addr> */
 	.byte		0x6			/*   DW_OP_deref */
 2:
 
	Modified   gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter.c
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter.c b/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter.c
index 361c44d..4474814 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter.c
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter.c
@@ -18,11 +18,17 @@
 int value = 0xdeadf00d;
 int *ptr = &value;
 
-void
+asm (".section	\".text\"");
+asm (".balign 8");
+asm ("func_start: .globl func_start");
+
+static void
 func (void)
 {
 }
 
+asm ("func_end: .globl func_end");
+
 int
 main (void)
 {

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-20 19:47                   ` Andreas Arnez
@ 2014-01-22 13:15                     ` Pedro Alves
  0 siblings, 0 replies; 15+ messages in thread
From: Pedro Alves @ 2014-01-22 13:15 UTC (permalink / raw)
  To: Andreas Arnez
  Cc: Edjunior Barbosa Machado, Jan Kratochvil, gdb-patches,
	Ulrich Weigand, Andreas Krebbel, Omair Javaid

On 01/20/2014 07:47 PM, Andreas Arnez wrote:

> * On s390x, the test case fails like this:
> 
>         Breakpoint 1, 0x00000000800005d2 in func (param=<error reading
>         variable: Cannot access memory at address 0x0>)
>         (gdb) p/x param
>         Cannot access memory at address 0x0
> 
>   Note that the debug info looks correct, and all addresses fit into
>   four bytes.  Still, the failure disappears when changing the DWARF
>   pointer size to 8.  Thus it seems that the address size mismatch
>   causes confusion somewhere in GDB on big-endian systems.

Hmm, yeah.  A little odd that dw2-dir-file-name also uses .byte 4
and doesn't have this issue.

> * On PPC64, 'func' and 'main' are function descriptors and don't point
>   to the actual code.  Thus the usage of these symbols in
>   dw2-ifort-parameter-debug.S is broken (similar to what has been
>   discussed in the thread, I think).  

Thanks.  I believe this alone would fix Thumb.  I think this bit should
go in as is (maybe with a comment), and independently of whatever
is the fix for the 64-bit issue.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Fix possible alignment issue with dw2-dir-file-name test case
  2014-01-17 17:58             ` Andreas Arnez
  2014-01-17 18:09               ` Pedro Alves
  2014-01-17 18:21               ` Edjunior Barbosa Machado
@ 2014-01-22 16:13               ` Andreas Krebbel
  2 siblings, 0 replies; 15+ messages in thread
From: Andreas Krebbel @ 2014-01-22 16:13 UTC (permalink / raw)
  To: Andreas Arnez; +Cc: gdb-patches

> testsuite/
> 2014-01-17  Andreas Arnez  <arnez@linux.vnet.ibm.com>
> 
> 	* gdb.dwarf2/dw2-dir-file-name.c (FUNC): Insert alignment and
> 	define "*_start" label.  Make "name" static.
> 
> 	* gdb.dwarf2/dw2-dir-file-name.exp: Replace references to
> 	${name} by references to ${name}_start.

Applied with the empty line removed.  Thanks!

Bye,

-Andreas-

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-01-22 16:13 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-08 18:00 [PATCH] Fix possible alignment issue with dw2-dir-file-name test case Andreas Arnez
2014-01-10 11:29 ` Pedro Alves
2014-01-10 14:30   ` Andreas Arnez
2014-01-10 14:53     ` Jan Kratochvil
2014-01-10 16:39       ` Pedro Alves
2014-01-15 21:33         ` Edjunior Barbosa Machado
2014-01-16 14:37           ` Pedro Alves
2014-01-17 17:58             ` Andreas Arnez
2014-01-17 18:09               ` Pedro Alves
2014-01-17 18:20                 ` Pedro Alves
2014-01-20 19:47                   ` Andreas Arnez
2014-01-22 13:15                     ` Pedro Alves
2014-01-17 18:21               ` Edjunior Barbosa Machado
2014-01-22 16:13               ` Andreas Krebbel
2014-01-10 15:32     ` Pedro Alves

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 read-only IMAP folder(s) and NNTP newsgroup(s).