From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 966163856DC0 for ; Fri, 21 Oct 2022 15:46:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 966163856DC0 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 29LFkqbw016298 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 21 Oct 2022 11:46:57 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 29LFkqbw016298 Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 50A081E0D5; Fri, 21 Oct 2022 11:46:52 -0400 (EDT) Message-ID: Date: Fri, 21 Oct 2022 11:46:51 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [PATCH 1/2] gdb: check for empty offsets vector in inherit_abstract_dies Content-Language: en-US To: Tom Tromey , Simon Marchi via Gdb-patches References: <20221021132104.1772565-1-simon.marchi@polymtl.ca> <87czalchpi.fsf@tromey.com> From: Simon Marchi In-Reply-To: <87czalchpi.fsf@tromey.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 21 Oct 2022 15:46:52 +0000 X-Spam-Status: No, score=-3032.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, 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: Fri, 21 Oct 2022 15:47:05 -0000 On 2022-10-21 11:35, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi via Gdb-patches writes: > > Simon> + if (!offsets.empty ()) > Simon> + { > Simon> + std::sort (offsets.begin (), offsets.end ()); > Simon> + sect_offset *offsets_end = offsets.data () + offsets.size (); > Simon> + for (sect_offset *offsetp = offsets.data () + 1; > Simon> + offsetp < offsets_end; > Simon> + offsetp++) > Simon> + if (offsetp[-1] == *offsetp) > Simon> + complaint (_("Multiple children of DIE %s refer " > Simon> + "to DIE %s as their abstract origin"), > Simon> + sect_offset_str (die->sect_off), > Simon> + sect_offset_str (*offsetp)); > Simon> + } > > Simon> sect_offset *offsetp = offsets.data (); > Simon> + sect_offset *offsets_end = offsets.data () + offsets.size (); > Simon> die_info *origin_child_die = origin_die->child; > Simon> while (origin_child_die != nullptr && origin_child_die->tag != 0) > Simon> { > > This subsequent loop does stuff like: > > while (offsetp < offsets_end > && *offsetp < origin_child_die->sect_off) > offsetp++; > > which in this scenario would be comparing null pointers using "<"? Is it undefined behavior to compre two null pointers (both offsetp and offset_end would be nullptr or both wouldn't be nullptr)? At least clang's UB sanitizer doesn't panic. > I'm guessing this whole loop should be hoisted into the 'if'. I don't understand enough what the code does to be convinced this would be a correct change. Note that the following patch changes this to use iterators, so it makes the worry about comparing null pointers being undefined behavior go away. It becomes comparing offsets.end() with offsets.end(). Simon