From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id zjQxBSXK5F9JPQAAWB0awg (envelope-from ) for ; Thu, 24 Dec 2020 12:04:37 -0500 Received: by simark.ca (Postfix, from userid 112) id 01F791F0AA; Thu, 24 Dec 2020 12:04:36 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id D7C441E99A for ; Thu, 24 Dec 2020 12:04:35 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4DB423898522; Thu, 24 Dec 2020 17:04:35 +0000 (GMT) Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by sourceware.org (Postfix) with ESMTPS id 168B0389851E for ; Thu, 24 Dec 2020 17:04:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 168B0389851E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x335.google.com with SMTP id a6so1938608wmc.2 for ; Thu, 24 Dec 2020 09:04:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=2xUFIL1DAkjHbBepuUSadhPC/xjTjdQNVQMM+P/KHV0=; b=GTphXL+8fyEHBj1+gykg+0u34E3edLqbn+r0P5zWuXvghjhAqq05FZDB3joFmW11Xk uMSrMn0uoh2nHIcJ38rKogkcICJ/dYiNxxtUuBZz2Ru45nRhvT04tZY55gnql569f+1G oFxu/lJGYiAgE+Ly4y9XaHzdSMFb3mBZNZ6Os7DHmOHxnqUPUUm4xYiFHk1y4N7obZDd mvDHs/VUaBidHVkLzv6xLBQ4EVayaQNzcW7ha7Lf7p7RgRwKeEwg21PJMnEc7zMo9/KX RFdKCYf+YEmoW/vAnNf2daYs4STujMJb+S1bqH4IuyqrF/ZEuOhwIL09s6/0EHau4thv ni9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=2xUFIL1DAkjHbBepuUSadhPC/xjTjdQNVQMM+P/KHV0=; b=eH909IkMTohGP5WbcL1syjyeOP/tyqnIQFuObBJ8Xa85nYYloXwTmeks8+W99sZmDY IUmDOQHOsClt3voidXmZirAKuvoMsRCOf77EYK1SzayhTCXAiJP/Js2oHh6FmEet8AnU /aHeN00YcaUuZT8QuvrE/HBN63GfrReiwHE77lFyqrmk+L11gOraWth/Q7qhP6qJ/oH4 Bpio2TzQybU+rIqlKN+nlM2TJnh7j7loAZi0CcqYvRFMOAGH60OsoCdqzybroMB0bHha 6DXjYnAi1okVo4wjHAAz3jY8CEN2BH/9rgfv+QzAhLNTaIixv4d6zWhURw4Z5tlQXLcz I6Fw== X-Gm-Message-State: AOAM531hwhnGnPLOVy/tavFfEfEIyK8zbOYWC2JloUDyC1cYgtgj2TrP yW83P+6LhRmsjgdAImY6RYZfoA== X-Google-Smtp-Source: ABdhPJycvXRK4hCFcVslkzeWyQFzVBet7/thPU+dRxZc6rf6wiouPcZ+rYWp0udWTNBmqbBrsaUQRw== X-Received: by 2002:a1c:2d92:: with SMTP id t140mr5225257wmt.114.1608829471104; Thu, 24 Dec 2020 09:04:31 -0800 (PST) Received: from localhost (host109-154-20-128.range109-154.btcentralplus.com. [109.154.20.128]) by smtp.gmail.com with ESMTPSA id o17sm41114136wrg.32.2020.12.24.09.04.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Dec 2020 09:04:30 -0800 (PST) Date: Thu, 24 Dec 2020 17:04:29 +0000 From: Andrew Burgess To: Simon Marchi Subject: Re: [PATCH 2/2] gdb: avoid resolving dynamic properties for non-allocated arrays Message-ID: <20201224170429.GO2945@embecosm.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 17:02:05 up 15 days, 21:46, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] 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: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" * Simon Marchi [2020-12-24 09:46:13 -0500]: > > > On 2020-12-23 6:53 p.m., Andrew Burgess wrote: > > In PR gdb/27059 an issue was discovered where GDB would sometimes > > trigger undefined behaviour in the form of signed integer overflow. > > The problem here is that GDB was reading random garbage from the > > inferior memory space, assuming this data was valid, and performing > > arithmetic on it. > > > > This bug raises an interesting general problem with GDB's DWARF > > expression evaluator, which is this: > > > > We currently assume that the DWARF expressions being evaluated are > > well formed, and well behaving. As an example, this is the expression > > that the bug was running into problems on, this was used as the > > expression for a DW_AT_byte_stride of a DW_TAG_subrange_type: > > > > DW_OP_push_object_address; > > DW_OP_plus_uconst: 88; > > DW_OP_deref; > > DW_OP_push_object_address; > > DW_OP_plus_uconst: 32; > > DW_OP_deref; > > DW_OP_mul > > > > Two values are read from the inferior and multiplied together. GDB > > should not assume that any value read from the inferior is in any way > > sane, as such the implementation of DW_OP_mul should be guarding > > against overflow and doing something semi-sane here. > > > > However, it turns out that the original bug PR gdb/27059, is hitting a > > more specific case, which doesn't require changes to the DWARF > > expression evaluator, so I'm going to leave the above issue for > > another day. > > Ok, so if I understand correctly, I could craft a DWARF expression that > makes an overflowing multiplication on purpose to hit that undefined > behavior bug, right? > > If so, it would be good to close 27059 with your fix and open a new bug > specifically for the fact that the DWARF expression evaluator does not > protect against multiplication overflows. Done, bug 27114. It's not just multiplication, but any arithmetic operator that might overflow (or have other undefined behaviour). We do already catch some of these cases, for example in scalar_binop we do check for divide by zero for example. > > > diff --git a/gdb/testsuite/gdb.dwarf2/dyn-type-unallocated.exp b/gdb/testsuite/gdb.dwarf2/dyn-type-unallocated.exp > > new file mode 100644 > > index 00000000000..60cf8abc899 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.dwarf2/dyn-type-unallocated.exp > > @@ -0,0 +1,122 @@ > > +# Copyright 2020 Free Software Foundation, Inc. > > + > > +# This program is free software; you can redistribute it and/or modify > > +# it under the terms of the GNU General Public License as published by > > +# the Free Software Foundation; either version 3 of the License, or > > +# (at your option) any later version. > > +# > > +# This program is distributed in the hope that it will be useful, > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +# GNU General Public License for more details. > > +# > > +# You should have received a copy of the GNU General Public License > > +# along with this program. If not, see . > > + > > +# Test for issue PR gdb/27059. The problem was that when resolving a > > +# dynamic type that was not-allocated GDB would still try to execute > > +# the DWARF expressions for the upper, lower, and byte-stride values. > > +# > > +# The problem is that, at least in some gfortran compiled programs, > > +# these values are undefined until the array is allocated. > > +# > > +# As a result, executing the dwarf expressions was triggering integer > > +# overflow in some cases. > > +# > > +# This test aims to make the sometimes occurring integer overflow a > > +# more noticeable error by creating an array that is always marked as > > +# not-allocated. > > +# > > +# The dwarf expressions for the various attributes then contains an > > +# infinite loop. If GDB ever tries to execute these expressions we > > +# will get a test timeout. With this issue fixed the expressions are > > +# never executed and the test completes as we'd expect. > > + > > +load_lib dwarf.exp > > + > > +if {![dwarf2_support]} { > > + return 0 > > +} > > + > > +standard_testfile .c -dw.S > > + > > +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { > > + return -1 > > +} > > + > > +set asm_file [standard_output_file $srcfile2] > > +Dwarf::assemble $asm_file { > > + cu {} { > > + global srcfile > > + > > + compile_unit { > > + {producer "gcc" } > > + {language @DW_LANG_Fortran90} > > + {name ${srcfile}} > > + {low_pc 0 addr} > > + } { > > + declare_labels array_type_label integer_type_label > > + > > + set int_size [get_sizeof "int" "UNKNOWN"] > > + set voidp_size [get_sizeof "void *" "UNKNOWN"] > > + > > + integer_type_label: DW_TAG_base_type { > > + {DW_AT_byte_size $int_size DW_FORM_sdata} > > + {DW_AT_encoding @DW_ATE_signed} > > + {DW_AT_name integer} > > + } > > + > > + array_type_label: DW_TAG_array_type { > > + {DW_AT_type :$integer_type_label} > > + {DW_AT_data_location { > > + DW_OP_push_object_address > > + DW_OP_deref > > + } SPECIAL_expr} > > + {DW_AT_allocated { > > + DW_OP_push_object_address > > + DW_OP_deref_size ${voidp_size} > > + DW_OP_lit0 > > + DW_OP_ne > > Can't this expression just be `DW_OP_lit0`, to make the array always > unallocated? Yes. I simplified this. > > I also looked at the fix itself, I can't really claim to understand > it perfectly, but nothing stood out as wrong to me, so LGTM. Thanks, I went ahead and merged this. Thanks, Andrew