Skip to content

Conversation

@islas
Copy link
Collaborator

@islas islas commented Dec 2, 2024

TYPE: bug fix

KEYWORDS: registry, path

SOURCE: internal

DESCRIPTION OF CHANGES:
Problem:
PR #1975 added the ability for registry code to be generated out-of-source. To do this, a provided path is used and prepended to include statements within registry files. The max path length checks to create the search path string in a char* does not take into account the combined prepend path + relative file path length. When this happens, no obvious errors occur, but a buffer overflow causes malformed registry code to be used, eventually leading to failed compilation.

Solution:
Use a max path length that checks both the relative file path length and combined length when using the prepended path. As failure to pass this will result in erroneous code, the error messages have been updated to convey that as well.

TESTS CONDUCTED:

  1. Tested with CMake build using an absolute path greater than ~128. Without these changes the registry code will pass but compilation will fail. With these changes, that path length works and paths > 256 will fail at the registry code generation step.

RELEASE NOTE: Bug fix for registry path length checks when doing out-of-source code generation

@islas islas requested a review from a team as a code owner December 2, 2024 23:06
@weiwangncar
Copy link
Collaborator

The regression test results:

Test Type              | Expected  | Received |  Failed
= = = = = = = = = = = = = = = = = = = = = = = =  = = = =
Number of Tests        : 23           24
Number of Builds       : 60           57
Number of Simulations  : 158           150        0
Number of Comparisons  : 95           86        0

Failed Simulations are: 
None
Which comparisons are not bit-for-bit: 
None

@islas
Copy link
Collaborator Author

islas commented Jan 1, 2025

@mgduda The failure mode was still implicit, so I've added return statements that end Registry processing when these errors are hit. This falls inline with the expected return of this function and the call chain handles it appropriately.

Copy link
Collaborator

@dudhia dudhia left a comment

Choose a reason for hiding this comment

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

I will approve this even though it is outside my scope. Is this just related to cmake? If so, the description should reflect that.

@islas
Copy link
Collaborator Author

islas commented Oct 21, 2025

@dudhia Just to close the loop on this thread, the fix isn't explicitly cmake related. Since cmake uses absolute paths this error would more likely occur in that configuration, but one could make an incredibly long registry file name and achieve the same effect.

@islas islas merged commit b791a59 into wrf-model:develop Oct 21, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants