Skip to content

C++: Fix a FP in cpp/wrong-number-format-arguments caused by an extraction error #18194

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

Merged
calumgrant merged 4 commits into from
Dec 6, 2024
Merged

C++: Fix a FP in cpp/wrong-number-format-arguments caused by an extraction error #18194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
08859be
C++: Test case for cpp/wrong-number-format-arguments
calumgrant Dec 3, 2024
99efff2
C++: Fix a FP in cpp/wrong-number-format-arguments caused by extracti…
calumgrant Dec 3, 2024
147d699
C++: Add another test case
calumgrant Dec 4, 2024
2da3d36
C++: Change note
calumgrant Dec 5, 2024
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
3 changes: 2 additions & 1 deletion cpp/ql/src/Likely Bugs/Format/WrongNumberOfFormatArguments.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ where
) and
// A typical problem is that string literals are concatenated, but if one of the string
// literals is an undefined macro, then this just leads to a syntax error.
not exists(SyntaxError e | e.affects(fl))
not exists(SyntaxError e | e.affects(fl)) and
not ffc.getArgument(_) instanceof ErrorExpr
select ffc,
"Format for " + ffcName + " expects " + expected.toString() + " arguments but given " +
given.toString()
4 changes: 4 additions & 0 deletions cpp/ql/src/change-notes/2024-12-05-wrong-number-format-arguments.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The "Too few arguments to formatting function" query (`cpp/wrong-number-format-arguments`) query no longer produces results if an argument has an extraction error.
1 change: 1 addition & 0 deletions ...sts/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
| macros.cpp:14:2:14:37 | call to printf | Format for printf (in a macro expansion) expects 4 arguments but given 3 |
| macros.cpp:21:2:21:36 | call to printf | Format for printf (in a macro expansion) expects 4 arguments but given 3 |
| macros.cpp:32:2:32:25 | call to printf | Format for printf (in a macro expansion) expects 1 arguments but given 0 |
| syntax_errors.c:15:5:15:10 | call to printf | Format for printf expects 2 arguments but given 0 |
| test.c:9:2:9:7 | call to printf | Format for printf expects 1 arguments but given 0 |
| test.c:12:2:12:7 | call to printf | Format for printf expects 2 arguments but given 1 |
| test.c:15:2:15:7 | call to printf | Format for printf expects 3 arguments but given 2 |
Expand Down
13 changes: 12 additions & 1 deletion cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/syntax_errors.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,16 @@
extern int printf(const char *fmt, ...);

void test_syntax_error() {
printf("Error code %d: " FMT_MSG, 0, "");
// GOOD
printf("Error code %d: " UNDEFINED_MACRO, 0, "");

// GOOD
printf("%d%d",
(UNDEFINED_MACRO)1,
(UNDEFINED_MACRO)2);

// GOOD [FALSE POSITIVE]
printf("%d%d"
UNDEFINED_MACRO,
Copy link
Contributor

@jketema jketema Dec 4, 2024

Choose a reason for hiding this comment

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

I assume this is an FP, because it neither occurs on the line with the literal, not occurs in any of the arguments?

Copy link
Contributor Author

@calumgrant calumgrant Dec 4, 2024 •
edited by jketema
Loading

Choose a reason for hiding this comment

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

Yes, so this is one of the FPs we saw on DCA that the current query or fixed query fails to catch. The problem is that the syntax error occurs on line 16, whereas the literal and function call are on line 15, and there is no obvious way to associate the two.

There is no ErrorExpr in the function call, and the extent of the function call is only line 15. So right now, I can't think of a good way to detect this case.

It looks like the frontend's syntax error recovery means dropping all of the expressions until the next ), which is pretty reasonable.

1, 2);
}
Loading