From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 53C683896C18 for ; Tue, 15 Nov 2022 14:59:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 53C683896C18 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 2AFExMqJ029967 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 15 Nov 2022 09:59:27 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 2AFExMqJ029967 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1668524367; bh=u16bMf5XQxyEcO97Jv3t/Z2s0GcalWkBedlVjbmK5L0=; h=Date:Subject:To:References:From:In-Reply-To:From; b=jfV6CrqG3YErfTZEV0SbueutLXoStihU/tCsCyBmYxMxxtAK2WN3Yo78g2w8ojTJy RMAqXjfbTJ5oInfvolLAZuoKnza3rXABl/XxxPSJ0W5pUHS14MNJ6bImXGRox4xEFi PSGZ3VZnKDKoIoG0RuTPfCPbMVExzDazbV0bAKd0= Received: from [172.16.0.64] (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id B4BA11E0D3; Tue, 15 Nov 2022 09:59:22 -0500 (EST) Message-ID: <33cbb0f0-72b2-7202-db84-2e3def9b126c@polymtl.ca> Date: Tue, 15 Nov 2022 09:59:22 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH] gdb/testsuite: get_set_option_choices: expect \r\n after each item Content-Language: fr To: Tom de Vries , gdb-patches@sourceware.org References: <20221114151416.372074-1-simon.marchi@polymtl.ca> <3c8249a8-4cc3-8fcb-05fa-bb70a2f55209@suse.de> From: Simon Marchi In-Reply-To: <3c8249a8-4cc3-8fcb-05fa-bb70a2f55209@suse.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Tue, 15 Nov 2022 14:59:22 +0000 X-Spam-Status: No, score=-3030.9 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 11/15/22 02:01, Tom de Vries wrote: > On 11/14/22 16:14, Simon Marchi via Gdb-patches wrote: >> I get some random failures since commit 8d45c3a82a0e ("[gdb/testsuite] >> Set completions to unlimited in get_set_option_choices"), which can be >> reproduced with: >> >>      $ make check-read1 TESTS="gdb.base/parse_number.exp" >> > > Hi, > > sorry, that's my fault.  I can reproduce this. > >> For instance: >> >>      set architecture A^M >>      Ambiguous item "A".^M >>      (gdb) FAIL: gdb.base/parse_number.exp: arch=A: set architecture A >> >> The problem is the regexp in get_set_option_choices, it can match only >> part of a completion result.  With check-read1, that is alwasy one >> letter.  Fix this by expecting the \r\n at the end of the line, so we >> only match entire results. >> > > Now, with --enable-targets=all I have 364 architectures: > ... > $ gdb -q -batch -ex "set max-completions unlimited" -ex "complete set architecture " 2>&1 | grep -v -c "set architecture" > 0 > $ gdb -q -batch -ex "set max-completions unlimited" -ex "complete set architecture " 2>&1 | grep -c "set architecture" > 364 > ... > > And using this: > ... > @@ -9156,6 +9156,7 @@ proc get_set_option_choices {set_cmd} { >         } >      } > > +    verbose -log "Found [llength $values] choices for $set_cmd" >      return $values >  } > > ... > we can see how many choices get_set_option_choices finds. > > Without this patch I get: > ... > (gdb) Found 364 choices for set architecture > ... > but with the patch: > ... > (gdb) Found 182 choices for set architecture > ... > > The problem is that by consuming \r\n both at the start and the end of the line, we drop about half of the choices. Ah, noob mistake, thanks for catching, it would have been bad. > > I usually solve this by using positive lookahead: > ... > -           -re "\r\n$set_cmd (\[^\r\n\]+)\r\n" { > +           -re "\r\n$set_cmd (\[^\r\n\]+)(?=\r\n)" { I had to look up what ?= does (positive lookahead), but that looks good to me. > ... > This fixes the number of choices for me, and still passes with check-read1. > > Note that in the original patch I did: > ... > -       -re "$set_cmd (\[^\r\n\]+)\r\n" { > +       -re "\r\n$set_cmd (\[^\r\n\]+)" { > ... > in order to be able to use -wrap (which requires a leading \r\n) instead of gdb_prompt. I remember typing that up in the commit message, but it seems to have gone missing. Since this let me make a mistake, I'm thinking that we could be a bit more paranoid in the match patterns, ensuring that we match at the beginning of the expect buffer, and that no output is left behind. Something like: with_set max-completions unlimited { gdb_test_multiple $cmd $test { -re "^[string_to_regexp $cmd]" { exp_continue } -re "^\r\n$set_cmd (\[^\r\n\]+)(?=\r\n)" { lappend values $expect_out(1,string) exp_continue } -re "^\r\n$::gdb_prompt $" { pass $gdb_test_name } } } Simon