[StaticAnalyzer] Fix non decimal macro values in tryExpandAsInteger #168632
Conversation
Values were parsed into an unsigned APInt with just enough of a bit width to hold the number then interpreted as signed values. This resulted in hex, octal and binary literals from being interpreted as negative when the most significant bit is 1. For example the `-0b11` would have a bit width of 2, would be interpreted as -1, then negated to become 1.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Colin Kinloch (ColinKinloch) ChangesValues were parsed into an unsigned APInt with just enough of a bit width to hold the number then interpreted as signed values. This resulted in hex, octal and binary literals from being interpreted as negative when the most significant bit is 1. For example the Full diff: https://github.com/llvm/llvm-project/pull/168632.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
index 8b404377186e9..9cf31af37f116 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -142,19 +142,25 @@ std::optional<int> tryExpandAsInteger(StringRef Macro, const Preprocessor &PP) {
if (InvalidSpelling)
return std::nullopt;
- llvm::APInt IntValue;
+ llvm::APSInt IntValue(0, true);
constexpr unsigned AutoSenseRadix = 0;
- if (ValueStr.getAsInteger(AutoSenseRadix, IntValue))
+ if (ValueStr.getAsInteger(AutoSenseRadix,
+ static_cast<llvm::APInt &>(IntValue)))
return std::nullopt;
// Parse an optional minus sign.
size_t Size = FilteredTokens.size();
if (Size >= 2) {
- if (FilteredTokens[Size - 2].is(tok::minus))
+ if (FilteredTokens[Size - 2].is(tok::minus)) {
+ // Make sure there's space for a sign bit
+ if (IntValue.isSignBitSet())
+ IntValue = IntValue.extend(IntValue.getBitWidth() + 1);
+ IntValue.setIsUnsigned(false);
IntValue = -IntValue;
+ }
}
- return IntValue.getSExtValue();
+ return IntValue.getExtValue();
}
OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
diff --git a/clang/test/Analysis/std-c-library-functions-eof-2-rad.c b/clang/test/Analysis/std-c-library-functions-eof-2-rad.c
new file mode 100644
index 0000000000000..6ead28b97fbf7
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-eof-2-rad.c
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -std=c23 -analyzer-checker=core,unix.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+
+void clang_analyzer_eval(int);
+
+typedef struct FILE FILE;
+/// Test that the static analyzer doesn't interpret the most significant bit as the sign bit.
+// Unorthodox EOF value with a power of 2 radix
+#define EOF (-0b11)
+
+int getc(FILE *);
+void test_getc(FILE *fp) {
+ int y = getc(fp);
+ if (y < 0) {
+ clang_analyzer_eval(y == EOF); // expected-warning{{TRUE}}
+ }
+}
|
|
@llvm/pr-subscribers-clang Author: Colin Kinloch (ColinKinloch) ChangesValues were parsed into an unsigned APInt with just enough of a bit width to hold the number then interpreted as signed values. This resulted in hex, octal and binary literals from being interpreted as negative when the most significant bit is 1. For example the Full diff: https://github.com/llvm/llvm-project/pull/168632.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
index 8b404377186e9..9cf31af37f116 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -142,19 +142,25 @@ std::optional<int> tryExpandAsInteger(StringRef Macro, const Preprocessor &PP) {
if (InvalidSpelling)
return std::nullopt;
- llvm::APInt IntValue;
+ llvm::APSInt IntValue(0, true);
constexpr unsigned AutoSenseRadix = 0;
- if (ValueStr.getAsInteger(AutoSenseRadix, IntValue))
+ if (ValueStr.getAsInteger(AutoSenseRadix,
+ static_cast<llvm::APInt &>(IntValue)))
return std::nullopt;
// Parse an optional minus sign.
size_t Size = FilteredTokens.size();
if (Size >= 2) {
- if (FilteredTokens[Size - 2].is(tok::minus))
+ if (FilteredTokens[Size - 2].is(tok::minus)) {
+ // Make sure there's space for a sign bit
+ if (IntValue.isSignBitSet())
+ IntValue = IntValue.extend(IntValue.getBitWidth() + 1);
+ IntValue.setIsUnsigned(false);
IntValue = -IntValue;
+ }
}
- return IntValue.getSExtValue();
+ return IntValue.getExtValue();
}
OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
diff --git a/clang/test/Analysis/std-c-library-functions-eof-2-rad.c b/clang/test/Analysis/std-c-library-functions-eof-2-rad.c
new file mode 100644
index 0000000000000..6ead28b97fbf7
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-eof-2-rad.c
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -std=c23 -analyzer-checker=core,unix.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+
+void clang_analyzer_eval(int);
+
+typedef struct FILE FILE;
+/// Test that the static analyzer doesn't interpret the most significant bit as the sign bit.
+// Unorthodox EOF value with a power of 2 radix
+#define EOF (-0b11)
+
+int getc(FILE *);
+void test_getc(FILE *fp) {
+ int y = getc(fp);
+ if (y < 0) {
+ clang_analyzer_eval(y == EOF); // expected-warning{{TRUE}}
+ }
+}
|
2 participants
Values were parsed into an unsigned APInt with just enough of a bit width to hold the number then interpreted as signed values. This resulted in hex, octal and binary literals from being interpreted as negative when the most significant bit is 1.
For example the
-0b11would have a bit width of 2, would be interpreted as -1, then negated to become 1.