[clang] Add support for consteval null terminated strings #173140
Conversation
ojhunt
force-pushed
the
users/ojhunt/consteval-null-terminated-string-constants
branch
from
0c70ec6 to
b769a72
Compare
Adds support for null terminated strings produced by constexpr evaluation. This makes it possible to perform analysis of format strings that previously were not possible, and is needed in the future to support __ptrauth qualifier options.
ojhunt
force-pushed
the
users/ojhunt/consteval-null-terminated-string-constants
branch
from
b769a72 to
2a7c39e
Compare
| if (Ptr.isDummy() || Ptr.isUnknownSizeArray()) | ||
| if (Ptr.isDummy() || Ptr.isUnknownSizeArray() || Ptr.isPastEnd()) | ||
| return false; | ||
|
|
||
| unsigned N = Ptr.getNumElems(); | ||
| if (Ptr.elemSize() == 1) { | ||
| Result = strnlen(reinterpret_cast<const char *>(Ptr.getRawAddress()), N); | ||
| return Result != N; | ||
| unsigned Size = N - Ptr.getIndex(); |
There was a problem hiding this comment.
It is not possible to hit this in the new interpreter prior to this change, the existing code fails to handle out of bounds pointer into a string literal (or presumably any array), or an in bounds offset.
It feels like it should be in a different PR, but because it can't currently be hit it can't be tested?
There was a problem hiding this comment.
@tbaederr maybe has ideas on testing it on it's own?
There was a problem hiding this comment.
Adding it to this PR is fine IMO.
There was a problem hiding this comment.
Actually, this might fix #173175, which got filed just 10 minutes ago.
There was a problem hiding this comment.
@tbaederr ah ha - I was trying to find a string function that would actually pass that expression to this code but I couldn't work out the correct way to make the call, I think I was trying to get the crash via the first expression, but huzzah we have a test case - will pull this bit out in a moment
| if (T->isPointerType() && T->getPointeeType()->isCharType()) | ||
| return EvaluateAsNullTerminatedCharBuffer( | ||
| SemaRef, Message, Result, Ctx, EvalContext, ErrorOnInvalidMessage); | ||
|
|
There was a problem hiding this comment.
The problem of doing that here is that it is a non-standard behavior.
So at the minimum we'd need extension warnings. But I'm a bit uncomfortable adding that extension to begin with.
I think the functionality is best left to format string / ptrauth for now.
Which brings the question... how do we test this?
There was a problem hiding this comment.
A gross option would be to have a langopt + "enable this" cli flag, but that would imply people be able to rely on being able to do this.
| SemaRef.Diag(Loc, diag::ext_consteval_string_constants); | ||
| return true; |
There was a problem hiding this comment.
Oh, you put that here! I would put it in the caller
There was a problem hiding this comment.
if we make evaluating a null terminated expression a flag or mode then we can remove this condition - maybe I should then just make the format string or similar routines use the "evaluate null terminated expressions" flag? it seems like such a change would be a minor change (he says with unfounded optimism)
Adds support for null terminated strings produced by constexpr evaluation. This makes it possible to perform analysis of format strings that previously were not possible, and is needed in the future to support __ptrauth qualifier options.