From patchwork Mon Dec 4 19:29:32 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 81300 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AD40C386D61E for ; Mon, 4 Dec 2023 19:33:25 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 4DB153858C41 for ; Mon, 4 Dec 2023 19:33:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4DB153858C41 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 4DB153858C41 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701718391; cv=none; b=TuJpS6q+7U8m1fQp/xOQko6a4tcPD6KdTo11ZxSj92ai3cshWi+oZXdyrpvy5qPgodRC28vs07+30XFqDEy5irmlaG0dnJfGJz7QI5hbAHSytZeEfz5205XDvw+NnmiKcJ+VH3P3/Y6QO1TBx6xhsVivgsxSpT+j34yFOw0txKo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701718391; c=relaxed/simple; bh=MvRzxoOp6aLkZkNH3R9/sCmlKBbfa5/nKpwwijm7XKg=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=QaomWBxtdzYxqvCUk764ARQ9O+oNEcyR7/uToW4KEeCcU8o/BMWwTQmpevct4OQTn+TqZEQSdni7C3VS+3GtJJP3zjrzm4JBj8rAJW+TIsWCSqWIVsYho1EvOTu9ai/JfnvfmaDvXQsyXzEbzBZTBa0C7VQiJl4D3vqUcWUZQW0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701718380; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=s0F9csuZx+ZirBC2ugS2ilUvJ1U/NBkUSm9wmhjDZDs=; b=bFt2yndMrMDsNbrTTmTGieLR5ehKV1rau2NaDcJclf9gfEPtnSi8KhErXJfcus80kKBpLH VX8MGiBg/ljledScXAO4lQ7j9lV3uD5c8xHOOLttB7vuCRxp8UPlZsDp877EFZ5SDmzBVI jAoM8ZT3UoTYPpak3YWbIErcMgthmA8= Received: from mail-ej1-f72.google.com (mail-ej1-f72.google.com [209.85.218.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-21-baJBTVpzOH6PecQnJOjkDw-1; Mon, 04 Dec 2023 14:29:39 -0500 X-MC-Unique: baJBTVpzOH6PecQnJOjkDw-1 Received: by mail-ej1-f72.google.com with SMTP id a640c23a62f3a-a00d0011a3bso359324266b.2 for ; Mon, 04 Dec 2023 11:29:39 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701718178; x=1702322978; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=s0F9csuZx+ZirBC2ugS2ilUvJ1U/NBkUSm9wmhjDZDs=; b=cg0B72S7WxPPg41HtiMGEGquP4jP891Uy7spv4KMc/vG060xDXSpmzB6C15HdgwPkz SQO2SoIIw/m216lFaxU1beh2MtGuDhEEO9Fn2LdwphRTjWiGNdFJQi7MbG9D4ta8u+w9 Ci87XiRJa146WSsHxi1GlNE8e6OkBOr/37QgJDTUQghOyFn4PLoQR2pLOjy5TCg6cc9i mPYdiiOcd4nuZ9DGGl+PfeJtGHpDduBHAWB0OHpQK1ikfjHlTInM2tURw9bB/J/15omZ FyG+pqCA5jFHVgb8NGjSWDztlfdRpoWfUSNiwlrJzi5DLaK4Nv/0aU51jRpEE3cpWgmy aKZw== X-Gm-Message-State: AOJu0YzTcxCwF+705kXhXb198wc6uciy9/7YcMBFqyfAZO85eDxramKY PabFOx+dbblRL9B4JOaPH/1CgtIQu6gr6SeClTLXCHX3ZNNOTmWBm6yUxcBhx39nGzsEZLkrxBM QIut3rJXrKio+G9c9+34GKDWwgPoWfm1PbXUroTpW3l2nrV/KdjsG0lW5eUMmcxlkjWkS6Olz8b PmQ26d3g== X-Received: by 2002:a17:906:4144:b0:a19:a19b:c735 with SMTP id l4-20020a170906414400b00a19a19bc735mr3954742ejk.133.1701718177847; Mon, 04 Dec 2023 11:29:37 -0800 (PST) X-Google-Smtp-Source: AGHT+IGrQXLcsTZwVHBevKjDGftSEjcNle96nKqL2/mnCKqpwV2G3Z3cX476s1FLxlFdwKE0D/+BCQ== X-Received: by 2002:a17:906:4144:b0:a19:a19b:c735 with SMTP id l4-20020a170906414400b00a19a19bc735mr3954719ejk.133.1701718176624; Mon, 04 Dec 2023 11:29:36 -0800 (PST) Received: from localhost (92.40.184.7.threembb.co.uk. [92.40.184.7]) by smtp.gmail.com with ESMTPSA id i21-20020a170906251500b009ff1997ce86sm5689674ejb.149.2023.12.04.11.29.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Dec 2023 11:29:36 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH] gdb: improve error reporting for 'save gdb-index' Date: Mon, 4 Dec 2023 19:29:32 +0000 Message-Id: <3182b7a3eb3764cf62a8bbff1318808188fdf261.1701718143.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_ABUSEAT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, RCVD_IN_SBL_CSS, 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 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+patchwork=sourceware.org@sourceware.org While making recent changes to 'save gdb-index' command I triggered some errors -- of the kind a user might be expected to trigger if they do something wrong -- and I didn't find GDB's output as helpful as it might be. For example: $ gdb -q /tmp/hello.x ... (gdb) save gdb-index /non_existing_dir Error while writing index for `/tmp/hello': mkstemp: No such file or directory. That the error message mentions '/tmp/hello', which does exit, but doesn't mention '/non_existing_dir', which doesn't is, I think, confusing. Also, I find the 'mkstemp' in the error message confusing for a user facing error. A user might not know what mkstemp means, and even if they do, that it appears in the error message is an internal GDB detail. The user doesn't care what function failed, why want to know what was wrong with their input, and what they should do to fix things. Similarly, for a directory that does exist, but can't be written to: (gdb) save gdb-index /no_access_dir Error while writing index for `/tmp/hello': mkstemp: Permission denied. In this case, the 'Permission denied' might make the user thing there is a permissions issue with '/tmp/hello', which is not the case. After this patch, the new errors are: (gdb) save gdb-index /non_existing_dir Error while writing index for `/tmp/hello': `/non_existing_dir': No such file or directory. and: (gdb) save gdb-index /no_access_dir Error while writing index for `/tmp/hello': `/no_access_dir': Permission denied. we also have: (gdb) save gdb-index /tmp/not_a_directory Error while writing index for `/tmp/hello': `/tmp/not_a_directory': Is not a directory. I think these do a better job of guiding the user towards fixing the problem. I've added a new test that exercises all of these cases, and also checks the case where a user tries to use an executable that already contains an index in order to generate an index. Nothing that worked correctly before this patch should give an error after this patch; I've only changed the output when the user was going to get an error anyway. Reviewed-By: Tom de Vries --- gdb/dwarf2/index-write.c | 10 +- gdb/testsuite/gdb.base/gdb-index-err.c | 22 +++++ gdb/testsuite/gdb.base/gdb-index-err.exp | 118 +++++++++++++++++++++++ 3 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 gdb/testsuite/gdb.base/gdb-index-err.c create mode 100644 gdb/testsuite/gdb.base/gdb-index-err.exp base-commit: 2c4caca9873ef4e97a51a538dfbecde8fe334082 diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c index b4a0117330e..4e5490e734e 100644 --- a/gdb/dwarf2/index-write.c +++ b/gdb/dwarf2/index-write.c @@ -1569,6 +1569,13 @@ struct index_wip_file index_wip_file (const char *dir, const char *basename, const char *suffix) { + /* Validate DIR is a valid directory. */ + struct stat buf; + if (stat (dir, &buf) == -1) + perror_with_name (string_printf (_("`%s'"), dir).c_str ()); + if ((buf.st_mode & S_IFDIR) != S_IFDIR) + error (_("`%s': Is not a directory."), dir); + filename = (std::string (dir) + SLASH_STRING + basename + suffix); @@ -1577,7 +1584,8 @@ struct index_wip_file scoped_fd out_file_fd = gdb_mkostemp_cloexec (filename_temp.data (), O_BINARY); if (out_file_fd.get () == -1) - perror_with_name (("mkstemp")); + perror_with_name (string_printf (_("couldn't open `%s'"), + filename_temp.data ()).c_str ()); out_file = out_file_fd.to_file ("wb"); diff --git a/gdb/testsuite/gdb.base/gdb-index-err.c b/gdb/testsuite/gdb.base/gdb-index-err.c new file mode 100644 index 00000000000..3a264f239ed --- /dev/null +++ b/gdb/testsuite/gdb.base/gdb-index-err.c @@ -0,0 +1,22 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2023 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +int +main () +{ + return 0; +} diff --git a/gdb/testsuite/gdb.base/gdb-index-err.exp b/gdb/testsuite/gdb.base/gdb-index-err.exp new file mode 100644 index 00000000000..9493c3aafd2 --- /dev/null +++ b/gdb/testsuite/gdb.base/gdb-index-err.exp @@ -0,0 +1,118 @@ +# Copyright 2023 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test some error messages that can arise from the 'save gdb-index' +# command. + +standard_testfile + +if {[build_executable "build executable" $testfile $srcfile] == -1} { + return -1 +} + +clean_restart $binfile + +# This test isn't going to work when the board file automatically adds +# an index section, or if the debug information is split into a +# separate objfile. +set has_cooked_index false +gdb_test_multiple "maint print objfiles ${binfile}" "check debug style" -lbl { + -re "\r\n\\.gdb_index: version ${decimal}(?=\r\n)" { + gdb_test_lines "" $gdb_test_name ".*" + } + -re "\r\n\\.debug_names: exists(?=\r\n)" { + gdb_test_lines "" $gdb_test_name ".*" + } + -re "\r\n(Cooked index in use:|Psymtabs)(?=\r\n)" { + set has_cooked_index true + gdb_test_lines "" $gdb_test_name ".*" + } + -re ".gdb_index: faked for \"readnow\"" { + gdb_test_lines "" $gdb_test_name ".*" + } + -re -wrap "" { + # Have no debug information! + } +} + +if { !$has_cooked_index } { + unsupported "cannot test without a cooked index" + return -1 +} + + +# The name of a directory that doesn't exist. +set bad_dir [standard_output_file "non-existent"] + +# Try to write the index into a non-existent directory. +foreach_with_prefix flag { "" "-dwarf-5" } { + gdb_test "save gdb-index ${flag} ${bad_dir}" \ + "Error while writing index for `[string_to_regexp $binfile]': `[string_to_regexp $bad_dir]': No such file or directory\\." \ + "try to write index to non-existent directory" +} + +# Create a text-file. +set text_file [standard_output_file "text-file"] +set fd [open $text_file w] +puts $fd "A line of text.\n" +close $fd + +# Try to write the index into something that is not a directory. +foreach_with_prefix flag { "" "-dwarf-5" } { + gdb_test "save gdb-index ${flag} ${text_file}" \ + "Error while writing index for `[string_to_regexp $binfile]': `[string_to_regexp $text_file]': Is not a directory\\." \ + "try to write index to something that's not a directory" +} + +# Create a directory which can't be written too. +set non_writable_dir [standard_output_file "private"] +remote_exec host "mkdir -p ${non_writable_dir}" +remote_exec host "chmod u-w,g-w ${non_writable_dir}" + +# Try to write the index into a non-writable directory. +foreach_with_prefix flag { "" "-dwarf-5" } { + gdb_test "save gdb-index ${flag} ${non_writable_dir}" \ + "Error while writing index for `[string_to_regexp $binfile]': couldn't open `[string_to_regexp $non_writable_dir]/${gdb_test_file_name}.*': Permission denied\\." \ + "try to write index to a non-writable directory" +} + +# Create copies of the executable, we will add an index section to +# each of these. +remote_exec host "cp $binfile ${binfile}.gdb_index" +remote_exec host "cp $binfile ${binfile}.dwarf_5" + +# Create a directory in which we can try to generate the index files. +set already_indexed_dir [standard_output_file "already_indexed"] +remote_exec host "mkdir -p $already_indexed_dir" + +foreach_with_prefix flag { "" "-dwarf-5" } { + if { $flag eq "" } { + set extension "gdb_index" + } else { + set extension "dwarf_5" + } + + # Add the index section to the executable. + clean_restart ${binfile}.${extension} + gdb_assert {[ensure_gdb_index ${binfile}.${extension} ${flag}] == 1} \ + "add index to executable" + + # Reload the executable (which now has an index), and try to + # generate and index from it. This will fail. + clean_restart ${binfile}.${extension} + gdb_test "save gdb-index ${flag} $already_indexed_dir" \ + "Error while writing index for `[string_to_regexp $binfile.$extension]': Cannot use an index to create the index" \ + "try to generate an index from a binary with an index" +}