From patchwork Tue Oct 14 20:22:05 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 3224 Received: (qmail 25161 invoked by alias); 14 Oct 2014 20:22:18 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 25149 invoked by uid 89); 14 Oct 2014 20:22:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 14 Oct 2014 20:22:15 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-02.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1Xe8bj-0004rh-KA from Maciej_Rozycki@mentor.com ; Tue, 14 Oct 2014 13:22:12 -0700 Received: from localhost (137.202.0.76) by SVR-IES-FEM-02.mgc.mentorg.com (137.202.0.106) with Microsoft SMTP Server (TLS) id 14.3.181.6; Tue, 14 Oct 2014 21:22:09 +0100 Date: Tue, 14 Oct 2014 21:22:05 +0100 From: "Maciej W. Rozycki" To: Doug Evans CC: gdb-patches Subject: Re: [PATCH] gdb.dwarf2: Testsuite 64-bit pointer truncation fixes In-Reply-To: Message-ID: References: User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 On Mon, 13 Oct 2014, Doug Evans wrote: > > 2014-10-12 Maciej W. Rozycki > > > > 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 . */ > > > > +#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 * 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 . */ +#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 . */ +#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 }