[Lldb-commits] [PATCH] D110553: [lldb] Remove non-stop mode code

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.

NetBSD and FreeBSD do not support non-stop in the kernel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110553

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D96766: [lldb] [Process/FreeBSD] Introduce mips64 FPU reg support

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.

Looks still fine.


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

https://reviews.llvm.org/D96766

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D101563: [lldb] [test] Extend aarch64-gp-read test to cover all registers

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski requested changes to this revision.
krytarowski added a comment.
This revision now requires changes to proceed.

There is an unhandled comment.


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

https://reviews.llvm.org/D101563

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D109326: [lldb] [Process/FreeBSD] Support SaveCore() using PT_COREDUMP

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.
This revision is now accepted and ready to land.

Looks good


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

https://reviews.llvm.org/D109326

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D101329: [lldb] Support SaveCore() from gdb-remote client

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.
This revision is now accepted and ready to land.

Looks correct.


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

https://reviews.llvm.org/D101329

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D101285: [lldb] [llgs server] Support creating core dumps on NetBSD

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.
This revision is now accepted and ready to land.

Looks good


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

https://reviews.llvm.org/D101285

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100503: [lldb] [client] Implement follow-fork-mode

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.
This revision is now accepted and ready to land.

Looks good


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

https://reviews.llvm.org/D100503

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100267: [lldb] [gdb-remote client] Remove breakpoints throughout vfork

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.

Let's see.


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

https://reviews.llvm.org/D100267

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100267: [lldb] [gdb-remote client] Remove breakpoints throughout vfork

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.
This revision is now accepted and ready to land.

Looks reasonable


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

https://reviews.llvm.org/D100267

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100263: [lldb] [gdb-remote client] Remove breakpoints in forked processes

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.
This revision is now accepted and ready to land.

Looks correct.


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

https://reviews.llvm.org/D100263

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100206: [lldb] [llgs client] Support minimal fork/vfork handling

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.
This revision is now accepted and ready to land.

Looks good.


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

https://reviews.llvm.org/D100206

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D97114: [lldb] [docs] Update platform support status

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.

NetBSD OK!


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

https://reviews.llvm.org/D97114

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95947: [lldb] [Process/FreeBSDRemote] Introduce powerpc support

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Host/common/NativeProcessProtocol.cpp:525-526
   static const uint8_t g_s390x_opcode[] = {0x00, 0x01};
+  static const uint8_t g_ppc_opcode[] = {0x7f, 0xc0, 0x00, 0x08};
   static const uint8_t g_ppc64le_opcode[] = {0x08, 0x00, 0xe0, 0x7f}; // trap
 

jrtc27 wrote:
> mgorny wrote:
> > mgorny wrote:
> > > jrtc27 wrote:
> > > > krytarowski wrote:
> > > > > jrtc27 wrote:
> > > > > > Why are these two different? Should it not always be `trap` ie `tw 
> > > > > > 31,0,0`? If not that should be explained here. These names also 
> > > > > > aren't great as it's unclear which ppc64 is using unless you read 
> > > > > > the code below (I'd expect either ppc and ppc64 or ppc and ppcle as 
> > > > > > the two "axes", but ppc and ppc64le are on a diagonal in the 2x2 
> > > > > > grid).
> > > > > On PPC we assume Big-Endian unless specified otherwise, so no need to 
> > > > > specify ppcbe or ppc64be.
> > > > I realise why there is no `be` suffix, that's not what I was asking. 
> > > > Currently there are four options (though ppcle isn't implemented in 
> > > > FreeBSD):
> > > > |  | Big | Little |
> > > > | 32 | ppc | ppcle |
> > > > | 64 | ppc64 | ppc64le |
> > > > 
> > > > If the difference between the two encodings is solely endianness then 
> > > > they should be called `g_ppc_opcode` and `g_ppcle_opcode`. If the 
> > > > difference between the two encodings is solely machine word size then 
> > > > they should be called `g_ppc_opcode` and g_ppc64_opcode`. But 
> > > > `g_ppc_opcode` vs g_ppc64le_opcode` has *both* differences in the name, 
> > > > which tells you nothing about *why* they are different, and thus does 
> > > > not obviously state which encoding ppc64 uses, if either of them.
> > > > 
> > > > As for the encodings themselves, they obviously differ in endianness, 
> > > > but there is also a difference in the second/third byte where 
> > > > `g_ppc_opcode[1]` is `0xc0` but `g_ppc64le_opcode[2]` is `0xe0`. That 
> > > > does not make sense to me, but if it's there for a reason it needs a 
> > > > comment.
> > > I don't really know what's the difference between the `0xc0` and `0xe0` 
> > > opcode. Both can be found in various places in LLDB sources. I've copied 
> > > this one from `source/Target/Platform.cpp`). Maybe `0xc0` works on Big 
> > > Endian ppc as well, I've copied `0xe0` because it was used by the 
> > > relevant code before. I'll simplify the code as suggested and try `0xc0`.
> > Sorry, I got the two exchanged. I meant I'm going to try `0xe0`.
> FWIW 0xc is `tw 30, 0, 0` whereas 0xe is `tw 31, 0, 0` i.e. the canonical 
> unconditional trap instruction, which `trap` is an alias for.
NetBSD uses:

`#define PTRACE_BREAKPOINT   ((const uint8_t[]) { 0x7f, 0xe0, 0x00, 0x08 
})`.

https://nxr.netbsd.org/xref/src/sys/arch/powerpc/include/ptrace.h#76


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

https://reviews.llvm.org/D95947

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95947: [lldb] [Process/FreeBSDRemote] Introduce powerpc support

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Host/common/NativeProcessProtocol.cpp:525-526
   static const uint8_t g_s390x_opcode[] = {0x00, 0x01};
+  static const uint8_t g_ppc_opcode[] = {0x7f, 0xc0, 0x00, 0x08};
   static const uint8_t g_ppc64le_opcode[] = {0x08, 0x00, 0xe0, 0x7f}; // trap
 

jrtc27 wrote:
> Why are these two different? Should it not always be `trap` ie `tw 31,0,0`? 
> If not that should be explained here. These names also aren't great as it's 
> unclear which ppc64 is using unless you read the code below (I'd expect 
> either ppc and ppc64 or ppc and ppcle as the two "axes", but ppc and ppc64le 
> are on a diagonal in the 2x2 grid).
On PPC we assume Big-Endian unless specified otherwise, so no need to specify 
ppcbe or ppc64be.


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

