From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf2a.google.com (mail-qv1-xf2a.google.com [IPv6:2607:f8b0:4864:20::f2a]) by sourceware.org (Postfix) with ESMTPS id E4FEC396EC43 for ; Thu, 20 May 2021 16:06:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E4FEC396EC43 Received: by mail-qv1-xf2a.google.com with SMTP id h7so8222457qvs.12 for ; Thu, 20 May 2021 09:06:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=kHAJf4w+LAIBc5fma5Fs+XjnAC181FXSVV78tDPuz1U=; b=bRUp3DWc8wHIdDTpg8MuAZ+27DXACWAP5Hd+bUf03CJMY4jJuCCnXmW5ADMPBNUZmA qZj/WY1nbwhjlmCSkYJwgoVS4LrJ47CAa+ACT/QIVUW5V173w6UacwRn/NsVrMm5JmjD rHLUGfI7B5TM90RKZXpHBqL6UOomuLjAMgxsds5sLjxfoEQ8aFNDU2YLorZfEozfCldS dqFEqIziTFwjb/hnAVfI2IlKYQI4+uo9V1UWuGo6DMceXBJnSVuaNvI9xCQu8I7PIfLM ExpmQJC/rIvoVZBfC+xWJpnMOvwmoAw8tfzxVGnn8H+rvuJzyRQxiPy6ogZRnR1bsHr4 fRIA== X-Gm-Message-State: AOAM531RAlumGBbqECBMcmM9mOjDGOshMStLL0k5SlybuBzpEBpDPRuP mM76jdg7iYf/oGxako7errR2g8X1RF/ICQ== X-Google-Smtp-Source: ABdhPJzKXioEGctpoAMwY8Ryw4mpZx43d/Wiv/lPtPLcrfhstayXKR43bMNZEn5d1AGl6PWOEu7zVw== X-Received: by 2002:a05:6214:b0b:: with SMTP id u11mr6577652qvj.9.1621526767826; Thu, 20 May 2021 09:06:07 -0700 (PDT) Received: from ?IPv6:2804:7f0:4841:40ad:4df5:156f:8548:18f7? ([2804:7f0:4841:40ad:4df5:156f:8548:18f7]) by smtp.gmail.com with ESMTPSA id c15sm2112505qtp.89.2021.05.20.09.06.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 20 May 2021 09:06:07 -0700 (PDT) Subject: Re: [PATCH] gdb: run 'maint selftest' with an executable loaded To: Andrew Burgess Cc: gdb-patches@sourceware.org References: <20210520132949.2575283-1-andrew.burgess@embecosm.com> <2124d974-53eb-5ebb-7ebc-39ad0bc29e61@linaro.org> <20210520154146.GC2672@embecosm.com> From: Luis Machado Message-ID: <01ac3e69-91c0-0eeb-ff1f-3eb98a4fbeca@linaro.org> Date: Thu, 20 May 2021 13:06:04 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210520154146.GC2672@embecosm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Thu, 20 May 2021 16:06:14 -0000 On 5/20/21 12:41 PM, Andrew Burgess wrote: > * Luis Machado [2021-05-20 11:29:07 -0300]: > >> On 5/20/21 10:29 AM, Andrew Burgess wrote: >>> I spotted that 'maint selftest' with an executable loaded into GDB, >>> would (when GDB was compiled for all targets) crash GDB. I fixed this >>> with a commit to bfd: >> >> I don't understand the purpose of being able to run selftests with an >> executable loaded, or, more generally, running selftests when any GDB state >> is capable of interfering with the tests. >> >> Should we force the selftests to cleanup the state before we run them? Or >> not allow running selftests when there is a debugging session already >> stablished? > > I'm not trying to claim such a thing is useful, but I do feel pretty > strongly that running 'maint selftest' (with an executable loaded) > shouldn't cause GDB to segfault. To ensure it doesn't get broken > again in the future I'd suggest we need add this configuration to the > testsuite. > > I guess one possible solution would have been to add a check within > the 'maint selftest' command that throws an error if an executable is > loaded, but, that would have meant we missed a real bug (I claim) in > bfd. > > You'll need to check out commit 427e4066afd1 for details of the issue > I found, but it could be reproduced (though I admit via a rather weird > set of steps) outside of 'maint selftest'. > > I understand your concerns about the selftests potentially depending > on GDB being in a vanilla state, which might be corrupted by loading > an executable, right now this doesn't seem to be the case (too much, I > guess the ARM failure is an example of this happening). > > If in the future we get more such cases, I'd have no problems with us > increasing the number of acceptable selftest failures. Right. I'm not against the fix, but this shows the selftest design in GDB is kinda broken in this regard. It is not self-contained and seems to rely on external state. Just wanted to point out that we shouldn't need to handle running the selftests with an executable loaded. It makes me wonder what other options we can tweak when GDB is running that will cause the selftests to hit some unpredictable situations. > >> >>> >>> commit 427e4066afd13d1bf52c849849475f536e285d66 >>> Date: Thu May 20 09:16:41 2021 +0100 >>> >>> gdb/bfd: avoid crash when architecture is forced to csky or riscv >>> >>> However, this issue was not spotted as we currently only run 'maint >>> selftest' without an executable loaded. >>> >>> This commit extends the testsuite to run 'maint selftest' both with >>> and without an executable loaded into GDB. >>> >>> Currently, when no executable is loaded into GDB all of the selftest >>> pass (i.e. the fail count is 0), however, when running with an >>> executable loaded, I am seeing 1 failure (on an x86-64 GNU/Linux >>> host). >>> >>> This failure is from the ARM disassembler tests, it appears that the >>> disassembler somehow gets itself into a state where it thinks it is in >>> thumb mode; when running the same test without an executable loaded >>> this doesn't happen. >> >> Do you have more information about that? > > Not really, the problem is the 'force_thumb' static global in > opcode/arm-dis.c. When running without an executable loaded this > variable remains 'false'. When run with an executable loaded I'm > seeing this get set 'true' during an earlier test, then when we get to > the print_one_insn tests we end up disassembling in thumb mode. > > At this point I stopped looking, for exactly the reasons you raised > above. Thanks, that's useful to know.