This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] gdb.dwarf2: Testsuite 64-bit pointer truncation fixes


On Mon, 13 Oct 2014, Doug Evans wrote:

> > 2014-10-12  Maciej W. Rozycki  <macro@codesourcery.com>
> >
> >         gdb/testsuite/
> >         * gdb.dwarf2/dw2-case-insensitive-debug.S: Handle 64-bit pointers.
> >         * gdb.dwarf2/dw2-case-insensitive.exp: Update accordingly.
> >         * gdb.dwarf2/dw2-skip-prologue.S: Handle 64-bit pointers.
> >         * gdb.dwarf2/dw2-skip-prologue.exp: Update accordingly.
> 
> I'm of two minds on this.
> On the one hand, what's the cost/benefit ratio of going down the road
> of portable assembler testcases?
> On the other hand, once someone has put in the effort for one
> particular testcase, as long as it doesn't place a long term burden on
> maintaining that testcase it's not that big a deal.  [If we're going
> to impose something on new tests then I would want it written down in
> the testsuite coding standards wiki, but that can be a separate
> discussion.  We should write something down anyway.]

 All the gdb.dwarf2 tests rely on handwritten assembly, be it standalone 
or interspersed with C code.  I think the tests have value in that they 
cover DWARF language in a manner independent to the compiler, mainly GCC, 
and as such prevent us from legitimising (and missing) bugs in compiler's 
DWARF generator.  We just need to take extra care not to assume too much 
about the target and sometimes handle quirks such as with targets having 
implied semantics like with compressed MIPS code handled with a patch sent 
separately.

 GAS also has a special pseudo-op to ease cross-target testing of generic 
code, `.dc.a', that outputs pointer-sized datum and could be used here.  
It has been created for the lone purpose of testing binutils I'm told.  I 
refrained from using it though and decided to explicitly switch between 
`.4byte' and `.8byte' as these are what normal DWARF code will use and we 
should be covering what real world uses.

> Although I think there's a limit to how far down the road we should
> go, I'm ok with this patch.

 Agreed and I'm glad to see your approval.  I've been a bit nervous with 
how our gdb.dwarf2 tests make some assumptions, but as I noted above I 
think their value outweighs their shortcomings and they just need a bit of 
care.  Maybe we could stress a little bit more on verifying that new such 
cases submitted work across both endiannesses and both ILP32 and LP64 
systems.  That should cover the usual cases with any exotics left to the 
respective platform maintainers.

> > @@ -15,6 +15,14 @@
> >     You should have received a copy of the GNU General Public License
> >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >
> > +#if PTRBITS == 64
> > +# define PTRBYTE .8byte
> > +#elif PTRBITS == 32
> > +# define PTRBYTE .4byte
> > +#else
> > +# error Unsupported pointer size
> > +#endif
> > +
> 
> Nit: The argument to #error is a series of preprocessor tokens, so
> 
> #error it's not ok
> 
> is not ok.  We don't have a convention (that I can remember) of always
> making the error text a string, but I do like this convention.
> 
> Can you change the #error's to use a string?
> [And I'll look into making and documenting this convention.
> After some grepping, it is more prevalent than not quoting.]
> 
> So, LGTM with that change.

 One thing that has always annoyed me about #error "foo" is that the error 
message produced includes the quotation marks.  I guess that's the obvious 
consequence of how the directive has been defined.  That's nothing I'm 
going to fight about though, this message is not supposed to trigger 
anyway.

 Here's the version I applied then, thanks for your review.

2014-10-14  Maciej W. Rozycki  <macro@codesourcery.com>

	* gdb.dwarf2/dw2-case-insensitive-debug.S: Handle 64-bit pointers.
	* gdb.dwarf2/dw2-case-insensitive.exp: Update accordingly.
	* gdb.dwarf2/dw2-skip-prologue.S: Handle 64-bit pointers.
	* gdb.dwarf2/dw2-skip-prologue.exp: Update accordingly.

  Maciej

