From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55626 invoked by alias); 1 Oct 2019 19:30:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 55483 invoked by uid 89); 1 Oct 2019 19:30:16 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.0 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy= X-HELO: mail-ot1-f65.google.com Received: from mail-ot1-f65.google.com (HELO mail-ot1-f65.google.com) (209.85.210.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 01 Oct 2019 19:30:14 +0000 Received: by mail-ot1-f65.google.com with SMTP id y39so12606393ota.7 for ; Tue, 01 Oct 2019 12:30:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=jsggm+YDE/UIP7afKjmP6Y4dwzCnbECr19g7qc1Ab/Q=; b=dLvGVInVlrcMMFgHvDbKovgeip34P6B/SOMm/VCQi/KSlw7eLv+lwr829opIwEdQfS OM2t4F5lp2zugiuS7cah5mvCBYUr9vN4VmFQyctle6mJnVggQhMY/Q1XcEr6/aWP0hNU w8uvrSVmf2IXeMGH9aBe2ZJsQYuLZc8t0UthVPC+WteOLrdLrLT2XNDAiBpz0FosFCG9 4h2dJ1FTv8FOKgkgO14AkqB36VtJZOIz4UK9YPYd0nXkV4iC6AHI6RAJa4mCIza5ksT2 tqhdbfsLX2o2QbJnZDAvEQY1JxO3DYXExt5pBsMVCUDKXtDVO/KQXuhRLf6mcL62P8eZ TGTw== MIME-Version: 1.0 References: <20191001173345.48753-1-cbiesinger@google.com> In-Reply-To: From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Tue, 01 Oct 2019 19:30:00 -0000 Message-ID: Subject: Re: [PATCH] Change some arguments to gdb::string_view instead of name+len To: Pedro Alves Cc: gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-10/txt/msg00037.txt.bz2 On Tue, Oct 1, 2019 at 2:10 PM Pedro Alves wrote: > > On 10/1/19 7:27 PM, Christian Biesinger via gdb-patches wrote: > > On Tue, Oct 1, 2019 at 1:23 PM Pedro Alves wrote: > >> > >> On 10/1/19 6:33 PM, Christian Biesinger via gdb-patches wrote: > >>> - if (linkage_name[len] != '\0') > >>> + /* Don't use string_view::operator[] because we are accessing beyond > >>> + the size of the string_view, which is technically unsupported. */ > >>> + if (linkage_name.data ()[linkage_name.length ()] != '\0') > >>> { > >>> char *alloc_name; > >> > >> It's more than just unsupported, it's undefined behavior. If we're promising > >> the string_view interface, then it's supposedly valid to pass in a string_view > >> that happens to point just at the end of a page, with the one-past-the-end > >> byte living in an unmapped page. Dereferencing the one-past-end byte in > >> that case SIGSEGVs. > > > > That's true (though also a pre-existing issue). > > > >>> - if (ms_type == mst_file_text && startswith (name, "__gnu_compiled")) > >>> + if (ms_type == mst_file_text && startswith (name.data (), "__gnu_compiled")) > >>> return (NULL); > >>> > >> > >> This, via startswith also assumes that name.data() is a null-terminated > >> string. > > > > Ah yes. I'll add a startswith version that takes string_views. > > > >> I wonder whether we should have a zstring_view type. like string_view, but > >> assumes/requires null-terminated. > > > > How does that solve anything? This function can (apparently) take > > non-null terminated strings, so zstring_view wouldn't work? > > Ah, right. > > Hmm. > > I wonder then, I assume that the caller up the stack should know whether > the string was originally null terminated? I wonder about tweaking the > interface to pass that info down somehow. Actually, let me come at this from a different direction... I looked at all the callers. In every case, they either passed a nullterminated string or a substring of an existing string. In other words, accessing name[length] was always valid, even if the string_view wasn't nullterminated. In light of that, how do you feel about just documenting that callers have to pass a string view where accessing [length] is guaranteed not to be an invalid memory access? Thanks, Christian