Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -1746,15 +1746,22 @@ def subst_user_defined_msg : TextSubstitution<
"%select{the message|the expression}0 in "
"%select{a static assertion|this asm operand}0">;

def err_user_defined_msg_invalid : Error<
"%sub{subst_user_defined_msg}0 must be a string literal or an "
"object with 'data()' and 'size()' member functions">;
def err_user_defined_msg_invalid
: Error<"%sub{subst_user_defined_msg}0 must be a null terminated constant "
"string or an "
"object with 'data()' and 'size()' member functions">;
def err_user_defined_msg_missing_member_function : Error<
"the %select{message|string}0 object in "
"%select{this static assertion|this asm operand}0 is missing %select{"
"a 'size()' member function|"
"a 'data()' member function|"
"'data()' and 'size()' member functions}1">;
def err_user_defined_msg_not_null_terminated_string
: Error<"%sub{subst_user_defined_msg}0 is not null terminated">;
def ext_consteval_string_constants
: Extension<"consteval string constants are an extension">,
DefaultWarn,
InGroup<DiagGroup<"consteval-string-constants-extension">>;
def err_user_defined_msg_invalid_mem_fn_ret_ty : Error<
"%sub{subst_user_defined_msg}0 must have a '%select{size|data}1()' member "
"function returning an object convertible to '%select{std::size_t|const char *}1'">;
Expand Down
9 changes: 5 additions & 4 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -831,10 +831,11 @@ enum class CCEKind {
ArrayBound, ///< Array bound in array declarator or new-expression.
ExplicitBool, ///< Condition in an explicit(bool) specifier.
Noexcept, ///< Condition in a noexcept(bool) specifier.
StaticAssertMessageSize, ///< Call to size() in a static assert
///< message.
StaticAssertMessageData, ///< Call to data() in a static assert
///< message.
StaticAssertMessageSize, ///< Call to size() in a static assert
///< message.
StaticAssertMessageData, ///< Call to data() in a static assert
///< message.
StaticAssertNullTerminatedString, ///< tryEvaluateStrLen
};

/// Enums for the diagnostics of target, target_version and target_clones.
Expand Down
8 changes: 5 additions & 3 deletions clang/lib/AST/ByteCode/Context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,13 +294,15 @@ bool Context::evaluateStrlen(State &Parent, const Expr *E, uint64_t &Result) {
if (!FieldDesc->isPrimitiveArray())
return false;

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();
Comment on lines -297 to +302
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tbaederr maybe has ideas on testing it on it's own?

Copy link
Contributor

Choose a reason for hiding this comment

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

Adding it to this PR is fine IMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, this might fix #173175, which got filed just 10 minutes ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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

Result =
strnlen(reinterpret_cast<const char *>(Ptr.getRawAddress()), Size);
return Result != Size;
}

PrimType ElemT = FieldDesc->getPrimType();
Expand Down
42 changes: 42 additions & 0 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17693,6 +17693,44 @@ void Sema::DiagnoseStaticAssertDetails(const Expr *E) {
}
}

template <typename ResultType>
static bool EvaluateAsNullTerminatedCharBuffer(
Sema &SemaRef, Expr *Message, ResultType &Result, ASTContext &Ctx,
Sema::StringEvaluationContext EvalContext, bool ErrorOnInvalidMessage) {
SourceLocation Loc = Message->getBeginLoc();
QualType SizeT = Ctx.getSizeType();
QualType ConstCharPtr = Ctx.getPointerType(Ctx.getConstType(Ctx.CharTy));
Expr::EvalResult Status;
SmallVector<PartialDiagnosticAt, 8> Notes;
Status.Diag = &Notes;

auto DiagnoseInvalidConstantString = [&]() {
SemaRef.Diag(Loc, diag::err_user_defined_msg_not_null_terminated_string)
<< EvalContext;
for (const auto &Note : Notes)
SemaRef.Diag(Note.first, Note.second);
return !ErrorOnInvalidMessage;
};
ExprResult EvaluatedData = SemaRef.BuildConvertedConstantExpression(
Message, ConstCharPtr, CCEKind::StaticAssertNullTerminatedString);
if (EvaluatedData.isInvalid())
return DiagnoseInvalidConstantString();

uint64_t Length = 0;
if (!EvaluatedData.get()->tryEvaluateStrLen(Length, Ctx))
return DiagnoseInvalidConstantString();

llvm::APInt SizeVal(Ctx.getIntWidth(SizeT), Length);
Expr *SizeExpr = IntegerLiteral::Create(Ctx, SizeVal, SizeT, Loc);

bool EvalResult = Message->EvaluateCharRangeAsString(
Result, SizeExpr, EvaluatedData.get(), Ctx, Status);
if (!EvalResult || !Notes.empty())
return DiagnoseInvalidConstantString();
SemaRef.Diag(Loc, diag::ext_consteval_string_constants);
return true;
Comment on lines +17730 to +17731
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, you put that here! I would put it in the caller

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)

}