gdb-dwarf2-test-64bit.diff
Index: gdb-fsf-trunk-quilt/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive-debug.S
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive-debug.S	2014-10-13 13:16:59.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive-debug.S	2014-10-13 22:45:16.668964665 +0100
@@ -15,6 +15,14 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#if PTRBITS == 64
+# define PTRBYTE .8byte
+#elif PTRBITS == 32
+# define PTRBYTE .4byte
+#else
+# error "Unsupported pointer size"
+#endif
+
 	.section .debug_info
 .Lcu1_begin:
 	/* CU header */
@@ -22,21 +30,21 @@
 .Lcu1_start:
 	.2byte	2				/* DWARF Version */
 	.4byte	.Labbrev1_begin			/* Offset into abbrev section */
-	.byte	4				/* Pointer size */
+	.byte	PTRBITS / 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	8				/* DW_AT_language (DW_LANG_Fortran90) */
-	.4byte		cu_text_start		/* DW_AT_low_pc */
-	.4byte		cu_text_end		/* DW_AT_high_pc */
+	PTRBYTE		cu_text_start		/* DW_AT_low_pc */
+	PTRBYTE		cu_text_end		/* DW_AT_high_pc */
 
 	.uleb128	3			/* Abbrev: DW_TAG_subprogram */
 	.byte		1			/* DW_AT_external */
 	.ascii		"FUNC_lang\0"		/* DW_AT_name */
-	.4byte		FUNC_lang_start		/* DW_AT_low_pc */
-	.4byte		FUNC_lang_end		/* DW_AT_high_pc */
+	PTRBYTE		FUNC_lang_start		/* DW_AT_low_pc */
+	PTRBYTE		FUNC_lang_end		/* DW_AT_high_pc */
 	.byte		1			/* DW_AT_prototyped */
 	.4byte		.Ltype - .Lcu1_begin	/* DW_AT_type */
 
Index: gdb-fsf-trunk-quilt/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp	2014-10-13 13:16:59.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp	2014-10-13 13:27:25.068279596 +0100
@@ -21,8 +21,15 @@ if {![dwarf2_support]} {
 
 standard_testfile .c dw2-case-insensitive-debug.S
 
+if [is_ilp32_target] {
+    set ptrbits 32
+} else {
+    set ptrbits 64
+}
+
 if { [prepare_for_testing ${testfile}.exp ${testfile} \
-	  [list $srcfile $srcfile2] {nodebug}] } {
+	  [list $srcfile $srcfile2] \
+	  [list nodebug additional_flags=-DPTRBITS=$ptrbits]] } {
     return -1
 }
 
Index: gdb-fsf-trunk-quilt/gdb/testsuite/gdb.dwarf2/dw2-skip-prologue.S
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/gdb.dwarf2/dw2-skip-prologue.S	2014-10-13 13:16:59.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/gdb.dwarf2/dw2-skip-prologue.S	2014-10-13 22:45:30.168015537 +0100
@@ -15,6 +15,14 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#if PTRBITS == 64
+# define PTRBYTE .8byte
+#elif PTRBITS == 32
+# define PTRBYTE .4byte
+#else
+# error "Unsupported pointer size"
+#endif
+
 	.section .debug_info
 .Lcu1_begin:
 	/* CU header */
@@ -22,13 +30,13 @@
 .Lcu1_start:
 	.2byte	2				/* DWARF Version */
 	.4byte	.Labbrev1_begin			/* Offset into abbrev section */
-	.byte	4				/* Pointer size */
+	.byte	PTRBITS / 8			/* Pointer size */
 
 	/* CU die */
 	.uleb128 1				/* Abbrev: DW_TAG_compile_unit */
 	.4byte	.Lline1_begin			/* DW_AT_stmt_list */
-	.4byte	func_start			/* DW_AT_low_pc */
-	.4byte	func_end			/* DW_AT_high_pc */
+	PTRBYTE	func_start			/* DW_AT_low_pc */
+	PTRBYTE	func_end			/* DW_AT_high_pc */
 	.ascii	"main.c\0"			/* DW_AT_name */
 	.ascii	"GNU C 4.5.0\0"			/* DW_AT_producer must be >= 4.5  */
 	.byte	2				/* DW_AT_language (DW_LANG_C) */
@@ -37,8 +45,8 @@
 	.byte		1			/* DW_AT_external */
 	.ascii		"func\0"		/* DW_AT_name */
 	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
-	.4byte		func_start		/* DW_AT_low_pc */
-	.4byte		func_end		/* DW_AT_high_pc */
+	PTRBYTE		func_start		/* DW_AT_low_pc */
+	PTRBYTE		func_end		/* DW_AT_high_pc */
 
 /* GDB `has_loclist' detection of -O2 -g code needs to see a DW_AT_location
    location list.  There may exist -O2 -g CUs still not needing/using any such
@@ -51,16 +59,16 @@
 
 	.uleb128	4			/* Abbrev: DW_TAG_inlined_subroutine */
 	.ascii		"inlined\0"		/* DW_AT_name */
-	.4byte		func0			/* DW_AT_low_pc */
-	.4byte		func1			/* DW_AT_high_pc */
+	PTRBYTE		func0			/* DW_AT_low_pc */
+	PTRBYTE		func1			/* DW_AT_high_pc */
 	.byte		3			/* DW_AT_inline (DW_INL_declared_inlined) */
 	.byte		1			/* DW_AT_call_file */
 	.byte		8			/* DW_AT_call_line */
 
 	.uleb128	4			/* Abbrev: DW_TAG_inlined_subroutine */
 	.ascii		"inlined2\0"		/* DW_AT_name */
-	.4byte		func2			/* DW_AT_low_pc */
-	.4byte		func3			/* DW_AT_high_pc */
+	PTRBYTE		func2			/* DW_AT_low_pc */
+	PTRBYTE		func3			/* DW_AT_high_pc */
 	.byte		3			/* DW_AT_inline (DW_INL_declared_inlined) */
 	.byte		1			/* DW_AT_call_file */
 	.byte		11			/* DW_AT_call_line */
@@ -68,8 +76,8 @@
 #ifdef INLINED
 	.uleb128	4			/* Abbrev: DW_TAG_inlined_subroutine */
 	.ascii		"otherinline\0"		/* DW_AT_name */
-	.4byte		func3			/* DW_AT_low_pc */
-	.4byte		func_end		/* DW_AT_high_pc */
+	PTRBYTE		func3			/* DW_AT_low_pc */
+	PTRBYTE		func_end		/* DW_AT_high_pc */
 	.byte		3			/* DW_AT_inline (DW_INL_declared_inlined) */
 	.byte		1			/* DW_AT_call_file */
 	.byte		9			/* DW_AT_call_line */
@@ -77,8 +85,8 @@
 
 #ifdef LEXICAL
 	.uleb128	5			/* Abbrev: DW_TAG_lexical_block */
-	.4byte		func3			/* DW_AT_low_pc */
-	.4byte		func_end		/* DW_AT_high_pc */
+	PTRBYTE		func3			/* DW_AT_low_pc */
+	PTRBYTE		func_end		/* DW_AT_high_pc */
 
 	/* GDB would otherwise ignore the DW_TAG_lexical_block.  */
 	.uleb128	6			/* Abbrev: DW_TAG_variable */
@@ -97,8 +105,8 @@
 	.byte		1			/* DW_AT_external */
 	.ascii		"func\0"		/* DW_AT_name */
 	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
-	.4byte		fund_start		/* DW_AT_low_pc */
-	.4byte		fund_end		/* DW_AT_high_pc */
+	PTRBYTE		fund_start		/* DW_AT_low_pc */
+	PTRBYTE		fund_end		/* DW_AT_high_pc */
 
 	.byte		0			/* End of children of DW_TAG_subprogram */
 
@@ -117,7 +125,7 @@
 	/* Reset the location list base address first.  */
 	.4byte		-1, 0
 
