[PATCH] D85525: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` is not a literal.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:806
         const auto &T = MI->tokens().back();
-        StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
-        llvm::APInt IntValue;
-        ValueStr.getAsInteger(10, IntValue);
-        AreSafeFunctionsWanted = IntValue.getZExtValue();
+        if (T.isLiteral()) {
+          StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
----------------
ArcsinX wrote:
> hokein wrote:
> > let's add the `getLiteralData` check here --  `if (T.isLiteral() && T.getLiteralData())`
> Could we also add clangd test here for that case (when `T.isLiteral() == true` and `T.getLiteralData() == nullptr`)?
I think it is fine to just change it here without a clangd test.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-stdc-want-lib-ext1-not-a-literal.c:3
+// RUN: -- -std=c11 -I %S/Inputs/bugprone-not-null-terminated-result
+
+#include "not-null-terminated-result-c.h"
----------------
ArcsinX wrote:
> hokein wrote:
> > I think you probably need a `// UNSUPPORTED: system-windows`, as the `bugprone-not-null-terminated-result-strlen.c` does.
> Seems we don't need this.  
> Problem with `bugprone-not-null-terminated-result-strlen.c` test on Windows related with `strncmp`, according with comment.
> ```
> // FIXME: Something wrong with the APInt un/signed conversion on Windows:
> // in 'strncmp(str6, "string", 7);' it tries to inject '4294967302' as length.
> 
> // UNSUPPORTED: system-windows
> ```
> 
> E.g. `bugprone-not-null-terminated-result-memcpy-safe` also uses `strlen`
I see, good catch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85525/new/

https://reviews.llvm.org/D85525