vsavchenko updated this revision to Diff 336077.
vsavchenko added a comment.
Require std::data to accept exactly 1 argument
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99260/new/
https://reviews.llvm.org/D99260
Files:
clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
clang/test/Analysis/inner-pointer.cpp
Index: clang/test/Analysis/inner-pointer.cpp
===================================================================
--- clang/test/Analysis/inner-pointer.cpp
+++ clang/test/Analysis/inner-pointer.cpp
@@ -17,6 +17,11 @@
string my_string = "default";
void default_arg(int a = 42, string &b = my_string);
+template <class T>
+T *addressof(T &arg);
+
+char *data(std::string &c);
+
} // end namespace std
void consume(const char *) {}
@@ -273,6 +278,15 @@
// expected-note@-1 {{Inner pointer of container used after re/deallocation}}
}
+void deref_after_std_data() {
+ const char *c;
+ std::string s;
+ c = std::data(s); // expected-note {{Pointer to inner buffer of 'std::string' obtained here}}
+ s.push_back('c'); // expected-note {{Inner buffer of 'std::string' reallocated by call to 'push_back'}}
+ consume(c); // expected-warning {{Inner pointer of container used after re/deallocation}}
+ // expected-note@-1 {{Inner pointer of container used after re/deallocation}}
+}
+
struct S {
std::string s;
const char *name() {
@@ -361,8 +375,24 @@
// expected-note@-1 {{Inner pointer of container used after re/deallocation}}
}
+void func_addressof() {
+ const char *c;
+ std::string s;
+ c = s.c_str();
+ addressof(s);
+ consume(c); // no-warning
+}
+
+void func_std_data() {
+ const char *c;
+ std::string s;
+ c = std::data(s);
+ consume(c); // no-warning
+}
+
struct T {
std::string to_string() { return s; }
+
private:
std::string s;
};
Index: clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
@@ -34,9 +34,9 @@
class InnerPointerChecker
: public Checker<check::DeadSymbols, check::PostCall> {
- CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn,
- InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn,
- ShrinkToFitFn, SwapFn;
+ CallDescription AppendFn, AssignFn, AddressofFn, ClearFn, CStrFn, DataFn,
+ DataMemberFn, EraseFn, InsertFn, PopBackFn, PushBackFn, ReplaceFn,
+ ReserveFn, ResizeFn, ShrinkToFitFn, SwapFn;
public:
class InnerPointerBRVisitor : public BugReporterVisitor {
@@ -73,9 +73,10 @@
InnerPointerChecker()
: AppendFn({"std", "basic_string", "append"}),
AssignFn({"std", "basic_string", "assign"}),
+ AddressofFn({"std", "addressof"}),
ClearFn({"std", "basic_string", "clear"}),
- CStrFn({"std", "basic_string", "c_str"}),
- DataFn({"std", "basic_string", "data"}),
+ CStrFn({"std", "basic_string", "c_str"}), DataFn({"std", "data"}, 1),
+ DataMemberFn({"std", "basic_string", "data"}),
EraseFn({"std", "basic_string", "erase"}),
InsertFn({"std", "basic_string", "insert"}),
PopBackFn({"std", "basic_string", "pop_back"}),
@@ -90,6 +91,9 @@
/// pointers referring to the container object's inner buffer.
bool isInvalidatingMemberFunction(const CallEvent &Call) const;
+ /// Check whether the called function returns a raw inner pointer.
+ bool isInnerPointerAccessFunction(const CallEvent &Call) const;
+
/// Mark pointer symbols associated with the given memory region released
/// in the program state.
void markPtrSymbolsReleased(const CallEvent &Call, ProgramStateRef State,
@@ -130,6 +134,12 @@
Call.isCalled(SwapFn));
}
+bool InnerPointerChecker::isInnerPointerAccessFunction(
+ const CallEvent &Call) const {
+ return (Call.isCalled(CStrFn) || Call.isCalled(DataFn) ||
+ Call.isCalled(DataMemberFn));
+}
+
void InnerPointerChecker::markPtrSymbolsReleased(const CallEvent &Call,
ProgramStateRef State,
const MemRegion *MR,
@@ -172,6 +182,11 @@
if (!ArgRegion)
continue;
+ // std::addressof function accepts a non-const reference as an argument,
+ // but doesn't modify it.
+ if (Call.isCalled(AddressofFn))
+ continue;
+
markPtrSymbolsReleased(Call, State, ArgRegion, C);
}
}
@@ -195,36 +210,49 @@
CheckerContext &C) const {
ProgramStateRef State = C.getState();
+ // TODO: Do we need these to be typed?
+ const TypedValueRegion *ObjRegion = nullptr;
+
if (const auto *ICall = dyn_cast<CXXInstanceCall>(&Call)) {
- // TODO: Do we need these to be typed?
- const auto *ObjRegion = dyn_cast_or_null<TypedValueRegion>(
+ ObjRegion = dyn_cast_or_null<TypedValueRegion>(
ICall->getCXXThisVal().getAsRegion());
- if (!ObjRegion)
- return;
- if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
- SVal RawPtr = Call.getReturnValue();
- if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) {
- // Start tracking this raw pointer by adding it to the set of symbols
- // associated with this container object in the program state map.
+ // Check [string.require] / second point.
+ if (isInvalidatingMemberFunction(Call)) {
+ markPtrSymbolsReleased(Call, State, ObjRegion, C);
+ return;
+ }
+ }
- PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>();
- const PtrSet *SetPtr = State->get<RawPtrMap>(ObjRegion);
- PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet();
- assert(C.wasInlined || !Set.contains(Sym));
- Set = F.add(Set, Sym);
+ if (isInnerPointerAccessFunction(Call)) {
- State = State->set<RawPtrMap>(ObjRegion, Set);
- C.addTransition(State);
- }
- return;
+ if (isa<SimpleFunctionCall>(Call)) {
+ // NOTE: As of now, we only have one free access function: std::data.
+ // If we add more functions like this in the list, hardcoded
+ // argument index should be changed.
+ ObjRegion =
+ dyn_cast_or_null<TypedValueRegion>(Call.getArgSVal(0).getAsRegion());
}
- // Check [string.require] / second point.
- if (isInvalidatingMemberFunction(Call)) {
- markPtrSymbolsReleased(Call, State, ObjRegion, C);
+ if (!ObjRegion)
return;
+
+ SVal RawPtr = Call.getReturnValue();
+ if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) {
+ // Start tracking this raw pointer by adding it to the set of symbols
+ // associated with this container object in the program state map.
+
+ PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>();
+ const PtrSet *SetPtr = State->get<RawPtrMap>(ObjRegion);
+ PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet();
+ assert(C.wasInlined || !Set.contains(Sym));
+ Set = F.add(Set, Sym);
+
+ State = State->set<RawPtrMap>(ObjRegion, Set);
+ C.addTransition(State);
}
+
+ return;
}
// Check [string.require] / first point.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
