testsuite: Fix some tests that write outside of the standard output/temp directories
Commit Message
I used the GDB_INOTIFY feature to find those. These should be all
obvious changes, except for standard_temp_file and save-trace.exp.
I changed standard_temp_file the same way we changed
standard_output_file (to always return a test file-specific directory).
However, standard_temp_file is sometimes called when gdb_test_file_name
is not set, such as in gdb_init. In that case, it returns the base temp
directory. When running in parallel, I guess it means there is still a
chance of conflict between parallel test invocations. I don't think I
am making things worst than before though, since it was already the
case.
In save-trace.exp, we want to test loading of a tracepoint definition
file with a relative path (I am not sure why in fact). We currently use
"savetrace-relative.tr", which ends up directly in testsuite/. If we
use [standard_output_file] on it, it becomes absolute. I decided to
just replace [pwd] with . (a dot) in that path to make it relative. It
wouldn't work if the [pwd] was not a prefix of the standard output
directory though. So I added a check to verify that precondition.
gdb/testsuite/ChangeLog:
* gdb.base/wrong_frame_bt_full.exp: Use standard_output_file to
define object file path.
* gdb.btrace/gcore.exp: Use standard_output_file to define core
file path.
* gdb.trace/save-trace.exp: Change relative path to be in the
standard output directory.
* lib/gdb.exp (standard_temp_file): Return a testfile-specific
path when possible.
* lib/opencl.exp (gdb_compile_opencl_hostapp): Use
standard_output_file to define binfile.
---
gdb/testsuite/gdb.base/wrong_frame_bt_full.exp | 5 +++--
gdb/testsuite/gdb.btrace/gcore.exp | 3 ++-
gdb/testsuite/gdb.trace/save-trace.exp | 13 ++++++++++++-
gdb/testsuite/lib/gdb.exp | 15 +++++++++++----
gdb/testsuite/lib/opencl.exp | 2 +-
5 files changed, 29 insertions(+), 9 deletions(-)
Comments
On 02/12/2016 09:01 PM, Simon Marchi wrote:
> I used the GDB_INOTIFY feature to find those. These should be all
> obvious changes, except for standard_temp_file and save-trace.exp.
I think you should push this in 3 patches. Go ahead and push the
obvious ones that just use standard_output_file first.
>
> I changed standard_temp_file the same way we changed
> standard_output_file (to always return a test file-specific directory).
> However, standard_temp_file is sometimes called when gdb_test_file_name
> is not set, such as in gdb_init. In that case, it returns the base temp
> directory. When running in parallel, I guess it means there is still a
> chance of conflict between parallel test invocations. I don't think I
> am making things worst than before though, since it was already the
> case.
As this is just a temp file, and we just need to make it unique,
how about (always) using runtest's PID instead of gdb_test_file_name?
That'd be a separate patch.
>
> In save-trace.exp, we want to test loading of a tracepoint definition
> file with a relative path (I am not sure why in fact). We currently use
> "savetrace-relative.tr", which ends up directly in testsuite/. If we
> use [standard_output_file] on it, it becomes absolute. I decided to
> just replace [pwd] with . (a dot) in that path to make it relative. It
> wouldn't work if the [pwd] was not a prefix of the standard output
> directory though. So I added a check to verify that precondition.
Seems fine, and would be best pushed as a separate patch.
Thanks,
Pedro Alves
On 16-02-12 06:28 PM, Pedro Alves wrote:
> On 02/12/2016 09:01 PM, Simon Marchi wrote:
>> I used the GDB_INOTIFY feature to find those. These should be all
>> obvious changes, except for standard_temp_file and save-trace.exp.
>
> I think you should push this in 3 patches. Go ahead and push the
> obvious ones that just use standard_output_file first.
>
>>
>> I changed standard_temp_file the same way we changed
>> standard_output_file (to always return a test file-specific directory).
>> However, standard_temp_file is sometimes called when gdb_test_file_name
>> is not set, such as in gdb_init. In that case, it returns the base temp
>> directory. When running in parallel, I guess it means there is still a
>> chance of conflict between parallel test invocations. I don't think I
>> am making things worst than before though, since it was already the
>> case.
>
> As this is just a temp file, and we just need to make it unique,
> how about (always) using runtest's PID instead of gdb_test_file_name?
>
> That'd be a separate patch.
>
>>
>> In save-trace.exp, we want to test loading of a tracepoint definition
>> file with a relative path (I am not sure why in fact). We currently use
>> "savetrace-relative.tr", which ends up directly in testsuite/. If we
>> use [standard_output_file] on it, it becomes absolute. I decided to
>> just replace [pwd] with . (a dot) in that path to make it relative. It
>> wouldn't work if the [pwd] was not a prefix of the standard output
>> directory though. So I added a check to verify that precondition.
>
> Seems fine, and would be best pushed as a separate patch.
>
> Thanks,
> Pedro Alves
>
You are right, I'll push the obvious ones and send the two other fixes
separately.
Simon
@@ -23,16 +23,17 @@
set main_testfile wrong_frame_bt_full-main
set opaque_testfile wrong_frame_bt_full-opaque
+set opaque_objfile [standard_output_file "$opaque_testfile.o"]
if {[gdb_compile "${srcdir}/${subdir}/$opaque_testfile.c" \
- $opaque_testfile.o \
+ $opaque_objfile \
object {}] != ""} {
untested "failed to compile $opaque_testfile.c"
return -1
}
if {[gdb_compile \
- [list ${srcdir}/${subdir}/$main_testfile.c $opaque_testfile.o] \
+ [list ${srcdir}/${subdir}/$main_testfile.c $opaque_objfile] \
[standard_output_file ${main_testfile}] \
executable {debug}] != ""} {
untested "failed to build $main_testfile"
@@ -38,4 +38,5 @@ gdb_test "next" ".*main\.3.*"
gdb_test "record goto begin" ".*main\.2.*"
# generate a core file - this used to assert
-gdb_test "generate-core-file core" "Saved corefile core"
+set corefile [standard_output_file core]
+gdb_test "generate-core-file $corefile" "Saved corefile $corefile"
@@ -151,7 +151,18 @@ proc do_save_load_test { save_path } {
gdb_verify_tracepoints "verify trace setup"
with_test_prefix "relative" {
- do_save_load_test "savetrace-relative.tr"
+ set filepath [standard_output_file "savetrace-relative.tr"]
+
+ # This only work because the pwd is a prefix of the standard output
+ # directory. If this assumption becomes false, then this test needs to be
+ # changed (the relative path from [pwd] to the standard output directory
+ # will become a bit more complicated to compute).
+ if {[string first [pwd] $filepath] != 0} {
+ error "[pwd] is not a prefix of $filepath."
+ }
+
+ set filepath [string map "[pwd] ." $filepath]
+ do_save_load_test "$filepath"
}
with_test_prefix "absolute" {
@@ -4339,13 +4339,20 @@ proc standard_output_file {basename} {
# Return the name of a file in our standard temporary directory.
proc standard_temp_file {basename} {
- global objdir GDB_PARALLEL
+ global objdir subdir gdb_test_file_name
+
+ # gdb_test_file_name may not be set, if we are not executing something
+ # specific to a test file (e.g. gdb_init). In that case, return the base
+ # temp directory.
- if {[info exists GDB_PARALLEL]} {
- return [make_gdb_parallel_path temp $basename]
+ if [info exists gdb_test_file_name] {
+ set dir [make_gdb_parallel_path temp $subdir $gdb_test_file_name]
} else {
- return $basename
+ set dir [make_gdb_parallel_path temp]
}
+
+ file mkdir $dir
+ return [file join $dir $basename]
}
# Set 'testfile', 'srcfile', and 'binfile'.
@@ -21,7 +21,7 @@
proc gdb_compile_opencl_hostapp {clsource executable options} {
global srcdir objdir subdir
set src "${srcdir}/lib/cl_util.c ${srcdir}/lib/opencl_hostapp.c"
- set binfile ${objdir}/${subdir}/${executable}
+ set binfile [standard_output_file ${executable}]
set compile_flags [concat additional_flags=-I${srcdir}/lib/ additional_flags=-DCL_SOURCE=$clsource]
set options_opencl [concat {debug} $compile_flags $options [list libs=-lOpenCL]]
return [gdb_compile ${src} ${binfile} "executable" ${options_opencl}]