testsuite: Remove no_fsanitize_address install directory dependency

The current no_fsanitize_address effective target check (implemented in
target-supports.exp rather than in asan.exp) has some problems with the
link path.

Because it is not called from in between asan_init and asan_finish the
link paths of the compiler are not changed to point at the build
directories.

That means that they point at the install directory that the current
build is configured for.  Hence this test passes if the current compiler
has ASAN support *and* if there are ASAN libraries in the directory that
this build is configured to install into.

That is an unnecessary requirement.  On looking through each of the
tests that currently use no_fsanitize_address it seems all are `compile`
tests.  Hence we can change the logical test of the effective target
from "can we link an ASAN executable" to "can we compile for ASAN" and
avoid the need to set up link paths correctly for this test.

N.b. one alternative would be to remove this effective target and try to
move all tests which currently use this into directories which run their
tests between calls to `asan_finish` and `asan_init`.  This seems like
it might ensure a clearer division of "asan tests must be run in X
directories" and avoid problems similar to the one here in the future.
I'm suggesting this change as it appears the easiest to make and I
didn't think the above too bad a risk to take -- especially if the name
of the test is clear.

In doing this I also inverted the meaning of this check.  Rather than
the function indicating whether we *do not* support something and
tests using `dg-skip-if` to avoid running a test if this returns true,
this function indicates whether we *do* support something and tests
use `dg-require-effective-target` to run a test if it returns true.

Testing done by checking that each of the affected testcases changes
from UNSUPPORTED to PASS when run individually without any install
directory available.

gcc/testsuite/ChangeLog:

	* g++.dg/warn/uninit-pr93100.C: Convert no_fsanitize_address use
	to fsanitize_address_compilation.
	* gcc.dg/pr91441.c: Likewise.
	* gcc.dg/pr96260.c: Likewise.
	* gcc.dg/pr96307.c: Likewise.
	* gcc.dg/uninit-pr93100.c: Likewise.
	* gnat.dg/asan1.adb: Likewise.
	* gcc.target/aarch64/sve/pr97696.c: Likewise.
	* lib/target-supports.exp (no_fsanitize_address): Rename to ...
	(fsanitize_address_compilation): Make this a compile-only test
	and avoid the need to set linker paths and invert semantics.



