Skip to content

fix tests and replace exec() in compute/_jit.py#8106

Closed
water0615 wants to merge 2 commits intoNVIDIA:mainfrom
water0615:branch19
Closed

fix tests and replace exec() in compute/_jit.py#8106
water0615 wants to merge 2 commits intoNVIDIA:mainfrom
water0615:branch19

Conversation

@water0615
Copy link
Copy Markdown

@water0615 water0615 commented Mar 19, 2026

Description

closes

fix tests and replace exec() in compute/_jit.py

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@water0615 water0615 requested review from a team as code owners March 19, 2026 08:51
@water0615 water0615 requested review from NaderAlAwar and tpn March 19, 2026 08:51
@github-project-automation github-project-automation bot moved this to Todo in CCCL Mar 19, 2026
@copy-pr-bot
Copy link
Copy Markdown
Contributor

copy-pr-bot bot commented Mar 19, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cccl-authenticator-app cccl-authenticator-app bot moved this from Todo to In Review in CCCL Mar 19, 2026
@shwina
Copy link
Copy Markdown
Contributor

shwina commented Mar 23, 2026

Thanks for the PR, @water0615!

By its nature, cuda.compute accepts arbitrary user callables and should be run in trusted environments.

We don't want to be too restrictive in terms of the contents of the user-provided callables. For instance, the __globals__ dict could contain e.g., other arbitrarily-named device functions. Thus, I think the restrictions in _validate_source_safety are a bit too strict and may prevent legitimate use-cases.

That being said, there are improvements we can make here in terms of validating, e.g., struct members to ensure they are identifiers. For example, I think the following patch would take us quite far in terms of better error reporting if someone passes something other than a identifier for the struct member name:

@@ -55,6 +55,13 @@ def gpu_struct(
     # At this point, field_dict must be a dict
     assert isinstance(field_dict, dict)

+    # Validate field names are valid Python identifiers before use in exec()
+    for key in field_dict:
+        if not isinstance(key, str) or not key.isidentifier():
+            raise ValueError(
+                f"gpu_struct field name {key!r} is not a valid Python identifier"
+            )
+
     # Normalize fields for storage on the struct class
     field_spec = {}
     for key, val in field_dict.items():

Do you think we could start there (and possibly some unit tests for the above)?

)
return namespace["impl"]
def impl(struct_val, idx):
return getattr(struct_val, field_name)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe numba's type system will need to see the attribute access rather than access via getattr; so this may not work as intended.

Instead, we could assert that all field names are valid Python identifiers via str.isidentifier().

@shwina
Copy link
Copy Markdown
Contributor

shwina commented Mar 31, 2026

I'm going to go ahead and close this PR, as #8235 addresses the issue without restricting the contents of user-provided callables.

@shwina shwina closed this Mar 31, 2026
@github-project-automation github-project-automation bot moved this from In Review to Done in CCCL Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants