From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d]) by sourceware.org (Postfix) with ESMTPS id 2EECB3858439 for ; Mon, 10 Oct 2022 13:22:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2EECB3858439 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 675951F8C7; Mon, 10 Oct 2022 13:22:44 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 489EE13ACA; Mon, 10 Oct 2022 13:22:44 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id EFOCEKQcRGP6fAAAMHmgww (envelope-from ); Mon, 10 Oct 2022 13:22:44 +0000 Content-Type: multipart/mixed; boundary="------------7UeX8kSUVfSLPXuOYvGgAgo0" Message-ID: Date: Mon, 10 Oct 2022 15:22:43 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 Subject: Re: [PATCH 2/3] gdb: improve disassembler styling when Pygments raises an exception Content-Language: en-US To: Andrew Burgess , Simon Marchi , gdb-patches@sourceware.org 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> <87o7ukdj47.fsf@redhat.com> From: Tom de Vries In-Reply-To: <87o7ukdj47.fsf@redhat.com> X-Spam-Status: No, score=-14.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, 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 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 13:23:04 -0000 This is a multi-part message in MIME format. --------------7UeX8kSUVfSLPXuOYvGgAgo0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 10/10/22 13:16, Andrew Burgess wrote: > 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. > Hi Andrew, thanks for following up. Luckily most of my time went to investigating the root cause (which had me puzzled for quite a bit), so that's not effort wasted. Also, I forgot to check the mailing list to see if the FAIL was already noted there, so that's on me. Anyway, I propose to error out in gdb_test_multiple on this specific pattern. Dealing with ^C / ^D for now. Not tested yet (I'll start testing in now in combination with the proposed patch to fix the build). WDYT? Thanks, - Tom --------------7UeX8kSUVfSLPXuOYvGgAgo0 Content-Type: text/x-patch; charset=UTF-8; name="0001-gdb-testsuite-Detect-trailing-C-D-in-command.patch" Content-Disposition: attachment; filename="0001-gdb-testsuite-Detect-trailing-C-D-in-command.patch" Content-Transfer-Encoding: base64 W2dkYi90ZXN0c3VpdGVdIERldGVjdCB0cmFpbGluZyBeQy9eRCBpbiBjb21tYW5kCgpEZXRl Y3QgYSB0cmFpbGluZyBeQy9eRCBpbiB0aGUgY29tbWFuZCBhcmd1bWVudCBvZiBnZGJfdGVz dCwgYW5kIGVycm9yIG91dC4KClRlc3RlZCBvbiB4ODZfNjQtbGludXguCgotLS0KIGdkYi90 ZXN0c3VpdGUvZ2RiLnRlc3RzdWl0ZS9nZGItdGVzdC5leHAgfCAzNCArKysrKysrKysrKysr KysrKysrKysrKysrKy0tLS0tLQogZ2RiL3Rlc3RzdWl0ZS9saWIvZ2RiLmV4cCAgICAgICAg ICAgICAgICB8ICA0ICsrKysKIDIgZmlsZXMgY2hhbmdlZCwgMzIgaW5zZXJ0aW9ucygrKSwg NiBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9nZGIvdGVzdHN1aXRlL2dkYi50ZXN0c3Vp dGUvZ2RiLXRlc3QuZXhwIGIvZ2RiL3Rlc3RzdWl0ZS9nZGIudGVzdHN1aXRlL2dkYi10ZXN0 LmV4cAppbmRleCAyY2U4ZWIzMWQ3My4uZTg5MWY4MWU3YjAgMTAwNjQ0Ci0tLSBhL2dkYi90 ZXN0c3VpdGUvZ2RiLnRlc3RzdWl0ZS9nZGItdGVzdC5leHAKKysrIGIvZ2RiL3Rlc3RzdWl0 ZS9nZGIudGVzdHN1aXRlL2dkYi10ZXN0LmV4cApAQCAtMTksMTAgKzE5LDMyIEBAIGNsZWFu X3Jlc3RhcnQKIAogIyBDaGVjayB0aGF0IGEgY29tbWFuZCB3aXRoIHRyYWlsaW5nIG5ld2xp bmUgdHJpZ2dlcnMgYW4gZXJyb3IuCiAKLXNldCByZXN1bHRzIFtjYXRjaCB7Ci0gICAgZ2Ri X3Rlc3QgInB3ZFxuIiAiLioiICJjbWQgd2l0aCB0cmFpbGluZyBuZXdsaW5lIgotfSBvdXRw dXRdCit3aXRoX3Rlc3RfcHJlZml4ICJjbWQgd2l0aCB0cmFpbGluZyBuZXdsaW5lIiB7Cisg ICAgc2V0IHJlc3VsdHMgW2NhdGNoIHsKKwlnZGJfdGVzdCAicHdkXG4iICIuKiIgInB3ZCIK KyAgICB9IG91dHB1dF0KIAotZ2RiX2Fzc2VydCB7ICRyZXN1bHRzID09IDEgfQotc2V0IGV4 cGVjdGVkX2Vycm9yX21zZyAiSW52YWxpZCB0cmFpbGluZyBuZXdsaW5lIGluIFwicHdkXG5c IiBjb21tYW5kIgotZ2RiX2Fzc2VydCB7IFtzdHJpbmcgZXF1YWwgJG91dHB1dCAkZXhwZWN0 ZWRfZXJyb3JfbXNnXSB9CisgICAgZ2RiX2Fzc2VydCB7ICRyZXN1bHRzID09IDEgfQorICAg IHNldCBleHBlY3RlZF9lcnJvcl9tc2cgIkludmFsaWQgdHJhaWxpbmcgbmV3bGluZSBpbiBc InB3ZFxuXCIgY29tbWFuZCIKKyAgICBnZGJfYXNzZXJ0IHsgW3N0cmluZyBlcXVhbCAkb3V0 cHV0ICRleHBlY3RlZF9lcnJvcl9tc2ddIH0KK30KKword2l0aF90ZXN0X3ByZWZpeCAiY21k IHdpdGggdHJhaWxpbmcgY29udHJvbCBjb2RlIiB7CisgICAgZm9yZWFjaF93aXRoX3ByZWZp eCBjb250cm9sX2NvZGUge15DIF5EfSB7CisJc3dpdGNoICRjb250cm9sX2NvZGUgeworCSAg ICBeQyB7CisJCXNldCBjbWQgIlwwMDMiCisJICAgIH0KKwkgICAgXkQgeworCQlzZXQgY21k ICJcMDA0IgorCSAgICB9CisJfQorCXNldCByZXN1bHRzIFtjYXRjaCB7CisJICAgIGdkYl90 ZXN0ICRjbWQgIi4qIiAiY29udHJvbCBjb2RlIgorCX0gb3V0cHV0XQorCisJZ2RiX2Fzc2Vy dCB7ICRyZXN1bHRzID09IDEgfQorCXNldCBleHBlY3RlZF9lcnJvcl9tc2cgIkludmFsaWQg dHJhaWxpbmcgY29udHJvbCBjb2RlIGluIFwiJGNtZFwiIGNvbW1hbmQiCisJZ2RiX2Fzc2Vy dCB7IFtzdHJpbmcgZXF1YWwgJG91dHB1dCAkZXhwZWN0ZWRfZXJyb3JfbXNnXSB9CisgICAg fQorfQpkaWZmIC0tZ2l0IGEvZ2RiL3Rlc3RzdWl0ZS9saWIvZ2RiLmV4cCBiL2dkYi90ZXN0 c3VpdGUvbGliL2dkYi5leHAKaW5kZXggNWYwYWNmYWE1MzAuLmFjMjhlZGUxYjA4IDEwMDY0 NAotLS0gYS9nZGIvdGVzdHN1aXRlL2xpYi9nZGIuZXhwCisrKyBiL2dkYi90ZXN0c3VpdGUv bGliL2dkYi5leHAKQEAgLTEwMDMsNiArMTAwMywxMCBAQCBwcm9jIGdkYl90ZXN0X211bHRp cGxlIHsgY29tbWFuZCBtZXNzYWdlIGFyZ3MgfSB7CiAJZXJyb3IgIkludmFsaWQgdHJhaWxp bmcgbmV3bGluZSBpbiBcIiRjb21tYW5kXCIgY29tbWFuZCIKICAgICB9CiAKKyAgICBpZiBb c3RyaW5nIG1hdGNoICIqXFtcMDAzXDAwNFxdIiAkY29tbWFuZF0geworCWVycm9yICJJbnZh bGlkIHRyYWlsaW5nIGNvbnRyb2wgY29kZSBpbiBcIiRjb21tYW5kXCIgY29tbWFuZCIKKyAg ICB9CisKICAgICBpZiBbc3RyaW5nIG1hdGNoICIqXFtcclxuXF0qIiAkbWVzc2FnZV0gewog CWVycm9yICJJbnZhbGlkIG5ld2xpbmUgaW4gXCIkbWVzc2FnZVwiIHRlc3QiCiAgICAgfQo= --------------7UeX8kSUVfSLPXuOYvGgAgo0--