[Lldb-commits] [PATCH] D29256: Do not pass non-POD type variables through variadic function
krytarowski added a comment. In https://reviews.llvm.org/D29256#660072, @ki.stfu wrote: > I don't know the point of this patch (probably it's something special for > NetBSD? @emaste) but I'm okay with that. It's undefined (implementation defined) behavior. C++11 5.2.2/7: > Passing a potentially-evaluated argument of class type having a non-trivial > copy constructor, a non-trivial move contructor, or a non-trivial destructor, > with no corresponding parameter, is conditionally-supported with > implementation-defined semantics. > This patch was created to address compiler warning. Repository: rL LLVM https://reviews.llvm.org/D29256 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29256: Do not pass non-POD type variables through variadic function
ki.stfu added a comment.
In https://reviews.llvm.org/D29256#660159, @krytarowski wrote:
> It's undefined (implementation defined) behavior.
>
> C++11 5.2.2/7:
>
>
> > Passing a potentially-evaluated argument of class type having a
> > non-trivial copy constructor, a non-trivial move contructor, or a
> > non-trivial destructor, with no corresponding parameter, is
> > conditionally-supported with implementation-defined semantics.
> >
Interesting. Passing to what? I thought it means we shouldn't pass non-trivial
types through variadic arguments (`...` expression), and in this case we don't
do it because `CMIUtilString` is the type of the parameter `vFormating` which
has a name.
Unfortunately I can't reproduce it on my Ubuntu using the following example:
#include
#include
struct Count {
int value;
explicit Count(int v) : value(v) { }
// Make it non-trivial
Count(const Count&) : value(0) { }
Count(Count&&) : value(0) { }
~Count() { if (value) value |= 1; }
};
int add_nums(Count count, ...)
{
int result = 0;
va_list args;
va_start(args, count);
for (int i = 0; i < count.value; ++i) {
result += va_arg(args, int);
}
va_end(args);
return result;
}
int main()
{
std::cout << add_nums(Count(4), 25, 25, 50, 50) << '\n';
return 0;
}
> This patch was created to address compiler warning.
Could you tell me what compiler and platform do you use and provide the exact
warning message?
Repository:
rL LLVM
https://reviews.llvm.org/D29256
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29256: Do not pass non-POD type variables through variadic function
krytarowski added a comment. Hmm, I'm not reproducing it right now either. It's a patch added a year ago. I've pinged Tobias. Repository: rL LLVM https://reviews.llvm.org/D29256 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29215: [LLDB][MIPS] Fix TestMiniDumpNew
nitesh.jain updated this revision to Diff 86268. nitesh.jain added a comment. Added setUp/tearDown code, which saves and restores the original platform after each run. Thanks https://reviews.llvm.org/D29215 Files: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py Index: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py === --- packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py +++ packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py @@ -23,6 +23,14 @@ _linux_x86_64_not_crashed_pid = 29939 _linux_x86_64_not_crashed_pid_offset = 0xD967 +def setUp(self): +super(MiniDumpNewTestCase, self).setUp() +self._initial_platform = lldb.DBG.GetSelectedPlatform() + +def tearDown(self): +lldb.DBG.SetSelectedPlatform(self._initial_platform) +super(MiniDumpNewTestCase, self).tearDown() + def test_process_info_in_minidump(self): """Test that lldb can read the process information from the Minidump.""" # target create -c linux-x86_64.dmp Index: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py === --- packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py +++ packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py @@ -23,6 +23,14 @@ _linux_x86_64_not_crashed_pid = 29939 _linux_x86_64_not_crashed_pid_offset = 0xD967 +def setUp(self): +super(MiniDumpNewTestCase, self).setUp() +self._initial_platform = lldb.DBG.GetSelectedPlatform() + +def tearDown(self): +lldb.DBG.SetSelectedPlatform(self._initial_platform) +super(MiniDumpNewTestCase, self).tearDown() + def test_process_info_in_minidump(self): """Test that lldb can read the process information from the Minidump.""" # target create -c linux-x86_64.dmp ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D28155: Force the installation of lldb-server and lldb-argdumper
sylvestre.ledru abandoned this revision. sylvestre.ledru added a comment. Thanks! https://reviews.llvm.org/D28155 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29288: Switch std::call_once to llvm::call_once
krytarowski created this revision.
krytarowski added a project: LLDB.
The std::call_once implementation in libstdc++ has problems on few systems:
NetBSD, OpenBSD and Linux PPC. LLVM ships with a homegrown implementation
llvm::call_once to help on these platforms.
This change is required in the NetBSD LLDB port. std::call_once with libstdc++
results with crashing the debugger.
Repository:
rL LLVM
https://reviews.llvm.org/D29288
Files:
include/lldb/Core/Debugger.h
source/Commands/CommandObjectPlatform.cpp
source/Core/ConstString.cpp
source/Core/Debugger.cpp
source/Core/ModuleList.cpp
source/Host/common/Editline.cpp
source/Host/common/HostInfoBase.cpp
source/Host/linux/HostInfoLinux.cpp
source/Host/windows/HostInfoWindows.cpp
source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/Go/GoLanguage.cpp
source/Plugins/Language/Java/JavaLanguage.cpp
source/Plugins/Language/ObjC/ObjCLanguage.cpp
source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
source/Plugins/Process/Linux/NativeProcessLinux.cpp
source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
source/Plugins/Process/POSIX/ProcessPOSIXLog.cpp
source/Plugins/Process/Windows/Common/ProcessWindows.cpp
source/Plugins/Process/Windows/Common/ProcessWindowsLog.cpp
source/Plugins/Process/elf-core/ProcessElfCore.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
source/Plugins/Process/gdb-remote/ProcessGDBRemoteLog.cpp
source/Plugins/Process/mach-core/ProcessMachCore.cpp
source/Plugins/Process/minidump/ProcessMinidump.cpp
source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.cpp
source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
source/Symbol/ClangASTContext.cpp
source/Symbol/GoASTContext.cpp
source/Target/Language.cpp
tools/debugserver/source/MacOSX/DarwinLog/DarwinLogCollector.cpp
Index: tools/debugserver/source/MacOSX/DarwinLog/DarwinLogCollector.cpp
===
--- tools/debugserver/source/MacOSX/DarwinLog/DarwinLogCollector.cpp
+++ tools/debugserver/source/MacOSX/DarwinLog/DarwinLogCollector.cpp
@@ -46,10 +46,10 @@
s_os_activity_stream_set_event_handler;
bool LookupSPICalls() {
- static std::once_flag s_once_flag;
+ LLVM_DEFINE_ONCE_FLAG(s_once_flag);
static bool s_has_spi;
- std::call_once(s_once_flag, [] {
+ llvm::call_once(s_once_flag, [] {
dlopen ("/System/Library/PrivateFrameworks/LoggingSupport.framework/LoggingSupport", RTLD_NOW);
s_os_activity_stream_for_pid = (os_activity_stream_for_pid_t)dlsym(
RTLD_DEFAULT, "os_activity_stream_for_pid");
Index: source/Target/Language.cpp
===
--- source/Target/Language.cpp
+++ source/Target/Language.cpp
@@ -20,29 +20,32 @@
#include "lldb/Symbol/TypeList.h"
#include "lldb/Target/Target.h"
+#include "llvm/Support/Threading.h"
+
using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::formatters;
+using namespace llvm;
typedef std::unique_ptr LanguageUP;
typedef std::map LanguagesMap;
static LanguagesMap &GetLanguagesMap() {
static LanguagesMap *g_map = nullptr;
- static std::once_flag g_initialize;
+ LLVM_DEFINE_ONCE_FLAG(g_initialize);
- std::call_once(g_initialize, [] {
+ llvm::call_once(g_initialize, [] {
g_map = new LanguagesMap(); // NOTE: INTENTIONAL LEAK due to global
// destructor chain
});
return *g_map;
}
static std::mutex &GetLanguagesMutex() {
static std::mutex *g_mutex = nullptr;
- static std::once_flag g_initialize;
+ LLVM_DEFINE_ONCE_FLAG(g_initialize);
- std::call_once(g_initialize, [] {
+ llvm::call_once(g_initialize, [] {
g_mutex = new std::mutex(); // NOTE: INTENTIONAL LEAK due to global
// destructor chain
});
Index: source/Symbol/GoASTContext.cpp
===
--- source/Symbol/GoASTContext.cpp
+++ source/Symbol/GoASTContext.cpp
@@ -25,10 +25,13 @@
#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/Target.h"
+#include "llvm/Support/Threading.h"
+
#include "Plugins/ExpressionParser/Go/GoUserExpression.h"
#include "Plugins/SymbolFile/DWARF/DWARFASTParserGo.h"
using namespace lldb;
+using namespace llvm;
namespace lldb_private {
class GoArray;
@@ -593,8 +596,8 @@
if (name) {
typedef UniqueCStringMap TypeNameToBasicTypeMap;
static TypeNameToBasicTypeMap g_type_map;
-static std::once_flag g_once_flag;
-std::call_once(g_once_flag
[Lldb-commits] [PATCH] D29288: Switch std::call_once to llvm::call_once
krytarowski added a comment. I can build and test this patch on NetBSD/amd64. I don't have access right now to a performant FreeBSD, Linux, Android, Windows and FreeBSD hosts to test build and execute tests for this patch on other platforms. Please check. I was in touch with `libstdc++` developers and the reason why `std::call_once` crashes is still cryptic. A combination of TLS, shared library and pointer to a function passed to `pthread_once`(3) results in a crash. Switch LLDB to the standard LLVM wrapper for `std::cal_once`. Few months back discussed with @mehdi_amini in the context of LLVM. Repository: rL LLVM https://reviews.llvm.org/D29288 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29288: Switch std::call_once to llvm::call_once
mehdi_amini added a comment. I'm fine with this change, but I'll leave the approval to one of the LLDB developer :) (Thanks for following-up with the libstdc++ on these platforms!) Repository: rL LLVM https://reviews.llvm.org/D29288 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29288: Switch std::call_once to llvm::call_once
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Be very careful when using this, you can't change member variables that used to
be std::once to be statics. We also don't need the llvm namespace to be
included with "using namespace llvm;" in many of the files.
Comment at: include/lldb/Core/Debugger.h:376
Broadcaster m_sync_broadcaster;
lldb::ListenerSP m_forward_listener_sp;
There must be an in ivar for m_clear_once. This can't be a static.
Debugger::Clear() can only be called once per debugger instance, not just once
ever.
Comment at: source/Core/Debugger.cpp:68
using namespace lldb_private;
+using namespace llvm;
Why was this added? Remove if not needed.
Comment at: source/Core/Debugger.cpp:768-769
//--
- std::call_once(m_clear_once, [this]() {
+ LLVM_DEFINE_ONCE_FLAG(m_clear_once);
+ llvm::call_once(m_clear_once, [this]() {
ClearIOHandlers();
This is wrong, it must be in ivar. We are trying to make sure Debugger::Clear()
gets calls only once for each instance of a Debugger. Read the comment.
Comment at: source/Core/ModuleList.cpp:33
using namespace lldb_private;
+using namespace llvm;
Why was this using added? Remove please.
Comment at: source/Host/common/Editline.cpp:27
using namespace lldb_private::line_editor;
+using namespace llvm;
Why was this added? Remove if not needed? I hope the LLVM_DEFINE_ONCE_FLAG
doesn't require the using directive. If it does, it should be fixed.
Comment at: source/Host/common/HostInfoBase.cpp:33
using namespace lldb_private;
+using namespace llvm;
Remove
Comment at: source/Host/linux/HostInfoLinux.cpp:22
using namespace lldb_private;
+using namespace llvm;
Remove
Comment at: source/Host/windows/HostInfoWindows.cpp:24
using namespace lldb_private;
+using namespace llvm;
Remove
Comment at: source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:38
using namespace lldb_private;
+using namespace llvm;
Remove
Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:47
using namespace lldb_private::formatters;
+using namespace llvm;
Remove
Comment at: source/Plugins/Language/Go/GoLanguage.cpp:30
using namespace lldb_private::formatters;
+using namespace llvm;
Remove
Comment at: source/Plugins/Language/Java/JavaLanguage.cpp:32
using namespace lldb_private::formatters;
+using namespace llvm;
Remove
Comment at: source/Plugins/Language/ObjC/ObjCLanguage.cpp:39
using namespace lldb_private::formatters;
+using namespace llvm;
Remove
Comment at: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:51
using namespace lldb_private;
+using namespace llvm;
Remove
Comment at: source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:51
using namespace lldb_private;
+using namespace llvm;
Remove
Comment at: source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp:53
using namespace lldb_private;
+using namespace llvm;
Remove
Comment at: source/Plugins/Process/POSIX/ProcessPOSIXLog.cpp:24
using namespace lldb_private;
+using namespace llvm;
Remove
Comment at: source/Plugins/Process/Windows/Common/ProcessWindows.cpp:44
using namespace lldb_private;
+using namespace llvm;
Remove
Comment at: source/Plugins/Process/Windows/Common/ProcessWindowsLog.cpp:20
using namespace lldb_private;
+using namespace llvm;
Remove
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:40
using namespace lldb_private;
+using namespace llvm;
rm
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp:48
using namespace lldb_private::process_gdb_remote;
+using namespace llvm;
rm
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:90
using namespace lldb_private::process_gdb_remote;
+using namespace llvm;
rm
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemoteLog.cpp:24
using namespace lldb_private::process_gdb_remote;
+using namespace llvm;
rm
Comment at: source/Plugins/Process/mach-core/ProcessMachCore.cpp:50
using namespace lldb_private;
+using namespace llvm;
rm
=
[Lldb-commits] [PATCH] D29288: Switch std::call_once to llvm::call_once
krytarowski added a comment.
In https://reviews.llvm.org/D29288#660636, @clayborg wrote:
> Be very careful when using this, you can't change member variables that used
> to be std::once to be statics. We also don't need the llvm namespace to be
> included with "using namespace llvm;" in many of the files.
`using namespace llvm;` is currently required in order to get this functional:
enum InitStatus { Uninitialized = 0, Wait = 1, Done = 2 };
typedef volatile sys::cas_flag once_flag;
/// This macro is the only way you should define your once flag for LLVM's
/// call_once.
#define LLVM_DEFINE_ONCE_FLAG(flag) static once_flag flag = Uninitialized
`sys::cas_flag` is a member of the `llvm` namespace.
Repository:
rL LLVM
https://reviews.llvm.org/D29288
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29288: Switch std::call_once to llvm::call_once
krytarowski added a comment.
namespace llvm {
namespace sys {
void MemoryFence();
#ifdef _MSC_VER
typedef long cas_flag;
#else
typedef uint32_t cas_flag;
#endif
cas_flag CompareAndSwap(volatile cas_flag* ptr,
cas_flag new_value,
cas_flag old_value);
}
}
- `include/llvm/Support/Atomic.h` of LLVM
Repository:
rL LLVM
https://reviews.llvm.org/D29288
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29288: Switch std::call_once to llvm::call_once
mehdi_amini added a comment. In https://reviews.llvm.org/D29288#660677, @krytarowski wrote: > In https://reviews.llvm.org/D29288#660636, @clayborg wrote: > > > Be very careful when using this, you can't change member variables that > > used to be std::once to be statics. We also don't need the llvm namespace > > to be included with "using namespace llvm;" in many of the files. > > > `using namespace llvm;` is currently required in order to get this functional: What about just updating the macro in LLVM to make it not needed? #define LLVM_DEFINE_ONCE_FLAG(flag) static llvm::once_flag flag = Uninitialized Repository: rL LLVM https://reviews.llvm.org/D29288 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29288: Switch std::call_once to llvm::call_once
krytarowski added a comment. In https://reviews.llvm.org/D29288#660703, @mehdi_amini wrote: > In https://reviews.llvm.org/D29288#660677, @krytarowski wrote: > > > In https://reviews.llvm.org/D29288#660636, @clayborg wrote: > > > > > Be very careful when using this, you can't change member variables that > > > used to be std::once to be statics. We also don't need the llvm namespace > > > to be included with "using namespace llvm;" in many of the files. > > > > > > `using namespace llvm;` is currently required in order to get this > > functional: > > > What about just updating the macro in LLVM to make it not needed? > > #define LLVM_DEFINE_ONCE_FLAG(flag) static llvm::once_flag flag = > Uninitialized > Right, this looks cleaner. I will propose this patch as a separated review. Repository: rL LLVM https://reviews.llvm.org/D29288 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29095: Open ELF core dumps with more than 64K sections
EugeneBi added a comment. So, what's now? Can somebody commit it, please? I do not see any option to do it myself. https://reviews.llvm.org/D29095 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29256: Do not pass non-POD type variables through variadic function
joerg added a comment.
#include
#include
void f(std::string msg, ...) {
va_list ap;
va_start(ap, msg);
}
compiled against libc++ gives:
test.cc:6:3: error: cannot pass object of non-POD type 'std::string' (aka
'basic_string,
allocator >') through variadic function; call will abort at runtime
[-Wnon-pod-varargs]
Joerg
Repository:
rL LLVM
https://reviews.llvm.org/D29256
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29256: Do not pass non-POD type variables through variadic function
labath accepted this revision. labath added a comment. > Interesting. Passing to what? I thought it means we shouldn't pass > non-trivial types through variadic arguments (... expression), and in this > case we don't do it because CMIUtilString is the type of the parameter > vFormating which has a name. Presumably the "undefinedness" comes from the fact that `va_start` uses the last non-... argument to locate the variadic args on the stack. And a non-POD type may cause the stack to be layed out in a way the the function does not expect. Repository: rL LLVM https://reviews.llvm.org/D29256 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29266: Synchronize PlatformNetBSD with Linux
labath requested changes to this revision. labath added a comment. This revision now requires changes to proceed. How much of the code is now actually different between the two classes? If the only changes are of the s/linux/netbsd type, then we should just create a new base class for the two, and pass the platform name in as a parameter. Repository: rL LLVM https://reviews.llvm.org/D29266 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29215: [LLDB][MIPS] Fix TestMiniDumpNew
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I am glad we could come closer to the cause of the problem. At one point we should have a common base class for all core tests, so that we don't have to do this manually (and also avoid doing things that don't make sense for core files), but this will do for now. https://reviews.llvm.org/D29215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
