Message ID | 20200211133710.6120-1-shahab.vahedi@gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 121430 invoked by alias); 11 Feb 2020 13:37:22 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 121415 invoked by uid 89); 11 Feb 2020 13:37:21 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=fledged, manipulating, our, HContent-Transfer-Encoding:8bit X-HELO: mail-lf1-f67.google.com Received: from mail-lf1-f67.google.com (HELO mail-lf1-f67.google.com) (209.85.167.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 11 Feb 2020 13:37:20 +0000 Received: by mail-lf1-f67.google.com with SMTP id 203so6997342lfa.12 for <gdb-patches@sourceware.org>; Tue, 11 Feb 2020 05:37:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=dyfUMcWUqf3q6Q9mcMqmxVmG9nuh9JqXz4VdPZwJptE=; b=AQZH3fuvscUHqmWrbw8P0L58s5+xflskfoqtdbPFx7OMJYxgd2E9FCAYsvaHnRFNeQ JMgEzoADUUvlggGEUUl2fGNa2S+EqBUEGwHIE85QtHAJHk15Xu6tQWLd+xdlT8RsVVJJ jF2XSFmTORQavg/lbxFF9RGP8+28SxiDTdwRTyeIY3oI0P6GGY36iZDY8bLwYQGjMPiU BcHFcl4Q1LhLAZXvQnfxja27FfOt7cFg6F9GsB5eJjdcVWkKTnyy8TlqrnMzs0R6t5nN R4ymF1YNnUX3k4polaLprOmsQ9AuoLJ4N+92rpZbHCKJX0b5HKNRJdKerEC5IjaBDAkH 7M2A== Return-Path: <shahab.vahedi@gmail.com> Received: from archie.internal.synopsys.com ([2a03:1b20:6:f011::2d]) by smtp.gmail.com with ESMTPSA id v26sm2699623ljh.90.2020.02.11.05.37.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Feb 2020 05:37:16 -0800 (PST) From: Shahab Vahedi <shahab.vahedi@gmail.com> To: gdb-patches@sourceware.org Cc: Shahab Vahedi <shahab@synopsys.com>, Shahab Vahedi <shahab.vahedi@gmail.com>, Francois Bedard <fbedard@synopsys.com> Subject: [PATCH] gdb/testsuite: Regenerate the testglue if it is not in path Date: Tue, 11 Feb 2020 14:37:10 +0100 Message-Id: <20200211133710.6120-1-shahab.vahedi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit |
Commit Message
Shahab Vahedi
Feb. 11, 2020, 1:37 p.m. UTC
From: Shahab Vahedi <shahab@synopsys.com>
For running the DejaGnu tests, some esoteric configurations
may require a testglue. This, for instance, is true about
testing ARC targets which uses its own DejaGnu board and
a simulator which does not support returning the pass or fail
through the exit code. Therefore, for those tests that use
"gdb_compile" a "gdb_tg.o" file is compiled and linked to the
final executable.
There are tests that invoke "gdb_compile" from different
directories. Let's take a look at an example test:
gdb.base/fullname.exp. The purpose of this test is to build
the executable from different directories (absolute vs. relative
vs. other) and then check if gdb can handle setting breakpoints
accordingly.
When "gdb_compile" generates the "gdb_tg.o", it does not
do it again for the same test. Although this might seem
efficient, it can lead to problems when changing directories
before the next compile:
gdb compile failed, arc-elf32-gcc: error: gdb_tg.o:
No such file or directory
This patch checks if the wrapper file ("gdb_tg.o") is still in
reach and if it is not, it will stimulate the generation of the
wrapper again.
It is worth mentioning that GCC's DejaGnu tests handle these
scenarios as well and they seem to be more efficient in doing so
by saving the library paths and manipulating them if necessary
[1]. However, for GDB tests, that require less compilations,
I think the proposed solution should be fine compared to a more
full fledged solution from GCC. The glue file in our case is
only 2 KiB.
Last but not least, I ran the x86_64 tests on an x86_64 host and
found no regression.
[1]
Avid coders may look for "set_ld_library_path_env_vars" in
gcc/testsuite/lib/target-libpath.exp.
gdb/testsuite/ChangeLog:
* lib/gdb.exp (gdb_compile): Reset
"gdb_wrapper_initialized" to 0 if "wrapper_file" does
not exist.
---
gdb/testsuite/lib/gdb.exp | 9 +++++++++
1 file changed, 9 insertions(+)
Comments
>>>>> "Shahab" == Shahab Vahedi <shahab.vahedi@gmail.com> writes:
Shahab> For running the DejaGnu tests, some esoteric configurations
Shahab> may require a testglue.
Thank you for the patch. I wasn't aware of this code at all :-)
Shahab> This patch checks if the wrapper file ("gdb_tg.o") is still in
Shahab> reach and if it is not, it will stimulate the generation of the
Shahab> wrapper again.
I wonder if it would make sense to instead set gdb_wrapper_file to the
absolute path.
It could perhaps be put in the cache directory. This might help avoid
recompiling it many times when running the tests in parallel.
What do you think of that?
Shahab> + # if the wrapper is initialized but the wrapper file cannot be
GNU style is to start a comment with an upper-case letter, so "If".
Shahab> + # found anymore, the wrapper file must be built again.
Shahab> + if { $gdb_wrapper_initialized == 1 && \
Shahab> + [info exists gdb_wrapper_file] && \
Shahab> + ![file exists $gdb_wrapper_file] } {
Shahab> + verbose "reinitializing the wrapper"
Shahab> + set gdb_wrapper_initialized 0
Shahab> + }
Shahab> +
Shahab> if { $gdb_wrapper_initialized == 0 } { gdb_wrapper_init }
If you don't like the absolute path idea, maybe just having all this
logic be in the wrapper code would be better, and this could just call
gdb_wrapper_init unconditionally.
Tom
On Tue, Feb 11, 2020 at 08:39:03AM -0700, Tom Tromey wrote: > >>>>> "Shahab" == Shahab Vahedi <shahab.vahedi@gmail.com> writes: > > Shahab> For running the DejaGnu tests, some esoteric configurations > Shahab> may require a testglue. > > Thank you for the patch. I wasn't aware of this code at all :-) > > Shahab> This patch checks if the wrapper file ("gdb_tg.o") is still in > Shahab> reach and if it is not, it will stimulate the generation of the > Shahab> wrapper again. > > I wonder if it would make sense to instead set gdb_wrapper_file to the > absolute path. This is how "gdb_wrapper_file" is set in gdb/testsuite/lib/gdb.exp: proc gdb_wrapper_init { args } { ... global gdb_wrapper_file ... set result [build_wrapper "testglue.o"] if { $result != "" } { set gdb_wrapper_file [lindex $result 0] ... } The "build_wrapper" is a procedure form DejaGnu itself (libgloss.exp), and it does not provide the full path. Maybe we can get the current working directory from somewhere else and append it here? > It could perhaps be put in the cache directory. This might help avoid > recompiling it many times when running the tests in parallel. > > What do you think of that? I absolutely like this idea and believe this is how it should be done, but am not sure where to begin. Any pointers? > > Shahab> + # if the wrapper is initialized but the wrapper file cannot be > > GNU style is to start a comment with an upper-case letter, so "If". I will fix this in next patch (whatever approach we take). > > Shahab> + # found anymore, the wrapper file must be built again. > Shahab> + if { $gdb_wrapper_initialized == 1 && \ > Shahab> + [info exists gdb_wrapper_file] && \ > Shahab> + ![file exists $gdb_wrapper_file] } { > Shahab> + verbose "reinitializing the wrapper" > Shahab> + set gdb_wrapper_initialized 0 > Shahab> + } > Shahab> + > Shahab> if { $gdb_wrapper_initialized == 0 } { gdb_wrapper_init } > > If you don't like the absolute path idea, maybe just having all this > logic be in the wrapper code would be better, and this could just call > gdb_wrapper_init unconditionally. I agree with your suggestion and if we opt for this approach, I will adapt it as such. > > Tom Cheers, Shahab
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index d5e22957039..8e91c91e715 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -3812,6 +3812,15 @@ proc gdb_compile {source dest type options} { verbose "options are $options" verbose "source is $source $dest $type $options" + # if the wrapper is initialized but the wrapper file cannot be + # found anymore, the wrapper file must be built again. + if { $gdb_wrapper_initialized == 1 && \ + [info exists gdb_wrapper_file] && \ + ![file exists $gdb_wrapper_file] } { + verbose "reinitializing the wrapper" + set gdb_wrapper_initialized 0 + } + if { $gdb_wrapper_initialized == 0 } { gdb_wrapper_init } if {[target_info exists needs_status_wrapper] && \