https://reviews.llvm.org/D95947

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95802: [lldb] [Process/FreeBSDRemote] Introduce mips64 support

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

Merging is a good idea as NetBSD might duplicate these code chunks too.


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

https://reviews.llvm.org/D95802

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95696: [lldb] [Process/FreeBSDRemote] Introduce arm (32-bit) support

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D95696#2532032 , @strejda wrote:

> Oki, I see, thanks for response. Please give me a few days to analyze (and 
> eventually to implement).

Meanwhile, does this patch look fine as is? Hardware assisted watchpoints could 
be added in a follow up patch.


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

https://reviews.llvm.org/D95696

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95297: [lldb] [Process/FreeBSDRemote] Introduce arm64 support

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

@clayborg please have a look!


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

https://reviews.llvm.org/D95297

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92667: [lldb] [Platform/POSIX] Use gdb-remote plugin when attaching

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D92667#2434223 , @emaste wrote:

> No objection here. I'm curious why the two modified tests work on Linux or 
> NetBSD today though?

This is a debt of having 2 process plugins. NetBSD and Linux ship with a single 
one only.


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

https://reviews.llvm.org/D92667

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92187: [lldb] [FreeBSD] Fix establishing DT_NEEDED libraries from dyld

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

Ideally we should iterate over the startup process and investigate the state of 
the `r_debug` structure. Once it gets initialized, set the software brakpoint 
to `r_brk` and keep track of the dynamic loading and unloading of libraries. 
The tricky part is to detect the right moment to plug into `r_brk`, early 
enough in the startup process before calling constructors, loading the 
libraries etc and late enough to become initialized.

Right now we place the breakpoints unconditionally, hardcoding the linker name, 
mostly ignoring the `r_brk`, but in practice it's not clear whether it's the 
simplest and sufficient approach.


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

https://reviews.llvm.org/D92187

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92187: [lldb] [FreeBSD] Fix establishing DT_NEEDED libraries from dyld

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D92187#2424109 , @emaste wrote:

>> One thing that FreeBSD should do, is to upgrade to the protocol version 1 
>> (stored in r_version), like Linux, NetBSD and OpenBSD.
>
> It looks like Linux has always used r_version=1 (since 1995).
>
> AFAICT we can just add r_ldbase and set version to 1.
> See https://reviews.freebsd.org/D27429

I propose to add `#define R_DEBUG_VERSION 1` too, to keep compat with NetBSD 
(and SunOS?).


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

https://reviews.llvm.org/D92187

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92187: [lldb] [FreeBSD] Fix establishing DT_NEEDED libraries from dyld

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D92187#2423018 , @emaste wrote:

> I'm curious how gdb handles this, and asked @bsdjhb if he knows off hand.
>
> Is there a brief description of how this works on Linux and/or NetBSD?

We are working on researching this, NetBSD, Linux and FreeBSD should behave 
almost in the same way. At least GNU gdbserver reused the Linux code on NetBSD 
practically as-is.

One thing that FreeBSD should do, is to upgrade to the protocol version 1 
(stored in r_version), like Linux, NetBSD and OpenBSD.


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

https://reviews.llvm.org/D92187

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91847: [lldb] [debugserver] Add stN aliases for stmmN for compatibility

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

I would mark stmmX as an alias to stX and keep stX as the default for all 
platforms. stmmX could be an alias for everybody for legacy reasons.


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

https://reviews.llvm.org/D91847

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91007: [lldb] [Process/FreeBSDRemote] Fix handling user-generated SIGTRAP

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

I propose to filter all user induced signals at once and check `& 0x1` and 
SI_USER.

http://src.illumos.org/source/xref/freebsd-head/sys/sys/signal.h#406

This way we will avoid all future fallout and crashing the debugger on unknown 
signal.


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

https://reviews.llvm.org/D91007

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D90938: [lldb] [Process/FreeBSDRemote] Handle exec() from inferior

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.
This revision is now accepted and ready to land.

I wonder why NetBSD fails having the same event handling.


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

https://reviews.llvm.org/D90938

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D90757: [lldb] Enable FreeBSDRemote plugin by default and update test status

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D90757#2378404 , @theraven wrote:

> Does the new plugin work with processes that are created with `pdfork`?  The 
> last time I tried this, it caused the old plugin to lock up the debugger 
> entirely.  Please can you ensure that there are tests that cover `pdfork` and 
> `cap_enter` in the child?  These are currently quite badly broken.

Can you show an example of a program (ideally minimal) that is supposed to work 
but hangs? Does GDB support pdfork-ed children?

There is also TRAP_CAP and I'm not sure what should we do upon it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90757

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D90298: [lldb] [Process/FreeBSDRemote] Implement thread GetName()

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp:151
 
