[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:787-791 + // Acquire the macro's name. + Token TheTok; + RawLexer.LexFromRawLexer(TheTok); + + std::string MacroName = PP.getSpelling(TheTok); NoQ wrote: > Not sure, random thought: Could this work in fact be done with the //static// > `Lexer::getImmediateMacroName()` helper? I need to create a lexer to lex the arguments of function-like macros (which is in part 3), so I can't get away with it I'm afraid :/ https://reviews.llvm.org/D52794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
xazax.hun added a comment.
Please add a test case where a bug path goes through a macro definition and
this macro is undefed at the end of the translation unit.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:667
+
+//===--===//
+// Declarations of helper functions and data structures for expanding macros.
Maybe it would be worth to move these helpers to a separate file and add some
documentation why we actually simulate the preprocessor rather than doing
something else.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:724
+ TokenPrinter Printer(OS, PP);
+ return { getExpandedMacroImpl(Printer, MacroLoc, PP), OS.str() };
+}
Maybe could directly return ExpansionInfo?
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:734
+ MacroNameAndInfo Info = getMacroNameAndInfo(SM.getExpansionLoc(MacroLoc),
PP);
+ std::string MacroName = std::move(Info.Name);
+ const MacroInfo *MI = Info.MI;
I think this local might be redundant.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:753
+// macro.
+if (const MacroInfo *MI = PP.getMacroInfo(II)) {
+ getExpandedMacroImpl(Printer, T.getLocation(), PP);
This might not be completely correct. We are using the preprocessor state after
processing the whole translation unit rather than the preprocessor state at the
expansion location. This might not cause any problems unless there is a
collision between macro names and non-macro names. Also, undefs, redefs might
cause troubles.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:796
+ const MacroInfo *MI = PP.getMacroInfo(II);
+ assert(MI && "This IdentifierInfo should refer to a macro!");
+
Could this assertion fail due to undefs in the code?
https://reviews.llvm.org/D52794
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
Szelethus planned changes to this revision.
Szelethus added a comment.
I'll put this patch on hold while I'm investigating the issue @xazax.hun
mentioned.
Mind you, again, other patches don't depend strictly on this part, I'll only
need to change the for loop. Hopefully. You never know.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:753
+// macro.
+if (const MacroInfo *MI = PP.getMacroInfo(II)) {
+ getExpandedMacroImpl(Printer, T.getLocation(), PP);
xazax.hun wrote:
> This might not be completely correct. We are using the preprocessor state
> after processing the whole translation unit rather than the preprocessor
> state at the expansion location. This might not cause any problems unless
> there is a collision between macro names and non-macro names. Also, undefs,
> redefs might cause troubles.
Yup, you're right, I'm afraid :/
I'll investigate whether I can gather the macro definition at the spelling
location with `Lexer`.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:796
+ const MacroInfo *MI = PP.getMacroInfo(II);
+ assert(MI && "This IdentifierInfo should refer to a macro!");
+
xazax.hun wrote:
> Could this assertion fail due to undefs in the code?
Yes... yes it could.
Repository:
rC Clang
https://reviews.llvm.org/D52794
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
Szelethus updated this revision to Diff 171962.
Szelethus added a comment.
Fixes according to @xazax.hun's remarks. Thanks for catching that, it was a
non-trivial cornercase!
- Now I'm acquiring the macro directive history, and choosing the appropriate
macro definition in `getMacroNameAndInfo`
- Rebased to the now commited first part.
https://reviews.llvm.org/D52794
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
test/Analysis/plist-macros-with-expansion.cpp
Index: test/Analysis/plist-macros-with-expansion.cpp
===
--- test/Analysis/plist-macros-with-expansion.cpp
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -27,8 +27,8 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// CHECK: name
-// CHECK-NEXT: expansion
+// CHECK: nameSET_PTR_VAR_TO_NULL
+// CHECK-NEXT: expansionptr = 0
#define NULL 0
#define SET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO \
@@ -40,5 +40,93 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// CHECK: name
-// CHECK-NEXT: expansion
+// CHECK: nameSET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO
+// CHECK-NEXT: expansionptr =0
+
+//===--===//
+// Tests for function-like macro expansions.
+//===--===//
+
+void setToNull(int **vptr) {
+ *vptr = nullptr;
+}
+
+#define TO_NULL(x) \
+ setToNull(x)
+
+void functionLikeMacroTest() {
+ int *ptr;
+ TO_NULL(&ptr);
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// TODO: Expand arguments.
+// CHECK: nameTO_NULL
+// CHECK: expansionsetToNull(x)
+
+#define DOES_NOTHING(x) \
+ { \
+int b; \
+b = 5; \
+ } \
+ print(x)
+
+#define DEREF(x) \
+ DOES_NOTHING(x); \
+ *x
+
+void functionLikeNestedMacroTest() {
+ int *a;
+ TO_NULL(&a);
+ DEREF(a) = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// TODO: Expand arguments.
+// CHECK: nameTO_NULL
+// CHECK-NEXT: expansionsetToNull(x)
+
+// TODO: Expand arguments.
+// CHECK: nameDEREF
+// CHECK-NEXT: expansion{ int b; b = 5; } print(x); *x
+
+//===--===//
+// Tests for undefining and/or redifining macros.
+//===--===//
+
+#define WILL_UNDEF_SET_NULL_TO_PTR(ptr) \
+ ptr = nullptr;
+
+void undefinedMacroByTheEndOfParsingTest() {
+ int *ptr;
+ WILL_UNDEF_SET_NULL_TO_PTR(ptr);
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+#undef WILL_UNDEF_SET_NULL_TO_PTR
+
+// TODO: Expand arguments.
+// CHECK: nameWILL_UNDEF_SET_NULL_TO_PTR
+// CHECK-NEXT: expansionptr = nullptr;
+
+//===---
+
+#define WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL(ptr) \
+ /* Nothing */
+#undef WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL
+#define WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL(ptr) \
+ ptr = nullptr;
+
+void macroRedefinedMultipleTimesTest() {
+ int *ptr;
+ WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL(ptr)
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+#undef WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL
+#define WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL(ptr) \
+ print("This string shouldn't be in the plist file at all. Or anywhere, " \
+"but here.");
+
+// TODO: Expand arguments.
+// CHECK: nameWILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL
+// CHECK-NEXT: expansionptr = nullptr;
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -143,8 +143,8 @@
col3
file0
- name
- expansion
+ nameSET_PTR_VAR_TO_NULL
+ expansionptr = 0
descriptionDereference of null pointer (loaded from variable 'ptr')
@@ -312,8 +312,8 @@
col3
file0
- name
- expansion
+ nameSET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO
+ expansionptr =0
descriptionDereference of null pointer (loaded from variable 'ptr')
@@ -342,6 +342,874 @@
+
+ path
+
+
+ kindcontrol
+ edges
+
+
+start
+
+
+ line58
+ col3
+ file0
+
+
+ line58
+ col5
+ file0
+
+
+end
+
+
+ line59
+ col3
+ file0
+
+
+ line59
+ col9
+ file0
+
+
+
+
+
+
+
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
Szelethus marked 10 inline comments as done.
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:667
+
+//===--===//
+// Declarations of helper functions and data structures for expanding macros.
xazax.hun wrote:
> Maybe it would be worth to move these helpers to a separate file and add some
> documentation why we actually simulate the preprocessor rather than doing
> something else.
I'm actually a little unsure about that -- I have had similar thought, but the
placement here can be justified with a simple "Where else?". But that's a
little vague, I'll admit.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:724
+ TokenPrinter Printer(OS, PP);
+ return { getExpandedMacroImpl(Printer, MacroLoc, PP), OS.str() };
+}
xazax.hun wrote:
> Maybe could directly return ExpansionInfo?
I renamed the function to be less confusing. Did this address your concern?
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:753
+// macro.
+if (const MacroInfo *MI = PP.getMacroInfo(II)) {
+ getExpandedMacroImpl(Printer, T.getLocation(), PP);
Szelethus wrote:
> xazax.hun wrote:
> > This might not be completely correct. We are using the preprocessor state
> > after processing the whole translation unit rather than the preprocessor
> > state at the expansion location. This might not cause any problems unless
> > there is a collision between macro names and non-macro names. Also, undefs,
> > redefs might cause troubles.
> Yup, you're right, I'm afraid :/
>
> I'll investigate whether I can gather the macro definition at the spelling
> location with `Lexer`.
I'm really-really happy it didn't have to come to that.
https://reviews.llvm.org/D52794
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Because i don't know much about macros, i shouldn't be blocking this review on waiting for myself to understand the code :) I guess we'll eventually figure out if something is wrong with it. https://reviews.llvm.org/D52794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
xazax.hun accepted this revision.
xazax.hun added a comment.
One question and one nit otherwise looks good. Feel free to commit once those
are resolved without another round of reviews.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:805
+// macro.
+if (const MacroInfo *MI = PP.getMacroInfo(II)) {
+ getMacroNameAndPrintExpansion(Printer, T.getLocation(), PP);
Can't we have troubles with getMacroInfo + undefs here? Wouldn't
`findDirectiveAtLoc` be a safer choice here?
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:883
+
+inline void TokenPrinter::printToken(const Token &Tok) {
+ // If the tokens were already space separated, or if they must be to avoid
I would remove the inline keyword here. I rarely see this in clang codebase.
Let's just trust the compiler :)
https://reviews.llvm.org/D52794
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
Szelethus updated this revision to Diff 172459.
Szelethus marked an inline comment as done.
Szelethus added a comment.
- Fixes according to @xazax.hun's observations
Thanks! I'll commit when I have time watch buildbots.
https://reviews.llvm.org/D52794
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
test/Analysis/plist-macros-with-expansion.cpp
Index: test/Analysis/plist-macros-with-expansion.cpp
===
--- test/Analysis/plist-macros-with-expansion.cpp
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -27,8 +27,8 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// CHECK: name
-// CHECK-NEXT: expansion
+// CHECK: nameSET_PTR_VAR_TO_NULL
+// CHECK-NEXT: expansionptr = 0
#define NULL 0
#define SET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO \
@@ -40,5 +40,109 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// CHECK: name
-// CHECK-NEXT: expansion
+// CHECK: nameSET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO
+// CHECK-NEXT: expansionptr =0
+
+//===--===//
+// Tests for function-like macro expansions.
+//===--===//
+
+void setToNull(int **vptr) {
+ *vptr = nullptr;
+}
+
+#define TO_NULL(x) \
+ setToNull(x)
+
+void functionLikeMacroTest() {
+ int *ptr;
+ TO_NULL(&ptr);
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// TODO: Expand arguments.
+// CHECK: nameTO_NULL
+// CHECK: expansionsetToNull(x)
+
+#define DOES_NOTHING(x) \
+ { \
+int b; \
+b = 5; \
+ } \
+ print(x)
+
+#define DEREF(x) \
+ DOES_NOTHING(x); \
+ *x
+
+void functionLikeNestedMacroTest() {
+ int *a;
+ TO_NULL(&a);
+ DEREF(a) = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// TODO: Expand arguments.
+// CHECK: nameTO_NULL
+// CHECK-NEXT: expansionsetToNull(x)
+
+// TODO: Expand arguments.
+// CHECK: nameDEREF
+// CHECK-NEXT: expansion{ int b; b = 5; } print(x); *x
+
+//===--===//
+// Tests for undefining and/or redifining macros.
+//===--===//
+
+#define WILL_UNDEF_SET_NULL_TO_PTR(ptr) \
+ ptr = nullptr;
+
+void undefinedMacroByTheEndOfParsingTest() {
+ int *ptr;
+ WILL_UNDEF_SET_NULL_TO_PTR(ptr);
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+#undef WILL_UNDEF_SET_NULL_TO_PTR
+
+// TODO: Expand arguments.
+// CHECK: nameWILL_UNDEF_SET_NULL_TO_PTR
+// CHECK-NEXT: expansionptr = nullptr;
+
+#define WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL(ptr) \
+ /* Nothing */
+#undef WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL
+#define WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL(ptr) \
+ ptr = nullptr;
+
+void macroRedefinedMultipleTimesTest() {
+ int *ptr;
+ WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL(ptr)
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+#undef WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL
+#define WILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL(ptr) \
+ print("This string shouldn't be in the plist file at all. Or anywhere, " \
+"but here.");
+
+// TODO: Expand arguments.
+// CHECK: nameWILL_REDIFINE_MULTIPLE_TIMES_SET_TO_NULL
+// CHECK-NEXT: expansionptr = nullptr;
+
+#define WILL_UNDEF_SET_NULL_TO_PTR_2(ptr) \
+ ptr = nullptr;
+
+#define PASS_PTR_TO_MACRO_THAT_WILL_BE_UNDEFD(ptr) \
+ WILL_UNDEF_SET_NULL_TO_PTR_2(ptr)
+
+void undefinedMacroInsideAnotherMacroTest() {
+ int *ptr;
+ PASS_PTR_TO_MACRO_THAT_WILL_BE_UNDEFD(ptr);
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// TODO: Expand arguments.
+// CHECK: namePASS_PTR_TO_MACRO_THAT_WILL_BE_UNDEFD
+// CHECK-NEXT: expansionptr = nullptr;
+
+#undef WILL_UNDEF_SET_NULL_TO_PTR_2
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -143,8 +143,8 @@
col3
file0
- name
- expansion
+ nameSET_PTR_VAR_TO_NULL
+ expansionptr = 0
descriptionDereference of null pointer (loaded from variable 'ptr')
@@ -312,8 +312,8 @@
col3
file0
- name
- expansion
+ nameSET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO
+ expansionptr =0
descriptionDereference of null pointer (loaded from variable 'ptr')
@@ -342,10 +342,1047 @@
+
+ path
+
+
+ kindcontrol
+ edges
+
+
+start
+
+
+ line58
+ col3
+ file0
+
+
+
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
This revision was automatically updated to reflect the committed changes. Closed by commit rL346095: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and… (authored by Szelethus, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52794?vs=172459&id=172519#toc Repository: rL LLVM https://reviews.llvm.org/D52794 Files: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp Index: cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist === --- cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist +++ cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist @@ -143,8 +143,8 @@ col3 file0 - name - expansion + nameSET_PTR_VAR_TO_NULL + expansionptr = 0 descriptionDereference of null pointer (loaded from variable 'ptr') @@ -312,8 +312,8 @@ col3 file0 - name - expansion + nameSET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO + expansionptr =0 descriptionDereference of null pointer (loaded from variable 'ptr') @@ -342,10 +342,1047 @@ + + path + + + kindcontrol + edges + + +start + + + line58 + col3 + file0 + + + line58 + col5 + file0 + + +end + + + line59 + col3 + file0 + + + line59 + col9 + file0 + + + + + + + kindevent + location + + line59 + col3 + file0 + + ranges + + + + line59 + col3 + file0 + + + line59 + col15 + file0 + + + + depth0 + extended_message + Calling 'setToNull' + message + Calling 'setToNull' + + + kindevent + location + + line50 + col1 + file0 + + depth1 + extended_message + Entered call from 'functionLikeMacroTest' + message + Entered call from 'functionLikeMacroTest' + + + kindcontrol + edges + + +start + + + line50 + col1 + file0 + + + line50 + col4 + file0 + + +end + + + line51 + col3 + file0 + + + line51 + col3 + file0 + + + + + + + kindevent + location + + line51 + col3 + file0 + + ranges + + + + line51 + col3 + file0 + + + line51 + col17 + file0 + + + + depth1 + extended_message + Null pointer value stored to 'ptr' + message + Null pointer value stored to 'ptr' + + + kindevent + location + + line59 + col3 + file0 + + ranges + + + + line59 + col3 + file0 + + + line59 + col15 + file0 + + + + depth0 + extended_message + Returning from 'setToNull' + message + Returning from 'setToNull' + + + kindcontrol + edges + + +start + + + line60 + col3 + file0 + + + line60 + col3 + file0 + + +end + + + line60 + col8 + file0 + + + line60 + col8 + file0 + + + + + + + kindevent + location + + line60 + col8 + file0 + + ranges + + + + line60 + col4 + file0 + + + line60 + col6 + file0 + + + + depth0 + extended_message + Dereference of null pointer (loaded from variable 'ptr') + message + Dereference of null pointer (loaded from variable 'ptr') + + + macro_expansions + + + location + + line59 + col3 + file0 + + nameTO_NULL + expansionsetToNull(x) + + + descriptionDereference of null pointer (loaded from variable 'ptr') + categoryLogic error + typeDereference of null pointer
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
Szelethus reopened this revision. Szelethus added a comment. This revision is now accepted and ready to land. Here's to losing a couple more handfuls of hair, tests break on many platforms, so I reverted it. Repository: rL LLVM https://reviews.llvm.org/D52794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
Szelethus closed this revision. Szelethus added a comment. Woohoo, it seems to be fine! ^-^ I thought the evaluation order in braced initializer lists are from left to right, but apparently not. Certainly not on windows. Repository: rL LLVM https://reviews.llvm.org/D52794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, rnkovacs, dkrupp, whisperity,
martong, baloghadamsoftware.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet,
xazax.hun.
Szelethus added a dependency: D52742: [analyzer][PlistMacroExpansion] Part 1.:
New expand-macros flag.
This patch adds a couple new functions to acquire the macro's name, and also
expands it, although it doesn't expand the arguments, as seen from the test
files.
Repository:
rC Clang
https://reviews.llvm.org/D52794
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
test/Analysis/plist-macros-with-expansion.cpp
Index: test/Analysis/plist-macros-with-expansion.cpp
===
--- test/Analysis/plist-macros-with-expansion.cpp
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -27,7 +27,7 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// CHECK: Expanding macro '' to ''
+// CHECK: Expanding macro 'SET_PTR_VAR_TO_NULL' to 'ptr = 0 '
#define NULL 0
#define SET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO \
@@ -39,4 +39,47 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// CHECK: Expanding macro '' to ''
+// CHECK: Expanding macro 'SET_PTR_VAR_TO_NULL' to 'ptr = 0 '
+
+//===--===//
+// Tests for function-like macro expansions.
+//===--===//
+
+void setToNull(int **vptr) {
+ *vptr = nullptr;
+}
+
+#define TO_NULL(x) \
+ setToNull(x)
+
+void functionLikeMacroTest() {
+ int *ptr;
+ TO_NULL(&ptr);
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// TODO: Expand macro arguments.
+// CHECK: Expanding macro 'TO_NULL' to 'setToNull ( x ) '
+
+#define DOES_NOTHING(x) \
+ { \
+int b; \
+b = 5; \
+ } \
+ print(x)
+
+#define DEREF(x) \
+ DOES_NOTHING(x); \
+ *x
+
+void functionLikeNestedMacroTest() {
+ int *a;
+ TO_NULL(&a);
+ DEREF(a) = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// TODO: Expand macro arguments.
+// CHECK: Expanding macro 'TO_NULL' to 'setToNull ( x ) '
+
+// TODO: Expand macro arguments.
+// CHECK: Expanding macro 'DEREF' to '{ int b ; b = 5 ; } print ( x ) ; * x '
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -3,7 +3,7 @@
clang_version
-clang version 8.0.0 (http://mainstream.inf.elte.hu/Szelethus/clang 85a6dda64587a5a18482f091cbcf020fbd3ec1dd) (https://github.com/llvm-mirror/llvm 1ffbf26a1a0a190d69327af875a3337b74a2ce82)
+clang version 8.0.0 (http://mainstream.inf.elte.hu/Szelethus/clang 7bb09c0bd99537e54637926df8b2814fd9e02947) (https://github.com/llvm-mirror/llvm 1ffbf26a1a0a190d69327af875a3337b74a2ce82)
diagnostics
@@ -52,9 +52,9 @@
file0
extended_message
- Expanding macro '' to ''
+ Expanding macro 'SET_PTR_VAR_TO_NULL' to 'ptr = 0 '
message
- Expanding macro '' to ''
+ Expanding macro 'SET_PTR_VAR_TO_NULL' to 'ptr = 0 '
kindevent
@@ -221,9 +221,9 @@
file0
extended_message
- Expanding macro '' to ''
+ Expanding macro 'SET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO' to 'ptr = 0 '
message
- Expanding macro '' to ''
+ Expanding macro 'SET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO' to 'ptr = 0 '
kindevent
@@ -344,10 +344,543 @@
+
+ path
+
+
+ kindcontrol
+ edges
+
+
+start
+
+
+ line56
+ col3
+ file0
+
+
+ line56
+ col5
+ file0
+
+
+end
+
+
+ line57
+ col3
+ file0
+
+
+ line57
+ col9
+ file0
+
+
+
+
+
+
+ kindmacro_expansion
+ location
+
+ line57
+ col3
+ file0
+
+ extended_message
+ Expanding macro 'TO_NULL' to 'setToNull ( x ) '
+ message
+ Expanding macro 'TO_NULL' to 'setToNull ( x ) '
+
+
+ kindevent
+ location
+
+ line57
+ col3
+ file0
+
+ ranges
+
+
+
+ line57
+ col3
+ file0
+
+
+ line57
+ col15
+ file0
+
+
+
+ depth0
+ extended_message
+ Calling 'setToNull'
+ message
+ Calling 'setToNull'
+
+
+ kindevent
+ loc
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
Szelethus updated this revision to Diff 168334.
https://reviews.llvm.org/D52794
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
test/Analysis/plist-macros-with-expansion.cpp
Index: test/Analysis/plist-macros-with-expansion.cpp
===
--- test/Analysis/plist-macros-with-expansion.cpp
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -27,8 +27,8 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// CHECK: name
-// CHECK: expansion
+// CHECK: nameSET_PTR_VAR_TO_NULL
+// CHECK: expansionptr = 0
#define NULL 0
#define SET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO \
@@ -40,5 +40,51 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// CHECK: name
-// CHECK: expansion
+// CHECK: nameSET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO
+// CHECK: expansionptr = 0
+
+//===--===//
+// Tests for function-like macro expansions.
+//===--===//
+
+void setToNull(int **vptr) {
+ *vptr = nullptr;
+}
+
+#define TO_NULL(x) \
+ setToNull(x)
+
+void functionLikeMacroTest() {
+ int *ptr;
+ TO_NULL(&ptr);
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// TODO: Expand argumnts.
+// CHECK: nameTO_NULL
+// CHECK: expansionsetToNull ( x )
+
+#define DOES_NOTHING(x) \
+ { \
+int b; \
+b = 5; \
+ } \
+ print(x)
+
+#define DEREF(x) \
+ DOES_NOTHING(x); \
+ *x
+
+void functionLikeNestedMacroTest() {
+ int *a;
+ TO_NULL(&a);
+ DEREF(a) = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// TODO: Expand argumnts.
+// CHECK: nameTO_NULL
+// CHECK: expansionsetToNull ( x )
+
+// TODO: Expand argumnts.
+// CHECK: nameDEREF
+// CHECK: expansion{ int b ; b = 5 ; } print ( x ) ; * x
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -3,7 +3,7 @@
clang_version
-clang version 8.0.0 (http://mainstream.inf.elte.hu/Szelethus/clang 80e1678b9f598ca78bb3b71cf546a63414a37b11) (https://github.com/llvm-mirror/llvm 1ffbf26a1a0a190d69327af875a3337b74a2ce82)
+clang version 8.0.0 (http://mainstream.inf.elte.hu/Szelethus/clang 54f58baf2c799080816023e7f78b92e4f9460078) (https://github.com/llvm-mirror/llvm 1ffbf26a1a0a190d69327af875a3337b74a2ce82)
diagnostics
@@ -51,8 +51,8 @@
col3
file0
- name
- expansion
+ nameSET_PTR_VAR_TO_NULL
+ expansionptr = 0
kindevent
@@ -218,8 +218,8 @@
col3
file0
- name
- expansion
+ nameSET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO
+ expansionptr = 0
kindevent
@@ -340,10 +340,537 @@
+
+ path
+
+
+ kindcontrol
+ edges
+
+
+start
+
+
+ line58
+ col3
+ file0
+
+
+ line58
+ col5
+ file0
+
+
+end
+
+
+ line59
+ col3
+ file0
+
+
+ line59
+ col9
+ file0
+
+
+
+
+
+
+ kindmacro_expansion
+ location
+
+ line59
+ col3
+ file0
+
+ nameTO_NULL
+ expansionsetToNull ( x )
+
+
+ kindevent
+ location
+
+ line59
+ col3
+ file0
+
+ ranges
+
+
+
+ line59
+ col3
+ file0
+
+
+ line59
+ col15
+ file0
+
+
+
+ depth0
+ extended_message
+ Calling 'setToNull'
+ message
+ Calling 'setToNull'
+
+
+ kindevent
+ location
+
+ line50
+ col1
+ file0
+
+ depth1
+ extended_message
+ Entered call from 'functionLikeMacroTest'
+ message
+ Entered call from 'functionLikeMacroTest'
+
+
+ kindcontrol
+ edges
+
+
+start
+
+
+ line50
+ col1
+ file0
+
+
+ line50
+ col4
+ file0
+
+
+end
+
+
+ line51
+ col3
+ file0
+
+
+ line51
+ col3
+ file0
+
+
+
+
+
+
+ kindevent
+ location
+
+ line51
+ col3
+ file0
+
+ ranges
+
+
+
+ line51
+
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
Szelethus updated this revision to Diff 169862.
Szelethus added a comment.
Herald added a subscriber: donat.nagy.
- Removed the version entry from the plist file,
- Now using `TokenConcatenation` to print spaces only when needed (this needed
for https://reviews.llvm.org/D52988),
- A bit more doc,
- Reorganized ("refactored" would be too strong of a word)
`getExpandedMacroImpl` to make it easier to read.
https://reviews.llvm.org/D52794
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
test/Analysis/plist-macros-with-expansion.cpp
Index: test/Analysis/plist-macros-with-expansion.cpp
===
--- test/Analysis/plist-macros-with-expansion.cpp
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -27,8 +27,8 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// CHECK: name
-// CHECK: expansion
+// CHECK: nameSET_PTR_VAR_TO_NULL
+// CHECK: expansionptr = 0
#define NULL 0
#define SET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO \
@@ -40,5 +40,51 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// CHECK: name
-// CHECK: expansion
+// CHECK: nameSET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO
+// CHECK: expansionptr =0
+
+//===--===//
+// Tests for function-like macro expansions.
+//===--===//
+
+void setToNull(int **vptr) {
+ *vptr = nullptr;
+}
+
+#define TO_NULL(x) \
+ setToNull(x)
+
+void functionLikeMacroTest() {
+ int *ptr;
+ TO_NULL(&ptr);
+ *ptr = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// TODO: Expand argumnts.
+// CHECK: nameTO_NULL
+// CHECK: expansionsetToNull(x)
+
+#define DOES_NOTHING(x) \
+ { \
+int b; \
+b = 5; \
+ } \
+ print(x)
+
+#define DEREF(x) \
+ DOES_NOTHING(x); \
+ *x
+
+void functionLikeNestedMacroTest() {
+ int *a;
+ TO_NULL(&a);
+ DEREF(a) = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// TODO: Expand argumnts.
+// CHECK: nameTO_NULL
+// CHECK: expansionsetToNull(x)
+
+// TODO: Expand argumnts.
+// CHECK: nameDEREF
+// CHECK: expansion{ int b; b = 5; } print(x); *x
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -49,8 +49,8 @@
col3
file0
- name
- expansion
+ nameSET_PTR_VAR_TO_NULL
+ expansion ptr = 0
kindevent
@@ -216,8 +216,8 @@
col3
file0
- name
- expansion
+ nameSET_PTR_VAR_TO_NULL_WITH_NESTED_MACRO
+ expansion ptr =0
kindevent
@@ -338,6 +338,533 @@
+
+ path
+
+
+ kindcontrol
+ edges
+
+
+start
+
+
+ line58
+ col3
+ file0
+
+
+ line58
+ col5
+ file0
+
+
+end
+
+
+ line59
+ col3
+ file0
+
+
+ line59
+ col9
+ file0
+
+
+
+
+
+
+ kindmacro_expansion
+ location
+
+ line59
+ col3
+ file0
+
+ nameTO_NULL
+ expansion setToNull(x)
+
+
+ kindevent
+ location
+
+ line59
+ col3
+ file0
+
+ ranges
+
+
+
+ line59
+ col3
+ file0
+
+
+ line59
+ col15
+ file0
+
+
+
+ depth0
+ extended_message
+ Calling 'setToNull'
+ message
+ Calling 'setToNull'
+
+
+ kindevent
+ location
+
+ line50
+ col1
+ file0
+
+ depth1
+ extended_message
+ Entered call from 'functionLikeMacroTest'
+ message
+ Entered call from 'functionLikeMacroTest'
+
+
+ kindcontrol
+ edges
+
+
+start
+
+
+ line50
+ col1
+ file0
+
+
+ line50
+ col4
+ file0
+
+
+end
+
+
+ line51
+ col3
+ file0
+
+
+ line51
+ col3
+ file0
+
+
+
+
+
+
+ kindevent
+ location
+
+ line51
+ col3
+ file0
+
+ ranges
+
+
+
+ line51
+ col3
+ file0
+
+
+ line51
+ col17
+ file0
+
[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:727 + +static std::string getExpandedMacroImpl(TokenPrinter &Printer, +SourceLocation MacroLoc, Whoa, that's so easy! Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:787-791 + // Acquire the macro's name. + Token TheTok; + RawLexer.LexFromRawLexer(TheTok); + + std::string MacroName = PP.getSpelling(TheTok); Not sure, random thought: Could this work in fact be done with the //static// `Lexer::getImmediateMacroName()` helper? Comment at: test/Analysis/plist-macros-with-expansion.cpp:31 +// CHECK: nameSET_PTR_VAR_TO_NULL +// CHECK: expansionptr = 0 Hmm, i think we can use `CHECK-NEXT:` here, in order to make sure these lines go sequentially. Otherwise this second line may accidentally match someone else's expansion(?). Comment at: test/Analysis/plist-macros-with-expansion.cpp:84 + +// TODO: Expand argumnts. +// CHECK: nameTO_NULL Typo: `arguments`. https://reviews.llvm.org/D52794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
