[clang][deps] Call getMemBuffer with RequiresNullTerminator false - #194867
Closed
smithp35 wants to merge 1 commit into
Closed
[clang][deps] Call getMemBuffer with RequiresNullTerminator false#194867smithp35 wants to merge 1 commit into
smithp35 wants to merge 1 commit into
Conversation
smithp35
added
the
skip-precommit-approval
The getMemBuffer() has a default parameter RequiresNullTerminator which is set to true. In ModuleCache the MemoryBuffer::getOpenFile is called with /* RequiresNullTerminator=*/false. This means that initial contents of the MemoryBuffer may not have a trailing 0x0 at the end of the file. When assertions are enabled and RequiresNullTerminator is true the MemoryBuffer will trigger a "Buffer is not null terminated!" assertion failure if BufEnd[0] != 0. We have at one build with assertions enabled that is triggering this MemoryBuffer assertion failure in the check-clang tests: * ClangScanDeps/modules-dep-args.c * Driver/modules-driver-import-std.cpp The failure is specific to one particular machine, we have not been able to reproduce locally. It is possible that the failure is filesystem type or path length dependent. Changing the RequiresNullTerminator in getMemBuffer to false to match the value of RequiresNullTerminator in getOpenFile fixes the problem and all tests pass.
smithp35
force-pushed
the
read-without-requiring-null-terminate
branch
from
April 30, 2026 12:43
15e8806 to
41f1091
Compare
Contributor
Author
|
Closing in favour of full PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The getMemBuffer() has a default parameter RequiresNullTerminator which is set to true.
In ModuleCache the MemoryBuffer::getOpenFile is called with /* RequiresNullTerminator=*/false.
We have at one build with assertions enabled that is tripping over an assertion in MemoryBuffer that is checking for null termination. This failure is currently specific to this single machine and all attempts to reproduce have failed.
My working theory is that the length of path names are important so on most machines we may be adding trailing 0x0 bytes of padding which are being counted as null terminators.
Patch is in a draft state to see if we can tell if this fixes the problem.