-	.4byte		func_start, func_end
+	PTRBYTE		func_start, func_end
 	.2byte		2f-1f
 1:	.byte		0x50	/* DW_OP_reg0 */
 2:
@@ -277,7 +285,7 @@
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		func_start
+	PTRBYTE		func_start
 	.byte		3	/* DW_LNS_advance_line */
 	.sleb128	4	/* ... to 5 */
 	.byte		1	/* DW_LNS_copy */
@@ -285,7 +293,7 @@
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		func0
+	PTRBYTE		func0
 	.byte		4	/* DW_LNS_set_file */
 	.uleb128	2
 	.byte		3	/* DW_LNS_advance_line */
@@ -295,7 +303,7 @@
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		func1
+	PTRBYTE		func1
 	.byte		4	/* DW_LNS_set_file */
 	.uleb128	1
 	.byte		3	/* DW_LNS_advance_line */
@@ -305,7 +313,7 @@
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		func2
+	PTRBYTE		func2
 	.byte		4	/* DW_LNS_set_file */
 	.uleb128	2
 	.byte		3	/* DW_LNS_advance_line */
@@ -315,7 +323,7 @@
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		func3
+	PTRBYTE		func3
 	.byte		4	/* DW_LNS_set_file */
 	.uleb128	1
 	.byte		3	/* DW_LNS_advance_line */
@@ -325,14 +333,14 @@
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		func_end
+	PTRBYTE		func_end
 
 /* Equivalent copy but renamed s/func/fund/.  */
 
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		fund_start
+	PTRBYTE		fund_start
 	.byte		3	/* DW_LNS_advance_line */
 	.sleb128	-4	/* ... to 5 */
 	.byte		1	/* DW_LNS_copy */
@@ -340,7 +348,7 @@
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		fund0
+	PTRBYTE		fund0
 	.byte		4	/* DW_LNS_set_file */
 	.uleb128	2
 	.byte		3	/* DW_LNS_advance_line */
@@ -350,7 +358,7 @@
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		fund1
+	PTRBYTE		fund1
 	.byte		4	/* DW_LNS_set_file */
 	.uleb128	1
 	.byte		3	/* DW_LNS_advance_line */
@@ -360,7 +368,7 @@
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		fund2
+	PTRBYTE		fund2
 	.byte		4	/* DW_LNS_set_file */
 	.uleb128	2
 	.byte		3	/* DW_LNS_advance_line */
@@ -370,7 +378,7 @@
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		fund3
+	PTRBYTE		fund3
 	.byte		4	/* DW_LNS_set_file */
 	.uleb128	1
 	.byte		3	/* DW_LNS_advance_line */
@@ -380,7 +388,7 @@
 	.byte		0	/* DW_LNE_set_address */
 	.uleb128	5
 	.byte		2
-	.4byte		fund_end
+	PTRBYTE		fund_end
 
 /* Line numbering end.  */
 
Index: gdb-fsf-trunk-quilt/gdb/testsuite/gdb.dwarf2/dw2-skip-prologue.exp
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/gdb.dwarf2/dw2-skip-prologue.exp	2014-10-13 13:16:59.000000000 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/gdb.dwarf2/dw2-skip-prologue.exp	2014-10-13 13:27:25.068279596 +0100
@@ -39,7 +39,16 @@ if {![dwarf2_support]} {
 standard_testfile
 set executable ${testfile}
 
-if {[build_executable ${testfile}.exp ${executable} "${testfile}.c ${testfile}.S" {additional_flags=-DINLINED}] == -1} {
+if [is_ilp32_target] {
+    set ptrbits 32
+} else {
+    set ptrbits 64
+}
+
+if { [build_executable ${testfile}.exp ${executable} \
+	  "${testfile}.c ${testfile}.S" \
+	  [list additional_flags=-DINLINED \
+		additional_flags=-DPTRBITS=$ptrbits]] == -1 } {
     return -1
 }
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]