[UR] Add lifetime validation to validation layer #1121

kswiecicki · 2023-11-24T10:44:54Z

... and refactor the validation layer mako template.

Relies on: #1029.
Solves: #867.

scripts/core/common.yml

scripts/templates/helper.py

codecov-commenter · 2023-11-29T09:54:04Z

Codecov Report

Attention: 623 lines in your changes are missing coverage. Please review.

Comparison is base (32e2533) 15.36% compared to head (c0f0a70) 15.33%.

Files	Patch %	Lines
source/loader/layers/validation/ur_valddi.cpp	2.68%	617 Missing ⚠️
test/layers/validation/fixtures.hpp	75.00%	4 Missing ⚠️
source/loader/layers/validation/ur_leak_check.hpp	93.10%	2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1121      +/-   ##
==========================================
- Coverage   15.36%   15.33%   -0.04%     
==========================================
  Files         240      241       +1     
  Lines       34182    34820     +638     
  Branches     3788     3989     +201     
==========================================
+ Hits         5252     5339      +87     
- Misses      28879    29430     +551     
  Partials       51       51

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pbalcer

right now this checks whether the provided ptr is a valid ptr to something, not necessarily an object of the expected type. Shouldn't be too hard to add?

scripts/core/INTRO.rst

scripts/templates/valddi.cpp.mako

pbalcer · 2023-12-05T09:58:45Z

scripts/core/INTRO.rst

@@ -256,6 +256,8 @@ Layers currently included with the runtime are as follows:
     - Enables non-adapter-specific parameter validation (e.g. checking for null values).
   * - UR_LAYER_LEAK_CHECKING
     - Performs some leak checking for API calls involving object creation/destruction.
+   * - UR_LAYER_LIFETIME_VALIDATION
+     - Performs lifetime validation on objects used in API calls. Requires UR_LAYER_LEAK_CHECKING.


please explain what "lifetime validation" is.

I've added an explanation, take a look.

kswiecicki · 2023-12-07T15:44:11Z

right now this checks whether the provided ptr is a valid ptr to something, not necessarily an object of the expected type. Shouldn't be too hard to add?

I've added std::type_index info to the RefRuntimeInfo and check for the index equality when checking a reference validity.
I was trying to also print handle type names but they were mangled and showed internal type names like ur_adapter_handle_t_ * instead of ur_adapter_handle_t.

pbalcer · 2023-12-11T11:20:56Z

test/layers/validation/fixtures.hpp