###############     Attachment also inlined for ease of reply    ###############
diff --git a/gcc/testsuite/g++.dg/warn/uninit-pr93100.C b/gcc/testsuite/g++.dg/warn/uninit-pr93100.C
index e08a36d68a91ba9620ab44d5772017d598f50826..2f6056cc3c3e8fd4aa8e88236a31b8f4344b89ca 100644
--- a/gcc/testsuite/g++.dg/warn/uninit-pr93100.C
+++ b/gcc/testsuite/g++.dg/warn/uninit-pr93100.C
@@ -1,7 +1,7 @@
 /* PR tree-optimization/98508 - Sanitizer disable -Wall and -Wextra
    { dg-do compile }
    { dg-options "-O0 -Wall -fsanitize=address" }
-   { dg-skip-if "no address sanitizer" { no_fsanitize_address } } */
+   { dg-require-effective-target fsanitize_address_compilation } */
 
 struct S
 {
diff --git a/gcc/testsuite/gcc.dg/pr91441.c b/gcc/testsuite/gcc.dg/pr91441.c
index 4c785f61e597533202f9d3a42ce5a94aa3fd758f..4bd4c295913262463e2029f4a0466719474cc77a 100644
--- a/gcc/testsuite/gcc.dg/pr91441.c
+++ b/gcc/testsuite/gcc.dg/pr91441.c
@@ -1,7 +1,7 @@
 /* PR target/91441 */
 /* { dg-do compile  } */
 /* { dg-options "--param asan-stack=1 -fsanitize=kernel-address" } */
-/* { dg-skip-if "no address sanitizer" { no_fsanitize_address } } */
+/* { dg-require-effective-target fsanitize_address_compilation } */
 
 int *bar(int *);
 int *f( int a)
diff --git a/gcc/testsuite/gcc.dg/pr96260.c b/gcc/testsuite/gcc.dg/pr96260.c
index 587afb76116c5759751d5d6a2ceb1b4a392bc38a..a2b326c8e567972494e6e64d33a3e7f6c514f91d 100644
--- a/gcc/testsuite/gcc.dg/pr96260.c
+++ b/gcc/testsuite/gcc.dg/pr96260.c
@@ -1,7 +1,7 @@
 /* PR target/96260 */
 /* { dg-do compile } */
 /* { dg-options "--param asan-stack=1 -fsanitize=kernel-address -fasan-shadow-offset=0x100000" } */
-/* { dg-skip-if "no address sanitizer" { no_fsanitize_address } } */
+/* { dg-require-effective-target fsanitize_address_compilation } */
 
 int *bar(int *);
 int *f( int a)
diff --git a/gcc/testsuite/gcc.dg/pr96307.c b/gcc/testsuite/gcc.dg/pr96307.c
index 89002b85c8ea6829e6b78679eedde653bb16753e..f49a9f642d35ffdfb16ac4f715dd8a25793a4817 100644
--- a/gcc/testsuite/gcc.dg/pr96307.c
+++ b/gcc/testsuite/gcc.dg/pr96307.c
@@ -1,7 +1,7 @@
 /* PR target/96307 */
 /* { dg-do compile } */
 /* { dg-additional-options "-fsanitize=kernel-address --param=asan-instrumentation-with-call-threshold=8" } */
-/* { dg-skip-if "no address sanitizer" { no_fsanitize_address } } */
+/* { dg-require-effective-target fsanitize_address_compilation } */
 
 #include <limits.h>
 enum a {test1, test2, test3=INT_MAX};
diff --git a/gcc/testsuite/gcc.dg/uninit-pr93100.c b/gcc/testsuite/gcc.dg/uninit-pr93100.c
index 7cb02227bff8a448fd582efdee4ecf955ccaf16f..57838e61b1d3a4442700cd3f257a978f71589018 100644
--- a/gcc/testsuite/gcc.dg/uninit-pr93100.c
+++ b/gcc/testsuite/gcc.dg/uninit-pr93100.c
@@ -1,7 +1,7 @@
 /* PR tree-optimization/93100 - gcc -fsanitize=address inhibits -Wuninitialized
    { dg-do compile }
    { dg-options "-Wall -fsanitize=address" }
-   { dg-skip-if "no address sanitizer" { no_fsanitize_address } } */
+   { dg-require-effective-target fsanitize_address_compilation } */
 
 struct A
 {
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr97696.c b/gcc/testsuite/gcc.target/aarch64/sve/pr97696.c
index 8b7de18a07d8c855919e27c683007a70b1076aac..f63ad1455bce04d0e6568a07335c1b3107282770 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/pr97696.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/pr97696.c
@@ -1,4 +1,4 @@
-/* { dg-skip-if "" { no_fsanitize_address } } */
+/* { dg-require-effective-target fsanitize_address_compilation  } */
 /* { dg-options "-fsanitize=address -fsanitize-address-use-after-scope" } */
 
 #include <arm_sve.h>
diff --git a/gcc/testsuite/gnat.dg/asan1.adb b/gcc/testsuite/gnat.dg/asan1.adb
index 26bc1a404475ad7811f607de15d2dfd8f237d2e3..eef58efa4baa8f79e9c6b650f0e41cf8d90362d5 100644
--- a/gcc/testsuite/gnat.dg/asan1.adb
+++ b/gcc/testsuite/gnat.dg/asan1.adb
@@ -1,7 +1,7 @@
 --  { dg-do compile }
 --  { dg-additional-sources asan1_pkg.ads }
 --  { dg-options "-fsanitize=address" }
---  { dg-skip-if "no address sanitizer" { no_fsanitize_address } }
+--  { dg-require-effective-target fsanitize_address_compilation }
 
 with Asan1_Pkg;
 
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index d3edc7d839ec2d9460501c157c1129176542d870..d9ea048d01212c4d28e68860a38c523cea8f2393 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -13166,16 +13166,16 @@ proc check_effective_target_movdir { } {
     } "-mmovdiri -mmovdir64b" ]
 }
 
-# Return 1 if the target does not support address sanitizer, 0 otherwise
+# Return 1 if the target supports address sanitizer, 0 otherwise
 
