[Lldb-commits] [PATCH] D25681: [PseudoTerminal] Fix PseudoTerminal MSVC release crash

[Zachary Turner via lldb-commits]

It will return true in this case. Should it be returning false to indicate
failure?
On Tue, Oct 18, 2016 at 11:26 PM Carlo Kok <ck at remobjects.com> wrote:

> fwiw this looks good to me, a much better fix than I had. but it'z
> zturner who objected to my approach.
>
> On 2016-10-17 17:51, Rudy Pons via lldb-commits wrote:
> > Ilod created this revision.
> > Ilod added reviewers: zturner, carlokok.
> > Ilod added a subscriber: lldb-commits.
> >
> > Since r278177, the Posix functions in
> PseudoTerminal::OpenFirstAvailableMaster on windows are now inlined
> LLVM_UNREACHABLE instead of return 0.
> > This causes __assume(0) to be inlined in OpenFirstAvailableMaster,
> allowing the optimizer to change code because this function should never be
> executed. In particular, on Visual 2015 Update 3 Win32 Release builds, the
> optimizer skips the if (error_str) test, causing a crash if error_str is
> nullptr.
> > The added #if !defined(LLDB_DISABLE_POSIX) restore the previous
> behaviour (which was doing nothing and returning true, as every function
> was returning 0, and prevent crashes.
> > The crash was 100% when launching a test x86 executable (built with
> clang, linked with lld-link) in lldb.
> > I don't know if there is another fix in progress (not calling the
> function on Win32?), but it seems to be called from several places, so it
> may be simpler to fix it in PseudoTerminal.
> >
> >
> > https://reviews.llvm.org/D25681
> >
> > Files:
> >   source/Utility/PseudoTerminal.cpp
> >
> >
> > Index: source/Utility/PseudoTerminal.cpp
> > ===================================================================
> > --- source/Utility/PseudoTerminal.cpp
> > +++ source/Utility/PseudoTerminal.cpp
> > @@ -88,6 +88,7 @@
> >    if (error_str)
> >      error_str[0] = '\0';
> >
> > +#if !defined(LLDB_DISABLE_POSIX)
> >    // Open the master side of a pseudo terminal
> >    m_master_fd = ::posix_openpt(oflag);
> >    if (m_master_fd < 0) {
> > @@ -111,6 +112,7 @@
> >      CloseMasterFileDescriptor();
> >      return false;
> >    }
> > +#endif
> >
> >    return true;
> >  }
> >
> >
> >
> >
> > _______________________________________________
> > lldb-commits mailing list
> > lldb-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> >
>
> --
> Carlo Kok
> RemObjects Software
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20161019/9ff0b7e9/attachment.html>