From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id NxbkADf/omhDOAYAWB0awg (envelope-from ) for ; Mon, 18 Aug 2025 06:23:51 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=KkAwmo3b; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id E31DB1E0B3; Mon, 18 Aug 2025 06:23:50 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.8 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_LOW,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=no autolearn_force=no version=4.0.1 Received: from server2.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 ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 24F821E047 for ; Mon, 18 Aug 2025 06:23:50 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B38F5385843A for ; Mon, 18 Aug 2025 10:23:49 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B38F5385843A Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=KkAwmo3b Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id E68FB3858413 for ; Mon, 18 Aug 2025 10:23:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E68FB3858413 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E68FB3858413 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1755512588; cv=none; b=d1C/J2HGGjbIouBaNroq8A6gxwEn6uFgotpyUviglwiIgcsZQLuSsK510p4N4YFqIDKDZ6fy0ahE9w/FQGgM0JA5cKQqPmwwvUBtbb4OzLsBiS+bMh/jlic6MsZmYRbRwvQh/T/z5NdEtJuDsIVRQOhhaB9Xkd1TmhB9wiUvkYo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1755512588; c=relaxed/simple; bh=9nRS7VlNWvlg+t5ZVInpoT5CttbH3GyC1pfLNudnuKA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=unoN8QfHVPRXRqD55hEO0Isud2aZVztZB6q4x3v2Q/74JQRh0z62DWhvXJ6ATmUWfUzvvSWiNzE+dgKYjDAAVtvuoNLd7ULNDTWsG04xCbV+8u92Lv1TZeJWIeAxQTOlPLRzmb4BUpopWBz706L6bSwmMZWYJUrn4BVslbhO8Hg= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E68FB3858413 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1755512587; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=xyu6dhY+6mdKmo2MsMTJ7RyXATnasLqXciHy8OzNZVg=; b=KkAwmo3bVxAyfqO5mKDmiBlrrIZzXfu5rtdlUl290Oz7lF8UpYqrBL7UXY80l0GrMTtwZw jRf2SqyfRiKQuTcFexo/n2u138zwr8ORF294k6qad9fViDEsB8QaPyYWnzHIFQXgq7eeiI wxJVpX2iYzfhPtcP8/1oXLWZZJA/Srk= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-81-fIAuHKuPM7ytlWMLyZyTvw-1; Mon, 18 Aug 2025 06:23:06 -0400 X-MC-Unique: fIAuHKuPM7ytlWMLyZyTvw-1 X-Mimecast-MFC-AGG-ID: fIAuHKuPM7ytlWMLyZyTvw_1755512585 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-45a1ba17bbcso16106475e9.0 for ; Mon, 18 Aug 2025 03:23:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1755512585; x=1756117385; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=xyu6dhY+6mdKmo2MsMTJ7RyXATnasLqXciHy8OzNZVg=; b=UxRd7O8Ld0NOqf/zRDklZ8qae0KcGaUl15mdW2jPYAEfQzFHH9VADBRY1SW1HtJ4I2 veyTHGd6FlankT5GrSuxdOK6XEuiRnRGvcqnuU2PcgtyeeapbC0NKqwr96THseIl8DuE Kaze7MmrSiA46dmYUyKOrNV7zHstpu4cMNvL+EZttiw8YRX1U2tNy1I+dkXYTgdIVj5Z rjN6XYLL2miDwFc/ipjQ3KjQL5piEzXP3hAQ60fpeFKHwMavXpIRH6zvbwikVUoWDYmD ilab8beR5dTlKp2vb8Vh3aAjybTMA2jzhPV1Re45eXnT6uDK9Lvt2XKAQ3uidjfsKrLw m23A== X-Forwarded-Encrypted: i=1; AJvYcCXV7RcbgpWggnWGzZcZnWwC8AuTWuauuaKuh7WgCklJsrqH5CWl4aOaTRypCWNotLTYCMX84sXQHkIKrA==@sourceware.org X-Gm-Message-State: AOJu0YwTbLFlAreQ4xMQpu+XO5RViV52KBuZN1UNiov7iGR4llTGWizW e13Qh+vd/quleeA9fc0qLpTwaQMf73Y+fNuhVwPrkoxxxHGBpwGe/Ve06826ekXL+gedH3dWyN2 YrBu71PYhD+v12s+vLEzx79LTDjMqa+JklKO5gThWcpjdX/Q/JD/giSXqF6jWkos= X-Gm-Gg: ASbGncvorx+9Ud0NDjUum1qeyc+hH491qLGj4hiK+pabOxbuxPWxuB0Oa8dFsNfWDtV zaqtUXrSd35QZTFlMF7/3VK3eAG5EYkpdtKsyaVMzwuw8atzLVWJJdXKd/HF6dfGLZVGmts5CiB /T36U9RywSJTMp5VGGdCp56QYPLaRBnpTia3jhF8F46emBBfQO+mvb6JPgrcZ94EM5VizA1gzAu ka9d/h0IWYcfmxjDK12ygVXk11jaxLVIVLrWH/g3C3VI/8GKW8ao/WhAmRyLDBdcQankDtQ/C6C 5n0NZakox9vxbY2jO0L3qhsydsQAhMdftGI= X-Received: by 2002:a05:600c:b90:b0:459:eeaf:d6c7 with SMTP id 5b1f17b1804b1-45a218578fbmr82790605e9.26.1755512584917; Mon, 18 Aug 2025 03:23:04 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG0q6a5ulTFIZHJT7wYMsu6AMfnZvmto4PrwAsr3NCgD4CG5iwBIFtq4aekzJxaaQV10fS6Wg== X-Received: by 2002:a05:600c:b90:b0:459:eeaf:d6c7 with SMTP id 5b1f17b1804b1-45a218578fbmr82790445e9.26.1755512584436; Mon, 18 Aug 2025 03:23:04 -0700 (PDT) Received: from localhost ([31.111.84.207]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-45a1c6d89b9sm175242825e9.12.2025.08.18.03.23.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 Aug 2025 03:23:03 -0700 (PDT) From: Andrew Burgess To: Tom de Vries , gdb-patches@sourceware.org Subject: Re: [PATCH v2] [gdb] Handle EINTR in fgets In-Reply-To: <20250818083208.18860-1-tdevries@suse.de> References: <20250818083208.18860-1-tdevries@suse.de> Date: Mon, 18 Aug 2025 11:23:02 +0100 Message-ID: <87y0rh6i9l.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: l-KnBpaenGchKA3frq-B_eIjWLGVYy2rDAISlh6QlEg_1755512585 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Tom de Vries writes: > Usually, find_charset_names calls iconv -l to get the list of supported > charsets, allowing us to use a charset not in the default list: > ... > $ /usr/bin/gdb -q -batch -ex "set charset CP858" > $ > ... > > If the call to iconv -l fails somehow, gdb silently falls back to using the > default list, which gets us instead: > ... > $ PATH= /usr/bin/gdb -q -batch -ex "set charset CP858" > Undefined item: "CP858". > $ > ... > > PR gdb/33274 reports that gdb occasionally fails in the same way, because > fgets returns nullptr before the output of iconv -l is read entirely. > > We asked the reporter to try out a patch handling errno == EINTR after > fgets returns nullptr, and that fixed the problem. > > Fix this by: > - adding an inline function gdb::fgets that handles EINTR, and > - using it instead of fgets. > > Tested on x86_64-linux. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33274 > --- > gdb/charset.c | 3 ++- > gdb/linux-nat.c | 3 ++- > gdb/nat/linux-btrace.c | 3 ++- > gdb/nat/linux-osdata.c | 13 +++++++------ > gdb/nat/linux-procfs.c | 7 ++++--- > gdbserver/linux-i386-ipa.cc | 3 ++- > gdbsupport/eintr.h | 15 +++++++++++++++ > 7 files changed, 34 insertions(+), 13 deletions(-) > > diff --git a/gdb/charset.c b/gdb/charset.c > index 259362563b2..abf3fe800d2 100644 > --- a/gdb/charset.c > +++ b/gdb/charset.c > @@ -25,6 +25,7 @@ > #include "gdbsupport/environ.h" > #include "arch-utils.h" > #include > +#include "gdbsupport/eintr.h" > > #ifdef USE_WIN32API > #include > @@ -842,7 +843,7 @@ find_charset_names (void) > char *start, *r; > int len; > > - r = fgets (buf, sizeof (buf), in); > + r = gdb::fgets (buf, sizeof (buf), in); > if (!r) > break; > len = strlen (r); > diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c > index f3179279d92..39f1fb59ae0 100644 > --- a/gdb/linux-nat.c > +++ b/gdb/linux-nat.c > @@ -64,6 +64,7 @@ > #include "gdbsupport/scope-exit.h" > #include "gdbsupport/gdb-sigmask.h" > #include "gdbsupport/common-debug.h" > +#include "gdbsupport/eintr.h" > #include > > /* This comment documents high-level logic of this file. > @@ -4301,7 +4302,7 @@ linux_proc_pending_signals (int pid, sigset_t *pending, > if (procfile == NULL) > error (_("Could not open %s"), fname); > > - while (fgets (buffer, PATH_MAX, procfile.get ()) != NULL) > + while (gdb::fgets (buffer, PATH_MAX, procfile.get ()) != NULL) > { > /* Normal queued signals are on the SigPnd line in the status > file. However, 2.6 kernels also have a "shared" pending > diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c > index 9eadd46c9be..08c7b795a86 100644 > --- a/gdb/nat/linux-btrace.c > +++ b/gdb/nat/linux-btrace.c > @@ -26,6 +26,7 @@ > #include "gdbsupport/filestuff.h" > #include "gdbsupport/scoped_fd.h" > #include "gdbsupport/scoped_mmap.h" > +#include "gdbsupport/eintr.h" > > #include > > @@ -202,7 +203,7 @@ linux_determine_kernel_start (void) > uint64_t addr; > int match; > > - line = fgets (buffer, sizeof (buffer), file.get ()); > + line = gdb::fgets (buffer, sizeof (buffer), file.get ()); > if (line == NULL) > break; > > diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c > index b52a8ed5f36..bfdc26f2795 100644 > --- a/gdb/nat/linux-osdata.c > +++ b/gdb/nat/linux-osdata.c > @@ -35,6 +35,7 @@ > #include > #include > #include "gdbsupport/filestuff.h" > +#include "gdbsupport/eintr.h" > #include > #include "linux-procfs.h" > > @@ -562,7 +563,7 @@ linux_xfer_osdata_cpus () > > do > { > - if (fgets (buf, sizeof (buf), fp.get ())) > + if (gdb::fgets (buf, sizeof (buf), fp.get ())) > { > char *key, *value; > int i = 0; > @@ -780,7 +781,7 @@ print_sockets (unsigned short family, int tcp, std::string &buffer) > > do > { > - if (fgets (buf, sizeof (buf), fp.get ())) > + if (gdb::fgets (buf, sizeof (buf), fp.get ())) > { > uid_t uid; > unsigned int local_port, remote_port, state; > @@ -961,7 +962,7 @@ linux_xfer_osdata_shm () > > do > { > - if (fgets (buf, sizeof (buf), fp.get ())) > + if (gdb::fgets (buf, sizeof (buf), fp.get ())) > { > key_t key; > uid_t uid, cuid; > @@ -1057,7 +1058,7 @@ linux_xfer_osdata_sem () > > do > { > - if (fgets (buf, sizeof (buf), fp.get ())) > + if (gdb::fgets (buf, sizeof (buf), fp.get ())) > { > key_t key; > uid_t uid, cuid; > @@ -1137,7 +1138,7 @@ linux_xfer_osdata_msg () > > do > { > - if (fgets (buf, sizeof (buf), fp.get ())) > + if (gdb::fgets (buf, sizeof (buf), fp.get ())) > { > key_t key; > PID_T lspid, lrpid; > @@ -1231,7 +1232,7 @@ linux_xfer_osdata_modules () > > do > { > - if (fgets (buf, sizeof (buf), fp.get ())) > + if (gdb::fgets (buf, sizeof (buf), fp.get ())) > { > char *name, *dependencies, *status, *tmp, *saveptr; > unsigned int size; > diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c > index d4f9af32bb9..1927a21314e 100644 > --- a/gdb/nat/linux-procfs.c > +++ b/gdb/nat/linux-procfs.c > @@ -19,6 +19,7 @@ > #include "linux-procfs.h" > #include "gdbsupport/filestuff.h" > #include "gdbsupport/unordered_set.h" > +#include "gdbsupport/eintr.h" > #include > #include > #include > @@ -42,7 +43,7 @@ linux_proc_get_int (pid_t lwpid, const char *field, int warn) > return -1; > } > > - while (fgets (buf, sizeof (buf), status_file.get ())) > + while (gdb::fgets (buf, sizeof (buf), status_file.get ())) > if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':') > { > retval = strtol (&buf[field_len + 1], NULL, 10); > @@ -140,7 +141,7 @@ linux_proc_pid_get_state (pid_t pid, int warn, enum proc_state *state) > } > > have_state = 0; > - while (fgets (buffer, sizeof (buffer), procfile.get ()) != NULL) > + while (gdb::fgets (buffer, sizeof (buffer), procfile.get ()) != NULL) > if (startswith (buffer, "State:")) > { > have_state = 1; > @@ -317,7 +318,7 @@ linux_proc_tid_get_name (ptid_t ptid) > if (comm_file == NULL) > return NULL; > > - comm_val = fgets (comm_buf, sizeof (comm_buf), comm_file.get ()); > + comm_val = gdb::fgets (comm_buf, sizeof (comm_buf), comm_file.get ()); > > if (comm_val != NULL) > { > diff --git a/gdbserver/linux-i386-ipa.cc b/gdbserver/linux-i386-ipa.cc > index 17af6eb3610..7d6b9600c34 100644 > --- a/gdbserver/linux-i386-ipa.cc > +++ b/gdbserver/linux-i386-ipa.cc > @@ -21,6 +21,7 @@ > #include > #include "tracepoint.h" > #include "gdbsupport/x86-xstate.h" > +#include "gdbsupport/eintr.h" > #include "arch/i386-linux-tdesc.h" > #include "arch/x86-linux-tdesc-features.h" > > @@ -138,7 +139,7 @@ initialize_fast_tracepoint_trampoline_buffer (void) > return; > } > > - if (fgets (buf, IPA_BUFSIZ, f)) > + if (gdb::fgets (buf, IPA_BUFSIZ, f)) > sscanf (buf, "%llu", &mmap_min_addr); > > fclose (f); > diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h > index 04f86c8c3df..800cab561b4 100644 > --- a/gdbsupport/eintr.h > +++ b/gdbsupport/eintr.h > @@ -116,6 +116,21 @@ write (int fd, const void *buf, size_t count) > return gdb::handle_eintr (-1, ::write, fd, buf, count); > } > > +inline char* > +fgets (char *str, int count, FILE* stream) > +{ > + char *ret; > + > + do > + { > + errno = 0; > + ret = ::fgets (str, count, stream); > + } > + while (ret == nullptr && ferror (stream) && errno == EINTR); Having read through the v1 thread, I just want to make sure I'm clear on the motivation for not using handle_eintr: your concern is that errno might be set in the EOF case? I'm not arguing with your reading of the spec, but that does seem unlikely. So much so that I do think that's worth a comment explaining the logic here. It's also worth the comment explaining that we've never _seen_ such behaviour, just that we're worried such behaviour _might_ be in-spec. That comment could fill in the missing header comment for this function. Thanks, Andrew