From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13891 invoked by alias); 19 May 2014 17:22:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 13879 invoked by uid 89); 19 May 2014 17:22:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 19 May 2014 17:22:45 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s4JHMhMN022651 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 19 May 2014 13:22:43 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s4JHMfmo004715; Mon, 19 May 2014 13:22:42 -0400 Message-ID: <537A3DE1.6070805@redhat.com> Date: Mon, 19 May 2014 17:22:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: "Metzger, Markus T" CC: "gdb-patches@sourceware.org" Subject: Re: [PATCH] btrace, vdso: add vdso target sections References: <1396601586-24380-1-git-send-email-markus.t.metzger@intel.com> <53760BDF.2080500@redhat.com> In-Reply-To: Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2014-05/txt/msg00360.txt.bz2 On 05/19/2014 09:05 AM, Metzger, Markus T wrote: >> -----Original Message----- >> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches- >> owner@sourceware.org] On Behalf Of Pedro Alves >> Sent: Friday, May 16, 2014 3:00 PM > > Thanks for your review. > > >> Looks largely fine, but ... >> >> On 04/04/2014 09:53 AM, Markus Metzger wrote: >> >>> @@ -131,6 +133,27 @@ symbol_file_add_from_memory (struct bfd >> *templ, CORE_ADDR addr, >>> from_tty ? SYMFILE_VERBOSE : 0, >>> sai, OBJF_SHARED, NULL); >>> >>> + if (add_sections) >>> + { >> ... >> >> Why is this conditional? Why not add the sections if the symbols are >> added manually with add-symbol-file-from-memory? The use case for the >> command was originally exactly to add the vdso's symbols to GDB. > > This function is used by the add-symbol-file-from-memory command, as well. > > I'm not sure if we want to add the target sections there, as well. If there's no good reason, then I'd rather the command installed the sections as well. add-symbol-file-from-memory was invented to add the vdso manually. And in that case, we'd want btrace to work just the same. >>> +# disassemble the code around the current PC >>> +gdb_test "disassemble \$pc, +10" [join [list \ >>> + ".*" \ >>> + "End of assembler dump\." \ >>> + ] "\r\n"] >> >> What is the error one gets without the fix? Doesn't >> GDB say "End of assembler dump" in that case too? > > No. GDB says "Cannot access memory at address ...". Alright. I was thinking that that might be a little brittle: if GDB changes that for some reason, the test will no longer catch problems. I think the best test is to make sure the disassemble output when replaying is the same as when not replaying at all. Say, borrow the idea of capture_command_output from gcore.exp, and do proc disassemble { what test } { global gdb_prompt global expect_out set output_string "" set command "disassemble $what" gdb_test_multiple "$command" "$command" { -re "${command}\[\r\n\]+(.*)End of assembler dump\.\r\n$gdb_prompt $" { set output_string $expect_out(1,string) pass $command } } return $output_string } set pre_btrace_disasm [disassemble "gettimeofday"] # trace the test code gdb_test_no_output "record btrace" gdb_test "next" # start replaying gdb_test "reverse-stepi" with_test_prefix "replay" { set post_btrace_disasm [disassemble "gettimeofday"] set test "disassembly output matched" if ![string pre_btrace_disasm $post_btrace_disasm] { pass $test } else { fail $test } } Also, how about making sure we're actually testing the vdso code, by e.g., expecting "Dump of assembler code for function __vdso_gettimeofday", or making sure the disassembled region is within what "info proc maps" shows is the vdso region? The idea would be that if the kernel ever changes, we'd realize that the test is no longer disassembling the vdso (due to a FAIL/UNSUPPORTED, etc.), thus no longer exercising what the test was meant to exercise. -- Pedro Alves