This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] gdb.dwarf2: Testsuite 64-bit pointer truncation fixes
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Doug Evans <dje at google dot com>
- Cc: gdb-patches <gdb-patches at sourceware dot org>
- Date: Tue, 14 Oct 2014 21:22:05 +0100
- Subject: Re: [PATCH] gdb.dwarf2: Testsuite 64-bit pointer truncation fixes
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot DEB dot 1 dot 10 dot 1410122308450 dot 19130 at tp dot orcam dot me dot uk> <CADPb22Q-8fbfV+Yu7jy7xX3o9CS7VJvGr4q0=NsphtE-RwJAtA at mail dot gmail dot com>
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
}