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 A17543857B91 for ; Fri, 10 Jun 2022 18:24:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A17543857B91 Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-101-6vGAztPyO5CcP_9-QnwqiQ-1; Fri, 10 Jun 2022 14:24:14 -0400 X-MC-Unique: 6vGAztPyO5CcP_9-QnwqiQ-1 Received: by mail-wr1-f69.google.com with SMTP id n5-20020adf8b05000000b00219ece7272bso393895wra.8 for ; Fri, 10 Jun 2022 11:24:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:in-reply-to:references:date :message-id:mime-version; bh=hIqUw6ILfxjh/UXuONUWKHAa2ICouf0oUDwePWyFEOo=; b=seTxUlxbUlMJ9B5/pgVjJ2+lA5xMausUB4oB8p6FFOdyKUHhP0/T5A7GY29JCqHAwl kRx1W0hL9PF0PJuHphFj9pva8ojHmy7kvwy3s8iVrsud5UX7wS/M9OTa4asHuCqIPFPR T8uNxsLsIUrcXfUOXSz/G1j2CPiPmfXtbVFZvgoCudvN7o0bUMgaLiiWUCIV3j05HlAZ tZ6YI2bU39W+nTaCtTwlnXPl09EpsIzNCoQn0rWoXkasdP4+J85rSHGzGW1u6RDu3IQp +qE3b2M0AIXX8uOGCSnWQ9Rj5FEjMFKMlpjf3j68td3rjzfyHkxkX/KKlRLIHN0fbNdQ kV4w== X-Gm-Message-State: AOAM530GX3+P1VF1uwjaNFvbLsOl+lUqqjJCIMCk5c+B9IKlFitI58nz fxXlv0ydOmd8h9CnE8RxE0MoDQHFRhBpRAB3KZHZV0rRr/C/9axKMfhLoTqgwM8QyPTABpY6ZBb EcPiuDNGvllcBGHW+vWodGA== X-Received: by 2002:a05:600c:a4c:b0:39c:34d0:fd25 with SMTP id c12-20020a05600c0a4c00b0039c34d0fd25mr992190wmq.172.1654885452648; Fri, 10 Jun 2022 11:24:12 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw1oakYU4Xlv5jMR4fzY+MeMsmoASL6gUEE6JHF4+DNrgaI9qBP3NLAYGaM3g1OfzZWuga9aA== X-Received: by 2002:a05:600c:a4c:b0:39c:34d0:fd25 with SMTP id c12-20020a05600c0a4c00b0039c34d0fd25mr992172wmq.172.1654885452451; Fri, 10 Jun 2022 11:24:12 -0700 (PDT) Received: from localhost (host109-152-215-36.range109-152.btcentralplus.com. [109.152.215.36]) by smtp.gmail.com with ESMTPSA id q3-20020a7bce83000000b003976fbfbf00sm3710295wmj.30.2022.06.10.11.24.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jun 2022 11:24:11 -0700 (PDT) From: Andrew Burgess To: Bruno Larsen , gdb-patches@sourceware.org Subject: Re: [PATCH v3 04/14] change gdb.base/nodebug.c to not fail with clang In-Reply-To: <20220526151041.23223-5-blarsen@redhat.com> References: <20220526151041.23223-1-blarsen@redhat.com> <20220526151041.23223-5-blarsen@redhat.com> Date: Fri, 10 Jun 2022 19:24:10 +0100 Message-ID: <87v8t8e5rp.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=-10.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_BARRACUDACENTRAL, 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 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: Fri, 10 Jun 2022 18:24:17 -0000 Bruno Larsen via Gdb-patches writes: > Clang organizes the variables differently to gcc in the original version > of this code, leading to the following differences when testing > p (int*) &dataglobal + 1 > > gcc: > $16 = (int *) 0x404034 > > clang: > $16 = (int *) 0x404034 > > The code change to nodebug.c makes it so gcc and clang will place the > same variable under dataglobal8, generating the same output. > --- > > No change in v3. > > No change in v2. > > --- > gdb/testsuite/gdb.base/nodebug.c | 2 +- > gdb/testsuite/gdb.base/nodebug.exp | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/nodebug.c b/gdb/testsuite/gdb.base/nodebug.c > index c7bc93991b8..572255697bb 100644 > --- a/gdb/testsuite/gdb.base/nodebug.c > +++ b/gdb/testsuite/gdb.base/nodebug.c > @@ -3,8 +3,8 @@ > > /* Test that things still (sort of) work when compiled without -g. */ > > -int dataglobal = 3; /* Should go in global data */ > static int datalocal = 4; /* Should go in local data */ > +int dataglobal = 3; /* Should go in global data */ > int bssglobal; /* Should go in global bss */ > static int bsslocal; /* Should go in local bss */ Something about just reordering the symbols like this makes me a little uncomfortable. I spent some time thinking about this test, wondering what the point of the check was. The test was added in commit: commit 46a4882b3c7d9ec981568b8b13a3c9c39c8f8e61 Date: Mon Sep 4 20:21:15 2017 +0100 Stop assuming no-debug-info variables have type int But there's no hint what this specific test was for. My best guess is that we're checking that the '+ 1' correctly increases the address by the size of an int. With that in mind, I wonder if the label after the address is really that important. I mean, right not it kind-of acts to confirm that we increased the address by the correct amount, but, we could achieve that same goal by just comparing addresses. And so, I wonder about the patch below instead? Instead of looking for a specific label I now accept any label. I then add an extra test afterwards that checks that '+ 1' increases the address by the size of an int. What do you think of this? Thanks, Andrew --- diff --git a/gdb/testsuite/gdb.base/nodebug.exp b/gdb/testsuite/gdb.base/nodebug.exp index 0dbd0ad153e..a4db483d6af 100644 --- a/gdb/testsuite/gdb.base/nodebug.exp +++ b/gdb/testsuite/gdb.base/nodebug.exp @@ -174,6 +174,7 @@ if [nodebug_runto inner] then { set unk_type_re "has unknown type.*to its declared type" set ptr_math_re "Cannot perform pointer math on incomplete type \"$data_var_type\", try casting to a known type, or void \\*\\." set not_mem_re "Attempt to take address of value not located in memory\\." + set any_label_regexp "<\[^>\]+>" set dataglobal_unk_re "dataglobal.*$unk_type_re" @@ -187,7 +188,7 @@ if [nodebug_runto inner] then { {"dataglobal + 1" "" $dataglobal_unk_re $dataglobal_unk_re} {"&dataglobal" "" "\\($data_var_type \\*\\) $hex " " = $data_var_type \\*"} {"&dataglobal + 1" "" $ptr_math_re $ptr_math_re} - {"(int *) &dataglobal + 1" "" " = \\(int \\*\\) $hex " "int \\*"} + {"(int *) &dataglobal + 1" "" " = \\(int \\*\\) $hex $any_label_regexp" "int \\*"} {"&(int) dataglobal + 1" "" $not_mem_re $not_mem_re} {"&dataglobal, &dataglobal" "" "\\($data_var_type \\*\\) $hex " " = $data_var_type \\*"} {"*dataglobal" "" $dataglobal_unk_re $dataglobal_unk_re} @@ -218,7 +219,14 @@ if [nodebug_runto inner] then { gdb_test "whatis $exp" $whatis gdb_test "ptype $exp" $whatis } - + + # Check that pointer arithmetic works as expected. + set addr1 [get_hexadecimal_valueof "&dataglobal" "*UNKNOWN*"] + set addr2 [get_hexadecimal_valueof "(int *) &dataglobal + 1" "*UNKNOWN*"] + set offset [expr $addr2 - $addr1] + set int_size [get_integer_valueof "sizeof (int)" "*UNKNOWN*"] + gdb_assert { $offset == $int_size } + # The only symbol xcoff puts out for statics is for the TOC entry. # Possible, but hairy, for gdb to deal. Right now it doesn't, it # doesn't know the variables exist at all.