Skip to content

[-Wunsafe-buffer-usage] Add absl::{Span,string_view} to UnsafeBufferUsage analysis #127698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[-Wunsafe-buffer-usage] Add absl::{Span,string_view} to UnsafeBufferUsage analysis #127698

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion clang/lib/Analysis/UnsafeBufferUsage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,9 @@ AST_MATCHER(CallExpr, hasUnsafeSnprintfBuffer) {

// Pattern 1:
static StringRef SizedObjs[] = {"span", "array", "vector",
"basic_string_view", "basic_string"};
"basic_string_view", "basic_string",
// Support absl::Span and absl::string_view
"Span", "string_view" };
Copy link
Contributor

Choose a reason for hiding this comment

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

I am afraid this will not work because of the following check on line 920:

          if (MCEPtr->getRecordDecl()->isInStdNamespace() &&
              MCEPtr->getRecordDecl()->getCanonicalDecl()->getName() ==
                  SizedObj)

You want to change this to take namespace into account somehow. I suggest creating a method IsInNamedNamespace that accepts the namespace name and passes it all the way down to this function that current passes a string constant "std".

llvm-project/clang/lib/AST/DeclBase.cpp

Line 1344 in 3dc1594

return II && II->isStr("std");

That way the IsInStdNamespace will be implemented as return IsInNamedNamespace("std")

PS a more sophisticated version would allow to follow multiple nested namespaces is probably also useful, but I'd not bother with it for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, please match namespaces. It also helps preventing false negatives. You probably do not want to match any type named Span.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. We need name space checks here.

Buf = Buf->IgnoreParenImpCasts();
Size = Size->IgnoreParenImpCasts();
if (auto *MCEPtr = dyn_cast<CXXMemberCallExpr>(Buf))
Expand Down