+
+    void SetUp() {
+        valAdaptersTest::SetUp();
+        adapter = adapters[0]; // TODO - which to choose?


the way the tests are run there will only ever be one adapter. so [0] is fine.

pbalcer · 2023-12-11T11:24:37Z

I was trying to also print handle type names but they were mangled and showed internal type names like ur_adapter_handle_t_ * instead of ur_adapter_handle_t.

I think that's fine. If you want to do this, one approach might be to pass the type name as a string from the generated layer to the class with runtime info. Would allow you to do away with typeindex and get correct type names. Not sure if it's worth it.

pbalcer · 2024-01-24T10:03:37Z

scripts/core/common.yml

@@ -102,7 +102,7 @@ name: "$x_kernel_handle_t"
 --- #--------------------------------------------------------------------------
 type: handle
 desc: "Handle of a queue object"
-class: $xPlatform
+class: $xQueue


this needs to be moved into another PR. Otherwise this one lgtm and we can merge it.

Done. The queue fix is here #1285.

The leak checking feature has been expanded to include handles obtained via the ...Get() functions.

Match files in leak checking tests also account for the ..Create(), ..Get() and ..Release() functions used in test setups.

... handle type checks.

pbalcer reviewed Nov 24, 2023

View reviewed changes

scripts/core/common.yml Outdated Show resolved Hide resolved

PatKamin reviewed Nov 27, 2023

View reviewed changes

scripts/templates/helper.py Show resolved Hide resolved

kswiecicki force-pushed the val-use-after-free branch 3 times, most recently from 43c1f19 to 93a709c Compare November 29, 2023 09:39

kswiecicki force-pushed the val-use-after-free branch 2 times, most recently from 65d5318 to 07e1009 Compare November 29, 2023 13:33

kswiecicki changed the title ~~[UR] Add support for handle tracking~~ [UR] Add lifetime validation to validation layer Nov 29, 2023

kswiecicki force-pushed the val-use-after-free branch from 07e1009 to 9f5ced0 Compare November 29, 2023 13:41

kswiecicki marked this pull request as ready for review November 29, 2023 13:41

kswiecicki force-pushed the val-use-after-free branch 2 times, most recently from 6f797f1 to 4487664 Compare December 4, 2023 09:49

pbalcer reviewed Dec 5, 2023

View reviewed changes

kswiecicki force-pushed the val-use-after-free branch from 4487664 to 7dbacd7 Compare December 6, 2023 13:45

kswiecicki requested a review from a team as a code owner December 6, 2023 13:45

kswiecicki force-pushed the val-use-after-free branch 2 times, most recently from e72dc55 to 510bd27 Compare December 8, 2023 15:12

pbalcer reviewed Dec 11, 2023

View reviewed changes

pbalcer approved these changes Dec 11, 2023

View reviewed changes

kswiecicki force-pushed the val-use-after-free branch 4 times, most recently from b79dbdd to 8b686dc Compare December 20, 2023 09:43

kswiecicki force-pushed the val-use-after-free branch 3 times, most recently from 952ba35 to 57d0a47 Compare December 27, 2023 14:09

kswiecicki force-pushed the val-use-after-free branch from 57d0a47 to 8540c10 Compare December 29, 2023 10:02

kswiecicki force-pushed the val-use-after-free branch 6 times, most recently from 7eac656 to b160633 Compare January 23, 2024 12:48

pbalcer reviewed Jan 24, 2024

View reviewed changes

kswiecicki force-pushed the val-use-after-free branch 2 times, most recently from fd803c8 to a08fc85 Compare January 26, 2024 09:04

kswiecicki mentioned this pull request Jan 26, 2024

[SPEC] Fix queue handle class #1285

Merged

kswiecicki force-pushed the val-use-after-free branch 8 times, most recently from 925c5e8 to 51be719 Compare February 5, 2024 15:19

kswiecicki force-pushed the val-use-after-free branch 3 times, most recently from e8dd668 to 1ba7c7b Compare February 8, 2024 15:51

kswiecicki added 7 commits February 9, 2024 10:29

[UR] Refactor validation layer refcount feature

9d24b81

The leak checking feature has been expanded to include handles obtained via the ...Get() functions.

[UR] Adjust validation layer leak tests

1c5e22b

Match files in leak checking tests also account for the ..Create(), ..Get() and ..Release() functions used in test setups.

[UR] Add lifetime validation to validation layer

f7bd6d9

[UR] Add tests for lifetime validation

f1dd200

[UR] Add lifetime validation documentation

dca6c88

[UR] Expand lifetime validation with

25225fd

... handle type checks.

[UR] Update copyright dates in changed files

c0f0a70

kswiecicki force-pushed the val-use-after-free branch from 1ba7c7b to c0f0a70 Compare February 9, 2024 09:29

pbalcer merged commit 186bfb9 into oneapi-src:main Feb 9, 2024
52 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UR] Add lifetime validation to validation layer #1121

[UR] Add lifetime validation to validation layer #1121

kswiecicki commented Nov 24, 2023 •

edited

Loading

codecov-commenter commented Nov 29, 2023 •

edited

Loading

pbalcer left a comment

pbalcer Dec 5, 2023

kswiecicki Dec 6, 2023

kswiecicki commented Dec 7, 2023

pbalcer Dec 11, 2023

pbalcer commented Dec 11, 2023

pbalcer Jan 24, 2024

kswiecicki Jan 26, 2024

[UR] Add lifetime validation to validation layer #1121

[UR] Add lifetime validation to validation layer #1121

Conversation

kswiecicki commented Nov 24, 2023 • edited Loading

codecov-commenter commented Nov 29, 2023 • edited Loading

Codecov Report

pbalcer left a comment

Choose a reason for hiding this comment

pbalcer Dec 5, 2023

Choose a reason for hiding this comment

kswiecicki Dec 6, 2023

Choose a reason for hiding this comment

kswiecicki commented Dec 7, 2023

pbalcer Dec 11, 2023

Choose a reason for hiding this comment

pbalcer commented Dec 11, 2023

pbalcer Jan 24, 2024

Choose a reason for hiding this comment

kswiecicki Jan 26, 2024

Choose a reason for hiding this comment

kswiecicki commented Nov 24, 2023 •

edited

Loading

codecov-commenter commented Nov 29, 2023 •

edited

Loading