[Lldb-commits] [PATCH] D33035: Tool for using Intel(R) Processor Trace hardware feature

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thank you for making the changes. I concede that I am not always clear in my communications, and I apologize if I was being rude.

I am happy with the cmake stuff, but I noticed a new issue now. Exceptions are banned in llvm, so we need to figure out how to remove them from this code.



================
Comment at: tools/intel-features/intel-mpx/CMakeLists.txt:9
+
+set(MPX_DEPS ${MPX_DEPS} LLVMSupport PARENT_SCOPE)
----------------
abhishek.aggarwal wrote:
> labath wrote:
> > What you want here is to define an INTERFACE dependency on the MPX library instead.
> > vanilla cmake way would be `target_link_libraries(lldbIntelMPX INTERFACE LLVMSupport)`. **However**, we should use the llvm function instead, as that also handles other llvm-specific magic (for example, this code will break if someone does a LLVM_LINK_LLVM_DYLIB build).
> > 
> > So, I am asking for the third time:
> > Have you tried using add_lldb_library instead?
> > 
> > The correct invocation should be `add_lldb_library(foo.cpp LINK_LIBS Support)` and the rest of this file can just go away.
> I am extremely sorry Pavel but I understood it now what you were trying to say in previous comments. Sorry about misinterpreting your comments before. I have used add_lldb_library function now. Please see them in the next patch set.
Looks much better. Thanks.


================
Comment at: tools/intel-features/intel-pt/Decoder.cpp:411
+        std::string image_path(image_complete_path, path_length);
+        try {
+          readExecuteSectionInfos.emplace_back(
----------------
We can't have exceptions in llvm code. You will have to achieve this differently. Your trick with manually adding -fexceptions will not work anyway if the rest of the code is compiled without exceptions. Although I'm not really sure why you need to protect this vector append in particular, as we don't do this for any other vector elsewhere.


https://reviews.llvm.org/D33035