-proc check_effective_target_no_fsanitize_address {} {
-    if ![check_no_compiler_messages fsanitize_address executable {
+proc check_effective_target_fsanitize_address_compilation {} {
+    if ![check_no_compiler_messages fsanitize_address assembly {
 	int main (void) { return 0; }
     } "-fsanitize=address" ] {
-	return 1;
+	return 0;
     }
 
-    return 0;
+    return 1;
 }
 
 # Return 1 if this target supports 'R' flag in .section directive, 0

... Oops, just after sending I noticed that
`check_effective_target_fsanitize_address_compilation` is caching its 
result under the same name as the original 
`check_effective_target_fsanitize_address` in `asan-dg.exp`.

Attaching an updated patch (with updated cover letter) that adjusts the 
property that uses a different name to cache under, given that this is
testing something different than the original function.
Have again tested that the problematic tests run without an install 
directory.


On 7/10/24 13:19, Matthew Malcomson wrote:
> The current no_fsanitize_address effective target check (implemented in
> target-supports.exp rather than in asan.exp) has some problems with the
> link path.
> 
> Because it is not called from in between asan_init and asan_finish the
> link paths of the compiler are not changed to point at the build
> directories.
> 
> That means that they point at the install directory that the current
> build is configured for.  Hence this test passes if the current compiler
> has ASAN support *and* if there are ASAN libraries in the directory that
> this build is configured to install into.
> 
> That is an unnecessary requirement.  On looking through each of the
> tests that currently use no_fsanitize_address it seems all are `compile`
> tests.  Hence we can change the logical test of the effective target
> from "can we link an ASAN executable" to "can we compile for ASAN" and
> avoid the need to set up link paths correctly for this test.
> 
> N.b. one alternative would be to remove this effective target and try to
> move all tests which currently use this into directories which run their
> tests between calls to `asan_finish` and `asan_init`.  This seems like
> it might ensure a clearer division of "asan tests must be run in X
> directories" and avoid problems similar to the one here in the future.
> I'm suggesting this change as it appears the easiest to make and I
> didn't think the above too bad a risk to take -- especially if the name
> of the test is clear.
> 
> In doing this I also inverted the meaning of this check.  Rather than
> the function indicating whether we *do not* support something and
> tests using `dg-skip-if` to avoid running a test if this returns true,
> this function indicates whether we *do* support something and tests
> use `dg-require-effective-target` to run a test if it returns true.
> 
> Testing done by checking that each of the affected testcases changes
> from UNSUPPORTED to PASS when run individually without any install
> directory available.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/warn/uninit-pr93100.C: Convert no_fsanitize_address use
> 	to fsanitize_address_compilation.
> 	* gcc.dg/pr91441.c: Likewise.
> 	* gcc.dg/pr96260.c: Likewise.
> 	* gcc.dg/pr96307.c: Likewise.
> 	* gcc.dg/uninit-pr93100.c: Likewise.
> 	* gnat.dg/asan1.adb: Likewise.
> 	* gcc.target/aarch64/sve/pr97696.c: Likewise.
> 	* lib/target-supports.exp (no_fsanitize_address): Rename to ...
> 	(fsanitize_address_compilation): Make this a compile-only test
> 	and avoid the need to set linker paths and invert semantics.
> 
> 
> 
> ###############     Attachment also inlined for ease of reply    ###############
> 
> 
> diff --git a/gcc/testsuite/g++.dg/warn/uninit-pr93100.C b/gcc/testsuite/g++.dg/warn/uninit-pr93100.C
> index e08a36d68a91ba9620ab44d5772017d598f50826..2f6056cc3c3e8fd4aa8e88236a31b8f4344b89ca 100644
> --- a/gcc/testsuite/g++.dg/warn/uninit-pr93100.C
> +++ b/gcc/testsuite/g++.dg/warn/uninit-pr93100.C
> @@ -1,7 +1,7 @@
>   /* PR tree-optimization/98508 - Sanitizer disable -Wall and -Wextra
>      { dg-do compile }
>      { dg-options "-O0 -Wall -fsanitize=address" }
> -   { dg-skip-if "no address sanitizer" { no_fsanitize_address } } */
> +   { dg-require-effective-target fsanitize_address_compilation } */
>   
>   struct S
>   {
> diff --git a/gcc/testsuite/gcc.dg/pr91441.c b/gcc/testsuite/gcc.dg/pr91441.c
> index 4c785f61e597533202f9d3a42ce5a94aa3fd758f..4bd4c295913262463e2029f4a0466719474cc77a 100644
> --- a/gcc/testsuite/gcc.dg/pr91441.c
> +++ b/gcc/testsuite/gcc.dg/pr91441.c
> @@ -1,7 +1,7 @@
>   /* PR target/91441 */
>   /* { dg-do compile  } */
>   /* { dg-options "--param asan-stack=1 -fsanitize=kernel-address" } */
> -/* { dg-skip-if "no address sanitizer" { no_fsanitize_address } } */
> +/* { dg-require-effective-target fsanitize_address_compilation } */
>   
>   int *bar(int *);
>   int *f( int a)
> diff --git a/gcc/testsuite/gcc.dg/pr96260.c b/gcc/testsuite/gcc.dg/pr96260.c
> index 587afb76116c5759751d5d6a2ceb1b4a392bc38a..a2b326c8e567972494e6e64d33a3e7f6c514f91d 100644
> --- a/gcc/testsuite/gcc.dg/pr96260.c
> +++ b/gcc/testsuite/gcc.dg/pr96260.c
> @@ -1,7 +1,7 @@
>   /* PR target/96260 */
>   /* { dg-do compile } */
>   /* { dg-options "--param asan-stack=1 -fsanitize=kernel-address -fasan-shadow-offset=0x100000" } */
> -/* { dg-skip-if "no address sanitizer" { no_fsanitize_address } } */
> +/* { dg-require-effective-target fsanitize_address_compilation } */
>   
>   int *bar(int *);
>   int *f( int a)
> diff --git a/gcc/testsuite/gcc.dg/pr96307.c b/gcc/testsuite/gcc.dg/pr96307.c
> index 89002b85c8ea6829e6b78679eedde653bb16753e..f49a9f642d35ffdfb16ac4f715dd8a25793a4817 100644
> --- a/gcc/testsuite/gcc.dg/pr96307.c
> +++ b/gcc/testsuite/gcc.dg/pr96307.c
> @@ -1,7 +1,7 @@
>   /* PR target/96307 */
>   /* { dg-do compile } */
>   /* { dg-additional-options "-fsanitize=kernel-address --param=asan-instrumentation-with-call-threshold=8" } */
> -/* { dg-skip-if "no address sanitizer" { no_fsanitize_address } } */
> +/* { dg-require-effective-target fsanitize_address_compilation } */
>   
>   #include <limits.h>
>   enum a {test1, test2, test3=INT_MAX};
> diff --git a/gcc/testsuite/gcc.dg/uninit-pr93100.c b/gcc/testsuite/gcc.dg/uninit-pr93100.c
> index 7cb02227bff8a448fd582efdee4ecf955ccaf16f..57838e61b1d3a4442700cd3f257a978f71589018 100644
> --- a/gcc/testsuite/gcc.dg/uninit-pr93100.c
> +++ b/gcc/testsuite/gcc.dg/uninit-pr93100.c
> @@ -1,7 +1,7 @@
>   /* PR tree-optimization/93100 - gcc -fsanitize=address inhibits -Wuninitialized
>      { dg-do compile }
>      { dg-options "-Wall -fsanitize=address" }
> -   { dg-skip-if "no address sanitizer" { no_fsanitize_address } } */
> +   { dg-require-effective-target fsanitize_address_compilation } */
>   
>   struct A
>   {
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr97696.c b/gcc/testsuite/gcc.target/aarch64/sve/pr97696.c
> index 8b7de18a07d8c855919e27c683007a70b1076aac..f63ad1455bce04d0e6568a07335c1b3107282770 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/pr97696.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr97696.c
> @@ -1,4 +1,4 @@
> -/* { dg-skip-if "" { no_fsanitize_address } } */
> +/* { dg-require-effective-target fsanitize_address_compilation  } */
>   /* { dg-options "-fsanitize=address -fsanitize-address-use-after-scope" } */
>   
>   #include <arm_sve.h>
> diff --git a/gcc/testsuite/gnat.dg/asan1.adb b/gcc/testsuite/gnat.dg/asan1.adb
> index 26bc1a404475ad7811f607de15d2dfd8f237d2e3..eef58efa4baa8f79e9c6b650f0e41cf8d90362d5 100644
> --- a/gcc/testsuite/gnat.dg/asan1.adb
> +++ b/gcc/testsuite/gnat.dg/asan1.adb
> @@ -1,7 +1,7 @@
>   --  { dg-do compile }
>   --  { dg-additional-sources asan1_pkg.ads }
>   --  { dg-options "-fsanitize=address" }
> ---  { dg-skip-if "no address sanitizer" { no_fsanitize_address } }
> +--  { dg-require-effective-target fsanitize_address_compilation }
>   
>   with Asan1_Pkg;
>   
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index d3edc7d839ec2d9460501c157c1129176542d870..d9ea048d01212c4d28e68860a38c523cea8f2393 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -13166,16 +13166,16 @@ proc check_effective_target_movdir { } {
>       } "-mmovdiri -mmovdir64b" ]
>   }
>   
> -# Return 1 if the target does not support address sanitizer, 0 otherwise
> +# Return 1 if the target supports address sanitizer, 0 otherwise
>   
> -proc check_effective_target_no_fsanitize_address {} {
> -    if ![check_no_compiler_messages fsanitize_address executable {
> +proc check_effective_target_fsanitize_address_compilation {} {
> +    if ![check_no_compiler_messages fsanitize_address assembly {
>   	int main (void) { return 0; }
>       } "-fsanitize=address" ] {
> -	return 1;
> +	return 0;
>       }
>   
> -    return 0;
> +    return 1;
>   }
>   
>   # Return 1 if this target supports 'R' flag in .section directive, 0
> 
> 
>

Hi Matthew,

> The current no_fsanitize_address effective target check (implemented in
> target-supports.exp rather than in asan.exp) has some problems with the
> link path.
>
> Because it is not called from in between asan_init and asan_finish the
> link paths of the compiler are not changed to point at the build
> directories.
>
> That means that they point at the install directory that the current
> build is configured for.  Hence this test passes if the current compiler
> has ASAN support *and* if there are ASAN libraries in the directory that
> this build is configured to install into.
>
> That is an unnecessary requirement.  On looking through each of the
> tests that currently use no_fsanitize_address it seems all are `compile`
> tests.  Hence we can change the logical test of the effective target
> from "can we link an ASAN executable" to "can we compile for ASAN" and
> avoid the need to set up link paths correctly for this test.
>
> N.b. one alternative would be to remove this effective target and try to
> move all tests which currently use this into directories which run their
> tests between calls to `asan_finish` and `asan_init`.  This seems like
> it might ensure a clearer division of "asan tests must be run in X
> directories" and avoid problems similar to the one here in the future.
> I'm suggesting this change as it appears the easiest to make and I
> didn't think the above too bad a risk to take -- especially if the name
> of the test is clear.

moving the tests would be clearer IMO, otherwise we have two separate
mechanisms for the same issue.  Especially since we're talking about 7
tests only.  The only complication would be the aarch64 test where
there's currently no asan subdir.

> In doing this I also inverted the meaning of this check.  Rather than
> the function indicating whether we *do not* support something and
> tests using `dg-skip-if` to avoid running a test if this returns true,
> this function indicates whether we *do* support something and tests
> use `dg-require-effective-target` to run a test if it returns true.
>
> Testing done by checking that each of the affected testcases changes
> from UNSUPPORTED to PASS when run individually without any install
> directory available.

Please remember to state on which target you've run the tests.

> gcc/testsuite/ChangeLog:
>
> 	* g++.dg/warn/uninit-pr93100.C: Convert no_fsanitize_address use
> 	to fsanitize_address_compilation.

*If* we go this route (and as I said I'd prefer not to; maybe Mike
differs), please rename the new keyword to fsanitize_address_compile in
line with e.g. "dg-do compile".  You also would need to adjust
sourcebuild.texi for the change.

> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index d3edc7d839ec2d9460501c157c1129176542d870..d9ea048d01212c4d28e68860a38c523cea8f2393 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -13166,16 +13166,16 @@ proc check_effective_target_movdir { } {
>      } "-mmovdiri -mmovdir64b" ]
>  }
>  
> -# Return 1 if the target does not support address sanitizer, 0 otherwise
> +# Return 1 if the target supports address sanitizer, 0 otherwise

Comment needs adjustment, too.

	Rainer

On 7/10/24 13:42, Rainer Orth wrote:
>> N.b. one alternative would be to remove this effective target and try to
>> move all tests which currently use this into directories which run their
>> tests between calls to `asan_finish` and `asan_init`.  This seems like
>> it might ensure a clearer division of "asan tests must be run in X
>> directories" and avoid problems similar to the one here in the future.
>> I'm suggesting this change as it appears the easiest to make and I
>> didn't think the above too bad a risk to take -- especially if the name
>> of the test is clear.
> 
> moving the tests would be clearer IMO, otherwise we have two separate
> mechanisms for the same issue.  Especially since we're talking about 7
> tests only.  The only complication would be the aarch64 test where
> there's currently no asan subdir.

I'm in the middle of doing this.

Just wanted to mention the other complications (in case that changes 
what looks like the best option) -- the compilation options that tests 
get run under change (because they're being run by a different test 
runner), and also there's no `asan` subdir for gnat.dg (the ada test).

I'm thinking that the change in options is not that bad (will eventually 
filter out the options that are not valid for the test, will likely run 
with slightly less variations than before), but still worth mentioning.

> 
>> In doing this I also inverted the meaning of this check.  Rather than
>> the function indicating whether we *do not* support something and
>> tests using `dg-skip-if` to avoid running a test if this returns true,
>> this function indicates whether we *do* support something and tests
>> use `dg-require-effective-target` to run a test if it returns true.
>>
>> Testing done by checking that each of the affected testcases changes
>> from UNSUPPORTED to PASS when run individually without any install
>> directory available.
> 
> Please remember to state on which target you've run the tests.

Good point ;-)
I ran the SVE test on AArch64, the ada test on x86 (because that's where 
I could install a host gnat compiler easily to build ada), and all the 
others on both these targets.