Matthew Malcomson <matthew.malcom...@arm.com> writes:
> Adding hwasan tests.
>
> Only interesting thing here is that we have to make sure the tagging mechanism
> is deterministic to avoid flaky tests.
Sorry for not reviewing this one earlier. TBH I only spot-checked
the tests themselves (they look good). But on hwasan-dg.exp: I think
we should try to avoid so much cut-&-paste between asan-dg.exp and
hwasan-dg.exp.
For one thing (and obviously not your fault), it seems odd to me that
check_effective_target_fsanitize_address is defined in asan-dg.exp.
I think it and the new check_effective_targets* should be defined
in target-supports.exp instead. On:
> +proc check_effective_target_hwaddress_exec {} {
> + if ![check_runtime hwaddress_exec {
> + int main (void) { return 0; }
> + }] {
> + return 0;
> + }
> + return 1;
> +
> + # hwasan doesn't work if there's a ulimit on virtual memory.
> + if ![is_remote target] {
> + if [catch {exec sh -c "ulimit -v"} ulimit_v] {
> + # failed to get ulimit
> + } elseif [regexp {^[0-9]+$} $ulimit_v] {
> + # ulimit -v gave a numeric limit
> + warning "skipping hwasan tests due to ulimit -v"
> + return 0;
> + }
> + }
> +}
either the “hwasan doesn't work” block or the early “return 1” should
be removed. (I'm guessing the former.)
> +proc hwasan_include_flags {} {
> + global srcdir
> + global TESTING_IN_BUILD_TREE
> +
> + set flags ""
> +
> + if { [is_remote host] || ! [info exists TESTING_IN_BUILD_TREE] } {
> + return "${flags}"
> + }
> +
> + set flags "-I$srcdir/../../libsanitizer/include"
> +
> + return "$flags"
> +}
This is identical to the asan version, but I guess it's small enough
that the cut-&-paste doesn't matter.
> +
> +#
> +# hwasan_link_flags -- compute library path and flags to find libhwasan.
> +# (originally from g++.exp)
> +#
> +
> +proc hwasan_link_flags { paths } {
> + global srcdir
> + global ld_library_path
> + global shlib_ext
> + global hwasan_saved_library_path
> +
> + set gccpath ${paths}
> + set flags ""
> +
> + set shlib_ext [get_shlib_extension]
> + set hwasan_saved_library_path $ld_library_path
> +
> + if { $gccpath != "" } {
> + if { [file exists "${gccpath}/libsanitizer/hwasan/.libs/libhwasan.a"]
> + || [file exists
> "${gccpath}/libsanitizer/hwasan/.libs/libhwasan.${shlib_ext}"] } {
> + append flags " -B${gccpath}/libsanitizer/ "
> + append flags " -B${gccpath}/libsanitizer/hwasan/ "
> + append flags " -L${gccpath}/libsanitizer/hwasan/.libs "
> + append ld_library_path ":${gccpath}/libsanitizer/hwasan/.libs"
> + }
> + } else {
> + global tool_root_dir
> +
> + set libhwasan [lookfor_file ${tool_root_dir} libhwasan]
> + if { $libhwasan != "" } {
> + append flags "-L${libhwasan} "
> + append ld_library_path ":${libhwasan}"
> + }
> + }
> +
> + set_ld_library_path_env_vars
> +
> + return "$flags"
> +}
Here I'd suggest:
- In asan-dg.exp, have:
# Compute library path and flags to find libsanitizer library LIB.
# (originally from g++.exp).
proc asan_link_flags_1 { paths lib } {
…body…
}
…existing comment…
proc asan_link_flags { paths } {
return [asan_link_flags_1 $paths asan]
}
where …body… is more or less the current body of asan_link_flags with
“asan” replaced by ${lib}. E.g.:
global ${lib}_saved_library_path
…
set ${lib}_saved_library_path $ld_library_path
is fine. For local variables like:
set libasan [lookfor_file ${tool_root_dir} libasan]
if { $libasan != "" } {
append flags "-L${libasan} "
append ld_library_path ":${libasan}"
}
it would make more sense to use a generic name instead, e.g.:
set libdir [lookfor_file ${tool_root_dir} lib${lib}]
if { $libdir != "" } {
append flags "-L${libdir} "
append ld_library_path ":${libdir}"
}
- Have hwasan-dg.exp include asan-dg.exp.
- Have:
proc hwasan_link_flags { paths } {
return [asan_link_flags_1 $paths hwasan]
}
> +
> +#
> +# hwasan_init -- called at the start of each subdir of tests
> +#
> +
> +proc hwasan_init { args } {
> + global TEST_ALWAYS_FLAGS
> + global ALWAYS_CXXFLAGS
> + global TOOL_OPTIONS
> + global hwasan_saved_TEST_ALWAYS_FLAGS
> + global hwasan_saved_ALWAYS_CXXFLAGS
> +
> + setenv HWASAN_OPTIONS "random_tags=0"
> +
> + set link_flags ""
> + if ![is_remote host] {
> + if [info exists TOOL_OPTIONS] {
> + set link_flags "[hwasan_link_flags [get_multilibs ${TOOL_OPTIONS}]]"
> + } else {
> + set link_flags "[hwasan_link_flags [get_multilibs]]"
> + }
> + }
> +
> + set include_flags "[hwasan_include_flags]"
> +
> + if [info exists TEST_ALWAYS_FLAGS] {
> + set hwasan_saved_TEST_ALWAYS_FLAGS $TEST_ALWAYS_FLAGS
> + }
> + if [info exists ALWAYS_CXXFLAGS] {
> + set hwasan_saved_ALWAYS_CXXFLAGS $ALWAYS_CXXFLAGS
> + set ALWAYS_CXXFLAGS [concat "{ldflags=$link_flags}" $ALWAYS_CXXFLAGS]
> + set ALWAYS_CXXFLAGS [concat "{additional_flags=-fsanitize=hwaddress
> --param hwasan-random-frame-tag=0 -g $include_flags}" $ALWAYS_CXXFLAGS]
> + } else {
> + if [info exists TEST_ALWAYS_FLAGS] {
> + set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=hwaddress --param
> hwasan-random-frame-tag=0 -g $include_flags $TEST_ALWAYS_FLAGS"
> + } else {
> + set TEST_ALWAYS_FLAGS "$link_flags -fsanitize=hwaddress --param
> hwasan-random-frame-tag=0 -g $include_flags"
> + }
> + }
> +}
I agree this one is different enough from the asan version that it
isn't worth commonising the code.
> +
> +#
> +# hwasan_finish -- called at the start of each subdir of tests
> +#
> +
> +proc hwasan_finish { args } {
> + global TEST_ALWAYS_FLAGS
> + global hwasan_saved_TEST_ALWAYS_FLAGS
> + global hwasan_saved_ALWAYS_CXXFLAGS
> + global hwasan_saved_library_path
> + global ld_library_path
> +
> + unsetenv HWASAN_OPTIONS
> +
> + if [info exists hwasan_saved_ALWAYS_CXXFLAGS ] {
> + set ALWAYS_CXXFLAGS $hwasan_saved_ALWAYS_CXXFLAGS
> + } else {
> + if [info exists hwasan_saved_TEST_ALWAYS_FLAGS] {
> + set TEST_ALWAYS_FLAGS $hwasan_saved_TEST_ALWAYS_FLAGS
> + } else {
> + unset TEST_ALWAYS_FLAGS
> + }
> + }
> + set ld_library_path $hwasan_saved_library_path
> + set_ld_library_path_env_vars
> + clear_effective_target_cache
> +}
And I guess then it would be more consistent to keep this separate too,
if the _init function is.
> +
> +# Symbolize lines like
> +# #2 0xdeadbeef (/some/path/libsanitizer.so.0.0.0+0xbeef)
> +# in $output using addr2line to
> +# #2 0xdeadbeef in foobar file:123
> +proc hwasan_symbolize { output } {
> + set addresses [regexp -inline -all -line "^ *#\[0-9\]+ 0x\[0-9a-f\]+
> \[(\](\[^)\]+)\[+\](0x\[0-9a-f\]+)\[)\]$" "$output"]
> + if { [llength $addresses] > 0 } {
> + set addr2line_name [find_binutils_prog addr2line]
> + set idx 1
> + while { $idx < [llength $addresses] } {
> + set key [regsub -all "\[\]\[\]" [lindex $addresses $idx] "\\\\&"]
> + set val [lindex $addresses [expr $idx + 1]]
> + lappend arr($key) $val
> + set idx [expr $idx + 3]
> + }
> + foreach key [array names arr] {
> + set args "-f -e $key $arr($key)"
> + set status [remote_exec host "$addr2line_name" "$args"]
> + if { [lindex $status 0] > 0 } continue
> + regsub -all "\r\n" [lindex $status 1] "\n" addr2line_output
> + regsub -all "\[\n\r\]BFD: \[^\n\r\]*" $addr2line_output ""
> addr2line_output
> + regsub -all "^BFD: \[^\n\r\]*\[\n\r\]" $addr2line_output ""
> addr2line_output
> + set addr2line_output [regexp -inline -all -line "^\[^\n\r]*"
> $addr2line_output]
> + set idx 0
> + foreach val $arr($key) {
> + if { [expr $idx + 1] < [llength $addr2line_output] } {
> + set fnname [lindex $addr2line_output $idx]
> + set fileline [lindex $addr2line_output [expr $idx + 1]]
> + if { "$fnname" != "??" } {
> + set newkey "$key+$val"
> + set repl($newkey) "$fnname $fileline"
> + }
> + set idx [expr $idx + 2]
> + }
> + }
> + }
> + set idx 0
> + set new_output ""
> + while {[regexp -start $idx -indices " #\[0-9\]+ 0x\[0-9a-f\]+
> \[(\](\[^)\]+\[+\]0x\[0-9a-f\]+)\[)\]" "$output" -> addr] > 0} {
> + set low [lindex $addr 0]
> + set high [lindex $addr 1]
> + set val [string range "$output" $low $high]
> + append new_output [string range "$output" $idx [expr $low - 2]]
> + if [info exists repl($val)] {
> + append new_output "in $repl($val)"
> + } else {
> + append new_output "($val)"
> + }
> + set idx [expr $high + 2]
> + }
> + append new_output [string range "$output" $idx [string length
> "$output"]]
> + return "$new_output"
> + }
> + return "$output"
> +}
> +
> +# Return a list of gtest tests, printed in the form
> +# DEJAGNU_GTEST_TEST AddressSanitizer_SimpleDeathTest
> +# DEJAGNU_GTEST_TEST AddressSanitizer_VariousMallocsTest
> +proc hwasan_get_gtest_test_list { output } {
> + set idx 0
> + set ret ""
> + while {[regexp -start $idx -indices "DEJAGNU_GTEST_TEST
> (\[^\n\r\]*)(\r\n|\n|\r)" "$output" -> testname] > 0} {
> + set low [lindex $testname 0]
> + set high [lindex $testname 1]
> + set val [string range "$output" $low $high]
> + lappend ret $val
> + set idx [expr $high + 1]
> + }
> + return $ret
> +}
> +
> +# Return a list of gtest EXPECT_DEATH tests, printed in the form
> +# DEJAGNU_GTEST_EXPECT_DEATH1 statement DEJAGNU_GTEST_EXPECT_DEATH1 regexp
> DEJAGNU_GTEST_EXPECT_DEATH1
> +# DEJAGNU_GTEST_EXPECT_DEATH2 other statement DEJAGNU_GTEST_EXPECT_DEATH2
> other regexp DEJAGNU_GTEST_EXPECT_DEATH2
> +proc hwasan_get_gtest_expect_death_list { output } {
> + set idx 0
> + set ret ""
> + while {[regexp -start $idx -indices
> "DEJAGNU_GTEST_EXPECT_DEATH(\[0-9\]*)" "$output" -> id ] > 0} {
> + set low [lindex $id 0]
> + set high [lindex $id 1]
> + set val_id [string range "$output" $low $high]
> + if {[regexp -start $low -indices "$val_id (.*)
> DEJAGNU_GTEST_EXPECT_DEATH$val_id (.*)
> DEJAGNU_GTEST_EXPECT_DEATH$val_id\[\n\r\]" "$output" whole statement regexpr
> ] == 0} { break }
> + set low [lindex $statement 0]
> + set high [lindex $statement 1]
> + set val_statement [string range "$output" $low $high]
> + set low [lindex $regexpr 0]
> + set high [lindex $regexpr 1]
> + set val_regexpr [string range "$output" $low $high]
> + lappend ret [list "$val_id" "$val_statement" "$val_regexpr"]
> + set idx [lindex $whole 1]
> + }
> + return $ret
> +}
AFAICT these three functions are identical to the asan-dg.exp versions,
with no asan/hwasan variance. I think it would be better to reuse the
asan versions for hwasan too.
> +
> +# Replace ${tool}_load with a wrapper so that we can symbolize the output.
> +if { [info procs ${tool}_load] != [list] \
> + && [info procs saved_hwasan_${tool}_load] == [list] } {
> + rename ${tool}_load saved_hwasan_${tool}_load
> +
> + proc ${tool}_load { program args } {
> + global tool
> + global hwasan_last_gtest_test_list
> + global hwasan_last_gtest_expect_death_list
> + set result [eval [list saved_hwasan_${tool}_load $program] $args]
> + set output [lindex $result 1]
> + set symbolized_output [hwasan_symbolize "$output"]
> + set hwasan_last_gtest_test_list [hwasan_get_gtest_test_list "$output"]
> + set hwasan_last_gtest_expect_death_list
> [hwasan_get_gtest_expect_death_list "$output"]
> + set result [list [lindex $result 0] $symbolized_output]
> + return $result
> + }
> +}
Also, I'm not sure we need separate hwasan versions of these globals.
> +
> +# Utility for running gtest hwasan emulation under dejagnu, invoked via
> dg-final.
> +# Call pass if variable has the desired value, otherwise fail.
> +#
> +# Argument 0 handles expected failures and the like
> +proc hwasan-gtest { args } {
> + global tool
> + global hwasan_last_gtest_test_list
> + global hwasan_last_gtest_expect_death_list
> +
> + if { ![info exists hwasan_last_gtest_test_list] } { return }
> + if { [llength $hwasan_last_gtest_test_list] == 0 } { return }
> + if { ![isnative] || [is_remote target] } { return }
> +
> + set gtest_test_list $hwasan_last_gtest_test_list
> + unset hwasan_last_gtest_test_list
> +
> + if { [llength $args] >= 1 } {
> + switch [dg-process-target [lindex $args 0]] {
> + "S" { }
> + "N" { return }
> + "F" { setup_xfail "*-*-*" }
> + "P" { }
> + }
> + }
> +
> + # This assumes that we are three frames down from dg-test, and that
> + # it still stores the filename of the testcase in a local variable
> "name".
> + # A cleaner solution would require a new DejaGnu release.
> + upvar 2 name testcase
> + upvar 2 prog prog
> +
> + set output_file "[file rootname [file tail $prog]].exe"
> +
> + foreach gtest $gtest_test_list {
> + set testname "$testcase $gtest"
> + set status -1
> +
> + setenv DEJAGNU_GTEST_ARG "$gtest"
> + set result [${tool}_load ./$output_file $gtest]
> + unsetenv DEJAGNU_GTEST_ARG
> + set status [lindex $result 0]
> + set output [lindex $result 1]
> + if { "$status" == "pass" } {
> + pass "$testname execution test"
> + if { [info exists hwasan_last_gtest_expect_death_list] } {
> + set gtest_expect_death_list $hwasan_last_gtest_expect_death_list
> + foreach gtest_death $gtest_expect_death_list {
> + set id [lindex $gtest_death 0]
> + set testname "$testcase $gtest [lindex $gtest_death 1]"
> + set regexpr [lindex $gtest_death 2]
> + set status -1
> +
> + setenv DEJAGNU_GTEST_ARG "$gtest:$id"
> + set result [${tool}_load ./$output_file "$gtest:$id"]
> + unsetenv DEJAGNU_GTEST_ARG
> + set status [lindex $result 0]
> + set output [lindex $result 1]
> + if { "$status" == "fail" } {
> + pass "$testname execution test"
> + if { ![regexp $regexpr ${output}] } {
> + fail "$testname output pattern test"
> + send_log "Output should match: $regexpr\n"
> + } else {
> + pass "$testname output pattern test"
> + }
> + } elseif { "$status" == "pass" } {
> + fail "$testname execution test"
> + } else {
> + $status "$testname execution test"
> + }
> + }
> + }
> + } else {
> + $status "$testname execution test"
> + }
> + unset hwasan_last_gtest_expect_death_list
> + }
> +
> + return
> +}
And if there are no separate versions of those variables, and if
hwasan-dg.exp does include asan-dg.exp, then I think it would be
enough to replace hwasan_symbolize onwards with:
# Utility for running gtest hwasan emulation under dejagnu, invoked via
dg-final.
# Call pass if variable has the desired value, otherwise fail.
#
# Argument 0 handles expected failures and the like
proc hwasan-gtest { args } {
asan-gtest {*}$args
}
Thanks,
Richard
