From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 1040338582A1 for ; Thu, 28 Sep 2023 15:23:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1040338582A1 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695914607; 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=43k6C743DvZlg4cPGMGIARxCWcBAluQ1cKlj3MHSebo=; b=gQWVV0CC+tc/RO00viIgNnBooab1NscaNckGwwqHGZluYDXh5V51EA1s2sDixiDzW20Yzs e0dITtiFMTwe2lPrl8Ag8JVd65x8GzZet+SPQ8ukjOVrXjDo0P6C1e4UDkHhrVuA3ttkDW v+Ws+d87MDYDfU0r+bFz2bvErD+K53w= Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-607-hi2NKeIoNquG5wkaAbXMmw-1; Thu, 28 Sep 2023 11:23:19 -0400 X-MC-Unique: hi2NKeIoNquG5wkaAbXMmw-1 Received: by mail-qt1-f198.google.com with SMTP id d75a77b69052e-4180c46f2abso175929891cf.1 for ; Thu, 28 Sep 2023 08:23:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695914597; x=1696519397; 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=43k6C743DvZlg4cPGMGIARxCWcBAluQ1cKlj3MHSebo=; b=G5dXEDy0scJ4xR83jvzEEEcPWfKuADVI3V/wryX6L4R3T41bI3HuMUEJ+Jz2YMwjFr aA5vgOcHmo8YPomcs/hWdmbC2nPjHZKmAS4nl00QnBO5xt+goi6YFupdzfqF7wnxt6X1 ia8IHLMK6RCDSrzxP2wL3dsveEDgj0rWxDxvRUWIcUNO0a+WmNuIndG5gHTmJY01/Z2W ePuU/Bnm0v7aWTx5a8LA5zKluygogJ2iILbrDmUtfR4L2O6bmtRqP6oDG4jSH6RwLXxn lbnGtC1v+rT3DFbDrDqDRF0OsdO9zfIjmqe230Y4f9oqIoxzvbMIWDNGdVQF7kVbD+Ar znTQ== X-Gm-Message-State: AOJu0Yzagi+6hyN5pgPkwfJri0NxgxBQL5bstqmCqp3LBnUDthCKSqCv PJNyd6NFL+BgmPVqA79AML1AdXdVW/DG9lVFiSVGbitB2zO9bZ2UGPH9/L3kDYzxw9Vbee5SzTd UuDhsyF6SfVqz/NL1D6rXMmuF9G1C9Q== X-Received: by 2002:ac8:59c3:0:b0:413:5dbd:a926 with SMTP id f3-20020ac859c3000000b004135dbda926mr1847522qtf.2.1695914597241; Thu, 28 Sep 2023 08:23:17 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGOaaq+YzrN75ve+8PatoPrY2kt9W1XimgmVcxal3XrSik/x8em+2kADyZmP6NXcRp+qDh7NA== X-Received: by 2002:ac8:59c3:0:b0:413:5dbd:a926 with SMTP id f3-20020ac859c3000000b004135dbda926mr1847505qtf.2.1695914596917; Thu, 28 Sep 2023 08:23:16 -0700 (PDT) Received: from localhost ([31.111.84.209]) by smtp.gmail.com with ESMTPSA id w8-20020ac843c8000000b00418122186ccsm3705810qtn.12.2023.09.28.08.23.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Sep 2023 08:23:16 -0700 (PDT) From: Andrew Burgess To: Tom Tromey , Andrew Burgess via Gdb-patches Subject: Re: [PATCH 9/9] gdb: use reopen_exec_file from reread_symbols In-Reply-To: <87msxi8dcv.fsf@tromey.com> References: <87msxi8dcv.fsf@tromey.com> Date: Thu, 28 Sep 2023 16:23:14 +0100 Message-ID: <877coawcd9.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,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 List-Id: Tom Tromey writes: >>>>>> "Andrew" == Andrew Burgess via Gdb-patches writes: > > Andrew> + reopen_exec_file (); > > This addition could probably use a comment explaining why it is > needed. I added a comment. > > Andrew> - new_modtime = new_statbuf.st_mtime; > Andrew> + long new_modtime = new_statbuf.st_mtime; > > Might as well change this to time_t now. I fixed this. > > I don't really understand how reread_symbols interacts with target: > files. It seems like this is just broken. Yup. > > At AdaCore we carry a patch that changes this code to use bfd_stat. > According to internal notes, this was pushed: > > https://sourceware.org/legacy-ml/gdb-patches/2020-01/msg00386.html > > ... but I don't see it in gdb. Maybe we never resolved the 'stat' issue > in gnulib? I guess not. I already posted a series to look at this: https://inbox.sourceware.org/gdb-patches/cover.1695824275.git.aburgess@redhat.com/ But after reviewing your series I've updated this: https://inbox.sourceware.org/gdb-patches/cover.1695909469.git.aburgess@redhat.com/ I didn't fully understand all the discussion w.r.t. gnulib stat vs system stat, but I'm hoping the change above, which is less that your originally proposed full change, might be mergable. Though even after that series reopen_exec_file is still broken for target: files, I guess I'll see how the above goes before fixing that too. > Anyway, the reason I bring this up is that reopen_exec_file calls > bfd_cache_close_all -- but then the loop in reread_symbols, in the > bfd_stat case, will reopen the files (BFD uses fstat). This seems > unfortunate. > > I don't think gdb has a very clear idea of when bfd_cache_close_all > ought to be called. It would be good to clear this up. I'm not very > clear on it myself. Maybe it is to avoid ETXTBUSY errors -- in which > case it seems like it should be called just before starting the > inferior. But, ISTR some error on Windows as well, though I don't know > exactly what... so maybe the files need to be guaranteed-closed in other > situations as well. I agree this stuff doesn't seem well defined at all, but I don't think I made anything particularly worse in this respect, so I'm leaving that as a problem for the future. I did take a quick look at when bfd_cache_close_all is currently called, and we do have a call in symbol_file_add_with_addrs, which is used when a solib is loaded -- which means in many cases, when an inferior starts we will end up calling bfd_cache_close_all anyway. Note: I'm not arguing that this is an actual well designed solution, just that in many cases we're not going to hit any problems because of this. Thanks, Andrew