-std::string NativeThreadFreeBSD::GetName() { return ""; }
+std::string NativeThreadFreeBSD::GetName() {
+  if (!m_thread_name) {

It is probably fine, but I would use:

1. `PT_GETNUMLWPS`
2. `PT_GETLWPLIST` with optional caching
3. Read `pl_tdname` from `ptrace_lwpinfo`



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp:164
+  if (ptr == nullptr || (error != 0 && errno == ENOMEM)) {
+// Add extra space in case threads are added before next call.
+kp.resize((len / sizeof(struct kinfo_proc)) + 10);

Isn't this call always synchronous and all the threads are stopped?


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

https://reviews.llvm.org/D90298

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D89874: [lldb] Unify x86 watchpoint implementation on Linux and NetBSD (and future FreeBSD plugin) [WIP]

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:580
 
-  if (IsGPR(reg_index))
+  if (IsGPROrDR(reg_index))
 return WriteRegisterRaw(reg_index, reg_value);

Can we have `IsGPR(reg_index) || IsDR(reg_index)`?


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

https://reviews.llvm.org/D89874

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D89193: [lldb] [Process/FreeBSDRemote] Support YMM reg via PT_*XSTATE

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:448-449
+
+assert(info.xsave_mask & XFEATURE_ENABLED_X87);
+assert(info.xsave_mask & XFEATURE_ENABLED_SSE);
+

emaste wrote:
> mgorny wrote:
> > mgorny wrote:
> > > emaste wrote:
> > > > I wonder if these should be an error rather than assertion?
> > > I suppose the question is if they ever happen in real use. If they do, we 
> > > should probably handle them gracefully. Otherwise, assertion should be 
> > > sufficient.
> > I can actually answer the first question myself. According to Intel's 
> > manual, it is impossible to disable x87 bit. IIRC attempt to unset it on 
> > XCR0 will raise some fault.
> > 
> > The second question is basically whether under any circumstances can 
> > FreeBSD kernel disable SSE on XCR0 (this code is only used on systems 
> > supporting XSAVE).
> I guess my point is that having these bits unset would indicate a kernel 
> issue or bug, or maybe hardware issue, but never indicate an error or invalid 
> operation in lldb itself.
> 
> Either way I think there is no practical impact, it's not actually going to 
> happen.
If we assert on this code we more trigger software bug in lldb rather than a 
hardware specifics.


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

https://reviews.llvm.org/D89193

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D89182: [lldb] [Process/FreeBSDRemote] Kill process via PT_KILL

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D89182#2324267 , @labath wrote:

> FWIW, PTHREAD_KILL is strongly discouraged on linux. But if the situation is 
> different on freebsd, then fine.

I think you mean PTRACE_KILL. PT_KILL is fine for BSDs and in case when it is 
not, it should be fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89182

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D89193: [lldb] [Process/FreeBSDRemote] Support YMM reg via PT_*XSTATE

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a reviewer: bsdjhb.
krytarowski added a subscriber: bsdjhb.
krytarowski added a comment.

+ @bsdjhb John, could you have a look?


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

https://reviews.llvm.org/D89193

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D88796: [lldb] Initial version of FreeBSD remote process plugin

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp:269-375
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM));
+  LLDB_LOG(log, "target {0}", target);
+
+  // If we're a remote host, use standard behavior from parent class.
+  if (!IsHost()) {
+printf("pare\n");
+return PlatformPOSIX::DebugProcess(launch_info, debugger, target, error);

labath wrote:
> I think it's time for a switcheroo -- move this code into 
> PlatformPOSIX::DebugProcess, and move that function into PlatformDarwin.
Please defer any refactoring to a separate, follow up commit. Here is a lot of 
room for code deduplication, at least for ELF platforms.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp:491
+
+  len = len * 4 / 3;
+  std::unique_ptr buf =

Assuming that the process is always stopped, we don't need to increment `len`.


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

https://reviews.llvm.org/D88796

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D88796: [lldb] Initial version of FreeBSD remote process plugin

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp:22
+
+//#include "Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h"
+#include "Plugins/Process/POSIX/ProcessPOSIXLog.h"

mgorny wrote:
> krytarowski wrote:
> > Why this line?
> Because otherwise it fails to compile? ;-)
OK, you have removed the unused include.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp:487
+  int count, i;
+  vm = kinfo_getvmmap(GetID(), );
+  if (vm == NULL) {

mgorny wrote:
> krytarowski wrote:
> > Maybe here and in other places: `struct kinfo_vmentry *vm =  
> > kinfo_getvmmap(GetID(), );`
> You've mentioned previously that you'd prefer for us not to use `-lutil`. 
> Should I inline the sysctls instead?
Yes, please.


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

https://reviews.llvm.org/D88796

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D88796: [lldb] Initial version of FreeBSD remote process plugin

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp:22
+
+//#include "Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h"
+#include "Plugins/Process/POSIX/ProcessPOSIXLog.h"

Why this line?



Comment at: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp:487
+  int count, i;
+  vm = kinfo_getvmmap(GetID(), );
+  if (vm == NULL) {

Maybe here and in other places: `struct kinfo_vmentry *vm =  
kinfo_getvmmap(GetID(), );`


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

https://reviews.llvm.org/D88796

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D88681: [lldb] [Process/NetBSD] Fix reading FIP/FDP registers

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp:954
   case lldb_foseg_x86_64:
-m_fpr.fxstate.fx_dp.fa_64 = reg_value.GetAsUInt64();
+m_fpr.fxstate.fx_dp.fa_32.fa_seg = reg_value.GetAsUInt64();
 break;

GetAsUInt32 above and GetAsUInt64 here?


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

https://reviews.llvm.org/D88681

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D88682: [lldb] [Process/NetBSD] Fix crash on unsupported i386 regs

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp:398
 assert(false && "Unhandled i386 register.");
-return 0;
+return -1;
   }

Use `llvm_unreachable` ? Same in other places where we add similar `assert()`.



Comment at: 
lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp:414
+  return -1; // MPXR
+if (reg_num >= k_first_mpxr_i386 && reg_num <= k_last_mpxc_i386)
+  return -1; // MPXC

k_first_mpxr_i386 -> k_first_mpxc_i386 ?



Comment at: 
lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp:428
   return -1; // MPXR
-else if (reg_num <= k_last_mpxc_x86_64)
+if (reg_num >= k_first_mpxr_x86_64 && reg_num <= k_last_mpxc_x86_64)
   return -1; // MPXC

k_first_mpxr_x86_64 -> k_first_mpxc_x86_64?


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

https://reviews.llvm.org/D88682

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D85820: Use find_library for ncurses

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

Please note that ncurses is not the only supported curses library. NetBSD uses 
its original BSD curses for LLVM projects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85820

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D73067: [lldb/CMake] Auto-generate the Initialize and Terminate calls for plugin

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

Autogeneration of the code puts extra burden on us for tracking what is defined 
where and for repackaging LLDB with custom build rules (we need plain Makefile 
in the distribution). I presume the same problem is for gn users, for FreeBSD 
etc.

The NetBSD buildbot is now broken:

  FAILED: lib/liblldb.so.11.0.0git 
  : && /home/motus/netbsd8/netbsd8/wrappers/clang++ -fPIC -fPIC 
-fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default 
-Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor 
-Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections 
-Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing 
-Wno-deprecated-register -Wno-vla-extension -O3 -DNDEBUG  -Wl,-z,defs 
-Wl,--color-diagnostics   -Wl,-O3 -Wl,--gc-sections  
-Wl,--version-script,"/home/motus/netbsd8/netbsd8/build-stage2/tools/lldb/source/API/liblldb.exports"
 -shared -Wl,-soname,liblldb.so.11git -o lib/liblldb.so.11.0.0git 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBAddress.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBAttachInfo.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBBlock.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBBreakpoint.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBBreakpointLocation.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBBreakpointName.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBBreakpointOptionCommon.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBBroadcaster.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBCommandInterpreter.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBCommandReturnObject.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBCommunication.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBCompileUnit.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBData.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBDebugger.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBDeclaration.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBError.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBEvent.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBExecutionContext.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBExpressionOptions.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBFileSpec.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBFile.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBFileSpecList.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBFrame.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBFunction.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBHostOS.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBInstruction.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBInstructionList.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBLanguageRuntime.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBLaunchInfo.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBLineEntry.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBListener.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBMemoryRegionInfo.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBMemoryRegionInfoList.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBModule.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBModuleSpec.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBPlatform.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBProcess.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBProcessInfo.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBQueue.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBQueueItem.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBReproducer.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBSection.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBSourceManager.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBStream.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBStringList.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBStructuredData.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBSymbol.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBSymbolContext.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBSymbolContextList.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBTarget.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBThread.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBThreadCollection.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBThreadPlan.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBTrace.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBTraceOptions.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBType.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBTypeCategory.cpp.o 
tools/lldb/source/API/CMakeFiles/liblldb.dir/SBTypeEnumMember.cpp.o 

[Lldb-commits] [PATCH] D73802: [lldb] Introduce i386 support in NetBSD Process plugin

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp:62
+  GPR gpr;
+  FPR_i386 i387;
+  uint32_t u_debugreg[8]; // Debug registers (DR0 - DR7).

krytarowski wrote:
> mgorny wrote:
> > krytarowski wrote:
> > > mgorny wrote:
> > > > krytarowski wrote:
> > > > > Please add `uint32_t tlsbase;`.
> > > > To what purpose? It's not being used anywhere.
> > > Consistency with amd64 AND we will want to make use of it in future and 
> > > present as a 'tlsbase' register or anything.
> > The only reason I didn't remove it from amd64 is because I don't believe 
> > it's worth the effort. I'm against proactively adding undocumented 
> > features, especially given how much time I've spent trying to figure out 
> > what those structures were doing. We can add it when we start using it.
> This was discussed when we were adding amd64 fields and it was decided to add 
> tlsbase. Please add it here too.
Also there is now ptrace(2) API ready (`PT_LWPSTATUS`) to read tlsbase and I 
wish I saw its support in LLDB sooner than later.


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

https://reviews.llvm.org/D73802



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D73802: [lldb] Introduce i386 support in NetBSD Process plugin

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp:62
+  GPR gpr;
+  FPR_i386 i387;
+  uint32_t u_debugreg[8]; // Debug registers (DR0 - DR7).

mgorny wrote:
> krytarowski wrote:
> > mgorny wrote:
> > > krytarowski wrote:
> > > > Please add `uint32_t tlsbase;`.
> > > To what purpose? It's not being used anywhere.
> > Consistency with amd64 AND we will want to make use of it in future and 
> > present as a 'tlsbase' register or anything.
> The only reason I didn't remove it from amd64 is because I don't believe it's 
> worth the effort. I'm against proactively adding undocumented features, 
> especially given how much time I've spent trying to figure out what those 
> structures were doing. We can add it when we start using it.
This was discussed when we were adding amd64 fields and it was decided to add 
tlsbase. Please add it here too.


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

https://reviews.llvm.org/D73802



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D73802: [lldb] Introduce i386 support in NetBSD Process plugin

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp:62
+  GPR gpr;
+  FPR_i386 i387;
+  uint32_t u_debugreg[8]; // Debug registers (DR0 - DR7).

mgorny wrote:
> krytarowski wrote:
> > Please add `uint32_t tlsbase;`.
> To what purpose? It's not being used anywhere.
Consistency with amd64 AND we will want to make use of it in future and present 
as a 'tlsbase' register or anything.


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

https://reviews.llvm.org/D73802



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D73802: [lldb] Introduce i386 support in NetBSD Process plugin

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp:62
+  GPR gpr;
+  FPR_i386 i387;
+  uint32_t u_debugreg[8]; // Debug registers (DR0 - DR7).

Please add `uint32_t tlsbase;`.


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

https://reviews.llvm.org/D73802



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70025: [lldb] [Process/NetBSD] Fix handling concurrent watchpoint events

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D70025#1758800 , @labath wrote:

> LGTM, since this seems to be the best we can do given the current netbsd 
> behavior.
>
> However, I'd like to repeat what I said on the IRC, that I consider this 
> behavior of netbsd to be unreasonable.


Thanks for the feedback. Evaluating the options, we will keep using our current 
model. There are no plans for any modifications in this domain.

The end user of a debugger is just expected to see no difference.


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

https://reviews.llvm.org/D70025



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70022: [lldb] [Process/NetBSD] Improve threading support

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_vContThreads.py:68-72
+"read packet: $vCont;C{0:x}:{1:x};C{0:x}:{2:x}#00".format(
+lldbutil.get_signal_number('SIGUSR1'),
+threads[0], threads[1]),
+{"direction": "send", "regex": r"^\$W00#b7$"},
+], True)

mgorny wrote:
> labath wrote:
> > It might be nice to actually verify how many signals did the process 
> > receive here. I guess you could do that by just checking the output of the 
> > signal handler.
> I've tried but it claimed to timeout before receiving any output. I suspect 
> the interrupt terminates `sleep()` in thread, and they exit before the signal 
> handler manages to get run. Or maybe I was doing something wrong.
There is a bug in the kernel with `sleep()` that it does not return EINTR when 
interrupted. It shall be fixes as it is imho fatal.


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

https://reviews.llvm.org/D70022



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70363: [lldb] [Process/NetBSD] Implement thread name getting

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp:122
+  if (::sysctl(mib, 5, nullptr, , nullptr, 0) == -1 || size == 0) {
+LLDB_LOG(log, "sysctl() for LWP info size failed: {0}", strerror(errno));
+return "";

labath wrote:
> Thread-safety technically doesn't matter in this part of the code, but I 
> think it would be good practice to use `llvm::StrError` 
> (`llvm/Support/Errno.h`) anyway. 
`strerror` is thread safe on NetBSD, but probably better to pick llvm 
interfaces.


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

https://reviews.llvm.org/D70363



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70363: [lldb] [Process/NetBSD] Implement thread name getting

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp:22
 
+#include 
+

Please include before this header ``. If it works, that header is 
likely pulled from some indirect location.


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

https://reviews.llvm.org/D70363



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70335: [lldb] [test] Enable lldb-server tests on NetBSD, and set XFAILs

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py:33
 @skipIfWindows # the test is not updated for Windows.
+@skipIfNetBSD # build failure due to pthread_setname_np prototype
 @llgs_test

Can we fix the prototype in use on NetBSD?


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

https://reviews.llvm.org/D70335



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70025: [lldb] [Process/NetBSD] Fix handling concurrent watchpoint events

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:334
 
 thread->SetStoppedByTrace();
 SetState(StateType::eStateStopped, true);

I presume that in this code path we land into a scenario that:

1. Trap on a different LWP
2. User sets new watchpoints
3. We land here with a SIGTRAP on old watchpoint that was wiped out.

If so, we shall ignore this report, bail out and resume execution with 
`PT_CONTINUE`.

I think that this path could be some remnant from Linux shared trap reasons.


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

https://reviews.llvm.org/D70025



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70023: [lldb] [Process/NetBSD] Copy watchpoints to newly-created threads

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

How does it deal with `security.models.extensions.user_set_dbregs`? If there is 
a handled error than it's fine.


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

https://reviews.llvm.org/D70023



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70022: [lldb] [Process/NetBSD] Improve threading support

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:241
   case TRAP_BRKPT:
-for (const auto  : m_threads) {
-  static_cast(*thread).SetStoppedByBreakpoint();
-  FixupBreakpointPCAsNeeded(static_cast(*thread));
+if (thread) {
+  thread->SetStoppedByBreakpoint();

This shall be always true unless there is a kernel issue.
But this check is fine, I would just add a comment.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:248
   case TRAP_TRACE:
-for (const auto  : m_threads)
-  static_cast(*thread).SetStoppedByTrace();
+if (thread)
+  thread->SetStoppedByTrace();

Same here. Isn't there a fixup for PC?

It's not needed in x86 and can be delayed into future.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:268
+ptrace_state_t pst;
+Status error = PtraceWrapper(PT_GET_PROCESS_STATE, GetID(), ,
+ sizeof(pst));

Maybe `GetID()` -> `pid`? Same later.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:790
+void NativeProcessNetBSD::RemoveThread(lldb::tid_t thread_id) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
+  LLDB_LOG(log, "pid {0} removing thread with tid {1}", GetID(), thread_id);

I would add an assert `thread_id > 0`.


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

https://reviews.llvm.org/D70022



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69320: [lldb] [Python] Do not attempt to flush() a read-only fd

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

Same here, I cannot commit.


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

https://reviews.llvm.org/D69320



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68856: convert SBDebugger::***FileHandle() wrappers to native files.

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.



In D68856#1718096 , @lawrence_danna 
wrote:

> now I'm stuck on  this trying to install cmake.
>
>   pkg_add: no pkg found for 'libunistring>=0.9.4', sorry.
>   pkg_add: Can't install dependency libunistring>=0.9.4
>   pkg_add: 1 package addition failed
>   *** Error code 1
>


I recommend to just use pkgsrc from source code. As of today packages are 
imperfect and always there is a set of things that are not prebuilt.

If there are mixed packages for binary packages vs source ones, it's reasonable 
to `rm -rf /usr/pkg /var/db/pkg` before starting.

Steps:

1. Download pkgsrc from tarball.

http://cdn.netbsd.org/pub/pkgsrc/pkgsrc-2019Q3/pkgsrc-2019Q3.tar.bz2

2. Unpack, e.g. into /usr/pkgsrc
3. cd /usr/pkgsrc/devel/git-base && make install
4. cd /usr/pkgsrc/security/mozilla-rootcerts && make install

And follow the MESSAGE commands to finish the installation.

5. Follow this process for all the dependencies (if I remember correctly: 
cmake, swig3, ninja-build).

In D68856#1718075 , @lawrence_danna 
wrote:

> In D68856#1717767 , @krytarowski 
> wrote:
>
> > In D68856#1717683 , 
> > @lawrence_danna wrote:
> >
> > > @mgorny
> > >
> > > I can't get anything to work.   I've tried running  a local VM with 
> > > virtualbox but it's networking driver crashes my kernel.   I've tried a 
> > > local VM with VMware but it won't boot netbsd.   I've tried AWS but they 
> > > only have netbsd 7, which is too old.   I've tried google cloud, but 
> > > their image creator script only works for netbsd 9, and python won't 
> > > build because x11 isn't installed.   I've tried installing the pkgsrc 
> > > binaries from netbsd 8 onto netbsd 9, but that doesn't work either.   I'm 
> > > completely at a loss.  I can't figure out how to make a netbsd VM that 
> > > can actually build LLDB.
> > >
> > > Do you have  a machine image on AWS or google cloud or even a VMDK or 
> > > something that I could use?
> >
> >
> > For the python part, you need to either install all basesystem sets 
> > (including x11) or set `X11_TYPE=modular` in `/etc/mk.conf`.
>
>
> I tried `X11_TYPE`, it does nothing.  I'm a bit confused there because I 
> did not have a file called `/etc/mk.conf`.
>
> Is there a way to install those missing basesystem sets once the machine is 
> up?   Or do I have to go back and reinstall from scratch?


The typical approach is to fetch set file and untarball it into /, all other 
approaches are at most wrappers to this. The list of installed sets is stored 
in `/etc/mtree/`.

Please remember to use tar options to preserve file properties.

http://pub.nethence.com/bsd/wo.sysinst notes: `tar xzphfe $set.tgz -C /`.

Another option is to disable x11 option either for all packages or for python. 
In my opinion it's easier to just set the `modular` in the `X11_TYPE` variable.

> PS: oh,  //of course// I have to set the environment variable 
> `MAKECONF=/etc/mk.conf`

`/etc/mk.conf` is the default file for MAKECONF, no need to set this variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68856



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68856: convert SBDebugger::***FileHandle() wrappers to native files.

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

For the git part you will need to install mozilla-rootcerts and follow 
post-install instructions, as otherwise `https://` won't work nicely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68856



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68856: convert SBDebugger::***FileHandle() wrappers to native files.

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D68856#1717683 , @lawrence_danna 
wrote:

> @mgorny
>
> I can't get anything to work.   I've tried running  a local VM with 
> virtualbox but it's networking driver crashes my kernel.   I've tried a local 
> VM with VMware but it won't boot netbsd.   I've tried AWS but they only have 
> netbsd 7, which is too old.   I've tried google cloud, but their image 
> creator script only works for netbsd 9, and python won't build because x11 
> isn't installed.   I've tried installing the pkgsrc binaries from netbsd 8 
> onto netbsd 9, but that doesn't work either.   I'm completely at a loss.  I 
> can't figure out how to make a netbsd VM that can actually build LLDB.
>
> Do you have  a machine image on AWS or google cloud or even a VMDK or 
> something that I could use?


For the python part, you need to either install all basesystem sets (including 
x11) or set `X11_TYPE=modular` in `/etc/mk.conf`.

All my NetBSD setups are either native or qemu+haxm. I cannot test virtualbox 
or vmware as of now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68856



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D66566: [lldb] Replace std::once_flag with llvm::once_flag.

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

This breaks NetBSD as it shall be paired with `llvm::call_once`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66566



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D65555: [lldb] [Process/NetBSD] Enable reporting of new and exited threads [WIP]

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:104
+  // TODO: PTRACE_FORK | PTRACE_VFORK | PTRACE_POSIX_SPAWN?
+  events.pe_set_event = PTRACE_LWP_CREATE | PTRACE_LWP_EXIT;
+  status = PtraceWrapper(PT_SET_EVENT_MASK, pid, , sizeof(events));

I would go for more generic `PT_GET_EVENT_MASK` and then `|=`. In theory we 
could add more bits and make them enabled by default.


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

https://reviews.llvm.org/D6



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Implement per-thread execution control

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:394
+siginfo.psi_siginfo.si_signo = signal;
+siginfo.psi_siginfo.si_code = SI_NOINFO;
+if (signaled_threads == m_threads.size()) // signal aimed at all threads

For extra completeness, all basic signals (without specified siginfo) from 
debugger (that weren't intercepted by a debugger as they were routed into 
debuggee) that are emitted should be of type `SI_USER` with filled `si_pid` and 
`si_uid` of the debugger.

We thought have the power to set it to whatever value we want, but LLDB 
probably doesn't allow to set defailed siginfo.


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

https://reviews.llvm.org/D64647



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Implement per-thread execution control

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D64647#1584340 , @mgorny wrote:

> In D64647#1583429 , @krytarowski 
> wrote:
>
> > Something that we do not cover here is that once a tracee reports a signal 
> > (like someone poked it with SIGUSR1) and we want to pass it over to the 
> > tracee, we will reset siginfo.
> >
> > This scenario should be covered by a test and we should handle it properly..
> >
> > The solution in NetBSD for passing over signals without changing siginfo, 
> > is to not calling PT_SET_SIGINFO as the default one will be kept by the 
> > kernel. Optionally pick old siginfo with PT_GET_SIGINFO and optionally 
> > change destination lwp.
>
>
> How is 'properly'? Should we reject resuming with a signal when there's 
> already another signal pending?


We need to pass the same signal (with unchanged siginfo) to the tracee.

The easiest way is to not changing it, however as we can emit a signal over to 
tracee specifying LWP. we can go for the sequence of PT_GET_SIGINFO, change 
lwp, PT_SET_SIGINFO, PT_CONTINUE with a signal.


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

https://reviews.llvm.org/D64647



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Implement per-thread execution control

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

Something that we do not cover here is that once a tracee reports a signal 
(like someone poked it with SIGUSR1) and we want to pass it over to the tracee, 
we will reset siginfo.

This scenario should be covered by a test and we should handle it properly..

The solution in NetBSD for passing over signals without changing siginfo, is to 
not calling PT_SET_SIGINFO as the default one will be kept by the kernel.


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

https://reviews.llvm.org/D64647



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Implement per-thread execution control

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:354
+  if (signal != LLDB_INVALID_SIGNAL_NUMBER && signal != action->signal)
+return Status("NetBSD does not support passing multiple signals 
simultaneously");
 

mgorny wrote:
> labath wrote:
> > Is this "passing multiple signals simultaneously", or "passing multiple 
> > *distinct* signals simultaneously". (E.g,. thread 1 gets a SIGALRM, thread 
> > 2 gets SIGIO, etc.).
> The former. Basically there's one siginfo slot, so you can either put a 
> signal for whole process, or for one LWP.
Once we emit a single signal to all threads, it's still technically a single 
signal that hits the process.


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

https://reviews.llvm.org/D64647



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Implement per-thread execution control

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

I think it looks OK. there are some nits that could be optimized in future or 
handled additionally.. but for now it should be fine.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D64647



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63792: [lldb] [Process/NetBSD] Use global enable bits for watchpoints

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

I would follow the same kernel behavior here as Linux, but that can be done 
independently.


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

https://reviews.llvm.org/D63792



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63791: [lldb] [Process/NetBSD] Fix segfault when handling watchpoint

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:250
+if (!thread) {
+  LLDB_LOG(log,
+   "thread not found in m_threads, pid = {0}, LWP = {1}",

We should always have a fallback and whenever we register an event for an 
unknown thread, we shall create it in place. There are potential races in the 
kernel that might lead to this order of reporting events. This is less likely 
for DB Registers, but still we shall be prepared for it just in case.

I'm fine to leave this for later once we will handle debuggees with multiple 
threads.


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

https://reviews.llvm.org/D63791



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63545: [lldb] [Process/NetBSD] Support reading YMM registers via PT_*XSTATE [DO NOT MERGE]

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp:239
+
+  if (byte_order == lldb::eByteOrderBig) {
+::memcpy(m_xstate_x86_64.xs_fxsave.fx_xmm[reg_index].xmm_bytes,

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > Why would we ever want to do this?
> > I was asking myself the exact same thing! But the code is present both on 
> > Linux and FreeBSD, so I presumed there is some fancy use case I have no 
> > clue about.
> Yeah, I don't think that's a safe assumption around here. :)
Right now x86 supports only LE on NetBSD.


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

https://reviews.llvm.org/D63545



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63380: [lldb] [test] Skip watchpoint tests on NetBSD if userdbregs is disabled

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.
Herald added a subscriber: ormris.

This check should contain additional check for uid==root. If we are root we can 
read and write to DB registers.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63380



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

I would leave the NetBSD version as it is in this patch and let us to fix/test 
it later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62502: Implement xfer:libraries-svr4:read packet

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

https://nxr.netbsd.org/xref/src/include/link_elf.h#9

In general this code should be close to functional on NetBSD (if not already 
compatible).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62499: Create a generic handler for Xfer packets

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D62499#1520654 , @labath wrote:

> In D62499#1520610 , @krytarowski 
> wrote:
>
> > > One time just before it's loaded (so lldb can check which modules are 
> > > loaded) and another right after it's loaded (so lldb can check again 
> > > which ones are loaded and calculate the difference).
> >
> > There is on NetBSD and on a selection of other OSs: `_rtld_debug_state` 
> > integrated as a part of ELF dynamic loader.
> >
> > Is there something like that on Android that could be reused?
>
>
> Yes, there is, and it's being used now. The question here is what do we do 
> *after* we hit the dynamic loader breakpoint...


First I will need to make research of it locally as it was probably not used in 
some time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62499



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D62499: Create a generic handler for Xfer packets

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

> One time just before it's loaded (so lldb can check which modules are loaded) 
> and another right after it's loaded (so lldb can check again which ones are 
> loaded and calculate the difference).

There is on NetBSD and on a selection of other OSs: `_rtld_debug_state` 
integrated as a part of ELF dynamic loader.

Is there something like that on Android that could be reused?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62499



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D61310: [lldb] [Process/NetBSD] Fix handling EOF in PT_IO when reading memory

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski requested changes to this revision.
krytarowski added a comment.
This revision now requires changes to proceed.

I think there is a bug thought in the code.

1. bytes_read should return the bytes transferred from the function, it returns 
the value from the previous iteration only
2. `bytes_read != 0` should be replaced with `io.piod_len != 0`


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

https://reviews.llvm.org/D61310



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60034: [lldb] [Process/elf-core] Support aarch64 NetBSD core dumps

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/TestNetBSDCore.py:236
+
+@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64(self):

I would move it above X86.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60034



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D60034: [lldb] [Process/elf-core] Support aarch64 NetBSD core dumps

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp:657
   } break;
+  case llvm::Triple::aarch64: {
+// Assume order PT_GETREGS, PT_GETFPREGS

I would move it above x86_64 for sorting reasons.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60034



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59427: [lldb] [API] Split SBRegistry into smaller files

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

It looks good to me, but maybe @JDevlieghere has a better idea how to 
optimize it.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59427



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59177: [lldb] [test] Make 2lwp_process_SIGSEGV test more portable

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.c:9
   char F = 'b';
-  kill(getpid(), SIGSEGV); // Frame bar
+  while (1); // Frame bar
 }

Just style, but I would use `while (1) continue;`



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/netbsd-core/2lwp_process_SIGSEGV.c:31
   _lwp_create(, 0, );
+  sleep(1);
+  kill(getpid(), SIGSEGV);

It's a style but I would replace `sleep(1)` with a global volatile int that is 
switched in `bar` and here we could wait with: `while (!initialized) continue;`


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59177



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D32149: [lldb] [Process] Add proper support for NetBSD core files with threads

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

Looks good to me.


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

https://reviews.llvm.org/D32149



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58527: [lldb] [test] Mark failing tests XFAIL on NetBSD

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.

Feel free to commit this and next without review.


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

https://reviews.llvm.org/D58527



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42870: [lldb] [ObjectFile/ELF] Correct recognition of NetBSD images

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/lit/Modules/ELF/netbsd-core.test:3
+
+# RUN: lldb-test object-file %S/Inputs/netbsd.core | FileCheck %s
+# CHECK: Architecture: x86_64-unknown-netbsd

krytarowski wrote:
> I propose to keep it as netbsd$VERSION-$ARCH.core
> 
> We will want multiple core(5) files and possible with variations (FPU layout).
> We will want multiple core(5) files and possible with variations (FPU layout).

Actually such variations will be applicable for https://reviews.llvm.org/D32149 
Here we just want `$ARCH`-`$VERSION`.


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

https://reviews.llvm.org/D42870



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42870: [lldb] [ObjectFile/ELF] Correct recognition of NetBSD images

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/lit/Modules/ELF/netbsd-core.test:3
+
+# RUN: lldb-test object-file %S/Inputs/netbsd.core | FileCheck %s
+# CHECK: Architecture: x86_64-unknown-netbsd

I propose to keep it as netbsd$VERSION-$ARCH.core

We will want multiple core(5) files and possible with variations (FPU layout).


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

https://reviews.llvm.org/D42870



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42870: [lldb] [ObjectFile/ELF] Correct recognition of NetBSD images

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1359
+  // p = patchlevel
+  const uint32_t version_major = version_info / 1;
+  const uint32_t version_minor = (version_info % 1) / 100;

mgorny wrote:
> Would it be crazy to rewrite this into `std::div` (i.e. to get both quotient 
> and remainder in one call)?
If it will be more readable OK.


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

https://reviews.llvm.org/D42870



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58230: [lldb] [MainLoop] Add kevent() EINTR handling

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

@mgorny let's go through tech-kern@ and later checking FreeBSD/Darwin/OpenBSD. 
I think it's worth to clarify this in the documentation.

As an intermediate version we can land this patch.


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

https://reviews.llvm.org/D58230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58230: [lldb] [MainLoop] Add kevent() EINTR handling

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D58230#1398529 , @mgorny wrote:

> In D58230#1398020 , @krytarowski 
> wrote:
>
> > For EINTR we shall use `llvm::sys::RetryAfterSignal`
>
>
> `kevent()` man page indicates:
>
> > All changes contained in the changelist are applied before any pending 
> > events are read from the queue.
>
> Also:
>
> > [EINTR]A signal was delivered before the timeout expired and 
> > before any events were placed on the kqueue for return.
>
> So while it's not stated explicitly, I think `in_events` is always consumed, 
> even if EINTR is returned. In which case, `llvm::sys::RetryAfterSignal` would 
> be wrong whenever `in_events` is not empty.


Please bring it to tech-kern@ to clear this and improve documentation. I think 
that we need to use `llvm::sys::RetryAfterSignal`.


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

https://reviews.llvm.org/D58230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58227: [lldb] [MainLoop] Remove redundant termination clause (NFCI)

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

It looks good to me.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58227



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42206: If kevent() is interrupted by signal (or is being debugged) and we get EINTR, retry

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.
Herald added a project: LLVM.

'attaching a debugger produces an observable side-effect (EINTR) in the 
debugged process is considered a bug by the linux kernel folks'

There are differences between Linux and BSD.

On BSD we try hard to make return of such syscall peacefully valid input (like 
returning already received bytes int read(2)), while on Linux there is hard 
interruption of operation.

Also we can introduce EINTR easily on BSD in arbitrary moments with ctrl-T 
(SIGINFO) that is used in many programs to pass information and the unprepared 
ones can misbehave due to EINTR.

So even if this could be a bug to cause EINTR in some scenarios on BSD under a 
debugger, it's a program fault to not handle it.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D42206



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58230: [lldb] [MainLoop] Add kevent() EINTR handling

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

For EINTR we shall use `llvm::sys::RetryAfterSignal`


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D58131: [lldb] [unittest] Avoid mixing '127.0.0.1' and 'localhost'

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.

Short term this looks fine.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58131



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Kamil Rytarowski via Phabricator via lldb-commits]

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 to differentiate Windows (out of UNIX [and probably next to Fuchsia 
today, but it's not supported in LLDB].


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

https://reviews.llvm.org/D57959



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Kamil Rytarowski via Phabricator via lldb-commits]

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/new/

https://reviews.llvm.org/D57959



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Kamil Rytarowski via Phabricator via lldb-commits]

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 as of now.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57912



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Kamil Rytarowski via Phabricator via lldb-commits]

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  : loop.m_signals)
 sigdelset(, sig.first);
 #endif

