njames93 added inline comments.

================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:313
+                          AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc),
+                          std::string, MacroName) {
   // Verifies that the statement' beginning and ending are both expanded from
----------------
aaron.ballman wrote:
> You mentioned that the change from `StringRef` to `std::string` was to avoid 
> lifetime issues while matching, but I'm wondering if you can expound on that 
> situation a bit more. I would have assumed that any memoization that involves 
> `StringRef` would be responsible for the lifetime issues rather than the 
> matchers themselves, but maybe I'm thinking about a different way you can hit 
> lifetime issues than you are.
Take this as contrived, however using ASAN reports a heap-use-after-free for 
this code when isExpandedFromMacro takes a `StringRef` but works fine when 
using `std::string`.
```lang=c++
TEST(IsExpandedFromMacro, IsExpandedFromMacro_MatchesDecls) {
  auto Matcher = 
isExpandedFromMacro(std::string("MY_MACRO_OVERFLOW_SMALL_STRING_SIZE"));// 
<-Temporary string created and destroyed here.
  StringRef input = R"cc(
#define MY_MACRO_OVERFLOW_SMALL_STRING_SIZE(a) int i = a;
    void Test() { MY_MACRO_OVERFLOW_SMALL_STRING_SIZE(4); }
  )cc";
  EXPECT_TRUE(matches(input, varDecl(Matcher))); // <- heap-use-after-free 
detected down the callstack from here.
}
```
Obviously this is very contrived to ensure asan detects the use-after-free and 
to ensure the temporary string is destroyed before the matcher. For these tests 
it doesn't look like a problem, but in other instances these matchers will 
outlive a string being passed to them
```lang=c++
void addVarFromMacro(ASTMatchFinder* Finder, std::string MacroName) {
  Finder->addMatcher(varDecl(isExpandedFromMacro(MacroName)), this);
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90303/new/

https://reviews.llvm.org/D90303

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to