Fix CreateFunctionTemplateSpecialization to prevent dangling poiner to stack memory

[shafik]

Summary

In ClangASTContext::CreateFunctionTemplateSpecializationInfo a TemplateArgumentList is allocated on the stack but is treated as if it is persistent in subsequent calls. When we exit the function func_decl will still point to the stack allocated memory. We will use TemplateArgumentList::CreateCopy instead which will allocate memory out of the DeclContext.

[JDevlieghere]

packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_args/main.cpp

1 ↗

(On Diff #209965)

Clang format?

source/Symbol/ClangASTContext.cpp

1619 ↗

(On Diff #209965)

Out of curiosity, who does the cleanup of this pointer?

[teemperor]

I assume we never tested this and that's how didn't found this in sanitized builds?

But this patch LGTM. Thanks Shafik!

packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_args/main.cpp

1 ↗

(On Diff #209965)

Pretty sure that file is clang-formatted (at least my clang-format doesn't modify this file)

source/Symbol/ClangASTContext.cpp

1619 ↗

(On Diff #209965)

It's like an AST object stored in the ASTContext.

[labath]

packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_args/main.cpp

1 ↗

(On Diff #209965)

We have a .clang-format file for the test directory which effectively disables clang-formatting. So depending on how you run clang-format, the invocation might be completely ignored.

The .clang-format was put there before the Great Reformat to avoid it messing with the line numbers in tests. Maybe the time has come to do something about it...

[JDevlieghere]

packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_args/main.cpp

1 ↗

(On Diff #209965)

Sounds like a good idea. Most test should be using the // break here anyway, and removing the special .clang-format might flush out the ones that don't.

source/Symbol/ClangASTContext.cpp

1619 ↗

(On Diff #209965)

Thanks

[labath]

packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_args/main.cpp

1 ↗

(On Diff #209965)

I'm afraid that won't be enough. All of these tests were using // break here comments, but that wasn't enough because

some(very, long, statement) // break here

breaks at a slightly different place than

some(very,
  long, statement) // break here

Also, things like step-in/over are affected by how lines are broken up, and sometimes even comment it self is so long it doesn't fit ("please break on this line to inspect the state of foo"). However, I think we could do something via some combination of telling clang-format to not break certain comments (there's a way to set a regex to match non-breakable comments), increasing the line length, and making the comments themselves much shorter...

[shafik]

packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_args/main.cpp

1 ↗

(On Diff #209965)

Ahhh that makes sense, I was going to reply the same way @teemperor did.

source/Symbol/ClangASTContext.cpp

1619 ↗

(On Diff #209965)

If you dig into the CreateCopy it does a Context.Allocate and these allocations will be released when the ASTContext is destroyed.

[teemperor]

packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_args/main.cpp

1 ↗

(On Diff #209965)

Oh, I was totally unaware of that. That probably means I should double-check if some of my tests were actually clang-formatted :)

[shafik]

Applying clang-format to test

lldb/packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_args/Makefile

+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules

lldb/packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_args/TestFunctionTemplateSpecializationTempArgs.py

+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestFunctionTemplateSpecializationTempArgs(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def test_function_template_specialization_temp_args(self):
+        self.build()
+
+        (self.target, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(self, '// break here',
+                lldb.SBFileSpec("main.cpp", False))
+
+        self.expect("expr p0",
+                substrs=['(VType) $0 = {}'])

lldb/packages/Python/lldbsuite/test/expression_command/function_template_specialization_temp_args/main.cpp

+template <typename T> struct M {};
+
+template <typename T> void f(T &t);
+
+template <> void f<int>(int &t) {
+  typedef M<int> VType;
+
+  VType p0; // break here
+}
+
+int main() {
+  int x;
+
+  f(x);
+
+  return 0;
+}

lldb/source/Symbol/ClangASTContext.cpp

clang::FunctionTemplateDecl *ClangASTContext::CreateFunctionTemplateDecl(
   }
 
   return func_tmpl_decl;
 }
 
 void ClangASTContext::CreateFunctionTemplateSpecializationInfo(
     FunctionDecl *func_decl, clang::FunctionTemplateDecl *func_tmpl_decl,
     const TemplateParameterInfos &infos) {
-  TemplateArgumentList template_args(TemplateArgumentList::OnStack, infos.args);
+  TemplateArgumentList *template_args_ptr =
+      TemplateArgumentList::CreateCopy(func_decl->getASTContext(), infos.args);
 
-  func_decl->setFunctionTemplateSpecialization(func_tmpl_decl, &template_args,
-                                               nullptr);
+  func_decl->setFunctionTemplateSpecialization(func_tmpl_decl,
+                                               template_args_ptr, nullptr);
 }
 
 ClassTemplateDecl *ClangASTContext::CreateClassTemplateDecl(
     DeclContext *decl_ctx, lldb::AccessType access_type, const char *class_name,
     int kind, const TemplateParameterInfos &template_param_infos) {
   ASTContext *ast = getASTContext();
 
   ClassTemplateDecl *class_template_decl = nullptr;