Shouldn't we initialize sigmask always before adding/deleting particular 
signals?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57959



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Host/common/MainLoop.cpp:149
   for (const auto  : loop.m_signals)
 sigdelset(, sig.first);
 #endif

krytarowski wrote:
> Shouldn't we initialize sigmask always before adding/deleting particular 
> signals?
Ah, I can see it now. It's passed to `pthread_sigmask(3)` so it's fine.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D57959



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for building lldb-server on Windows

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: include/lldb/Host/windows/PosixApi.h:78
 
+#define WNOHANG 1
+#define WUNTRACED 2

I think that these symbols should not be leaked here in the first place.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56233



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55706: ELF: more section creation cleanup

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski edited reviewers, added: joerg; removed: krytarowski.
krytarowski added a comment.

I recommend joerg@ as a reviewer for ELF specifics.


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

https://reviews.llvm.org/D55706



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

On NetBSD one has to check PaX MPROTECT property of a traced process.

Something like:

  bool IsMPROTECT(pid_t pid) {
  #if defined(__NetBSD__)
int mib[3];
int paxflags;
size_t len = sizeof(paxflags);
  
mib[0] = CTL_PROC;
mib[1] = pid;
mib[2] = PROC_PID_PAXFLAGS;
  
if (sysctl(mib, 3, , , NULL, 0) != 0)
  err(EXIT_FAILURE, "sysctl"); /* or return true */
  
return !!(paxflags & CTL_PROC_PAXFLAGS_MPROTECT);
  #else
return false;
  #endif
  }

