[TySan] Improved compatability for tests #96507

gbMattN · 2024-06-24T15:14:21Z

All the violation tests failed when running on my machine. This was due to changes in file structure, and my machine using different underlying types for types like uint64_t. By changing some check lines to regular expressions, we can account for file locality, and hardware specific type differences. There was also an include needed for better integer types.

github-actions · 2024-06-24T15:14:43Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

gbMattN · 2024-06-24T15:14:48Z

@fhahn (Sorry for the ping, I can't manually add reviewers yet)

llvmbot · 2024-06-24T15:15:13Z

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (gbMattN)

Changes

All the violation tests failed when running on my machine. By changing some check lines to regular expressions, we can account for file locality, and hardware specific type differences. There was also an include needed for better integer types.

Full diff: https://github.com/llvm/llvm-project/pull/96507.diff

6 Files Affected:

(modified) compiler-rt/test/tysan/violation-pr45282.c (+1-1)
(modified) compiler-rt/test/tysan/violation-pr47137.c (+3-2)
(modified) compiler-rt/test/tysan/violation-pr62544.c (+1-1)
(modified) compiler-rt/test/tysan/violation-pr62828.cpp (+1-1)
(modified) compiler-rt/test/tysan/violation-pr68655.cpp (+1-1)
(modified) compiler-rt/test/tysan/violation-pr86685.c (+1-1)

diff --git a/compiler-rt/test/tysan/violation-pr45282.c b/compiler-rt/test/tysan/violation-pr45282.c
index 2cbc37b3d1832..37e710ae18deb 100644
--- a/compiler-rt/test/tysan/violation-pr45282.c
+++ b/compiler-rt/test/tysan/violation-pr45282.c
@@ -18,7 +18,7 @@ int main(void) {
 
 // CHECK:      TypeSanitizer: type-aliasing-violation on address
 // CHECK-NEXT: WRITE of size 8 at {{.+}} with type double accesses an existing object of type float
-// CHECK-NEXT:   in main violation-pr45282.c:25
+// CHECK-NEXT:   in main {{.*violation-pr45282.c:25.*}}
 
   // loop of problems
   for (j = 2; j <= 4; ++j) {
diff --git a/compiler-rt/test/tysan/violation-pr47137.c b/compiler-rt/test/tysan/violation-pr47137.c
index 04d68d1dd936e..7b996d39f81e2 100644
--- a/compiler-rt/test/tysan/violation-pr47137.c
+++ b/compiler-rt/test/tysan/violation-pr47137.c
@@ -4,6 +4,7 @@
 // https://github.com/llvm/llvm-project/issues/47137
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdint.h>
 
 void f(int m) {
   int n = (4 * m + 2) / 3;
@@ -23,8 +24,8 @@ void f(int m) {
   }
 
 // CHECK:      TypeSanitizer: type-aliasing-violation on address
-// CHECK-NEXT: READ of size 2 at {{.+}} with type short accesses an existing object of type long long
-// CHECK-NEXT:    in f violation-pr47137.c:30
+// CHECK-NEXT: READ of size 2 at {{.+}} with type short accesses an existing object of type {{(long)+}}
+// CHECK-NEXT:    in f {{.*violation-pr47137.c:31.*}}
   for (int i = 0, j = 0; j < 4 * m; i += 4, j += 3) {
     for (int k = 0; k < 3; k++) {
       ((uint16_t *)a)[j + k] = ((uint16_t *)a)[i + k];
diff --git a/compiler-rt/test/tysan/violation-pr62544.c b/compiler-rt/test/tysan/violation-pr62544.c
index 4187a91bde3fc..7c6cb59156d15 100644
--- a/compiler-rt/test/tysan/violation-pr62544.c
+++ b/compiler-rt/test/tysan/violation-pr62544.c
@@ -18,7 +18,7 @@ int main() {
 
 // CHECK:      TypeSanitizer: type-aliasing-violation on address
 // CHECK-NEXT: WRITE of size 2 at {{.+}} with type short accesses an existing object of type int
-// CHECK-NEXT:   in main violation-pr62544.c:22
+// CHECK-NEXT:   in main {{.*violation-pr62544.c:22.*}}
   *e = 3;
   printf("%d\n", a);
 }
diff --git a/compiler-rt/test/tysan/violation-pr62828.cpp b/compiler-rt/test/tysan/violation-pr62828.cpp
index 879200c8069b0..f815227d86248 100644
--- a/compiler-rt/test/tysan/violation-pr62828.cpp
+++ b/compiler-rt/test/tysan/violation-pr62828.cpp
@@ -24,7 +24,7 @@ short *test1(int_v8 *cast_c_array, short_v8 *shuf_c_array1, int *ptr) {
 
 // CHECK:      ERROR: TypeSanitizer: type-aliasing-violation on address
 // CHECK-NEXT: READ of size 2 at {{.+}} with type short accesses an existing object of type int
-// CHECK-NEXT:    in test1(int (*) [8], short (*) [8], int*) violation-pr62828.cpp:29
+// CHECK-NEXT:    in test1(int (*) [8], short (*) [8], int*) {{.*violation-pr62828.cpp:29.*}}
   for (int i3 = 0; i3 < 4; ++i3) {
     output2[i3] = input2[(i3 * 2)];
   }
diff --git a/compiler-rt/test/tysan/violation-pr68655.cpp b/compiler-rt/test/tysan/violation-pr68655.cpp
index ac20f8c94e1ff..615971c75d20e 100644
--- a/compiler-rt/test/tysan/violation-pr68655.cpp
+++ b/compiler-rt/test/tysan/violation-pr68655.cpp
@@ -9,7 +9,7 @@ struct S1 {
 
 // CHECK: TypeSanitizer: type-aliasing-violation on address
 // CHECK-NEXT:  READ of size 4 at {{.+}} with type int accesses an existing object of type long long (in S1 at offset 0)
-// CHECK-NEXT: in copyMem(S1*, S1*) violation-pr68655.cpp:19
+// CHECK-NEXT: in copyMem(S1*, S1*) {{.*violation-pr68655.cpp:19.*}}
 
 void inline copyMem(S1 *dst, S1 *src) {
   unsigned *d = reinterpret_cast<unsigned *>(dst);
diff --git a/compiler-rt/test/tysan/violation-pr86685.c b/compiler-rt/test/tysan/violation-pr86685.c
index b5198c440fa44..6667fc1805195 100644
--- a/compiler-rt/test/tysan/violation-pr86685.c
+++ b/compiler-rt/test/tysan/violation-pr86685.c
@@ -13,7 +13,7 @@ void foo(int *s, float *f, long n) {
 
 // CHECK:      TypeSanitizer: type-aliasing-violation on address
 // CHECK-NEXT: WRITE of size 4 at {{.+}} with type int accesses an existing object of type float
-// CHECK-NEXT:   #0 {{.+}} in foo violation-pr86685.c:17
+// CHECK-NEXT:   #0 {{.+}} in foo {{.*violation-pr86685.c:17.*}}
     *s = 4;
   }
 }

gbMattN · 2024-07-12T16:22:19Z

@fhahn updated to remove merge conflicts

cjappl

This seems reasonable enough to me, I'm far from a tysan expert, but I requested review for you. 👍

compiler-rt/test/tysan/violation-pr47137.c

jmorse · 2024-11-11T17:28:27Z

compiler-rt/test/tysan/violation-pr45282.c

  // CHECK:      TypeSanitizer: type-aliasing-violation on address
  // CHECK-NEXT: WRITE of size 8 at {{.+}} with type double accesses an existing object of type float
-  // CHECK-NEXT:   in main violation-pr45282.c:25
+  // CHECK-NEXT:   in main {{.*violation-pr45282.c:25.*}}


Nit: IMO having {{.*}} to swallow extra path components is slightly preferable, as that avoids the filename being part of a regex. Almost certainly not ever going to be a problem, but it reduces what can go wrong. Does your environment also have trailing components after the line number? If there's any whitespace between '5' and the extra text then FileCheck will (99% confident) ignore those anyway.

Similar thoughts apply to all the other similar regexes.

Checked and the trailing .* can indeed be left out, I'll change that

Compiler-rt tests tend to have the file name in the check lines because you want to make sure that the sanitizer correctly informs the user where the problem is.

jmorse · 2024-11-11T17:31:17Z

compiler-rt/test/tysan/violation-pr47137.c

  // CHECK:      TypeSanitizer: type-aliasing-violation on address
-  // CHECK-NEXT: READ of size 2 at {{.+}} with type short accesses an existing object of type long long
-  // CHECK-NEXT:    in f violation-pr47137.c:30
+  // CHECK-NEXT: READ of size 2 at {{.+}} with type short accesses an existing object of type {{(long)+}}


IMO having an optional (with ?) long is preferable to having an unbound number of longs.

Good catch, will change that 👍

Created using spr 1.3.4

github-actions · 2024-11-27T16:00:11Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 8e6e62d0dee48a696afd0c7d53d74eaccef97b5e 11c62d601626d9da1fb3ed0c9cadab2d106681ab --extensions cpp,c -- compiler-rt/test/tysan/violation-pr45282.c compiler-rt/test/tysan/violation-pr47137.c compiler-rt/test/tysan/violation-pr62544.c compiler-rt/test/tysan/violation-pr62828.cpp compiler-rt/test/tysan/violation-pr68655.cpp compiler-rt/test/tysan/violation-pr86685.c

View the diff from clang-format here.

diff --git a/compiler-rt/test/tysan/violation-pr47137.c b/compiler-rt/test/tysan/violation-pr47137.c
index 72693afe66..e488c23fe1 100644
--- a/compiler-rt/test/tysan/violation-pr47137.c
+++ b/compiler-rt/test/tysan/violation-pr47137.c
@@ -2,9 +2,9 @@
 // RUN: FileCheck %s < %t.out
 
 // https://github.com/llvm/llvm-project/issues/47137
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <stdint.h>
 
 void f(int m) {
   int n = (4 * m + 2) / 3;

fhahn · 2024-12-17T18:56:07Z

Sorry I didn't mean to close this PR, looks like it happened automatically once I deleted the users/fhahn/tysan-a-type-sanitizer-runtime-library branch :(

llvmbot added compiler-rt compiler-rt:sanitizer labels Jun 24, 2024

fhahn force-pushed the users/fhahn/tysan-a-type-sanitizer-runtime-library branch from e2caf4e to 733b3ed Compare June 27, 2024 16:10

gbMattN force-pushed the tysan-test-violation-fix branch from 712aa7f to 9df832f Compare June 28, 2024 16:49

cjappl requested a review from fhahn October 22, 2024 16:57

cjappl reviewed Oct 22, 2024

View reviewed changes

compiler-rt/test/tysan/violation-pr47137.c Show resolved Hide resolved

jmorse reviewed Nov 11, 2024

View reviewed changes

gbMattN force-pushed the tysan-test-violation-fix branch from 9df832f to c684ebd Compare November 12, 2024 11:19

gbMattN requested a review from jmorse November 12, 2024 17:06

fhahn added 6 commits November 22, 2024 14:19

[𝘀𝗽𝗿] initial version

be460ed

Created using spr 1.3.4

!fixup address comments, thanks

9a6e027

!fiupx address comments, thanks!

a6d44dd

[TySan] A Type Sanitizer (Clang)

ab8d005

!fixup: add test

9cbb116

[TySan] A Type Sanitizer (Runtime Library)

8e6e62d

fhahn force-pushed the users/fhahn/tysan-a-type-sanitizer-runtime-library branch from 733b3ed to 8e6e62d Compare November 22, 2024 19:34

[TySan] Improves compatability for tests

11c62d6

gbMattN force-pushed the tysan-test-violation-fix branch from c684ebd to 11c62d6 Compare November 27, 2024 15:56

fhahn force-pushed the users/fhahn/tysan-a-type-sanitizer-runtime-library branch 3 times, most recently from 5c0e520 to 6f48a28 Compare December 10, 2024 12:22

fhahn deleted the branch llvm:users/fhahn/tysan-a-type-sanitizer-runtime-library December 17, 2024 18:49

fhahn closed this Dec 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[TySan] Improved compatability for tests #96507

[TySan] Improved compatability for tests #96507

Uh oh!

gbMattN commented Jun 24, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Jun 24, 2024

Uh oh!

gbMattN commented Jun 24, 2024

Uh oh!

llvmbot commented Jun 24, 2024

Uh oh!

gbMattN commented Jul 12, 2024

Uh oh!

cjappl left a comment

Uh oh!

Uh oh!

jmorse Nov 11, 2024

Uh oh!

gbMattN Nov 12, 2024

Uh oh!

jmorse Nov 11, 2024

Uh oh!

gbMattN Nov 12, 2024

Uh oh!

github-actions bot commented Nov 27, 2024

Uh oh!

fhahn commented Dec 17, 2024

Uh oh!

Uh oh!

[TySan] Improved compatability for tests #96507

[TySan] Improved compatability for tests #96507

Uh oh!

Conversation

gbMattN commented Jun 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 24, 2024

Uh oh!

gbMattN commented Jun 24, 2024

Uh oh!

llvmbot commented Jun 24, 2024

Uh oh!

gbMattN commented Jul 12, 2024

Uh oh!

cjappl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jmorse Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

gbMattN Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

jmorse Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

gbMattN Nov 12, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 27, 2024

Uh oh!

fhahn commented Dec 17, 2024

Uh oh!

Uh oh!

gbMattN commented Jun 24, 2024 •

edited

Loading