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

[Kamil Rytarowski via lldb-commits]

Thanks! I'm working on this, I will test new code soon and submit to review.

On 20.04.2017 15:51, Zachary Turner wrote:
> 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 <mailto: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 --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170422/4d5edf69/attachment.sig>