If IsMPROTECT is true, then we must use hardware assisted/emulated breakpoints.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54221



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54135: Add convenience method in FileSystem to check if a path/filespec is a directory.

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

Why? We already put a lot of effort into reusing code from LLVM.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54135



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52941: NativeProcessProtocol: Simplify breakpoint setting code

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

This looks fine to me.


https://reviews.llvm.org/D52941



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52941: NativeProcessProtocol: Simplify breakpoint setting code

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski accepted this revision.
krytarowski added a comment.
This revision is now accepted and ready to land.

The NetBSD part looks fine. I will be out of the office soon as I will visit 
California for GSoC Mentor Summit and MeetBSDCa (until October 23rd).


https://reviews.llvm.org/D52941



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52532: Pull GetSoftwareBreakpointPCOffset into base class

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In https://reviews.llvm.org/D52532#1250404, @labath wrote:

> In https://reviews.llvm.org/D52532#1246173, @krytarowski wrote:
>
> > I was wondering whether we want to normalize this inside the kernel and 
> > always advance the Program Counter.. but it's easier to manage it in 
> > userland.
>
>
> I am generally in favour of keeping the kernel simple, particularly when 
> ptrace is concerned. However, there is one difference in behaviour that 
> annoys me. Right now, if an application itself inserts a trap into it's 
> source code, on intel it will be easy to resume it from debugger simply by 
> continuing. OTOH, on arm, the user will have to manually update the PC first. 
> So I am not sure which is better here...


