From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by sourceware.org (Postfix) with ESMTPS id 1CFAA3982403 for ; Tue, 16 Feb 2021 17:50:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1CFAA3982403 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x431.google.com with SMTP id r21so14184876wrr.9 for ; Tue, 16 Feb 2021 09:50:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=RrSvV3ItgCiPgqaq/8v6h4MC2Pci42dZfg28os4XQ1M=; b=gBa5ULBFA9DrN7Tm2u5KVKAPvlETGDrDbOzI/JeW4Hr8+b5hs73u/0xlNkb6llyCuY TBut0VnnZB/vdAoKjeE05+pzp3cOG6Y1ypYGINtGQ4KbbjsL34cNohtUlj/q/5rl4FtC +ojFtElNEnWjjkkGzF1kIQE1h7mpVy9itsS5sc6hNd0HYUh3gTp/r2wUe4agOua1i1ET YGdKwOcy/jkNrSgdfFPOYm+QtEwstHggw1JfkxUzXz8VnY3VH+uJLo79R0Oh204ya9ev 147uRlKV81UvQzyJvJzZqUs7P51zN+z6BMUCTUktZfpRGFp6Np48/KlQcMhld0V5YLNN xVyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=RrSvV3ItgCiPgqaq/8v6h4MC2Pci42dZfg28os4XQ1M=; b=g/tX9/suaXypoXOAxNjbQkImvXS36idRs3YGcq8Nxvx5iMEBvWTQ0acAVUCr1nXICw Nzpah/gdPr7wJQYdd5+znlK23iCMQPS5NakH5ScUojnxuQVVXDmnFhCR59NlRWBlT5yO z32clKzkcAHjqPb9Cl79BPx0aDSOKbtMpsIDUZwOkxr1moIQVYTkYQyw8HAXhv+88ksu unvxWxPZkYhhp09drPsyE0QCTfkJelKoD1N44YnJPnlwtTS3GrnZXdLLH0doMWJT5vmP okOZSLG7v+sChszpguxbMYIhmnJ1v8ELtDqTSMMxx5cf7LYTtzZSgbRZeSIObg8LjpbN DTIQ== X-Gm-Message-State: AOAM530tneix1462gdCkoto2b2JT3SaMlG8Yv4k75DJWb7sueqFuxL26 eSyJgSuvxuwNd1GdbYXicijnM4v429W9IQ== X-Google-Smtp-Source: ABdhPJxykB961Z8vTFZbRuGQ/bBDSGJ8EFyuh1VTFWANbUTsb4Pmui1oEMz/iQluhKaoRZr7EhOyPQ== X-Received: by 2002:adf:f701:: with SMTP id r1mr24922894wrp.353.1613497842828; Tue, 16 Feb 2021 09:50:42 -0800 (PST) Received: from localhost (host86-186-80-154.range86-186.btcentralplus.com. [86.186.80.154]) by smtp.gmail.com with ESMTPSA id v5sm4628075wmh.2.2021.02.16.09.50.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Feb 2021 09:50:42 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Subject: [PATCH 4/4] gdb: move get_section_table from exec_target to dummy_target Date: Tue, 16 Feb 2021 17:50:30 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Feb 2021 17:50:46 -0000 The only target that implements target_ops::get_section_table in a meaningful way is exec_target. This target calls back into the program space to return the current global section_table. The global section table is populated whenever the user provides GDB with an executable, or when a symbol file is loaded, e.g. when a dynamic library is loaded, or when the user does add-symbol-file. I recently ran into a situation where a user, debugging a remote target, was not supplying GDB with a main executable at all. Instead the user attached to the target then did add-symbol-file, and then proceeded to debug the target. This works fine, but it was noticed that even when trust-readonly-sections was on GDB was still accessing the target to get the contents of readonly sections. The problem is that by not providing an executable there was no exec_target in the target stack, and so when GDB calls the target_ops::get_section_table function GDB ends up in dummy_target::get_section_table, which just returns NULL. What I want is that even when GDB doesn't have an exec_target in the target stack, a call to target_ops::get_section_table will still return the section_table from the current program space. When considering how to achieve this my first though was, why is the request for the section table going via the target stack at all? The set of sections loaded is a property of the program space, not the target. This is, after all, why the data is being stored in the program space. So I initially tried changing target_get_section_table so that, instead of calling into the target it just returns current_program_space->target_sections (). This would be fine except for one issue, target_bfd (from bfd-target.c). This code is used from solib-svr4.c to create a temporary target_ops structure that implements two functions target_bfd::xfer_partial and target_bfd::get_section_table. The purpose behind the code is to enable two targets, ppc64 and frv to decode function descriptors from the dynamic linker, based on the non-relocated addresses from within the dynamic linker bfd object. Both of the implemented functions in target_bfd rely on the target_bfd object holding a section table, and the ppc64 target requires that the target_bfd implement ::get_section_table. The frv target doesn't require ::get_section_table, instead it requires the ::xfer_partial. We could in theory change the ppc64 target to use the same approach as frv, however, this would be a bad idea. I believe that the frv target approach is broken. I'll explain: The frv target calls get_target_memory_unsigned to read the function descriptor. The address being read is the non-relocated address read from the dynamic linker in solib-srv4.c:enable_break. Calling get_target_memory_unsigned eventually ends up in target_xfer_partial with an object type of TARGET_OBJECT_RAW_MEMORY. This will then call memory_xfer_check_region. I believe that it is quite possible that a the non-relocated addresses pulled from the dynamic linker could be in a memory region that is not readable, while the relocated addresses are in a readable memory region. If this was ever the case for the frv target then GDB would reject the attempt to read the non-relocated function pointer. In contrast the ppc64 target calls target_section_by_addr, which calls target_get_section_table, which then calls the ::get_section_table function on the target. Thus, when reflecting on target_bfd we see two functions, ::xfer_partial and ::get_section_table. The former is required by the frv target, but that target is (I think) potentially broken. While the latter is required by the ppc64 target, but this forces ::get_section_table to exist as a target_ops member function. So my original plan, have target_get_section_table NOT call a target_ops member function appears to be flawed. My next idea was to remove exec_target::get_section_table, and instead move the implementation into dummy_target::get_section_table. Currently the dummy_target implementation always returns NULL indicating no section table, but plenty of other dummy_target member functions do more than just return null values. So now, dummy_target::get_section_table returns the section table from the current program space. This allows target_bfd to remain unchanged, so ppc64 and frv should not be affected. Making this change removes the requirement for the user to provide an executable, GDB can now always access the section_table, as the dummy_target always exists in the target stack. Finally, there's a test that the target_section table is not empty in the case where the user does add-symbol-file without providing an executable. gdb/ChangeLog: * exec.c (exec_target::get_section_table): Delete member function. (section_table_read_available_memory): Use current_top_target, not just the exec_ops target. * target-delegates.c: Regenerate. * target.c (default_get_section_table): New function. * target.h (target_ops::get_section_table): Change default behaviour to call default_get_section_table. (default_get_section_table): Declare. --- gdb/ChangeLog | 11 +++++++++++ gdb/exec.c | 10 ++-------- gdb/target-delegates.c | 2 +- gdb/target.c | 7 +++++++ gdb/target.h | 6 +++++- gdb/testsuite/gdb.base/maint-info-sections.exp | 4 +--- 6 files changed, 27 insertions(+), 13 deletions(-) diff --git a/gdb/exec.c b/gdb/exec.c index 2ad89af4798..e613409e8ef 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -75,7 +75,6 @@ struct exec_target final : public target_ops const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) override; - const target_section_table *get_section_table () override; void files_info () override; bool has_memory () override; @@ -788,7 +787,8 @@ enum target_xfer_status section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - const target_section_table *table = target_get_section_table (&exec_ops); + const target_section_table *table + = target_get_section_table (current_top_target ()); std::vector available_memory = section_table_available_memory (offset, len, *table); @@ -897,12 +897,6 @@ section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, return TARGET_XFER_EOF; /* We can't help. */ } -const target_section_table * -exec_target::get_section_table () -{ - return ¤t_program_space->target_sections (); -} - enum target_xfer_status exec_target::xfer_partial (enum target_object object, const char *annex, gdb_byte *readbuf, diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index 69fbc0f3b23..1c7999724c7 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -2030,7 +2030,7 @@ target_ops::get_section_table () const target_section_table * dummy_target::get_section_table () { - return NULL; + return default_get_section_table (); } const target_section_table * diff --git a/gdb/target.c b/gdb/target.c index 78535b89e58..ba445e7fd34 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -836,6 +836,13 @@ target_section_by_addr (struct target_ops *target, CORE_ADDR addr) return NULL; } +/* See target.h. */ + +const target_section_table * +default_get_section_table () +{ + return ¤t_program_space->target_sections (); +} /* Helper for the memory xfer routines. Checks the attributes of the memory region of MEMADDR against the read or write being attempted. diff --git a/gdb/target.h b/gdb/target.h index c11a8c91aa8..45b92f7d83f 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -688,7 +688,7 @@ struct target_ops virtual void log_command (const char *) TARGET_DEFAULT_IGNORE (); virtual const target_section_table *get_section_table () - TARGET_DEFAULT_RETURN (NULL); + TARGET_DEFAULT_RETURN (default_get_section_table ()); /* Provide default values for all "must have" methods. */ virtual bool has_all_memory () { return false; } @@ -2422,6 +2422,10 @@ const struct target_section *target_section_by_addr (struct target_ops *target, extern const target_section_table *target_get_section_table (struct target_ops *target); +/* Default implementation of get_section_table for dummy_target. */ + +extern const target_section_table *default_get_section_table (); + /* From mem-break.c */ extern int memory_remove_breakpoint (struct target_ops *, diff --git a/gdb/testsuite/gdb.base/maint-info-sections.exp b/gdb/testsuite/gdb.base/maint-info-sections.exp index b86812e4f12..06508de366d 100644 --- a/gdb/testsuite/gdb.base/maint-info-sections.exp +++ b/gdb/testsuite/gdb.base/maint-info-sections.exp @@ -223,9 +223,7 @@ gdb_test_multiple "maint info sections -all-objects" "" { } } -# NOTE: We would like to check 'maint info target-sections' again -# here, but GDB currently doesn't display the target sections table in -# this case. This is a bug and fill be fixed shortly!! +check_maint_info_target_sections_output "with loaded symbol file" # Test the command line completion on 'maint info sections'. First # the command line flag. -- 2.25.4