From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f46.google.com (mail-wm1-f46.google.com [209.85.128.46]) by sourceware.org (Postfix) with ESMTPS id 263723858401 for ; Wed, 1 Dec 2021 13:52:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 263723858401 Received: by mail-wm1-f46.google.com with SMTP id i8-20020a7bc948000000b0030db7b70b6bso22329803wml.1 for ; Wed, 01 Dec 2021 05:52:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=TZJ1zGuVZGRVLvt+GviC24x8aeTv9HLps3I20xcChQY=; b=iDV4dvQ5dkpsIy6Wq4Bm0UpFJ0C43l84JuPjRUPM6nw/R1t01qy9zgM0HYDFf2A/17 pq+ysASXyz/dwgNTha4dz+eX5z2bu9v93APH9saMpT6v7qhXp00xQwOB8Vv7+JnJFZzi vTepNMOVR2/7DasGpFcwnPJQOoi+pX2b57NCHQ+2OPnhgaP70OyhHMfYbimofitsbnOG 8RBCWHHj5dv+yB0FZ9D80wAKE21o5k7uxLHq4zqXIR3L3Zu26eonlndAE3f4wffQzuzN hi0fATBgQPfvAWIWpVtsAzubc50/3aLI+bNl53j5Z9QC/uepQEYzwFs3siJhMkI/sfD4 jf0g== X-Gm-Message-State: AOAM532nPkm3sSVxXn4+YsL6rbPU95vMWLOfkRYHaJYozoW5ckdyy6LJ 5EWJ/c4k7U1/hIbTdGkABQX7GwNXJ5k= X-Google-Smtp-Source: ABdhPJyClw4g7RTsJcDU0Tl6rdmdo5u1QhXws0b2GwsYltUGY4fMo/7pyoOecIPlY3LnKCD8vDvvXA== X-Received: by 2002:a05:600c:3658:: with SMTP id y24mr7199919wmq.161.1638366777131; Wed, 01 Dec 2021 05:52:57 -0800 (PST) Received: from [192.168.8.216] ([83.219.56.252]) by smtp.gmail.com with ESMTPSA id g18sm19306523wrv.42.2021.12.01.05.52.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Dec 2021 05:52:56 -0800 (PST) Message-ID: <1a2347f01cad930ba5845b75abd32a67f640ebfe.camel@labware.com> Subject: Re: [PATCH v2 2/2] ppc: recognize all program traps From: Jan Vrany To: Pedro Alves , gdb-patches@sourceware.org Cc: lsix@lancelotsix.com Date: Wed, 01 Dec 2021 13:52:55 +0000 In-Reply-To: References: <20211123154237.2335848-1-jan.vrany@labware.com> <20211124130926.2412617-1-jan.vrany@labware.com> <20211124130926.2412617-3-jan.vrany@labware.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.3-1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00, BODY_8BITS, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Dec 2021 13:52:59 -0000 On Tue, 2021-11-30 at 12:17 +0000, Pedro Alves wrote: > On 2021-11-24 13:09, Jan Vrany via Gdb-patches wrote: > > +# Number of expected SIGTRAP's to get.  This needs to be kept in > > sync > > +# with the source file. > > +set expected_traps 3 > > +set keep_going 1 > > +set count 0 > > + > > +# Make sure we have a lower timeout in case GDB doesn't support a > > particular > > +# instruction.  Such instruction will cause GDB to loop > > infinitely. > > +while {$keep_going} { > > +    # Continue to next program breakpoint instruction. > > +    gdb_test_multiple "continue" "trap instruction $count causes > > SIGTRAP" { > > +       -re "Program received signal SIGTRAP, Trace/breakpoint > > trap.*$gdb_prompt $" { > > +           pass $gdb_test_name > > + > > +           # Advance PC to next instruction > > +           gdb_test "set \$pc = \$pc + 4" "" "advance past trap > > instruction $count" > > + > > +           incr count > > +       } > > +       # We've reached the end of the test. > > +       -re "exited with code 01.*$gdb_prompt $" { > > +           set keep_going 0 > > +       } > > +       timeout { > > +           fail $gdb_test_name > > +           set keep_going 0 > > +       } > > +    } > > +} > > + > > > This while loop will iterate forever if gdb_test_multiple trips on > some internal match, like > some unexpected text out of gdb that ends with a prompt, or an > internal error.  The logic of > "keep_going" should be reversed so that the loop breaks if anything > goes wrong.   Also, the > last continue to "exited with code 01" doesn't itself issue a pass > unlike the other continues, > which seems inconsistent.  BTW, if we flip the logic, we no longer > need the timeout check. > > Something like this (completely untested): > > ~~~~~~~~~~~~ > # Number of expected SIGTRAP's to get.  This needs to be kept in sync > # with the source file. > set expected_traps 3 > set count 0 > set keep_going 1 > > # Make sure we have a lower timeout in case GDB doesn't support a > particular > # instruction.  Such instruction will cause GDB to loop infinitely. > while {$keep_going} { > >     set keep_going 0 > >     # Continue to next program breakpoint instruction. >     gdb_test_multiple "continue" "trap instruction $count causes > SIGTRAP" { >         -re "Program received signal SIGTRAP, Trace/breakpoint > trap.*$gdb_prompt $" { >             pass $gdb_test_name > >             # Advance PC to next instruction >             gdb_test "set \$pc = \$pc + 4" "" "advance past trap > instruction $count" > >             incr count >             if {$count < $expected_traps} { >                set keep_going 1 >             } >         } >     } > > # Verify we stopped at the expected number of SIGTRAP's. > gdb_assert {$count == $expected_traps} "all trap instructions > triggered" > > # One last continue to reach the end of the test, to make sure we > don't get > # another SIGTRAP. > gdb_test "continue" "exited with code 01.*" "continue to end" > ~~~~~~~~~~~~ > > > Some thing in the other file.  Or you could merge the files, and make > the exp file > set a different .S filename and different "expected_traps" depending > on arch, to > avoid duplication. Good point. I'll do as suggested in next version. > > > BTW, I don't understand what the "Make sure we have a lower timeout" > comment is referring > to -- I don't see anything changing the timeout. Neither I do, to be honest. These tests are shameless hacked copies of existing gdb.arch/aarch64-brk-patterns.exp test so this and the  comment above also apply to it as well.  I prefer not to change aarch64 test now since I do not have aarch64  test environment set up yet. > > BTW², seems strange to expect that the program exits with exit code 1 > instead of 0 on a success run. > I can't write PPC assembly to save my life, but I don't see any "1", > or writing to r3 in in > the assembly, so I guess that is assuming the value is already 1 on > entry.  I wonder whether that's a > good assumption in all environments. This works because main gets "int argc" as first parameter and test runs program with no aguments so argc is always 1. AFAIK, in all calling conventions used on PPC first argument is passed in r3 which is also a return register. But I agree this is little to hacky. I'll change it. Thanks! Jan