Right now I defer this into future. We still need to fixup PC in kernel 
debuggers... on the other hand in order to advance the register manually on an 
arbitrary ISA we might need to guess instruction size. It's better to guess it 
in userland.


Repository:
  rL LLVM

https://reviews.llvm.org/D52532



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52532: Pull GetSoftwareBreakpointPCOffset into base class

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

I was wondering whether we want to normalize this inside the kernel and always 
advance the Program Counter.. but it's easier to manage it in userland.


https://reviews.llvm.org/D52532



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49110: [testsuite] Implement a category to skip libstdcxx tests

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

Through pkgsrc a user can install LLDB with either style of distribution, 
including GCC with libstdc++ one (I'm using this myself also for the 
development of LLVM related code).

BTW. NetBSD uses GPLv3 GNU toolchain.


Repository:
  rL LLVM

https://reviews.llvm.org/D49110



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49110: [testsuite] Implement a category to skip libstdcxx tests

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

Probably the former with ship GDB at least for long time, the latter will 
switch to LLDB.


Repository:
  rL LLVM

https://reviews.llvm.org/D49110



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49110: [testsuite] Implement a category to skip libstdcxx tests

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

NetBSD ships with GCC style and Clang style userland. The former one ships with 
GNU libstdc++, the latter with libc++.


Repository:
  rL LLVM

https://reviews.llvm.org/D49110



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits