From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id bkz/LuMco2i2SAYAWB0awg (envelope-from ) for ; Mon, 18 Aug 2025 08:30:27 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=WqTDOng7; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=DRoVxOqE; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=WqTDOng7; dkim=neutral header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=DRoVxOqE; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id B020C1E047; Mon, 18 Aug 2025 08:30:27 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, 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 970A61E047 for ; Mon, 18 Aug 2025 08:30:25 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 253063857833 for ; Mon, 18 Aug 2025 12:30:25 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 253063857833 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=WqTDOng7; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=DRoVxOqE; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=WqTDOng7; dkim=neutral header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=DRoVxOqE Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2a07:de40:b251:101:10:150:64:1]) by sourceware.org (Postfix) with ESMTPS id 3591C3858419 for ; Mon, 18 Aug 2025 12:29:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3591C3858419 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 3591C3858419 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a07:de40:b251:101:10:150:64:1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1755520186; cv=none; b=NQaOiAxlvXqw1SooKfQmAMcZbT3LqRqCnEqpRX+NpOoGiI2LBvPQU5M9lvoD1ojNk/jjhQxnWXxFHZDx0ebn3k5mIamjfFafKdhr1ia48vKJkIu1sukjVzrnUrbHQS7S4mjMykTnG9qSW+cP2IQP54/Ovybmux0ToIUeaygS3lo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1755520186; c=relaxed/simple; bh=3+lyobwimeQENreDKNWzW320ewthH6Dxgn1RMppPtJc=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature: Message-ID:Date:MIME-Version:Subject:To:From; b=TMEEM50whkzUOggPhMvTUsI4DOj2tN5Ih9WzptwOjyepZhFDeQzmmL+je/+I8JoaHm0fLoJ09797xjzr3OIHDb22RtJUPtGUvY5tQQHKgylLfv0Nous/Jp7u9BBTMatZxD91dNU7Sq84vZ0WHyWY9JMUcRAW83jQxonwXnrasT0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3591C3858419 Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 30381216EC; Mon, 18 Aug 2025 12:29:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1755520185; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=VhQb23A1Gm5nWdpXfqD26piX+yGV5nsUqCGu8o4UY40=; b=WqTDOng758zFwgE1UdqWps4KqRCd9ssi90wuJUMRuPJv+ERb9FVQ6XX0Vu3kl4SjzlzEUA DHPmR45nomHLDC7xJw+jfmAJPu2ehEl9X5kMpFha0NlNplxb7cWzPF4ZjAHroCZfm43VeE R/MCNfQMbCyIXz+UsFdMQ7ic8M4aTz8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1755520185; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=VhQb23A1Gm5nWdpXfqD26piX+yGV5nsUqCGu8o4UY40=; b=DRoVxOqEIhB1N2JJms7engEUR7LjVufzBmg3gaBe4UR8wVqMK7GSW2gEV+crHHnlODIDOr H9VGsSqf7KfLEKAg== Authentication-Results: smtp-out1.suse.de; dkim=pass header.d=suse.de header.s=susede2_rsa header.b=WqTDOng7; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=DRoVxOqE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1755520185; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=VhQb23A1Gm5nWdpXfqD26piX+yGV5nsUqCGu8o4UY40=; b=WqTDOng758zFwgE1UdqWps4KqRCd9ssi90wuJUMRuPJv+ERb9FVQ6XX0Vu3kl4SjzlzEUA DHPmR45nomHLDC7xJw+jfmAJPu2ehEl9X5kMpFha0NlNplxb7cWzPF4ZjAHroCZfm43VeE R/MCNfQMbCyIXz+UsFdMQ7ic8M4aTz8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1755520185; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=VhQb23A1Gm5nWdpXfqD26piX+yGV5nsUqCGu8o4UY40=; b=DRoVxOqEIhB1N2JJms7engEUR7LjVufzBmg3gaBe4UR8wVqMK7GSW2gEV+crHHnlODIDOr H9VGsSqf7KfLEKAg== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 1772413686; Mon, 18 Aug 2025 12:29:45 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id ICBMBLkco2jNdAAAD6G6ig (envelope-from ); Mon, 18 Aug 2025 12:29:45 +0000 Message-ID: <9bdfdcba-4256-45c1-b012-60c69b1272ed@suse.de> Date: Mon, 18 Aug 2025 14:29:47 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] [gdb] Handle EINTR in fgets To: Andrew Burgess , gdb-patches@sourceware.org References: <20250818083208.18860-1-tdevries@suse.de> <87y0rh6i9l.fsf@redhat.com> Content-Language: en-US From: Tom de Vries In-Reply-To: <87y0rh6i9l.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 30381216EC X-Rspamd-Action: no action X-Rspamd-Server: rspamd1.dmz-prg2.suse.org X-Spamd-Result: default: False [-4.51 / 50.00]; BAYES_HAM(-3.00)[100.00%]; NEURAL_HAM_LONG(-1.00)[-1.000]; R_DKIM_ALLOW(-0.20)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-0.20)[-1.000]; MIME_GOOD(-0.10)[text/plain]; MX_GOOD(-0.01)[]; RBL_SPAMHAUS_BLOCKED_OPENRESOLVER(0.00)[2a07:de40:b281:104:10:150:64:97:from]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; FUZZY_RATELIMITED(0.00)[rspamd.com]; RECEIVED_SPAMHAUS_BLOCKED_OPENRESOLVER(0.00)[2a07:de40:b281:106:10:150:64:167:received]; TO_DN_SOME(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_TLS_ALL(0.00)[]; RCPT_COUNT_TWO(0.00)[2]; SPAMHAUS_XBL(0.00)[2a07:de40:b281:104:10:150:64:97:from]; FROM_EQ_ENVFROM(0.00)[]; FROM_HAS_DN(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; RCVD_COUNT_TWO(0.00)[2]; TO_MATCH_ENVRCPT_ALL(0.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[imap1.dmz-prg2.suse.org:rdns,imap1.dmz-prg2.suse.org:helo,suse.de:dkim,suse.de:mid,suse.de:email]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; DKIM_TRACE(0.00)[suse.de:+] 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 On 8/18/25 12:23, Andrew Burgess wrote: > 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? > Hi Andrew, thanks for the review. Yes, that is my concern. > I'm not arguing with your reading of the spec, but that does seem > unlikely. Agreed. > 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. > Good point. I've added a comment in a v3 ( https://sourceware.org/pipermail/gdb-patches/2025-August/220017.html ). > That comment could fill in the missing header comment for this function. > Since it's a comment about the implementation of the function, I've added it in the function. [ The missing header comment seems to be a feature of all the functions in the file, probably because they're wrappers and the interface is defined by what they're wrapping, and there's only one difference in behaviour implemented by the wrapper, which is described in detail at the top of the file. ] Thanks, - Tom > Thanks, > Andrew >