From patchwork Sun Mar 1 17:42:03 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Wei-cheng, Wang" X-Patchwork-Id: 5363 Received: (qmail 62458 invoked by alias); 1 Mar 2015 17:42:14 -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 62445 invoked by uid 89); 1 Mar 2015 17:42:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.5 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, KAM_FROM_URIBL_PCCC, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-pa0-f51.google.com Received: from mail-pa0-f51.google.com (HELO mail-pa0-f51.google.com) (209.85.220.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sun, 01 Mar 2015 17:42:12 +0000 Received: by padet14 with SMTP id et14so11078516pad.0 for ; Sun, 01 Mar 2015 09:42:11 -0800 (PST) X-Received: by 10.70.128.49 with SMTP id nl17mr40687167pdb.67.1425231731001; Sun, 01 Mar 2015 09:42:11 -0800 (PST) Received: from [192.168.2.3] ([123.110.214.155]) by mx.google.com with ESMTPSA id pl8sm9598541pdb.72.2015.03.01.09.42.08 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 01 Mar 2015 09:42:09 -0800 (PST) Message-ID: <54F34F6B.2090105@gmail.com> Date: Mon, 02 Mar 2015 01:42:03 +0800 From: Wei-cheng Wang User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Ulrich Weigand , palves@redhat.com CC: gdb-patches@sourceware.org Subject: Re: [PATCH 1/2] Fast tracepoint for powerpc64le References: <201502271952.t1RJqq7X018591@d03av02.boulder.ibm.com> In-Reply-To: <201502271952.t1RJqq7X018591@d03av02.boulder.ibm.com> On 2015/2/28 上午 03:52, Ulrich Weigand wrote: > The tspeed.exp file already has: > # Typically we need a little extra time for this test. > set timeout 180 > Is that still not enough? It should include the time spent in trying different loop counts, so it would be 11 + 22 + 45 + 90 + 180 = at least 348 seconds in my environment. (for 10000, 20000, 40000, 80000, 160000 iterations respectively) If I set timeout to 360, the case will pass. >> * tfind.exp: One of the tracepoint is inserted at >> `*gdb_recursion_test'. It's not hit because local-entry is called >> instead. The 18 FAILs are off-by-one error. > This test case seem a bit more complicated, we may need to split it > in two parts; one that uses a normal "trace gdb_recursion_test" > without the "*", and possibly a second one that specifically tests > that "trace *func" works, using a source file that makes sure to > call func via a function pointers (as in step-bt.c). How about simply change the code to this? It wouldn't hurt other cases. And all the failed cases in tfind.exp now pass. >> need to be fixed. For example, >> * write_inferior_data_ptr should be fixed for big-endian. >> If sizeof (CORE_ADDR) is larger than sizeof (void*), zeros are written. >> BTW, I thnink write_inferior_data_pointer provides the same functionality >> without this issue. I'm not sure why write_inferior_data_ptr is needed? > This is odd, I don't see the point of this either. Of course, as the > comment says, much of this stuff will break anyway if gdbserver is > compiled differently than the inferior (e.g. a 64-bit gdbserver > debugging a 32-bit inferior), because it assumes the structure layout > is identical. However, if we do have a 32-bit gdbserver, then I don't > see why it shouldn't be possible to debug a 32-bit inferior, just > because CORE_ADDR is a 64-bit type ... For example, CORE_ADDR ptr = 0x11223344, a 32-bit address, and sizeof (void *) = 4-byte |------------ 64-bit CORE_ADDR ---------| MSB LSB | 00 | 00 | 00 | 00 | 11 | 22 | 33 | 44 | Low High Address |-- 32-bit(void*) --| &ptr,4 means these zeros are written to inferior. static int write_inferior_data_ptr (CORE_ADDR where, CORE_ADDR ptr) { return write_inferior_memory (where, (unsigned char *) &ptr, sizeof (void *)); ^^^^ ^^^^^^^^^^^^^^^ } CORE_ADDR is declared as "unsigned long long" for gdbserver (in common/gdb/common-types.h) > Ugh. That's a strange construct, and extremely dependent on alignment > rules (as you noticed). I'm not really sure what the best way to fix > this would be. My preference right now would be get rid of "ops" on > the gdbserver side too, and just switch on "type" in the two places > where the ops->send and ops->download routines are called right now. > > This makes the data structures the same on gdbserver and IPA, which > simplifies downloading quite a bit. (Also, it keeps the data structure > identical in IPA, which should avoid compatibility issues between > versions.) That sounds great to me! Thanks Wei-cheng, --- a/gdb/testsuite/gdb.trace/actions.c +++ b/gdb/testsuite/gdb.trace/actions.c @@ -46,6 +46,8 @@ static union GDB_UNION_TEST } gdb_union1_test; void gdb_recursion_test (int, int, int, int, int, int, int); +typedef void (*gdb_recursion_test_fp) (int, int, int, int, int, int, int); +gdb_recursion_test_fp gdb_recursion_test_ptr = gdb_recursion_test; void gdb_recursion_test (int depth, int q1, @@ -64,7 +66,7 @@ void gdb_recursion_test (int depth, q5 = q6; /* gdbtestline 6 */ q6 = q; /* gdbtestline 7 */ if (depth--) /* gdbtestline 8 */ - gdb_recursion_test (depth, q1, q2, q3, q4, q5, q6); /* gdbtestline 9 */ + gdb_recursion_test_ptr (depth, q1, q2, q3, q4, q5, q6); /* gdbtestline 9 */ } @@ -103,7 +105,7 @@ unsigned long gdb_c_test( unsigned long *parm ) gdb_structp_test = &gdb_struct1_test; gdb_structpp_test = &gdb_structp_test; - gdb_recursion_test (3, (long) parm[1], (long) parm[2], (long) parm[3], + gdb_recursion_test_ptr (3, (long) parm[1], (long) parm[2], (long) parm[3], (long) parm[4], (long) parm[5], (long) parm[6]); >> For powerpc32 to work, some data structure/function in tracepoint.c