From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 1D1F13858D37 for ; Mon, 10 Oct 2022 11:16:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1D1F13858D37 Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-344-q2jZbQ5aOvGOND31AGVWZw-1; Mon, 10 Oct 2022 07:16:10 -0400 X-MC-Unique: q2jZbQ5aOvGOND31AGVWZw-1 Received: by mail-wm1-f70.google.com with SMTP id i5-20020a1c3b05000000b003c47c8569easo3683610wma.1 for ; Mon, 10 Oct 2022 04:16:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=1EH4u6Wk9mkD/RN5xPTx7Q4YYM3cus8RpvI7ay3zaeY=; b=kGx81B+/DyopK+dV2sNLFh9BGBkQdYMrVsltFgASaMcRtljh4KiRcnrCf+QFByekje eQv4t7p4VzOIpBv7GC6bp63Nn4rNSGGT4SqJSf/m7Ay4G21nO2JpFslkVCaVszRPAxAo gTYzIAvAVF3fTCbTBrE1X7qGKr7Xq+hcP6zoDwzBJs5ZMUx3ZB1EoVS6bmhnTy2q/CHc JDnwZtHgh7y07DtLZ9DIFcoIULqDr3gJCshreCBubXc1bSPFsukSkNpdjv9Spt17An5d l76iVUfm/F9+D6oTpootOmy+uu2CYCl/OiBDfmpkI9PTuRV32LcgvmxOCYL6GI79p+TJ k1sQ== X-Gm-Message-State: ACrzQf1uCl0jptR8sUvogtu6ltoTviVo6YOJ6zDUQkX60Rws71YqOiOo 0TYojadyiMTYjKO2EoC0R6p4lp0rjFPfZzUC1Km95PCCyMI9AerAcjDzOcnXnXdyU38XwyNwRJr ey0FvpRJ0hJZIWrNgw84nIg== X-Received: by 2002:adf:d204:0:b0:22e:397:d489 with SMTP id j4-20020adfd204000000b0022e0397d489mr9809461wrh.639.1665400569602; Mon, 10 Oct 2022 04:16:09 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7z57xPtjCvxewBGa7RspFQTSidFMndg9BTlndvESKZIST99J23sW6Kkwo5NBXTQeHYo5AzJQ== X-Received: by 2002:adf:d204:0:b0:22e:397:d489 with SMTP id j4-20020adfd204000000b0022e0397d489mr9809451wrh.639.1665400569355; Mon, 10 Oct 2022 04:16:09 -0700 (PDT) Received: from localhost (52.72.115.87.dyn.plus.net. [87.115.72.52]) by smtp.gmail.com with ESMTPSA id b21-20020a05600c151500b003c6b9749505sm1659771wmg.30.2022.10.10.04.16.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Oct 2022 04:16:08 -0700 (PDT) From: Andrew Burgess To: Tom de Vries , Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCH 2/3] gdb: improve disassembler styling when Pygments raises an exception In-Reply-To: <32ac73a1-c0b8-75cf-72d3-bd11471d0fe6@suse.de> References: <15cf0c9f-81ae-9bc8-79ba-e5b4eb1f0412@simark.ca> <87wn9ae25u.fsf@redhat.com> <9ea099ee-e984-63ad-0400-dd9a6f9fa6ef@simark.ca> <87tu4cdnws.fsf@redhat.com> <32ac73a1-c0b8-75cf-72d3-bd11471d0fe6@suse.de> Date: Mon, 10 Oct 2022 12:16:08 +0100 Message-ID: <87o7ukdj47.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Mon, 10 Oct 2022 11:16:13 -0000 Tom de Vries writes: > On 10/10/22 11:32, Andrew Burgess via Gdb-patches wrote: >> Simon Marchi writes: >> >>> On 2022-10-08 12:00, Andrew Burgess wrote: >>>> Simon Marchi writes: >>>> >>>>>> +# Check that, if the user is using Python Pygments for disassembler >>>>>> +# styling, then the styling correctly switches off when an error is >>>>>> +# detected in the Python code. >>>>>> +proc test_disassembler_error_handling { } { >>>>>> + >>>>>> + # This test requires the Python Pygments module to be installed >>>>>> + # and used by GDB. >>>>>> + if { !$::python_disassembly_styling } { >>>>>> + return >>>>>> + } >>>>>> + >>>>>> + save_vars { env(TERM) } { >>>>>> + # We need an ANSI-capable terminal to get the output. >>>>>> + setenv TERM ansi >>>>>> + >>>>>> + # Restart GDB with the correct TERM variable setting, this >>>>>> + # means that GDB will enable styling. >>>>>> + clean_restart_and_disable "restart 4" $::binfile >>>>>> + >>>>>> + # Disable use of libopcodes for styling. As this function is >>>>>> + # only called when Python Pygments module is available, we >>>>>> + # should now be using that module to style the disassembler >>>>>> + # output. >>>>>> + gdb_test_no_output "maint set libopcodes-styling enabled off" >>>>>> + >>>>>> + # Disassemble a single instruction and ensure that the output >>>>>> + # has styling markers in it. >>>>>> + set insn_before [get_single_disassembled_insn] >>>>>> + gdb_assert { [regexp "\033" $insn_before] } \ >>>>>> + "have style markers when Pygments is working fine" >>>>>> + >>>>>> + # Now replace the standard function that colorizes the >>>>>> + # disassembler output, with a new function that always returns >>>>>> + # None, this should cause GDB to stop using the Pygments >>>>>> + # module for disassembler styling. >>>>>> + gdb_py_test_silent_cmd \ >>>>>> + [multi_line_input \ >>>>>> + "python" \ >>>>>> + "def replacement_colorize_disasm(content,gdbarch):" \ >>>>>> + " return None" \ >>>>>> + "gdb.styling.colorize_disasm = replacement_colorize_disasm" \ >>>>>> + "\004"] \ >>>>> >>>>> Any reason you are using \004 here, instead of end? I don't quite >>>>> understand why, but it seems to cause some random failures. Running the >>>>> test under `taskset -c 2` makes it fail most of the time. Running it >>>>> with check-read1 makes it fail consistently: >>>>> >>>>> FAIL: gdb.base/style.exp: capture_command_output for x/1i *main >>>>> >>>>> When changing \004 for end, it passes. I don't have an explanation why >>>>> though. >>>> >>>> It'll be a copy&paste. There's a couple of other places in the >>>> testsuite where this pattern is used. >>>> >>>> The patch below changes all three to use 'end'. How's this? >>>> >>>> Thanks, >>>> Andrew >>> >>> This is fine with me, but again, I didn't dig into it so I don't really >>> know what sending a \004 was causing, >> >> Hi, >> >> Given you were seeing problems with the use of \004, and the tests >> definitely were not about whether \004 could be used or not, I've pushed >> the patch I proposed (exact version is below). >> > > I found this independently and filed a PR ( > https://sourceware.org/bugzilla/show_bug.cgi?id=29664 ) and wrote the > same fix (in the same three test-cases), and was about to commit ... > > The FAIL is caused by using gdb_py_test_silent_cmd, which adds a "\n" at > the end of the command in combination with "\004", causing a stray "\n" > that generates an extra prompt, which makes the prompt matching > unpredictable. Apologies for the duplicated effort. Also, thanks for the explanation, which, if I'd spotted sooner would have saved me investigating the problem. Again, thanks for picking up after me, and apologies for wasting your time. thanks, Andrew