[Lldb-commits] [PATCH] D32149: Correct handling NetBSD core(5) files with threads

[Zachary Turner via lldb-commits]

Note that getAsInteger returns false on success, so be careful!
On Thu, Apr 20, 2017 at 6:09 AM Pavel Labath via Phabricator <
reviews at reviews.llvm.org> wrote:

> labath added a comment.
>
> In https://reviews.llvm.org/D32149#731920, @krytarowski wrote:
>
> > In https://reviews.llvm.org/D32149#731887, @labath wrote:
> >
> > > A test would infinitely times more valuable then a demo script. What
> is the tiniest core file you can produce on NetBSD? (on linux we've gotten
> them down to about 20K) Then we could check that in and write a test for
> it...
> >
> >
> > This is something I wanted to bring to the dev mailing list.
> >
> > I wanted to prepare at least three tests, if possible four:
> >
> > - one thread (if possible two variations: signal to one particular
> thread + signal to all threads)
> > - multiple threads (signal to one particular thread + signal to all
> threads)
> >
> >   And this in combination of all supported targets (x86_64, i386, etc).
> >
> >   Emitting SIGABRT for such program gives core of size 97kilobytes:
> >
> >   ``` int main(){for(;;);} ```
> >
> >   I will write assembly programs for the above cases, without libc.
>
>
> Cool, I am looking forward to the results.
>
>
>
> ================
> Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:795
> +
> +    if ((note.n_name == "NetBSD-CORE") &&
> +        (note.n_type == NETBSD::NT_PROCINFO)) {
> ----------------
> How about `StringRef name = note.n_name;`
>
>
> ================
> Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:803
> +      m_auxv = DataExtractor(note_data);
> +    } else if ((note.n_name.substr(0, 12) == "NetBSD-CORE@")) {
> +      switch (arch.GetMachine()) {
> ----------------
> Then this can be
> ```
> else if (name.consume_front("NetBSD-CORE@")) {
>   ...
>   if (name.getAsInteger(0, tid))
>     error...
> ```
>
>
> ================
> Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:848
> +  if (siglwp == 0) {
> +    std::for_each(
> +        m_thread_data.begin(), m_thread_data.end(),
> ----------------
> `for (auto &data: m_thread_data) data.signo = signo` seems shorter, more
> understandable, and consistent with other usages in this file.
>
>
> ================
> Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:856
> +
> +    for (auto it = m_thread_data.begin(); it != m_thread_data.end();
> ++it) {
> +      if (it->tid == siglwp) {
> ----------------
> This could also be a range-based for.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D32149
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170420/06b3c803/attachment-0001.html>