[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365853: [Expression] Move IRDynamicChecks to 
ClangExpressionParser (authored by xiaobai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64591?vs=209351=209388#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64591

Files:
  lldb/trunk/include/lldb/Expression/DynamicCheckerFunctions.h
  lldb/trunk/include/lldb/Expression/IRDynamicChecks.h
  lldb/trunk/source/Expression/CMakeLists.txt
  lldb/trunk/source/Expression/IRDynamicChecks.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
  lldb/trunk/source/Target/Process.cpp
  lldb/trunk/source/Target/ThreadPlanCallUserExpression.cpp

Index: lldb/trunk/source/Target/ThreadPlanCallUserExpression.cpp
===
--- lldb/trunk/source/Target/ThreadPlanCallUserExpression.cpp
+++ lldb/trunk/source/Target/ThreadPlanCallUserExpression.cpp
@@ -13,7 +13,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Target/LanguageRuntime.h"
Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -22,7 +22,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -62,6 +62,7 @@
 #include "ClangHost.h"
 #include "ClangModulesDeclVendor.h"
 #include "ClangPersistentVariables.h"
+#include "IRDynamicChecks.h"
 #include "IRForTarget.h"
 #include "ModuleDependencyCollector.h"
 
@@ -69,7 +70,6 @@
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/StreamFile.h"
-#include "lldb/Expression/IRDynamicChecks.h"
 #include "lldb/Expression/IRExecutionUnit.h"
 #include "lldb/Expression/IRInterpreter.h"
 #include "lldb/Host/File.h"
@@ -1281,8 +1281,8 @@
 (execution_policy != eExecutionPolicyTopLevel && !can_interpret)) {
   if (m_expr.NeedsValidation() && process) {
 if (!process->GetDynamicCheckers()) {
-  DynamicCheckerFunctions *dynamic_checkers =
-  new DynamicCheckerFunctions();
+  ClangDynamicCheckerFunctions *dynamic_checkers =
+  new ClangDynamicCheckerFunctions();
 
   DiagnosticManager install_diagnostics;
 
@@ -1302,23 +1302,26 @@
 "Finished installing dynamic checkers ==");
 }
 
-IRDynamicChecks ir_dynamic_checks(*process->GetDynamicCheckers(),
-  function_name.AsCString());
-
-llvm::Module *module = execution_unit_sp->GetModule();
-if (!module || !ir_dynamic_checks.runOnModule(*module)) {
-  err.SetErrorToGenericError();
-  err.SetErrorString("Couldn't add dynamic checks to the expression");
-  return err;
-}
+if (auto *checker_funcs = llvm::dyn_cast(
+process->GetDynamicCheckers())) {
+  IRDynamicChecks ir_dynamic_checks(*checker_funcs,
+function_name.AsCString());
+
+  llvm::Module *module = execution_unit_sp->GetModule();
+  if (!module || !ir_dynamic_checks.runOnModule(*module)) {
+err.SetErrorToGenericError();
+err.SetErrorString("Couldn't add dynamic checks to the expression");
+return err;
+  }
 
-if (custom_passes.LatePasses) {
-  if (log)
-log->Printf("%s - Running Late IR Passes from LanguageRuntime on "
-"expression module '%s'",
-__FUNCTION__, m_expr.FunctionName());
+  if (custom_passes.LatePasses) {
+if (log)
+  log->Printf("%s - Running Late IR 

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D64591#1581878 , @jingham wrote:

> That would be cleaner.
>
> OTOH, the original reason for these checkers was to help people understand 
> crashes in their expressions more clearly.  Supposedly, modern languages 
> "don't have pointers" and can't have bad objects, so the kind of crashes this 
> instrumentation was supposed to help with "can't happen" and checkers for 
> such languages wouldn't be all that helpful...
>
> So while cleaner, maybe generalizing this more fully isn't a high priority 
> change?  In which case, just getting them out of generic code seems fine as a 
> stopping point.  Your choice.


I don't think of it as high priority. That might change at some in the future, 
but for the time being I think that there are bigger fish to fry.


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

That would be cleaner.

OTOH, the original reason for these checkers was to help people understand 
crashes in their expressions more clearly.  Supposedly, modern languages "don't 
have pointers" and can't have bad objects, so the kind of crashes this 
instrumentation was supposed to help with "can't happen" and checkers for such 
languages wouldn't be all that helpful...

So while cleaner, maybe generalizing this more fully isn't a high priority 
change?  In which case, just getting them out of generic code seems fine as a 
stopping point.  Your choice.


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D64591#1581820 , @jingham wrote:

> Then the IRDynamicCheck part would go with LLVMUserExpression, and the 
> C-specific checks in Clang...


Except IRDynamicChecks has special knowledge of the clang-specific 
instrumenters. I can think of another way in which DynamicCheckerFunctions and 
IRDynamicChecks are more generic classes that will work with any language. I 
can try to explore that option to see what I come up with, maybe the end result 
will be better?


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Then the IRDynamicCheck part would go with LLVMUserExpression, and the 
C-specific checks in Clang...


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D64591#1581775 , @jingham wrote:

> This is a little unclear to me.  LLVMUserExpression is the class that 
> represents "anything that uses LLVM at its back end."  Both the Swift & Clang 
> user expressions are instances of this.  Running through IR and inserting 
> little callouts is an LLVMUserExpression type thing.  So it seems like this 
> move is in the right direction, but to be really clean needs to represent 
> LLVMUserExpression in the Plugin Hierarchy.


Right, this makes sense to me. My understanding was that LLVMUserExpression 
would eventually invoke ClangExpressionParser, which actually handles dealing 
with IR instrumentation. With swift in the picture, things get a little more 
complicated because you could want to deal with both ObjC code and Swift code. 
I think that in that case, LLDB would want to use both Clang and Swift to 
instrument a user expression. I don't think that the abstractions are set up to 
account for this use case right now, so it would require some planning and 
refactoring.


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

This is a little unclear to me.  LLVMUserExpression is the class that 
represents "anything that uses LLVM at its back end."  Both the Swift & Clang 
user expressions are instances of this.  Running through IR and inserting 
little callouts is an LLVMUserExpression type thing.  So it seems like this 
move is in the right direction, but to be really clean needs to represent 
LLVMUserExpression in the Plugin Hierarchy.


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai updated this revision to Diff 209351.
xiaobai added a comment.

Remove documentation boilerplate


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

https://reviews.llvm.org/D64591

Files:
  include/lldb/Expression/DynamicCheckerFunctions.h
  include/lldb/Expression/IRDynamicChecks.h
  source/Expression/CMakeLists.txt
  source/Expression/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
  source/Target/Process.cpp
  source/Target/ThreadPlanCallUserExpression.cpp

Index: source/Target/ThreadPlanCallUserExpression.cpp
===
--- source/Target/ThreadPlanCallUserExpression.cpp
+++ source/Target/ThreadPlanCallUserExpression.cpp
@@ -13,7 +13,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Target/LanguageRuntime.h"
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -22,7 +22,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
Index: source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
===
--- source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
+++ source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
@@ -9,46 +9,32 @@
 #ifndef liblldb_IRDynamicChecks_h_
 #define liblldb_IRDynamicChecks_h_
 
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/lldb-types.h"
 #include "llvm/Pass.h"
 
 namespace llvm {
 class BasicBlock;
-class CallInst;
-class Constant;
-class Function;
-class Instruction;
 class Module;
-class DataLayout;
-class Value;
 }
 
 namespace lldb_private {
 
-class ClangExpressionDeclMap;
 class ExecutionContext;
 class Stream;
 
-/// \class DynamicCheckerFunctions IRDynamicChecks.h
-/// "lldb/Expression/IRDynamicChecks.h" Encapsulates dynamic check functions
-/// used by expressions.
-///
-/// Each of the utility functions encapsulated in this class is responsible
-/// for validating some data that an expression is about to use.  Examples
-/// are:
-///
-/// a = *b; // check that b is a valid pointer [b init];   // check that b
-/// is a valid object to send "init" to
-///
-/// The class installs each checker function into the target process and makes
-/// it available to IRDynamicChecks to use.
-class DynamicCheckerFunctions {
+class ClangDynamicCheckerFunctions
+: public lldb_private::DynamicCheckerFunctions {
 public:
   /// Constructor
-  DynamicCheckerFunctions();
+  ClangDynamicCheckerFunctions();
 
   /// Destructor
-  ~DynamicCheckerFunctions();
+  virtual ~ClangDynamicCheckerFunctions();
+
+  static bool classof(const DynamicCheckerFunctions *checker_funcs) {
+return checker_funcs->GetKind() == DCF_Clang;
+  }
 
   /// Install the utility functions into a process.  This binds the instance
   /// of DynamicCheckerFunctions to that process.
@@ -63,9 +49,9 @@
   /// True on success; false on failure, or if the functions have
   /// already been installed.
   bool Install(DiagnosticManager _manager,
-   ExecutionContext _ctx);
+   ExecutionContext _ctx) override;
 
-  bool DoCheckersExplainStop(lldb::addr_t addr, Stream );
+  bool DoCheckersExplainStop(lldb::addr_t addr, Stream ) override;
 
   std::shared_ptr m_valid_pointer_check;
   std::shared_ptr m_objc_object_check;
@@ -94,7 +80,7 @@
   /// \param[in] decl_map
   /// The mapping used to look up entities in the target process. In
   /// this case, used to find objc_msgSend
-  IRDynamicChecks(DynamicCheckerFunctions _functions,
+  IRDynamicChecks(ClangDynamicCheckerFunctions _functions,
   const char *func_name = "$__lldb_expr");
 
   /// Destructor
@@ -136,7 +122,7 @@
   bool FindDataLoads(llvm::Module , llvm::BasicBlock );
 
   std::string m_func_name; ///< The name of the function to add checks to
-  DynamicCheckerFunctions
+  ClangDynamicCheckerFunctions
   _checker_functions; ///< The checker functions for the process
 };
 
Index: source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
===
--- 

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: include/lldb/Expression/DynamicCheckerFunctions.h:19
+
+/// \class DynamicCheckerFunctions DynamicCheckerFunctions.h
+/// "lldb/Expression/DynamicCheckerFunctions.h" Encapsulates dynamic check

xiaobai wrote:
> JDevlieghere wrote:
> > Can we skip this boilerplate? 
> Do you mean the whole thing? Or just the `\class` line?
Everything up to "Encapsulates dynamic check...", Doxygen is smart enough to 
associate the comment with the class. 


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai added inline comments.



Comment at: include/lldb/Expression/DynamicCheckerFunctions.h:19
+
+/// \class DynamicCheckerFunctions DynamicCheckerFunctions.h
+/// "lldb/Expression/DynamicCheckerFunctions.h" Encapsulates dynamic check

JDevlieghere wrote:
> Can we skip this boilerplate? 
Do you mean the whole thing? Or just the `\class` line?


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai updated this revision to Diff 209335.
xiaobai added a comment.

Reflow comment
Remove newline


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

https://reviews.llvm.org/D64591

Files:
  include/lldb/Expression/DynamicCheckerFunctions.h
  include/lldb/Expression/IRDynamicChecks.h
  source/Expression/CMakeLists.txt
  source/Expression/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
  source/Target/Process.cpp
  source/Target/ThreadPlanCallUserExpression.cpp

Index: source/Target/ThreadPlanCallUserExpression.cpp
===
--- source/Target/ThreadPlanCallUserExpression.cpp
+++ source/Target/ThreadPlanCallUserExpression.cpp
@@ -13,7 +13,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Target/LanguageRuntime.h"
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -22,7 +22,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
Index: source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
===
--- source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
+++ source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
@@ -9,46 +9,32 @@
 #ifndef liblldb_IRDynamicChecks_h_
 #define liblldb_IRDynamicChecks_h_
 
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/lldb-types.h"
 #include "llvm/Pass.h"
 
 namespace llvm {
 class BasicBlock;
-class CallInst;
-class Constant;
-class Function;
-class Instruction;
 class Module;
-class DataLayout;
-class Value;
 }
 
 namespace lldb_private {
 
-class ClangExpressionDeclMap;
 class ExecutionContext;
 class Stream;
 
-/// \class DynamicCheckerFunctions IRDynamicChecks.h
-/// "lldb/Expression/IRDynamicChecks.h" Encapsulates dynamic check functions
-/// used by expressions.
-///
-/// Each of the utility functions encapsulated in this class is responsible
-/// for validating some data that an expression is about to use.  Examples
-/// are:
-///
-/// a = *b; // check that b is a valid pointer [b init];   // check that b
-/// is a valid object to send "init" to
-///
-/// The class installs each checker function into the target process and makes
-/// it available to IRDynamicChecks to use.
-class DynamicCheckerFunctions {
+class ClangDynamicCheckerFunctions
+: public lldb_private::DynamicCheckerFunctions {
 public:
   /// Constructor
-  DynamicCheckerFunctions();
+  ClangDynamicCheckerFunctions();
 
   /// Destructor
-  ~DynamicCheckerFunctions();
+  virtual ~ClangDynamicCheckerFunctions();
+
+  static bool classof(const DynamicCheckerFunctions *checker_funcs) {
+return checker_funcs->GetKind() == DCF_Clang;
+  }
 
   /// Install the utility functions into a process.  This binds the instance
   /// of DynamicCheckerFunctions to that process.
@@ -63,9 +49,9 @@
   /// True on success; false on failure, or if the functions have
   /// already been installed.
   bool Install(DiagnosticManager _manager,
-   ExecutionContext _ctx);
+   ExecutionContext _ctx) override;
 
-  bool DoCheckersExplainStop(lldb::addr_t addr, Stream );
+  bool DoCheckersExplainStop(lldb::addr_t addr, Stream ) override;
 
   std::shared_ptr m_valid_pointer_check;
   std::shared_ptr m_objc_object_check;
@@ -94,7 +80,7 @@
   /// \param[in] decl_map
   /// The mapping used to look up entities in the target process. In
   /// this case, used to find objc_msgSend
-  IRDynamicChecks(DynamicCheckerFunctions _functions,
+  IRDynamicChecks(ClangDynamicCheckerFunctions _functions,
   const char *func_name = "$__lldb_expr");
 
   /// Destructor
@@ -136,7 +122,7 @@
   bool FindDataLoads(llvm::Module , llvm::BasicBlock );
 
   std::string m_func_name; ///< The name of the function to add checks to
-  DynamicCheckerFunctions
+  ClangDynamicCheckerFunctions
   _checker_functions; ///< The checker functions for the process
 };
 
Index: source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
===
--- 

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: include/lldb/Expression/DynamicCheckerFunctions.h:19
+
+/// \class DynamicCheckerFunctions DynamicCheckerFunctions.h
+/// "lldb/Expression/DynamicCheckerFunctions.h" Encapsulates dynamic check

Can we skip this boilerplate? 



Comment at: include/lldb/Expression/DynamicCheckerFunctions.h:27
+///
+/// a = *b; // check that b is a valid pointer [b init];   // check that b
+/// is a valid object to send "init" to

Reflow comment.



Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:1307
+process->GetDynamicCheckers())) {
 
+  IRDynamicChecks ir_dynamic_checks(*checker_funcs,

Spurious newline?


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

https://reviews.llvm.org/D64591



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

[Lldb-commits] [PATCH] D64591: [Expression] Move IRDynamicChecks to ClangExpressionParser

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: compnerd, clayborg, jingham, JDevlieghere.
Herald added a subscriber: mgorny.

IRDynamicChecks in its current form is specific to Clang since it deals
with the C language family. It is possible that we may want to
instrument code generated for other languages, but we can factor in a
more general mechanism to do so at a later time.

This decouples ObCLanguageRuntime from Expression!


https://reviews.llvm.org/D64591

Files:
  include/lldb/Expression/DynamicCheckerFunctions.h
  include/lldb/Expression/IRDynamicChecks.h
  source/Expression/CMakeLists.txt
  source/Expression/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
  source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
  source/Target/Process.cpp
  source/Target/ThreadPlanCallUserExpression.cpp

Index: source/Target/ThreadPlanCallUserExpression.cpp
===
--- source/Target/ThreadPlanCallUserExpression.cpp
+++ source/Target/ThreadPlanCallUserExpression.cpp
@@ -13,7 +13,7 @@
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Target/LanguageRuntime.h"
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -22,7 +22,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Expression/DiagnosticManager.h"
-#include "lldb/Expression/IRDynamicChecks.h"
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Expression/UtilityFunction.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
Index: source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
===
--- source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
+++ source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
@@ -9,46 +9,32 @@
 #ifndef liblldb_IRDynamicChecks_h_
 #define liblldb_IRDynamicChecks_h_
 
+#include "lldb/Expression/DynamicCheckerFunctions.h"
 #include "lldb/lldb-types.h"
 #include "llvm/Pass.h"
 
 namespace llvm {
 class BasicBlock;
-class CallInst;
-class Constant;
-class Function;
-class Instruction;
 class Module;
-class DataLayout;
-class Value;
 }
 
 namespace lldb_private {
 
-class ClangExpressionDeclMap;
 class ExecutionContext;
 class Stream;
 
-/// \class DynamicCheckerFunctions IRDynamicChecks.h
-/// "lldb/Expression/IRDynamicChecks.h" Encapsulates dynamic check functions
-/// used by expressions.
-///
-/// Each of the utility functions encapsulated in this class is responsible
-/// for validating some data that an expression is about to use.  Examples
-/// are:
-///
-/// a = *b; // check that b is a valid pointer [b init];   // check that b
-/// is a valid object to send "init" to
-///
-/// The class installs each checker function into the target process and makes
-/// it available to IRDynamicChecks to use.
-class DynamicCheckerFunctions {
+class ClangDynamicCheckerFunctions
+: public lldb_private::DynamicCheckerFunctions {
 public:
   /// Constructor
-  DynamicCheckerFunctions();
+  ClangDynamicCheckerFunctions();
 
   /// Destructor
-  ~DynamicCheckerFunctions();
+  virtual ~ClangDynamicCheckerFunctions();
+
+  static bool classof(const DynamicCheckerFunctions *checker_funcs) {
+return checker_funcs->GetKind() == DCF_Clang;
+  }
 
   /// Install the utility functions into a process.  This binds the instance
   /// of DynamicCheckerFunctions to that process.
@@ -63,9 +49,9 @@
   /// True on success; false on failure, or if the functions have
   /// already been installed.
   bool Install(DiagnosticManager _manager,
-   ExecutionContext _ctx);
+   ExecutionContext _ctx) override;
 
-  bool DoCheckersExplainStop(lldb::addr_t addr, Stream );
+  bool DoCheckersExplainStop(lldb::addr_t addr, Stream ) override;
 
   std::shared_ptr m_valid_pointer_check;
   std::shared_ptr m_objc_object_check;
@@ -94,7 +80,7 @@
   /// \param[in] decl_map
   /// The mapping used to look up entities in the target process. In
   /// this case, used to find objc_msgSend
-  IRDynamicChecks(DynamicCheckerFunctions _functions,
+  IRDynamicChecks(ClangDynamicCheckerFunctions _functions,
   const char *func_name = "$__lldb_expr");
 
   /// Destructor
@@ -136,7 +122,7 @@
   bool FindDataLoads(llvm::Module , llvm::BasicBlock );
 
   std::string m_func_name; ///< The name of the function to add checks to
-  DynamicCheckerFunctions
+