[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

jfb added a comment. Do you ever use weak pointers? The single allocation is good until you start holding on to more things (because you can't just keep the block around, you need the entire allocation), so you should make sure you won’t be delaying memory being reclaimed. Repository: rLLDB

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

Eugene.Zelenko added inline comments. Comment at: lldb/source/API/SBData.cpp:12 +#include + #include "lldb/API/SBData.h" JDevlieghere wrote: > Eugene.Zelenko wrote: > > JDevlieghere wrote: > > > jingham wrote: > > > > Eugene.Zelenko wrote: > > > > > Spaces betw

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/source/API/SBData.cpp:12 +#include + #include "lldb/API/SBData.h" Eugene.Zelenko wrote: > JDevlieghere wrote: > > jingham wrote: > > > Eugene.Zelenko wrote: > >

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

Eugene.Zelenko added inline comments. Comment at: lldb/source/API/SBData.cpp:12 +#include + #include "lldb/API/SBData.h" JDevlieghere wrote: > jingham wrote: > > Eugene.Zelenko wrote: > > > Spaces between include statements interfere with Clang-format. Same in

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/source/API/SBData.cpp:12 +#include + #include "lldb/API/SBData.h" jingham wrote: > Eugene.Zelenko wrote: > > Spaces between include statements interfere with Cla

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

jingham added inline comments. Comment at: lldb/source/API/SBData.cpp:12 +#include + #include "lldb/API/SBData.h" Eugene.Zelenko wrote: > Spaces between include statements interfere with Clang-format. Same in other > places. How? We always separate system hea

[Lldb-commits] [PATCH] D57990: Use std::make_shared in LLDB (NFC)

Eugene.Zelenko added inline comments. Comment at: lldb/source/API/SBData.cpp:9 #include cinttypes should be used instead. See Clang-tidy modernize-deprecated-headers. Same in other places. Comment at: lldb/source/API/SBData.cpp:12 +#incl

[Lldb-commits] [lldb] r353581 - Tiny fix spotted by static analyzer; GetPath() returns a std::string,

Author: jmolenda Date: Fri Feb 8 15:36:25 2019 New Revision: 353581 URL: http://llvm.org/viewvc/llvm-project?rev=353581&view=rev Log: Tiny fix spotted by static analyzer; GetPath() returns a std::string, we get a pointer to the c-string rep and then the temporary object is destructed and we still

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

krytarowski added a comment. In D57959#1390992 , @mgorny wrote: > But to what purpose? I think it's better to use consistent macros to refer to > the same scenario. I had an impression that LLVM_ON_UNIX is LLVM homegrown symbol for this exact purpose t

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

mgorny added a comment. But to what purpose? I think it's better to use consistent macros to refer to the same scenario. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57959/new/ https://reviews.llvm.org/D57959 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

krytarowski added a comment. In D57959#1390929 , @mgorny wrote: > Ok, I see that `_WIN32` actually redefines `sigset_t`, so I've added a > separate branch for it. Maybe `#ifdef LLVM_ON_UNIX`? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57959

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

mgorny updated this revision to Diff 186011. mgorny added a comment. Ok, I see that `_WIN32` actually redefines `sigset_t`, so I've added a separate branch for it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57959/new/ https://reviews.llvm.org/D57959 Files: lldb/source/Host/common

[Lldb-commits] [lldb] r353549 - [opaque pointer types] Update calls to CreateCall to pass the function

Author: jyknight Date: Fri Feb 8 11:30:46 2019 New Revision: 353549 URL: http://llvm.org/viewvc/llvm-project?rev=353549&view=rev Log: [opaque pointer types] Update calls to CreateCall to pass the function type in lldb and polly. Modified: lldb/trunk/source/Expression/IRDynamicChecks.cpp

[Lldb-commits] [PATCH] D57964: Fix potential UB when target_file directory is null

jingham added a comment. Might be clearer to do: if (target_file.GetDirectory().IsEmpty()) { error... } std::string directory = ... Avoids the extra variable and also adds the check for "", which I don't think we want to add to sys.path either. CHANGES SINCE LAST ACTION https://

[Lldb-commits] [PATCH] D57912: [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD

krytarowski added a comment. @labath our short-term goal is to enable execution of LLDB tests on the NetBSD buildbot. We are stuck temporarily with an older release of NetBSD on the machine for some time (1-2 months) so we need to live with it for now. No need to make it better than sufficient

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

krytarowski added a comment. I think that sigemptyset(2) is unsupported on Windows. Comment at: lldb/source/Host/common/MainLoop.cpp:149 for (const auto &sig : loop.m_signals) sigdelset(&sigmask, sig.first); #endif Shouldn't we initialize sigmask alway

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

krytarowski added inline comments. Comment at: lldb/source/Host/common/MainLoop.cpp:149 for (const auto &sig : loop.m_signals) sigdelset(&sigmask, sig.first); #endif krytarowski wrote: > Shouldn't we initialize sigmask always before adding/deleting parti

[Lldb-commits] [PATCH] D57964: Fix potential UB when target_file directory is null

sgraenitz updated this revision to Diff 186002. sgraenitz added a comment. Remove useless newline CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57964/new/ https://reviews.llvm.org/D57964 Files: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Index: source/Plugin

[Lldb-commits] [PATCH] D57912: [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD

This revision was automatically updated to reflect the committed changes. Closed by commit rL353545: [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD (authored by mgorny, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D57964: Fix potential UB when target_file directory is null

sgraenitz created this revision. sgraenitz added a reviewer: jingham. As seen in a crash report, the C-string returned for the directory component of `target_file` can null. It should not be assigned to `std::string` directly as this is undefined behavior. https://reviews.llvm.org/D57964 File

[Lldb-commits] [lldb] r353545 - [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD

Author: mgorny Date: Fri Feb 8 10:56:11 2019 New Revision: 353545 URL: http://llvm.org/viewvc/llvm-project?rev=353545&view=rev Log: [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD The NetBSD kernel currently does not support detecting closed slave pty via kevent on master pty. Thi

[Lldb-commits] [PATCH] D57751: minidump: Add ability to attach (breakpad) symbol files to placeholder modules

clayborg accepted this revision. clayborg added a comment. In D57751#1390345 , @labath wrote: > Hi Greg, what do you think of my replies to your `CreateModuleFromObjectFile` > comment? Your explanation makes sense. I missed the fact that this function w

[Lldb-commits] [PATCH] D57956: [www] Add ASTImporter fuzzer project.

JDevlieghere added a comment. Raphael, can you please also update the corresponding RST file? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57956/new/ https://reviews.llvm.org/D57956 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D57959: [lldb] [MainLoop] Initialize empty sigset_t correctly

mgorny created this revision. mgorny added reviewers: zturner, labath, krytarowski. Herald added a project: LLDB. Fix MainLoop::RunImpl::get_sigmask() to correctly return empty sigset_t when SIGNAL_POLLING_UNSUPPORTED is true. On NetBSD (and probably on some other platforms), integers are not imp

[Lldb-commits] [PATCH] D57956: [www] Add ASTImporter fuzzer project.

teemperor added subscribers: friss, LLDB. teemperor added a comment. Just trying to get some early feedback before I subscribe the mailing list. Also let me know if someone here also wants to mentor for this project and I'll add you. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57956/

[Lldb-commits] [PATCH] D57895: Breakpad: auto-detect path style of file entries

amccarth accepted this revision. amccarth added a comment. I think `absolute_path` is great. Thanks for checking on the `native`/`None`. I didn't want there to be any latent surprises. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57895/new/ https://reviews.llvm.org/D57895 ___

[Lldb-commits] [PATCH] D57928: Fix x86 return pattern detection

mortimer added a comment. For sure I can add a test. I found this only because I was looking to fix something else (prologue detection on OpenBSD with -fret-protector) and noticed that the return pattern detect function was wrong. So nothing was broken that I found, though the inclusion of 0xc

[Lldb-commits] [PATCH] D57912: [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD

mgorny added a comment. Ok, thanks. I consider this only temporary; I have a kernel patch ready already (and waiting for review), so we're going to look into reenabling it soon enough. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57912/new/ https://reviews.llvm.org/D57912 ___

[Lldb-commits] [PATCH] D57751: minidump: Add ability to attach (breakpad) symbol files to placeholder modules

labath added a comment. Hi Greg, what do you think of my replies to your `CreateModuleFromObjectFile` comment? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57751/new/ https://reviews.llvm.org/D57751 ___ lldb-commits mailing list lldb-comm

[Lldb-commits] [PATCH] D57895: Breakpad: auto-detect path style of file entries

labath updated this revision to Diff 185937. labath added a comment. Rename arg to `absolute_path`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57895/new/ https://reviews.llvm.org/D57895 Files: include/lldb/Utility/FileSpec.h lit/SymbolFile/Breakpad/Inputs/line-table-mixed-path-s

[Lldb-commits] [PATCH] D57895: Breakpad: auto-detect path style of file entries

labath marked 2 inline comments as done. labath added inline comments. Comment at: include/lldb/Utility/FileSpec.h:250 + /// unreliable (e.g. "c:\foo.txt" is a valid relative posix path). + static llvm::Optional

[Lldb-commits] [PATCH] D57928: Fix x86 return pattern detection

labath added a comment. Could you please write a test for this? You can take a look at unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp for other x86AssemblyInspectionEngine tests. Also, I'm curious how you found this bug. (i.e. which functionality was broken with the old imple

[Lldb-commits] [PATCH] D57912: [lldb] [unittests] Disable MainLoopTest::DetectsEOF on NetBSD

labath accepted this revision. labath added a comment. I don't remember if this tests some super-critical functionality (i.e., whether you will be able to function reasonably without it), but in case it does, it should be fairly easy to get this class to use one of the other event sources (psel

[Lldb-commits] [PATCH] D55122: [PDB] Fix location retrieval for function local variables and arguments that are stored relative to VFRAME

aleksandr.urakov added a comment. Fixed in r353503, thanks! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55122/new/ https://reviews.llvm.org/D55122 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://li

[Lldb-commits] [lldb] r353503 - [NFC] Fix license headers after r352845

Author: aleksandr.urakov Date: Fri Feb 8 00:48:15 2019 New Revision: 353503 URL: http://llvm.org/viewvc/llvm-project?rev=353503&view=rev Log: [NFC] Fix license headers after r352845 Modified: lldb/trunk/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.cpp lldb/trunk/source/Plu