template <typename ResultType>
static bool EvaluateAsStringImpl(Sema &SemaRef, Expr *Message,
ResultType &Result, ASTContext &Ctx,
Expand Down Expand Up @@ -17726,6 +17764,10 @@ static bool EvaluateAsStringImpl(Sema &SemaRef, Expr *Message,

SourceLocation Loc = Message->getBeginLoc();
QualType T = Message->getType().getNonReferenceType();
if (T->isPointerType() && T->getPointeeType()->isCharType())
return EvaluateAsNullTerminatedCharBuffer(
SemaRef, Message, Result, Ctx, EvalContext, ErrorOnInvalidMessage);

Comment on lines +17767 to +17770
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

auto *RD = T->getAsCXXRecordDecl();
if (!RD) {
SemaRef.Diag(Loc, diag::err_user_defined_msg_invalid) << EvalContext;
Expand Down
29 changes: 28 additions & 1 deletion clang/test/Parser/asm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ struct string_view {
int foo1 asm ((string_view("test"))); // expected-error {{expected string literal in 'asm'}}
int func() asm ((string_view("test"))); // expected-error {{expected string literal in 'asm'}}

constexpr const char* getConstantString(const char* s) {
return s;
}

void f2() {
asm(string_view("")); // expected-error {{expected string literal or parenthesized constant expression in 'asm'}}
Expand All @@ -44,6 +47,13 @@ void f2() {
asm("" :: string_view("")); // expected-error {{expected string literal or parenthesized constant expression in 'asm'}}
asm(::string_view("")); // expected-error {{expected string literal or parenthesized constant expression in 'asm'}}

asm(getConstantString("")); // expected-error {{expected string literal or parenthesized constant expression in 'asm'}}
asm("" : getConstantString("")); // expected-error {{expected string literal or parenthesized constant expression in 'asm'}}
asm("" : : getConstantString("")); // expected-error {{expected string literal or parenthesized constant expression in 'asm'}}
asm("" : : : getConstantString("")); // expected-error {{expected ')'}}
asm("" :: getConstantString("")); // expected-error {{expected string literal or parenthesized constant expression in 'asm'}}
asm(::getConstantString("")); // expected-error {{expected string literal or parenthesized constant expression in 'asm'}}

int i;

asm((string_view("")));
Expand All @@ -55,5 +65,22 @@ void f2() {
asm("" : (::string_view("+g")) (i) : (::string_view("g")) (0) : (string_view("memory")));


asm((0)); // expected-error {{the expression in this asm operand must be a string literal or an object with 'data()' and 'size()' member functions}}
asm((getConstantString("")));
// expected-warning@-1 {{consteval string constants are an extension}}
asm((::getConstantString("")));
// expected-warning@-1 {{consteval string constants are an extension}}
asm("" : (::getConstantString("+g")) (i));
// expected-warning@-1 {{consteval string constants are an extension}}
asm("" : (::getConstantString("+g"))); // expected-error {{expected '(' after 'asm operand'}}
// expected-warning@-1 {{consteval string constants are an extension}}
asm("" : (::getConstantString("+g")) (i) : (::getConstantString("g")) (0));
// expected-warning@-1 2 {{consteval string constants are an extension}}
asm("" : (::getConstantString("+g")) (i) : (::getConstantString("g"))); // expected-error {{expected '(' after 'asm operand'}}
// expected-warning@-1 2 {{consteval string constants are an extension}}
asm("" : (::getConstantString("+g")) (i) : (::getConstantString("g")) (0) : (getConstantString("memory")));
// expected-warning@-1 3 {{consteval string constants are an extension}}



asm((0)); // expected-error {{the expression in this asm operand must be a null terminated constant string or an object with 'data()' and 'size()' member functions}}
}
4 changes: 2 additions & 2 deletions clang/test/SemaCXX/gnu-asm-constexpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ struct string_view {


void f() {
asm(("")); // expected-error {{the expression in this asm operand must be a string literal or an object with 'data()' and 'size()' member functions}}
asm(("")); // expected-error {{the expression in this asm operand must be a null terminated constant string or an object with 'data()' and 'size()' member functions}}
asm((NotAString{})); // expected-error {{the string object in this asm operand is missing 'data()' and 'size()' member functions}}
asm((MessageInvalidData{})); // expected-error {{the expression in this asm operand must have a 'data()' member function returning an object convertible to 'const char *'}} \
// expected-error {{too few arguments to function call, expected 1, have 0}}
Expand Down Expand Up @@ -106,7 +106,7 @@ void test_dependent1(int i) {

template void test_dependent1<int>(int);
// expected-note@-1 {{in instantiation of function template specialization}}
// expected-error@#err-int {{the expression in this asm operand must be a string literal or an object with 'data()' and 'size()' member functions}}
// expected-error@#err-int {{the expression in this asm operand must be a null terminated constant string or an object with 'data()' and 'size()' member functions}}
// expected-error@#err-int2 {{cannot initialize a value of type 'int' with an lvalue of type 'const char[3]'}}
// expected-error@#err-int3 {{cannot initialize a value of type 'int' with an lvalue of type 'const char[2]'}}
// expected-error@#err-int4 {{cannot initialize a value of type 'int' with an lvalue of type 'const char[7]'}}
Expand Down
51 changes: 49 additions & 2 deletions clang/test/SemaCXX/static-assert-cxx26.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// RUN: %clang_cc1 -std=c++2c -triple=x86_64-linux -fsyntax-only %s -verify -fexperimental-new-constant-interpreter

static_assert(true, "");
static_assert(true, 0); // expected-error {{the message in a static assertion must be a string literal or an object with 'data()' and 'size()' member functions}}
static_assert(true, 0); // expected-error {{the message in a static assertion must be a null terminated constant string or an object with 'data()' and 'size()' member functions}}
struct Empty{};
static_assert(true, Empty{}); // expected-error {{the message object in this static assertion is missing 'data()' and 'size()' member functions}}
struct NoData {
Expand Down Expand Up @@ -288,7 +288,7 @@ struct Good {

template <typename Ty>
struct Bad {
static_assert(false, Ty{}); // expected-error {{the message in a static assertion must be a string literal or an object with 'data()' and 'size()' member functions}} \
static_assert(false, Ty{}); // expected-error {{the message in a static assertion must be a null terminated constant string or an object with 'data()' and 'size()' member functions}} \
// expected-error {{static assertion failed}}
};

Expand Down Expand Up @@ -416,3 +416,50 @@ static_assert(
// expected-note@-1 {{read of dereferenced one-past-the-end pointer is not allowed in a constant expression}}
);
}

static_assert(false, &(" basic test"[1]));
// expected-error@-1 {{static assertion failed: basic test}}
// expected-warning@-2 {{consteval string constants are an extension}}

constexpr const char *constexpr_global = "global_constexpr";
constexpr const char null_terminated_buffer[] = { 'n', 'u', 'l', 'l', 't', 'e', 'r', 'm', 0 };
constexpr const char no_null_buffer[] = { 'n', 'o', 'n', 'u', 'l', 'l', 't', 'e', 'r', 'm' };

constexpr const char *selector(int i) {
constexpr const char * a_constant = "a_constant";
const char *non_constexpr = "non-constexpr string";
switch (i) {
case 0: return "case 0";
case 1: return a_constant;
case 2: return constexpr_global;
case 3: return null_terminated_buffer;
case 4: return &(""[1]); // point to after the null terminator
case 5: return nullptr;
case 6: return no_null_buffer;
}
};

static_assert(false, selector(0));
// expected-error@-1 {{static assertion failed: case 0}}
// expected-warning@-2 {{consteval string constants are an extension}}
static_assert(false, selector(1));
// expected-error@-1 {{static assertion failed: a_constant}}
// expected-warning@-2 {{consteval string constants are an extension}}
static_assert(false, selector(2));
// expected-error@-1 {{static assertion failed: global_constexpr}}
// expected-warning@-2 {{consteval string constants are an extension}}
static_assert(false, selector(3));
// expected-error@-1 {{static assertion failed: nullterm}}
// expected-warning@-2 {{consteval string constants are an extension}}
static_assert(false, selector(4));
// expected-error@-1 {{the message in a static assertion is not null terminated}}
// expected-error@-2 {{static assertion failed}}
static_assert(false, selector(5));
// expected-error@-1 {{the message in a static assertion is not null terminated}}
// expected-error@-2 {{static assertion failed}}
static_assert(false, selector(6));
// expected-error@-1 {{the message in a static assertion is not null terminated}}
// expected-error@-2 {{static assertion failed}}
static_assert(false, selector(7));
// expected-error@-1 {{the message in a static assertion is not null terminated}}
// expected-error@-2 {{static assertion failed}}