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 7A2FF3858412 for ; Mon, 10 Oct 2022 09:32:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7A2FF3858412 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-596-RVgBoK4WPJiOXx005Gf0Fw-1; Mon, 10 Oct 2022 05:32:38 -0400 X-MC-Unique: RVgBoK4WPJiOXx005Gf0Fw-1 Received: by mail-wm1-f70.google.com with SMTP id b7-20020a05600c4e0700b003bde2d860d1so6735264wmq.8 for ; Mon, 10 Oct 2022 02:32:37 -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=wvdh4E66QDUE5amFkbN4Xevv48o/a3iIiSmt1z6PXBk=; b=BHx8GuMPHrmMPGHmQnNJvgPUuCoepWRK+/o2Y7RxiJ4PMW3UScUye2lmdmQqDhUBfs K63Ni++uiUDepTzVXBiUjRQjJAmkoi5R+5ejFG6LrAQFzPt0a2TNVz5aCVJXChf9Ei70 W3QAYcDIMtRI3ktBeNcsjJeYGFhmAwR/WvwxvfMJqv2+6q73q66pLPET4l1gxqzFF7Uc Woj9ojKVj07zCE8BvfiyKLWFiKXnBJfZ++YNx9RURHZNLAWTO2vTvlHRRZVxiNyktFVU tEVEM3UZbFgt2fJDwPuDmp/EFz3Fpev2akDvoYUWwBbnypPUWNxmj0V9V7gpMeGLHjo7 gO4A== X-Gm-Message-State: ACrzQf1L7eEIt0Fq6L0riHYpWDkfPSL/6AWB5bbm7SPvidfFtnevD8tr cSpykviEn91r1ZP06A3tEz1ilNku0n44URJ3U+nVELjR9QtpWwhRu+TNVoJOSLtIVrwxx4y4KYA kUPzYzcDZQGpnmHt6hsAarA== X-Received: by 2002:adf:db42:0:b0:22e:386e:f6af with SMTP id f2-20020adfdb42000000b0022e386ef6afmr10528970wrj.400.1665394356687; Mon, 10 Oct 2022 02:32:36 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4ULMjesMU3FwsqaPVZGldy7I7OTi+3L+0/mHbdncSPf6d6SQDCzRtXC82KVGby1HQjMwEEVg== X-Received: by 2002:adf:db42:0:b0:22e:386e:f6af with SMTP id f2-20020adfdb42000000b0022e386ef6afmr10528952wrj.400.1665394356348; Mon, 10 Oct 2022 02:32:36 -0700 (PDT) Received: from localhost (52.72.115.87.dyn.plus.net. [87.115.72.52]) by smtp.gmail.com with ESMTPSA id ck15-20020a5d5e8f000000b0022afe4fb459sm8605581wrb.51.2022.10.10.02.32.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Oct 2022 02:32:35 -0700 (PDT) From: Andrew Burgess To: Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCH 2/3] gdb: improve disassembler styling when Pygments raises an exception In-Reply-To: <9ea099ee-e984-63ad-0400-dd9a6f9fa6ef@simark.ca> References: <15cf0c9f-81ae-9bc8-79ba-e5b4eb1f0412@simark.ca> <87wn9ae25u.fsf@redhat.com> <9ea099ee-e984-63ad-0400-dd9a6f9fa6ef@simark.ca> Date: Mon, 10 Oct 2022 10:32:35 +0100 Message-ID: <87tu4cdnws.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=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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 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 09:32:41 -0000 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). Thanks, Andrew --- commit f91822c2b9f9fed0c2717b17f380e5216502ea32 Author: Andrew Burgess Date: Sat Oct 8 16:58:00 2022 +0100 gdb/testsuite: use 'end' at the end of python blocks Within the testsuite, use the keyword 'end' to terminate blocks of Python code being sent to GDB, rather than sending \004. I could only find three instances of this, all in tests that I originally wrote. I have no memory of there being any special reason why I used \004 instead of 'end' - I assume I copied this from somewhere else that has since changed. Non of the tests being changed here are specifically about whether \004 can be used to terminate a Python block, so I think switching to the more standard 'end' keyword is the right choice. diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp index c6ed996c280..146e2b6d757 100644 --- a/gdb/testsuite/gdb.base/style.exp +++ b/gdb/testsuite/gdb.base/style.exp @@ -444,7 +444,7 @@ proc test_disassembler_error_handling { } { "def replacement_colorize_disasm(content,gdbarch):" \ " return None" \ "gdb.styling.colorize_disasm = replacement_colorize_disasm" \ - "\004"] \ + "end"] \ "setup replacement colorize_disasm function" \ true diff --git a/gdb/testsuite/gdb.python/py-arch-reg-groups.exp b/gdb/testsuite/gdb.python/py-arch-reg-groups.exp index 9503e6c10f5..f2cf8b0e6ec 100644 --- a/gdb/testsuite/gdb.python/py-arch-reg-groups.exp +++ b/gdb/testsuite/gdb.python/py-arch-reg-groups.exp @@ -102,5 +102,5 @@ gdb_py_test_silent_cmd \ " raise gdb.GdbError (\"miss-matched names\")" \ " if (r1 != r2):" \ " raise gdb.GdbError (\"miss-matched objects\")" \ - "\004" ] \ + "end" ] \ "check names and objects match" 1 diff --git a/gdb/testsuite/gdb.python/py-arch-reg-names.exp b/gdb/testsuite/gdb.python/py-arch-reg-names.exp index d3c600ffc0f..62c47e8200e 100644 --- a/gdb/testsuite/gdb.python/py-arch-reg-names.exp +++ b/gdb/testsuite/gdb.python/py-arch-reg-names.exp @@ -102,7 +102,7 @@ gdb_py_test_silent_cmd \ " raise gdb.GdbError (\"miss-matched names\")" \ " if (r1 != r2):" \ " raise gdb.GdbError (\"miss-matched objects\")" \ - "\004" ] \ + "end" ] \ "check names and objects match" 1 # Ensure that the '.find' method on the iterator returns the same