Re: [PATCH v5 07/12] gdb: amd64 linux coredump support with shadow stack.

Thiago Jung Bauermann
2025-07-30T01:56:30Z

Hello Andrew,

Christina based the testcase in this patch on aarch64-gcs-core.exp from
my GCS patch series and your comments on it also apply to my patch, so
I'm replying on account of that.

Andrew Burgess <aburgess@redhat.com> writes:

> Christina Schimpe <christina.schimpe@intel.com> writes:
>
>> + # Generate the gcore core file.
>> + set gcore_filename [standard_output_file "${testfile}.gcore"]
>> + set gcore_generated [gdb_gcore_cmd "$gcore_filename" "generate gcore file"]
>> +
>> + # Obtain an OS-generated core file. Save test program output to
>> + # ${binfile}.out.
>> + set core_filename [core_find $binfile {} {} "${binfile}.out"]
>> + set core_generated [expr {$core_filename != ""}]
>> + set os_core_name "${binfile}.core"
>> + remote_exec build "mv $core_filename $os_core_name"
>> + set core_filename $os_core_name
>
> I'm wondering what the point of this core_filename / os_core_name stuff
> is? My reading of `core_find` is that the returned core_filename will
> be '${binfile}.core', so this whole thing feels redundant, but maybe I'm
> missing something here?

I wrote this part. The answer to your question is very simple: I don't
know what I was thinking. I just deleted the last 3 lines from the GCS
testcase.

>> +
>> + # At this point we have a couple of core files, the gcore one generated by
>> + # GDB and the one generated by the operating system. Make sure GDB can
>> + # read both correctly.
>> +
>> + if {$gcore_generated} {
>> + clean_restart $binfile
>> +
>> + with_test_prefix "gcore corefile" {
>> + check_core_file $gcore_filename $ssp_in_gcore
>> + }
>> + } else {
>> + fail "gcore corefile not generated"
>
> It's better, where possible, to avoid having pass/fail results that only
> show up down some code paths.
>
> In this case it's easy to avoid having a stray 'fail' by restructuring
> the code too:
>
> gdb_assert { $gcore_generated } "gcore corefile created"
> if { $gcore_generated } {
> ... etc ...
> }
>
> Now you'll always have either a pass or fail based on the gcore being
> generated.

Good idea. I did that for aarch64-gcs-core.exp.

> There is also the helper proc `gcore_cmd_available`. I'd guess for any
> x86 target that supports SSP, gcore will be available, but in theory you
> could consider using this to avoid a fail when gcore is not available
> maybe?

In the GCS core testcase, I moved the gcore tests after the OS corefile
ones, with an

if ![gcore_cmd_available] {
return
}

in the middle, and after that I had to redo some GDB setup:

clean_restart $binfile

if ![runto $linespec] {
return
}

But I agree it does make it more resilient/correct. Thanks for the
suggestion.

>> + }
>> +
>> + if {$core_generated} {
>> + clean_restart $binfile
>> +
>> + with_test_prefix "OS corefile" {
>> + # Read ssp value from saved output of the test program.
>> + set out_id [open ${binfile}.out "r"]
>> + set ssp_in_gcore [gets $out_id]
>> +
>> + close $out_id
>> + check_core_file $core_filename $ssp_in_gcore
>
> I'd move the blank line after the 'close' personally.

I also had that layout. Changed.

--
Thiago