[Lldb-commits] [PATCH] D70050: [lldb] [test] Add a test for watchpoint set error handling on NetBSD

[Michał Górny via Phabricator via lldb-commits]

mgorny marked 2 inline comments as done.
mgorny added inline comments.


================
Comment at: lldb/test/Shell/Watchpoint/Inputs/thread-dbreg.c:1-23
+#include <pthread.h>
+
+int g_watchme = 0;
+
+void *thread_func(void *arg) {
+  /* watchpoint trigger from subthread */
+  g_watchme = 2;
----------------
labath wrote:
> labath wrote:
> > mgorny wrote:
> > > labath wrote:
> > > > Maybe simplify this and remove the threads and stuff?
> > > Threads are intentional since new thread handler copies dbregs per the other patch. This makes sure that new thread handler will not crash when it is unable to set dbregs.
> > Hang on, isn't this test about what happens when you *cannot* set watchpoints? In that case there should be nothing to copy the dbregs from, right?
> Hmm... or is it that you're testing that the act of trying to copy this "nothing" works? In that case, this might be fine, but it'd probably be worth mentioning this in the test case.
Actually, since the whole dbreg support is rather stateless at the moment, we just unconditionally copy dbregs. I suppose this needs to be fixed, and now I see that my other patch misses error handling as well. However, the main point for this test is to prevent regressions, and this is a potential programmer's mistake.


================
Comment at: lldb/test/Shell/Watchpoint/netbsd-nouserdbregs.test:5
+# REQUIRES: native && system-netbsd && (target-x86 || target-x86_64) && !dbregs-set
+# RUN: %clang %p/Inputs/thread-dbreg.c -pthread -g -o %t.out
+# RUN: %lldb -b -o 'settings set interpreter.stop-command-source-on-error false' -s %s %t.out 2>&1 | FileCheck %s
----------------
labath wrote:
> mgorny wrote:
> > labath wrote:
> > > You should use %clang_host now. Are you sure that this even works on current master?
> > I have to admit that I'm at ~Oct 31 in my checkout. I've tried using `%clang_host` but it failed with `fg` saying that there is no job control.
> %clang_host was added *on* Oct 31, so maybe you need to sync a bit further... :)
Ok, I'll try.


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

https://reviews.llvm.org/D70050