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.129.124]) by sourceware.org (Postfix) with ESMTPS id AA38D3858C41 for ; Sat, 20 May 2023 06:31:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AA38D3858C41 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1684564310; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=V4IoRG9m8WWf6mqLYRrbybmiMYrIcSa2YX9dDd/0GOY=; b=byFkqfLo/d6ILis6m6QKKIq3FnPwJ0thDx3XZDWklKKr4YyeOrPCKZvSCX9I3Chn4os83e 0AOFzpYH1PSp1rvu9GiCY2yYRvqn+dgcAn8wF9D9jRz5HN4ChvMkXdU/3b6N4Y+F4fRDC5 iroT4+nBUx3kmlsWbTGtQNEoW6Zt7fA= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-519-tyMaNFauOMyxFToupaFtVg-1; Sat, 20 May 2023 02:31:48 -0400 X-MC-Unique: tyMaNFauOMyxFToupaFtVg-1 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-30479b764f9so1531052f8f.0 for ; Fri, 19 May 2023 23:31:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684564307; x=1687156307; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=V4IoRG9m8WWf6mqLYRrbybmiMYrIcSa2YX9dDd/0GOY=; b=UBwNNOoHU3YYGDNVybboe/cAFGJL2g2xBTfODAoQP0qDCHgvc37y+Zu8f1H2LOu8Cd MzmzOtmDRYaeonVYbvsonFup/Qo5asGz2lMe+CfaYU3uJkW+gxJxGSmE1qsgSYz8cuQ3 6pyEs0lxBIw5tpTyIimSREpz46uw5U4Zzj9vKpbuCZRmspN7zGOPI4kVzqLre43INbkV l4pOqNk4+gWFyr7v7FJhr9LIxVENzSobWOfdeqny1rhi6+wFcF2/8/Ju0KrMECNCunrx OkpNE766IIOIRvOSx+aIGKF5KQTlCHyMuX091tFrjRUhbR0Y0KhypiDOzBF0ic5POrL0 yLMw== X-Gm-Message-State: AC+VfDzIgG7ENmoeDgaQzBmsVCPfCgUUK8ESxiQ42/qP+QhwyOHx3n0n PmeurmHPOYI1akyPXPtTNq3ii5xEvtxU6De440oW7LkyEWUKyRSpsVth/Nxb5fZCes/0yCYIHNA HdXzxlOBhbEeOTWVU4gdM5JhcwXVU3C3Jp4V1aNhCAMNS/PP81cYrKg6tDbOYErYjwFp4unUgJd lbrGD82Q== X-Received: by 2002:a5d:4ed2:0:b0:309:3b52:4433 with SMTP id s18-20020a5d4ed2000000b003093b524433mr3103789wrv.25.1684564307421; Fri, 19 May 2023 23:31:47 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4d9h7EvxVdw9/c4phc40cHmI6sKqlPcP59J2ogRg5PpWHP9c8BbxX5uMt6MeyxMJG7UQByRw== X-Received: by 2002:a5d:4ed2:0:b0:309:3b52:4433 with SMTP id s18-20020a5d4ed2000000b003093b524433mr3103770wrv.25.1684564306982; Fri, 19 May 2023 23:31:46 -0700 (PDT) Received: from localhost (11.72.115.87.dyn.plus.net. [87.115.72.11]) by smtp.gmail.com with ESMTPSA id t5-20020a7bc3c5000000b003f4272c2d0csm1230316wmj.36.2023.05.19.23.31.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 19 May 2023 23:31:46 -0700 (PDT) From: Andrew Burgess To: Bruno Larsen via Gdb-patches , gdb-patches@sourceware.org Cc: Bruno Larsen Subject: Re: [PATCH] gdb/testsuite: change hardcoded assembly in gdb.arch/disp-step-insn-reloc.exp In-Reply-To: <87r0rc7zvm.fsf@redhat.com> References: <20230426132916.1988539-1-blarsen@redhat.com> <87r0rc7zvm.fsf@redhat.com> Date: Sat, 20 May 2023 07:31:45 +0100 Message-ID: <87o7mf8qem.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.7 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,T_SCC_BODY_TEXT_LINE 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: Andrew Burgess writes: > Bruno Larsen via Gdb-patches writes: > >> When testing gdb.arch.disp-step-insn-reloc.exp with clang in an x86_64 >> machine, the compiled test case would segfault when returning from >> the function can_relocate_call, with a suggestion of a broken stack. >> The example assembly in the commment was the following: >> >> f: >> MOV $1, %[ok] >> JMP end >> set_point0: >> CALL f ; tracepoint here. >> end: > > Honestly, I found the following paragraph really hard to grok. I had to > go look at the failure myself to see what was going on. Don't spend too > long on it, but it might be possible to make it a little clearer... > >> >> And the segmentation fault happening at the final "ret" instruction of >> the original function. > > s/the original function/the function cal_relocate_call/. > >> This suggests that gcc's compilation process would >> realize that no ret instruction ever happened after that call and doesn't > > s/after that call and/after the inner call and so/ > >> save the return address, while clang's process wouldn't. Looking at the > > Maybe: > > save the return address, while clang doesn't spot the missing 'ret' and > so does push the return address onto the stack. Looking at the > >> generated instructions, we can indeed see a difference: > >> >> clang's version: e8 f1 ff ff ff call 11a4 >> gcc's version: e8 f4 ff ff ff call 401125 >> >> Notice the difference on the second byte. > > Yes, but please, don't expect me to go read the x86-64 ISA manual -- > what does this difference mean? > >> >> Changing the assembly to use "ret" instead of "JMP end" does not change >> the behavior of the program and guarantees a compiler independent >> behavior. This commit does just that. > > The change itself is great, and I agree that this shouldn't change the > test. > > Feel free to tweak the commit message and push this. Actually, I retract this. Having thought about this a bit more I now worry that the explanation doesn't really explain what's going on. Is there really a call variant that doesn't push the return address? Even if that was the case, it is gas, not gcc that decides the encoding of the instruction, and gas isn't analysing the destination to decide if there's a ret or not. I suspect you fix is actually the right one, but I think this explanation is going in the wrong direction. Maybe if you reword things it might be clearer what's going on. Thanks, Andrew > > Approved-By: Andrew Burgess > > Thanks, > Andrew > >> --- >> gdb/testsuite/gdb.arch/insn-reloc.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/gdb/testsuite/gdb.arch/insn-reloc.c b/gdb/testsuite/gdb.arch/insn-reloc.c >> index f687c2c5631..365e6180057 100644 >> --- a/gdb/testsuite/gdb.arch/insn-reloc.c >> +++ b/gdb/testsuite/gdb.arch/insn-reloc.c >> @@ -49,10 +49,9 @@ fail (void) >> JMP set_point0 >> f: >> MOV $1, %[ok] >> - JMP end >> + RET >> set_point0: >> CALL f ; tracepoint here. >> - end: >> >> */ >> >> @@ -65,10 +64,9 @@ can_relocate_call (void) >> " jmp " SYMBOL (set_point0) "\n" >> "0:\n" >> " mov $1, %[ok]\n" >> - " jmp 1f\n" >> + " ret\n" >> SYMBOL (set_point0) ":\n" >> " call 0b\n" >> - "1:\n" >> : [ok] "=r" (ok)); >> >> if (ok == 1) >> -- >> 2.39.2