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 0263D3858403 for ; Fri, 2 Feb 2024 11:05:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0263D3858403 Authentication-Results: sourceware.org; dmarc=pass (p=none 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 0263D3858403 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=1706871930; cv=none; b=vUs8FD0nwLxXm4+S44/9ItWHAvn6SRSFrDk4UEc1PwfFHJY3KFVzLzmmf/nELxVdWXREMtyzbXih0sGq0eMJ/9YjCHrmxPp7nJjxzQRbQCnxoQeRRVkw2RfD8vS9SebDNQr0m1mYYklrfRogXpBq+KaU2fQZGVPaCiASFEGNg0A= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706871930; c=relaxed/simple; bh=a+8Ooi499bMFwPB8JNZ75DgOWzfBybdW+ayZl9GsgKE=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=xx/leknfsB4UmEXnJM1TQRcUGPSeElIyFOkYF2vzroyNYmm1BnnrmJPvheYeqtIpRsi/bpel67NiesG3eriWUztZIyS6ruz82194dQpnhboABS1AZ94fraPaqYiit05HVkix+YucjxTHboSHaUvH/sHzol5wzlq+2LzWn/i4lMY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1706871927; h=from:from:reply-to:subject:subject: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=ryqA+IJF6utAwOQOX0XxWLiz1MVup49i2pw5+RyhEck=; b=AFSp0tiG2TP5z8hWxChV4MTC6E1ySBSbPx8RV3ISkGA3EeeMqeznA1jqWUfLeoiQbt3kQT /G3/wjaepoiHZqAEgrgRqnBp7IaRr1X68nRwZOCm0g92HgJbPWq/Vel7K6oPhdvxmRZ5OR qQTiaZXcwQjMJUtdxJMMTXUgEvHlXBw= 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-99-IiQZkn-xOe-RZzwvGF5DVQ-1; Fri, 02 Feb 2024 06:05:26 -0500 X-MC-Unique: IiQZkn-xOe-RZzwvGF5DVQ-1 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-40fcb6ea753so299175e9.1 for ; Fri, 02 Feb 2024 03:05:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706871924; x=1707476724; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ryqA+IJF6utAwOQOX0XxWLiz1MVup49i2pw5+RyhEck=; b=AsJpWRWyF4Rw9w70lQCbbcUWWHE/5F/rIbUCdglYeWlsZhOJP9OZz695BJgyvtbk4Z DpyV9Ce80LLbqKd4kD8p/xqtbDus26Y8iRVj/pIl3qzaKTImp6dIcMe5e0hKN8KGulB5 NmKwgnlpL7gZNQYb4VxmWLRoH47y4KvxfpXv5kNuvcM4sXtGVcRNlcDt2OvOPi2FLIm9 vqdA7hFOPxTVCJaB+TpKv1ixtG5jRLaxJlvXPFaPXJ/w68hZlIzhi1rZCv1iLt39Efrx 18oY3YHdh+s/YnZlMU1ojTwJkmSZBDzhCWYF9YV5fFgYhEFfO6c4iSUh1lgwrT4Mraw/ J6Xg== X-Gm-Message-State: AOJu0Yx6ZifelkmqOiq4u9mzgbVrV6r7mV9kuNl8pJm1/L/kq67x57Z1 YeHMYUfdbbQ1uAcx8m2TDQgwTyE67Wp817FnkJi71rnFgGm7rz3Wo++pW0ommyREIa9Ah8h4RRm /dLtxkbMnVq0BrdiEQmco+tI7kHAS1vJIVEImFnLuXVoZEc5kS3U5sk6BN2orlRPHd7A= X-Received: by 2002:a05:600c:3544:b0:40f:c1fc:e5b6 with SMTP id i4-20020a05600c354400b0040fc1fce5b6mr2728591wmq.38.1706871924700; Fri, 02 Feb 2024 03:05:24 -0800 (PST) X-Google-Smtp-Source: AGHT+IE/GiFtJ85uPTibNKQROE+IgMVruyrLG+HGk+rWhQVS1wwerHGWOlzw7hvK1rqR2FVygzB8mw== X-Received: by 2002:a05:600c:3544:b0:40f:c1fc:e5b6 with SMTP id i4-20020a05600c354400b0040fc1fce5b6mr2728580wmq.38.1706871924338; Fri, 02 Feb 2024 03:05:24 -0800 (PST) X-Forwarded-Encrypted: i=0; AJvYcCWc8MV/Fer/IuZX/bdTljeEq1yFV4pmtRdRN91rfuam3+sKndPAndzLl5vOe7ikTkZAOeVAB259LnL2lDEujsI3xSvjnbFcgs2vLg== Received: from [192.168.0.129] (ip-94-112-227-180.bb.vodafone.cz. [94.112.227.180]) by smtp.gmail.com with ESMTPSA id p2-20020a05600c1d8200b0040fc771c864sm1442287wms.14.2024.02.02.03.05.23 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 02 Feb 2024 03:05:23 -0800 (PST) Message-ID: <4a3c8c97-4ba4-402e-993a-d4725bfd04dc@redhat.com> Date: Fri, 2 Feb 2024 12:05:23 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] gdb/testsuite: make gdb.base/list-nodebug.exp pass without libc symbols To: Simon Marchi , gdb-patches@sourceware.org References: <20240130093029.170544-2-blarsen@redhat.com> <08392c1c-41e4-4011-aa68-a8d6c6321556@redhat.com> <46ff7883-17b2-490c-83aa-2c8e0bb1e95f@simark.ca> <815736c9-66fa-4181-8847-4c94c455a905@redhat.com> <0ad8a940-a810-427a-9a8e-dd15897be468@simark.ca> From: Guinevere Larsen In-Reply-To: <0ad8a940-a810-427a-9a8e-dd15897be468@simark.ca> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00,BODY_8BITS,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,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 List-Id: On 31/01/2024 22:18, Simon Marchi wrote: > On 1/31/24 09:49, Guinevere Larsen wrote: >> On 30/01/2024 20:51, Simon Marchi wrote: >>> On 1/30/24 11:48, Guinevere Larsen wrote: >>> What I was wondering when looking at it the other day is: why do we >>> need to call get_current_source_symtab_and_line at all in a case >>> where we don't care about the "current" location? >> "current" is a poorly defined term in this context. what the `.` >> argument means with "current" is point of execution of the inferior, >> what get_current_source_symtab_and_line means by "current" is the >> location that is being printed. >> >> I say this based on the fact that list with no arguments or with '+' >> use this "current" sal to continue printing the file. > Right, but "list ." does not the "current sal". I said "why do we need > to call get_current_source_symtab_and_line", but in fact it's > "set_default_source_symtab_and_line" that throws the exception, so it's > "set_default_source_symtab_and_line" that we should avoid calling if we > are not going to use it. > >>> Early in the function, we have: >>> >>> symtab_and_line cursal = get_current_source_symtab_and_line (); >>> >>> But in the `else if (arg[0] == '.')` portion of the function, it >>> looks like we override `cursal` with something else. So, it is >>> possible to call get_current_source_symtab_and_line only in the >>> scopes that actually require it? >> As I mentioned, this gets used by no arg and '+', but I'm not sure if >> it is necessary. We can define cursal only when first printing and for >> '.', using get_last_line_printed for the rest. >> >> That's besides the point, the important part is that this is the only >> location (other than '.' recently added) that is able to tell if the >> inferior has debuginfo, so we have to handle its error somewhere if we >> want to have a single message for all list errors. > I wrote the patch below to illustrate what I mean. With it, I at least > get the same message for "list ." on the list-nodebug executable, > regardless of if I have libc debug symbols or not: > > (gdb) list . > No debug information available to print source lines at current PC (0x55555555511d). > > It is still different than if I use "list +", with no debug symbols: > > (gdb) list + > No symbol table is loaded. Use the "file" command. > > Perhaps that's still not ideal, I don't know, but I think it's already > better than having two different messages for "list .", based on some > unrelated variable (the presence of some debug info for another part of > the program). This still has 2 different errors, if a user tries "list ." before starting the inferior. This is because we also hit the same error in set_default_symtab_and_line further down, when we check if a target doesn't have a stack. I have attempted to fix it inline, but I'm not really happy with it yet, I just ran out of time due to FOSDEM and a week of vacation. > > Simon > > From 292d16f94423758bb08cd14209941a92218878c0 Mon Sep 17 00:00:00 2001 > From: Simon Marchi > Date: Wed, 31 Jan 2024 14:18:59 -0500 > Subject: [PATCH] Test change to list_command > > Change-Id: Ia536108a0c69d105cb3c6868c53afb12bb9c4ba9 > --- > gdb/cli/cli-cmds.c | 37 ++++++++++++++++++++++++------------- > 1 file changed, 24 insertions(+), 13 deletions(-) > > diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c > index df11f956245c..1df632594e8e 100644 > --- a/gdb/cli/cli-cmds.c > +++ b/gdb/cli/cli-cmds.c > @@ -1236,37 +1236,39 @@ list_command (const char *arg, int from_tty) > /* Pull in the current default source line if necessary. */ > if (arg == NULL || ((arg[0] == '+' || arg[0] == '-' || arg[0] == '.') && arg[1] == '\0')) > { > - set_default_source_symtab_and_line (); > - symtab_and_line cursal = get_current_source_symtab_and_line (); > - > /* If this is the first "list" since we've set the current > source line, center the listing around that line. */ > if (get_first_line_listed () == 0 && (arg == nullptr || arg[0] != '.')) > { > - list_around_line (arg, cursal); > + set_default_source_symtab_and_line (); > + list_around_line (arg, get_current_source_symtab_and_line ()); By applying this change here:          source line, center the listing around that line. */ -      if (get_first_line_listed () == 0 && (arg == nullptr || arg[0] != '.')) +      if (get_first_line_listed () == 0) { -         set_default_source_symtab_and_line (); + try + { +             set_default_source_symtab_and_line (); + } +         catch (const gdb_exception &e) + { +             error (_("No debug information available to " +                      "print source lines at current PC")); + }           list_around_line (arg, get_current_source_symtab_and_line ()); } we get a single error for all list commands if there are absolutely no symtabs (which is what happens if the inferior hasn't started) and we get a single error with "list ." I'm not a fan of "current PC" when the inferior may not be executing,though. And with this change, we don't need a try_catch around the other set_default_symtab_and_line because we'll only reach it if we managed to print something before... I think, but i'm not sure. -- Cheers, Guinevere Larsen She/Her/Hers > } > > /* "l" and "l +" lists the next few lines, unless we're listing past > the end of the file. */ > else if (arg == nullptr || arg[0] == '+') > { > + set_default_source_symtab_and_line (); > + const symtab_and_line cursal = get_current_source_symtab_and_line (); > if (last_symtab_line (cursal.symtab) >= cursal.line) > print_source_lines (cursal.symtab, > source_lines_range (cursal.line), 0); > else > - { > - error (_("End of the file was already reached, use \"list .\" to" > - " list the current location again")); > - } > + error (_("End of the file was already reached, use \"list .\" to" > + " list the current location again")); > } > > /* "l -" lists previous ten lines, the ones before the ten just > listed. */ > else if (arg[0] == '-') > { > + set_default_source_symtab_and_line (); > + const symtab_and_line cursal = get_current_source_symtab_and_line (); > + > if (get_first_line_listed () == 1) > error (_("Already at the start of %s."), > symtab_to_filename_for_display (cursal.symtab)); > + > source_lines_range range (get_first_line_listed (), > source_lines_range::BACKWARD); > print_source_lines (cursal.symtab, range, 0); > @@ -1275,13 +1277,19 @@ list_command (const char *arg, int from_tty) > /* "list ." lists the default location again. */ > else if (arg[0] == '.') > { > + std::optional cursal; > if (target_has_stack ()) > { > /* Find the current line by getting the PC of the currently > selected frame, and finding the line associated to it. */ > frame_info_ptr frame = get_selected_frame (nullptr); > CORE_ADDR curr_pc = get_frame_pc (frame); > - cursal = find_pc_line (curr_pc, 0); > + cursal.emplace (find_pc_line (curr_pc, 0)); > + > + if (cursal->symtab == nullptr) > + error > + (_("No debug information available to print source lines at current PC (%s)."), > + paddress (get_frame_arch (frame), curr_pc)); > } > else > { > @@ -1289,11 +1297,14 @@ list_command (const char *arg, int from_tty) > location to the default (usually the main function). */ > clear_current_source_symtab_and_line (); > set_default_source_symtab_and_line (); > - cursal = get_current_source_symtab_and_line (); > + cursal.emplace (get_current_source_symtab_and_line ()); > + > + // Not sure if always true, just guessing. > + gdb_assert (cursal->symtab != nullptr); > } > - if (cursal.symtab == nullptr) > - error (_("No debug information available to print source lines.")); > - list_around_line (arg, cursal); > + > + list_around_line (arg, *cursal); > + > /* Set the repeat args so just pressing "enter" after using "list ." > will print the following lines instead of the same lines again. */ > if (from_tty) > > base-commit: 249e54204b13f9acdd5fbca355fed305e8595f31