Reapply "[analyzer] Fix false positive in strchr/strrchr with constant args" - #212124
Open
steakhal wants to merge 2 commits into
Open
Reapply "[analyzer] Fix false positive in strchr/strrchr with constant args"#212124steakhal wants to merge 2 commits into
steakhal wants to merge 2 commits into
Conversation
…t args" (llvm#211857) This reverts commit ca78391.
The test uses u"" and U"" string literals, which require C11 or later. The RUN line didn't specify a standard, so it picked up the target default. That is gnu17 on most targets, but PS4 defaults to gnu99 (see getDefaultLanguageStandard in clang/lib/Basic/LangStandards.cpp), so the test failed on the SIE buildbot with "use of undeclared identifier 'u'". Reproduce with: clang -cc1 -triple x86_64-scei-ps4 -analyze -verify ... Pin the RUN line to -std=c17, and add a second RUN line that explicitly uses the PS4 triple, so that losing the -std= again is caught on every bot instead of only on the SIE ones.
llvmorg-github-actions
Bot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Balázs Benics (steakhal) ChangesRelands #211857 Patch is 30.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/212124.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 745297dd1f057..b47fca0e40af4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -164,23 +164,14 @@ class CStringChecker
{{CDM::CLibrary, {"strsep"}, 2}, &CStringChecker::evalStrsep},
{{CDM::CLibrary, {"strxfrm"}, 3}, &CStringChecker::evalStrxfrm},
{{CDM::CLibraryMaybeHardened, {"strchr"}, 2},
- llvm::bind_back(&CStringChecker::evalStrchrCommon, "strchr()",
- /*CanReturnNull=*/true)},
+ &CStringChecker::evalStrchr},
{{CDM::CLibraryMaybeHardened, {"strrchr"}, 2},
- llvm::bind_back(&CStringChecker::evalStrchrCommon, "strrchr()",
- /*CanReturnNull=*/true)},
+ &CStringChecker::evalStrrchr},
{{CDM::CLibraryMaybeHardened, {"memchr"}, 3},
- llvm::bind_back(&CStringChecker::evalStrchrCommon, "memchr()",
- /*CanReturnNull=*/true)},
- {{CDM::CLibrary, {"strstr"}, 2},
- llvm::bind_back(&CStringChecker::evalStrchrCommon, "strstr()",
- /*CanReturnNull=*/true)},
- {{CDM::CLibrary, {"strpbrk"}, 2},
- llvm::bind_back(&CStringChecker::evalStrchrCommon, "strpbrk()",
- /*CanReturnNull=*/true)},
- {{CDM::CLibrary, {"strchrnul"}, 2},
- llvm::bind_back(&CStringChecker::evalStrchrCommon, "strchrnul()",
- /*CanReturnNull=*/false)},
+ &CStringChecker::evalMemchr},
+ {{CDM::CLibrary, {"strstr"}, 2}, &CStringChecker::evalStrstr},
+ {{CDM::CLibrary, {"strpbrk"}, 2}, &CStringChecker::evalStrpbrk},
+ {{CDM::CLibrary, {"strchrnul"}, 2}, &CStringChecker::evalStrchrnul},
{{CDM::CLibrary, {"bcopy"}, 3}, &CStringChecker::evalBcopy},
{{CDM::CLibrary, {"bcmp"}, 3},
std::bind(&CStringChecker::evalMemcmp, _1, _2, _3, CK_Regular)},
@@ -244,8 +235,18 @@ class CStringChecker
void evalStrsep(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrchr(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrrchr(CheckerContext &C, const CallEvent &Call) const;
+ void evalMemchr(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrstr(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrpbrk(CheckerContext &C, const CallEvent &Call) const;
+ void evalStrchrnul(CheckerContext &C, const CallEvent &Call) const;
+
+ /// Shared transition logic for strchr-family functions.
+ /// ConstOffset: nullopt = unknown, npos = not found, other = exact offset.
void evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
- StringRef FnName, bool CanReturnNull) const;
+ bool CanReturnNull,
+ std::optional<size_t> ConstOffset) const;
void evalStdCopy(CheckerContext &C, const CallEvent &Call) const;
void evalStdCopyBackward(CheckerContext &C, const CallEvent &Call) const;
@@ -272,6 +273,8 @@ class CStringChecker
const MemRegion *MR,
bool hypothetical);
static const StringLiteral *getStringLiteralFromRegion(const MemRegion *MR);
+ // Like getStringLiteralFromRegion, but also handles ElementRegion offsets.
+ static std::optional<StringRef> getStringRefAtRegion(const MemRegion *R);
SVal getCStringLength(CheckerContext &C,
ProgramStateRef &state,
@@ -1047,7 +1050,7 @@ CStringChecker::getStringLiteralFromRegion(const MemRegion *MR) {
return cast<StringRegion>(MR)->getStringLiteral();
case MemRegion::NonParamVarRegionKind:
if (const VarDecl *Decl = cast<NonParamVarRegion>(MR)->getDecl();
- Decl->getType().isConstQualified() && Decl->hasGlobalStorage())
+ Decl->getType().isConstQualified())
return dyn_cast_or_null<StringLiteral>(Decl->getInit());
return nullptr;
default:
@@ -1055,6 +1058,29 @@ CStringChecker::getStringLiteralFromRegion(const MemRegion *MR) {
}
}
+std::optional<StringRef>
+CStringChecker::getStringRefAtRegion(const MemRegion *R) {
+ if (!R)
+ return std::nullopt;
+ size_t Offset = 0;
+ const MemRegion *Base = R->StripCasts();
+ if (const auto *ER = dyn_cast<ElementRegion>(Base)) {
+ if (auto Idx = ER->getIndex().getAs<nonloc::ConcreteInt>()) {
+ Offset = Idx->getValue().get()->getZExtValue();
+ Base = ER->getSuperRegion()->StripCasts();
+ } else {
+ return std::nullopt;
+ }
+ }
+ const StringLiteral *Lit = getStringLiteralFromRegion(Base);
+ if (!Lit)
+ return std::nullopt;
+ StringRef S = Lit->getBytes();
+ if (Offset > S.size())
+ return std::nullopt;
+ return S.substr(Offset);
+}
+
SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state,
const Expr *Ex, SVal Buf,
bool hypothetical) const {
@@ -2639,14 +2665,158 @@ void CStringChecker::evalStrsep(CheckerContext &C,
C.addTransition(State);
}
+/// Compute the constant search offset for strchr/strrchr/strchrnul.
+/// Try to resolve the source (first) argument to its string literal content.
+static std::optional<StringRef> getHaystack(CheckerContext &C,
+ const CallEvent &Call) {
+ ProgramStateRef State = C.getState();
+ const StackFrame *SF = C.getStackFrame();
+ SVal SrcVal = State->getSVal(Call.getArgExpr(0), SF);
+ return CStringChecker::getStringRefAtRegion(SrcVal.getAsRegion());
+}
+
+/// Get the null-terminated C string view of the haystack.
+static StringRef getCStr(StringRef Haystack) {
+ size_t NulPos = Haystack.find('\0');
+ return (NulPos != StringRef::npos) ? Haystack.substr(0, NulPos) : Haystack;
+}
+
+/// Try to extract the constant character from the second argument.
+static std::optional<char> getSearchChar(CheckerContext &C,
+ const CallEvent &Call) {
+ SValBuilder &SVB = C.getSValBuilder();
+ SVal Arg1Val = C.getState()->getSVal(Call.getArgExpr(1), C.getStackFrame());
+ const llvm::APSInt *CharInt = SVB.getKnownValue(C.getState(), Arg1Val);
+ if (!CharInt)
+ return std::nullopt;
+ return static_cast<char>(CharInt->getExtValue());
+}
+
+/// Resolve the haystack and delegate to a function-specific search lambda.
+using SearchFn = std::function<std::optional<size_t>(
+ CheckerContext &, const CallEvent &, StringRef)>;
+
+static std::optional<size_t>
+computeStringOffset(CheckerContext &C, const CallEvent &Call, SearchFn Search) {
+ auto Haystack = getHaystack(C, Call);
+ if (!Haystack)
+ return std::nullopt;
+ return Search(C, Call, *Haystack);
+}
+
+/// Search for a character in the null-terminated C string view.
+// SearchFn for strchr/strrchr/strchrnul.
+static std::optional<size_t> searchChar(CheckerContext &C,
+ const CallEvent &Call,
+ StringRef Haystack, bool Reverse,
+ bool NulOnMiss) {
+ auto Ch = getSearchChar(C, Call);
+ if (!Ch)
+ return std::nullopt;
+ StringRef CStr = getCStr(Haystack);
+ if (*Ch == '\0')
+ return CStr.size();
+ size_t Pos = Reverse ? CStr.rfind(*Ch) : CStr.find(*Ch);
+ if (Pos == StringRef::npos && NulOnMiss)
+ return CStr.size();
+ return Pos;
+}
+
+void CStringChecker::evalStrchr(CheckerContext &C,
+ const CallEvent &Call) const {
+ CurrentFunctionDescription = "strchr()";
+ evalStrchrCommon(
+ C, Call, /*CanReturnNull=*/true,
+ computeStringOffset(
+ C, Call,
+ llvm::bind_back(searchChar, /*Reverse=*/false, /*NulOnMiss=*/false)));
+}
+
+void CStringChecker::evalStrrchr(CheckerContext &C,
+ const CallEvent &Call) const {
+ CurrentFunctionDescription = "strrchr()";
+ evalStrchrCommon(
+ C, Call, /*CanReturnNull=*/true,
+ computeStringOffset(
+ C, Call,
+ llvm::bind_back(searchChar, /*Reverse=*/true, /*NulOnMiss=*/false)));
+}
+
+void CStringChecker::evalStrchrnul(CheckerContext &C,
+ const CallEvent &Call) const {
+ CurrentFunctionDescription = "strchrnul()";
+ evalStrchrCommon(
+ C, Call, /*CanReturnNull=*/false,
+ computeStringOffset(
+ C, Call,
+ llvm::bind_back(searchChar, /*Reverse=*/false, /*NulOnMiss=*/true)));
+}
+
+void CStringChecker::evalMemchr(CheckerContext &C,
+ const CallEvent &Call) const {
+ CurrentFunctionDescription = "memchr()";
+ auto Search = [](CheckerContext &C, const CallEvent &Call,
+ StringRef Haystack) -> std::optional<size_t> {
+ auto Ch = getSearchChar(C, Call);
+ if (!Ch || Call.getNumArgs() < 3)
+ return std::nullopt;
+ SValBuilder &SVB = C.getSValBuilder();
+ const llvm::APSInt *Len = SVB.getKnownValue(
+ C.getState(),
+ C.getState()->getSVal(Call.getArgExpr(2), C.getStackFrame()));
+ if (!Len)
+ return std::nullopt;
+ uint64_t N = Len->getZExtValue();
+ // Include the implicit null terminator in the searchable region.
+ SmallString<64> Buf(Haystack);
+ Buf.push_back('\0');
+ StringRef Region = StringRef(Buf.data(), Buf.size());
+ if (N > Region.size())
+ return std::nullopt;
+ return Region.substr(0, N).find(*Ch);
+ };
+ evalStrchrCommon(C, Call, /*CanReturnNull=*/true,
+ computeStringOffset(C, Call, Search));
+}
+
+void CStringChecker::evalStrstr(CheckerContext &C,
+ const CallEvent &Call) const {
+ CurrentFunctionDescription = "strstr()";
+ auto Search = [](CheckerContext &C, const CallEvent &Call,
+ StringRef Haystack) -> std::optional<size_t> {
+ SVal Arg1Val = C.getState()->getSVal(Call.getArgExpr(1), C.getStackFrame());
+ auto Needle = CStringChecker::getStringRefAtRegion(Arg1Val.getAsRegion());
+ if (!Needle)
+ return std::nullopt;
+ StringRef CStr = getCStr(Haystack);
+ StringRef CNeedle = getCStr(*Needle);
+ return CNeedle.empty() ? size_t{0} : CStr.find(CNeedle);
+ };
+ evalStrchrCommon(C, Call, /*CanReturnNull=*/true,
+ computeStringOffset(C, Call, Search));
+}
+
+void CStringChecker::evalStrpbrk(CheckerContext &C,
+ const CallEvent &Call) const {
+ CurrentFunctionDescription = "strpbrk()";
+ auto Search = [](CheckerContext &C, const CallEvent &Call,
+ StringRef Haystack) -> std::optional<size_t> {
+ SVal Arg1Val = C.getState()->getSVal(Call.getArgExpr(1), C.getStackFrame());
+ auto Accept = CStringChecker::getStringRefAtRegion(Arg1Val.getAsRegion());
+ if (!Accept)
+ return std::nullopt;
+ return getCStr(Haystack).find_first_of(getCStr(*Accept));
+ };
+ evalStrchrCommon(C, Call, /*CanReturnNull=*/true,
+ computeStringOffset(C, Call, Search));
+}
+
void CStringChecker::evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
- StringRef FnName,
- bool CanReturnNull) const {
- CurrentFunctionDescription = FnName;
+ bool CanReturnNull,
+ std::optional<size_t> ConstOffset) const {
const Expr *CE = Call.getOriginExpr();
assert(CE);
- // These functions always return a pointer.
if (!CE->getType()->isPointerType())
return;
@@ -2655,21 +2825,26 @@ void CStringChecker::evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
SValBuilder &SVB = C.getSValBuilder();
ASTContext &Ctx = C.getASTContext();
- // The first argument must be non-null for all functions in this family.
SourceArgExpr Src = {{Call.getArgExpr(0), 0}};
SVal SrcVal = State->getSVal(Src.Expression, SF);
State = checkNonNull(C, State, Src, SrcVal);
if (!State)
return;
- // NULL (no-match) branch.
- if (CanReturnNull) {
+ bool MustMatch = ConstOffset && *ConstOffset != StringRef::npos;
+ bool MustNotMatch = ConstOffset && *ConstOffset == StringRef::npos;
+
+ // NULL (no-match) branch — skip when the match is guaranteed.
+ if (CanReturnNull && !MustMatch) {
ProgramStateRef NullState =
State->BindExpr(CE, SF, SVB.makeNullWithType(CE->getType()));
C.addTransition(NullState);
}
- // Found branch: a pointer within the source; needs a Loc for the arithmetic.
+ // Found branch — skip when the match is impossible.
+ if (MustNotMatch)
+ return;
+
std::optional<Loc> SrcLoc = SrcVal.getAs<Loc>();
if (!SrcLoc) {
SVal Result = SVB.conjureSymbolVal(Call, C.blockCount());
@@ -2678,13 +2853,26 @@ void CStringChecker::evalStrchrCommon(CheckerContext &C, const CallEvent &Call,
return;
}
- // The result is: Src + SymOffset
+ // If we know the exact offset, use a concrete value.
+ if (MustMatch) {
+ NonLoc ConcreteOffset =
+ SVB.makeIntVal(*ConstOffset, Ctx.getSizeType()).castAs<NonLoc>();
+ SVal Result = SVB.evalBinOpLN(State, BO_Add, *SrcLoc, ConcreteOffset,
+ Src.Expression->getType());
+ State = State->BindExpr(CE, SF, Result);
+ C.addTransition(State);
+ return;
+ }
+
+ // Unknown match: use a symbolic offset constrained to be in bounds.
auto RemainingExtentBytes =
getDynamicExtentWithOffset(State, *SrcLoc).castAs<DefinedOrUnknownSVal>();
NonLoc SymOffset =
SVB.conjureSymbolVal(Call, Ctx.getSizeType(), C.blockCount())
.castAs<NonLoc>();
State = State->assumeInBound(SymOffset, RemainingExtentBytes, true);
+ if (!State)
+ return;
SVal Result = SVB.evalBinOpLN(State, BO_Add, *SrcLoc, SymOffset,
Src.Expression->getType());
diff --git a/clang/test/Analysis/string-search-modeling.c b/clang/test/Analysis/string-search-modeling.c
index a50ec439731a3..931f1bcbd526d 100644
--- a/clang/test/Analysis/string-search-modeling.c
+++ b/clang/test/Analysis/string-search-modeling.c
@@ -1,4 +1,13 @@
-// RUN: %clang_analyze_cc1 -verify %s \
+// The u"" and U"" string literals below need C11 or later. Pin the standard
+// because targets such as PS4 default to gnu99.
+// RUN: %clang_analyze_cc1 -std=c17 -verify %s \
+// RUN: -analyzer-checker=core,unix \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -analyzer-config eagerly-assume=false
+//
+// Also check a target whose default C standard is gnu99, so that dropping the
+// -std= above is caught everywhere instead of only on the SIE buildbots.
+// RUN: %clang_analyze_cc1 -triple x86_64-scei-ps4 -std=c17 -verify %s \
// RUN: -analyzer-checker=core,unix \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-config eagerly-assume=false
@@ -176,3 +185,407 @@ void no_invalidation_of_globals(const char *p) {
clang_analyzer_eval(local_unmodified == 10); // expected-warning {{TRUE}}
clang_analyzer_eval(global_unmodified == 20); // expected-warning {{TRUE}}
}
+
+//===----------------------------------------------------------------------===//
+// When both arguments are compile-time constants, only the correct branch is
+// taken: found when the target exists, null when it does not.
+// See: https://github.com/llvm/llvm-project/issues/209905
+//===----------------------------------------------------------------------===//
+
+// --- strchr / strrchr: target character IS in the literal ---
+const char *test_strrchr_const_no_fp(void) {
+ // This is the original reproducer from #209905.
+ return strrchr("/foo/bar.c", '/') ? strrchr("/foo/bar.c", '/') + 1 : "/foo/bar.c"; // no-warning
+}
+
+void test_strchr_const_found(void) {
+ clang_analyzer_eval(strchr("/foo/bar.c", '/') == 0); // expected-warning {{FALSE}}
+}
+
+void test_strrchr_const_found(void) {
+ clang_analyzer_eval(strrchr("/foo/bar.c", '/') == 0); // expected-warning {{FALSE}}
+}
+
+// --- strchr / strrchr: target character is NOT in the literal ---
+void test_strchr_const_not_found(void) {
+ clang_analyzer_eval(strchr("hello", 'z') == 0); // expected-warning {{TRUE}}
+}
+
+void test_strrchr_const_not_found(void) {
+ clang_analyzer_eval(strrchr("hello", 'z') == 0); // expected-warning {{TRUE}}
+}
+
+// --- memchr: character within bounds ---
+void test_memchr_const_found(void) {
+ clang_analyzer_eval(memchr("abcdef", 'c', 6) == 0); // expected-warning {{FALSE}}
+}
+
+// --- memchr: character beyond the specified length ---
+void test_memchr_const_not_in_range(void) {
+ clang_analyzer_eval(memchr("abcdef", 'f', 3) == 0); // expected-warning {{TRUE}}
+}
+
+// --- strstr: needle IS a substring ---
+void test_strstr_const_found(void) {
+ clang_analyzer_eval(strstr("hello world", "world") == 0); // expected-warning {{FALSE}}
+}
+
+// --- strstr: needle is NOT a substring ---
+void test_strstr_const_not_found(void) {
+ clang_analyzer_eval(strstr("hello world", "xyz") == 0); // expected-warning {{TRUE}}
+}
+
+// --- strpbrk: accept set has a match ---
+void test_strpbrk_const_found(void) {
+ clang_analyzer_eval(strpbrk("hello", "aeiou") == 0); // expected-warning {{FALSE}}
+}
+
+// --- strpbrk: no character from accept set in source ---
+void test_strpbrk_const_not_found(void) {
+ clang_analyzer_eval(strpbrk("hello", "xyz") == 0); // expected-warning {{TRUE}}
+}
+
+// --- Non-constant source: both branches must still exist ---
+void test_strchr_non_const_source(const char *p) {
+ clang_analyzer_eval(strchr(p, '/') == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+// --- Various constant source forms: const, static const, #define, __FILE__ ---
+static const char static_const_path[] = "/usr/local/bin/tool";
+
+void test_strchr_static_const(void) {
+ clang_analyzer_eval(strchr(static_const_path, '/') == 0); // expected-warning {{FALSE}}
+}
+
+const char global_const_path[] = "/etc/config";
+
+void test_strrchr_global_const(void) {
+ clang_analyzer_eval(strrchr(global_const_path, '/') == 0); // expected-warning {{FALSE}}
+}
+
+#define FIXED_PATH "/home/user/project/file.c"
+
+void test_strchr_define(void) {
+ clang_analyzer_eval(strchr(FIXED_PATH, '/') == 0); // expected-warning {{FALSE}}
+}
+
+#define MY_FILE_BASENAME (strrchr(__FILE__, '/') ? strrchr(__FILE__, '/') + 1 : __FILE__)
+
+void test_file_basename_macro(void) {
+ const char *base = MY_FILE_BASENAME; // no-warning
+ (void)base;
+}
+
+#define PREFIX "module"
+#define SUFFIX "_handler"
+// Adjacent string literal concatenation (the realistic preprocessor pattern):
+#define MODULE_PATH "/opt/" PREFIX "/" SUFFIX ".so"
+
+void test_strchr_concatenated_define(void) {
+ clang_analyzer_eval(strchr(MODULE_PATH, '/') == 0); // expected-warning {{FALSE}}
+}
+
+// --- Character argument via #define ---
+#define SEPARATOR '/'
+
+void test_strchr_define_char(void) {
+ clang_analyzer_eval(strchr("/foo/bar", SEPARATOR) == 0); // expected-warning {{FALSE}}
+}
+
+#define SEARCH_CHAR 'x'
+
+void test_strchr_define_char_not_found(void) {
+ clang_analyzer_eval(strchr("/foo/bar", SEARCH_CHAR) == 0); // expected-warning {{TRUE}}
+}
+
+// --- Edge cases: null character '\0' ---
+void test_strchr_null_char_always_found(void) {
+ // strchr(s, '\0') always finds the terminator.
+ clang_analyzer_eval(strchr("hello", '\0') == 0); // expected-warning {{FALSE}}
+}
+
+void test_strrchr_null_char_always_found(void) {
+ clang_analyzer_eval(strrchr("hello", '\0') == 0); // expected-warning {{FALSE}}
+}
+
+void test_memchr_null_char_within_bounds(void) {
+ // "abc" has terminator at index 3; searching 4 bytes includes it.
+ clang_analyzer_eval(memchr("abc", '\0', 4) == 0); // expected-warning {{FALSE}}
+}
+
+void test_memchr_null_char_out_of_bounds(void) {
+ // "abc" has terminator at index 3; searching only 3 bytes misses it.
+ clang_analyzer_eval(memchr("abc", '\0', 3) == 0); // expected-warning {{TRUE}}
+}
+
+// --- Edge cases: empty strings ---
+void test_strchr_empty_haystack(void) {
+ // Empty string only contains '\0'; '/' is not there.
+ clang_analyzer_eval(strchr("", '/') == 0); // expected-warning {{TRUE}}
+}
+
+void test_strchr_empty_haystack_null_char(void) {
+ // strchr("", '\0') finds the terminator.
+ clang_analyzer_eval(strchr("", '\0') == 0); // expected-warning {{FALSE}}
+}
+
+void test_strstr_empty_needle(void) {
+ // strstr(s, ...
[truncated]
|
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.
Relands #211857