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

[Abhishek via Phabricator via lldb-commits]

abhishek.aggarwal added inline comments.


================
Comment at: tools/intel-features/intel-pt/Decoder.cpp:411
+        std::string image_path(image_complete_path, path_length);
+        try {
+          readExecuteSectionInfos.emplace_back(
----------------
labath wrote:
> abhishek.aggarwal wrote:
> > labath wrote:
> > > 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.
> > I kept the exception handling around stl containers only to catch bad_alloc exception because if this exception occurs, I didn't want the code to just exit but provide user with whatever amount of instruction log is available in the vector. That much amount of instruction log might still be helpful to the user. What is your opinion on that?
> > 
> > Plus, If rest of the code is not being compiled with -fexceptions but just this file, will it not solve the purpose? Let me know what you think about it. I can make changes accordingly then.
> I don't think there's any negotiating on this point (not with me anyway, you'd need to take this much higher up). But here's what I think anyway:
> - the exception catch will be generally useless as most (all?) current OSs will overcommit virtual memory (and then just kill you if they really run out of memory and swap space)
> - even if you disable overcommit, chances are you will hit an OOM in one of the zillion other places which allocate memory (all of which are unchecked) instead of here. So this single catch will not make a difference.
Got it. Then I remove exception handling code from here. Submit the patch again. Thanks for elaborating.


https://reviews.llvm.org/D33035