[gdb/testsuite] Fix index-cache.exp with cc-with-{gdb-index,debug-names}
Commit Message
[ was: Re: [PATCH][gdb/testsuite] Fix index-cache.exp with
CC_WITH_TWEAKS_FLAGS=-i ]
On 03-05-19 23:17, Simon Marchi wrote:
> On 2019-05-03 6:43 a.m., Tom de Vries wrote:
>> Hi,
>>
>> When running gdb.base/index-cache.exp with target board cc-with-tweaks with
>> CC_WITH_TWEAKS_FLAGS set to "-i", we run into:
>> ...
>> FAIL: gdb.base/index-cache.exp: test_cache_enabled_miss: at least one file \
>> was created
>> FAIL: gdb.base/index-cache.exp: test_cache_enabled_miss: expected file is there
>> FAIL: gdb.base/index-cache.exp: test_cache_enabled_miss: check index-cache stats
>> FAIL: gdb.base/index-cache.exp: test_cache_enabled_hit: check index-cache stats
>> ...
>>
>> The problem is that the target board makes sure that the generated executable
>> contains a .gdb_index section, while the test assumes that the executable
>> doesn't contain this section.
>>
>> Fix this by removing the .gdb_index section from the generated executable.
>>
>> Tested on x86_64-linux with native and CC_WITH_TWEAKS_FLAGS=-i config.
>>
>> OK for trunk?
>>
>> Thanks,
>> - Tom
>
> Hi Tom,
>
> I would slightly prefer that instead of doing this, we would notice that that file
> already has an index (in the form of .gdb_index or .debug_names), and adjust our
> expectations in the test.
>
> In other words, we currently assert that loading the file in GDB will produce some
> files in the cache. However, if we know that the file already has an index, we
> should verify that no file was produced, as this is the behavior we expect when
> loading a file which already has an index.
>
> Stripping the index makes the test pass, but it just goes back to testing the same
> thing as with the default board file. Adjusting our expectation to the presence
> of an index makes the test cover a different use case.
I've implemented this approach, attached below.
OK for trunk?
Thanks,
- Tom
Comments
On 2019-05-04 4:35 a.m., Tom de Vries wrote:
> [ was: Re: [PATCH][gdb/testsuite] Fix index-cache.exp with
> CC_WITH_TWEAKS_FLAGS=-i ]
>
> On 03-05-19 23:17, Simon Marchi wrote:
>> On 2019-05-03 6:43 a.m., Tom de Vries wrote:
>>> Hi,
>>>
>>> When running gdb.base/index-cache.exp with target board cc-with-tweaks with
>>> CC_WITH_TWEAKS_FLAGS set to "-i", we run into:
>>> ...
>>> FAIL: gdb.base/index-cache.exp: test_cache_enabled_miss: at least one file \
>>> was created
>>> FAIL: gdb.base/index-cache.exp: test_cache_enabled_miss: expected file is there
>>> FAIL: gdb.base/index-cache.exp: test_cache_enabled_miss: check index-cache stats
>>> FAIL: gdb.base/index-cache.exp: test_cache_enabled_hit: check index-cache stats
>>> ...
>>>
>>> The problem is that the target board makes sure that the generated executable
>>> contains a .gdb_index section, while the test assumes that the executable
>>> doesn't contain this section.
>>>
>>> Fix this by removing the .gdb_index section from the generated executable.
>>>
>>> Tested on x86_64-linux with native and CC_WITH_TWEAKS_FLAGS=-i config.
>>>
>>> OK for trunk?
>>>
>>> Thanks,
>>> - Tom
>>
>> Hi Tom,
>>
>> I would slightly prefer that instead of doing this, we would notice that that file
>> already has an index (in the form of .gdb_index or .debug_names), and adjust our
>> expectations in the test.
>>
>> In other words, we currently assert that loading the file in GDB will produce some
>> files in the cache. However, if we know that the file already has an index, we
>> should verify that no file was produced, as this is the behavior we expect when
>> loading a file which already has an index.
>>
>> Stripping the index makes the test pass, but it just goes back to testing the same
>> thing as with the default board file. Adjusting our expectation to the presence
>> of an index makes the test cover a different use case.
>
> I've implemented this approach, attached below.
>
> OK for trunk?
>
> Thanks,
> - Tom
>
Thanks Tom, this LGTM.
Before pushing, could you just adjust the comments above each proc? They describe
what the test expects (at a high level), so maybe just add a precision about what is
expected when the binary already has an index.
Simon
[gdb/testsuite] Fix index-cache.exp with cc-with-{gdb-index,debug-names}
In gdb.base/index-cache.exp, handle the case that binfile contains either a
.gdb_index or .debug_names index section.
Tested on x86_64-linux with native, cc-with-gdb-index and cc-with-debug-names.
gdb/testsuite/ChangeLog:
2019-05-04 Tom de Vries <tdevries@suse.de>
* lib/gdb.exp (exec_has_index_section): New proc.
* gdb.base/index-cache.exp: Handle case that binfile contains an index
section.
---
gdb/testsuite/gdb.base/index-cache.exp | 30 +++++++++++++++++++++++++-----
gdb/testsuite/lib/gdb.exp | 11 +++++++++++
2 files changed, 36 insertions(+), 5 deletions(-)
@@ -22,6 +22,8 @@ if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
return
}
+set has_index_section [exec_has_index_section $binfile]
+
# List the files in DIR on the host (where GDB-under-test runs).
# Return a list of two elements:
# - 0 on success, -1 on failure
@@ -125,7 +127,7 @@ proc_with_prefix test_cache_disabled { cache_dir } {
# Test with the cache enabled, we expect to have exactly one file created.
proc_with_prefix test_cache_enabled_miss { cache_dir } {
- global testfile
+ global testfile has_index_section
lassign [ls_host $cache_dir] ret files_before
@@ -133,7 +135,11 @@ proc_with_prefix test_cache_enabled_miss { cache_dir } {
lassign [ls_host $cache_dir] ret files_after
set nfiles_created [expr [llength $files_after] - [llength $files_before]]
- gdb_assert "$nfiles_created > 0" "at least one file was created"
+ if { $has_index_section } {
+ gdb_assert "$nfiles_created == 0" "no file was created"
+ } else {
+ gdb_assert "$nfiles_created > 0" "at least one file was created"
+ }
set build_id [get_build_id [standard_output_file ${testfile}]]
if { $build_id == "" } {
@@ -143,11 +149,19 @@ proc_with_prefix test_cache_enabled_miss { cache_dir } {
set expected_created_file [list "${build_id}.gdb-index"]
set found_idx [lsearch -exact $files_after $expected_created_file]
- gdb_assert "$found_idx >= 0" "expected file is there"
+ if { $has_index_section } {
+ gdb_assert "$found_idx == -1" "no index cache file generated"
+ } else {
+ gdb_assert "$found_idx >= 0" "expected file is there"
+ }
remote_exec host rm "-f $cache_dir/$expected_created_file"
- check_cache_stats 0 1
+ if { $has_index_section } {
+ check_cache_stats 0 0
+ } else {
+ check_cache_stats 0 1
+ }
}
}
@@ -156,6 +170,8 @@ proc_with_prefix test_cache_enabled_miss { cache_dir } {
# same), but one cache read hit.
proc_with_prefix test_cache_enabled_hit { cache_dir } {
+ global has_index_section
+
# Just to populate the cache.
run_test_with_flags $cache_dir on {}
@@ -166,7 +182,11 @@ proc_with_prefix test_cache_enabled_hit { cache_dir } {
set nfiles_created [expr [llength $files_after] - [llength $files_before]]
gdb_assert "$nfiles_created == 0" "no files were created"
- check_cache_stats 1 0
+ if { $has_index_section } {
+ check_cache_stats 0 0
+ } else {
+ check_cache_stats 1 0
+ }
}
}
@@ -5107,6 +5107,17 @@ proc rerun_to_main {} {
}
}
+# Return true if EXECUTABLE contains a .gdb_index or .debug_names index section.
+
+proc exec_has_index_section { executable } {
+ set res [catch {exec readelf -S $executable \
+ | grep -E "\.gdb_index|\.debug_names" }]
+ if { $res == 0 } {
+ return 1
+ }
+ return 0
+}
+
# Return true if a test should be skipped due to lack of floating
# point support or GDB can't fetch the contents from floating point
# registers.