From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) by sourceware.org (Postfix) with ESMTPS id 667093857B9A for ; Mon, 6 Jun 2022 16:42:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 667093857B9A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f54.google.com with SMTP id t13so20613399wrg.9 for ; Mon, 06 Jun 2022 09:42:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=B6bd4RDCRzGqyJ0IjSp5ayd+X2T2nkL3OrttBX3cfc8=; b=zMorn98mOvmcDijVQpKZY6rS7sgBr9vrvYa1SqPCO4rjLH3SAwkCA/dqYhyB7f3tfO 2ifB/kKwpVYv8JhRwMJ4/bU2LsPegR74p7lmoLFE3ujHR0PRW8KNMwgvUhFMRS3gs9nU DkRgV37SsL1kkIrlGyb7hCQAcywPFwtq0gCyST3JK2z1JvkoJrDGrTLd6Yq3bGo3GitS +1RT+jm2zgcQnNvBuX0GMiEyk8DnmUMAZ+PcgL0uNO6TS4jLEe0oNLWA6jIUTG15MTjV vyGWdAegvoMQlwzx5loDwAp3hG8E6PYTDC0FPp0zC+07aRaJ7VY9xRc8fNAJfL/imcJ6 uJog== X-Gm-Message-State: AOAM5315433KMHjNQr7JDew7RpHfBvvdYVT8Nh2/gKYBEQToc1bZnGMx 7faygUShmL+v0OYwsFNGLRiy6Z4TJJ4= X-Google-Smtp-Source: ABdhPJzRQFoTe2uD/P0kCie2iIgUZ2yuIvgknAkKbS0RKHbFrX3N2U04CkvHf4iFQ/iiQB8lag94iA== X-Received: by 2002:a5d:4886:0:b0:20d:527:f98b with SMTP id g6-20020a5d4886000000b0020d0527f98bmr23540763wrq.70.1654533740935; Mon, 06 Jun 2022 09:42:20 -0700 (PDT) Received: from ?IPV6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id n187-20020a1c27c4000000b0039c151298b7sm21551074wmn.10.2022.06.06.09.42.19 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 06 Jun 2022 09:42:20 -0700 (PDT) Message-ID: <950882da-c5d2-d95c-d12f-6a2547727729@palves.net> Date: Mon, 6 Jun 2022 17:42:19 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH] gdb: Workaround stringop-overread warning in debuginfod-support.c on powerpc64 Content-Language: en-US To: Mark Wielaard , Simon Marchi , gdb-patches@sourceware.org References: <20220511225219.119067-1-mark@klomp.org> <0f5c5a76969801002e5b2f9b224e6bdc58b41c1d.camel@klomp.org> <99342989-cf95-0194-bd61-1f41292e8c72@palves.net> From: Pedro Alves In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_NUMSUBJECT, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, 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: Mon, 06 Jun 2022 16:42:24 -0000 On 2022-06-06 12:27, Mark Wielaard wrote: > Hi, > > On Tue, 2022-05-31 at 12:51 +0200, Mark Wielaard wrote: >> On Tue, 2022-05-31 at 10:46 +0100, Pedro Alves wrote: >>> I wonder whether sticking in an assert so that the compiler >>> knows the size of array, would make the warning go away: >> >>> diff --git c/gdb/debuginfod-support.c w/gdb/debuginfod-support.c >>> index 6dc08fc29b6..d6fab39eac8 100644 >>> --- c/gdb/debuginfod-support.c >>> +++ w/gdb/debuginfod-support.c >>> @@ -187,6 +187,8 @@ debuginfod_is_enabled () >>> "from the following URLs:\n")); >>> >>> gdb::string_view url_view (urls); >>> + gdb_assert (url_view.size () < PTRDIFF_MAX); >>> + >>> while (true) >>> { >>> size_t off = url_view.find_first_not_of (' '); >> >> I tried by hand, but that does result in the same warning (with the >> DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD removed): > > So, since that didn't work, OK to push the original patch that simply > suppresses the diagnostic always? The code currently looks like: gdb::string_view url_view (urls); while (true) { size_t off = url_view.find_first_not_of (' '); if (off == gdb::string_view::npos) break; url_view = url_view.substr (off); #if defined (__s390x__) || defined (__powerpc64__) /* g++ 11.2.1 on s390x and g++ 11.3.1 on ppc64le seem convinced url_view might be of SIZE_MAX length. And so complains because the length of an array can only be PTRDIFF_MAX. */ DIAGNOSTIC_PUSH DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD #endif off = url_view.find_first_of (' '); #if defined (__s390x__) || defined (__powerpc64__) DIAGNOSTIC_POP #endif I suppose that the fact that the warning appears in the find_first_of call, and not the find_first_not_of call above was already an indication that the assert at that spot wouldn't work, otherwise, I'd think we'd be seeing a similar warning in the find_first_not_of call. I suspect that for the gdb_assert to work, it would need to be put at the same spot where the DIAGNOSTIC_PUSH is, after the url_view.substr. The theory being that substr returns a new string_view and it's that one that would need the assert. Like: gdb::string_view url_view (urls); while (true) { size_t off = url_view.find_first_not_of (' '); if (off == gdb::string_view::npos) break; url_view = url_view.substr (off); + gdb_assert (url_view.size () < PTRDIFF_MAX); off = url_view.find_first_of (' '); OTOH, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97185 suggests that the root issue is with not eliminating builtin calls when sizes are 0, so maybe the right thing to do would be to assert that the string_view isn't empty, like: diff --git c/gdb/debuginfod-support.c w/gdb/debuginfod-support.c index 6dc08fc29b6..9a4301f018c 100644 --- c/gdb/debuginfod-support.c +++ w/gdb/debuginfod-support.c @@ -193,17 +193,8 @@ debuginfod_is_enabled () if (off == gdb::string_view::npos) break; url_view = url_view.substr (off); -#if defined (__s390x__) || defined (__powerpc64__) - /* g++ 11.2.1 on s390x and g++ 11.3.1 on ppc64le seem convinced - url_view might be of SIZE_MAX length. And so complains - because the length of an array can only be PTRDIFF_MAX. */ - DIAGNOSTIC_PUSH - DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD -#endif + gdb_assert (!url_view.empty ()); off = url_view.find_first_of (' '); -#if defined (__s390x__) || defined (__powerpc64__) - DIAGNOSTIC_POP -#endif Anyhow, your patch is OK with me.