Better handling for realpath() failures in windows_make_so() on Cygwin

  From: Jon Turney <jon.turney@dronecode.org.uk>

Fix a memory leak which would occur in the case when the result of realpath() is
greater than or equal to SO_NAME_MAX_PATH_SIZE.

Distinguish between realpath() failing (returning NULL), and returning a path
longer than SO_NAME_MAX_PATH_SIZE

Warn rather than stopping with an error in those cases.

Original patch from Tim Chick.  Memory leak fix by Corinna Vinschen.
---
 gdb/windows-nat.c | 4 ++++
 1 file changed, 4 insertions(+)
  

On 21/03/2024 06:53, Orgad Shaneh wrote:
> From: Jon Turney <jon.turney@dronecode.org.uk>

Not sure where this is coming from, but this doesn't seem to be my 
latest version of this patch.

> Fix a memory leak which would occur in the case when the result of realpath() is
> greater than or equal to SO_NAME_MAX_PATH_SIZE.
> 
> Distinguish between realpath() failing (returning NULL), and returning a path
> longer than SO_NAME_MAX_PATH_SIZE
> 
> Warn rather than stopping with an error in those cases.

This line in the patch commentary, and the title, refers to the part of 
the patch submitted [1], which is already applied as commit 
a0e9b53238c3033222c53b1654da535c0743ab6e.

I separated that out because of the discussion starting at [2] ("Remove 
SO_NAME_PATH_SIZE instead"...)

[1] https://sourceware.org/pipermail/gdb-patches/2020-January/164695.html
[2] https://sourceware.org/pipermail/gdb-patches/2016-January/130435.html
  

On 2024-03-21 14:45, Jon Turney wrote:
> On 21/03/2024 06:53, Orgad Shaneh wrote:
>> From: Jon Turney <jon.turney@dronecode.org.uk>
> 
> Not sure where this is coming from, but this doesn't seem to be my latest version of this patch.
> 
>> Fix a memory leak which would occur in the case when the result of realpath() is
>> greater than or equal to SO_NAME_MAX_PATH_SIZE.
>>
>> Distinguish between realpath() failing (returning NULL), and returning a path
>> longer than SO_NAME_MAX_PATH_SIZE
>>
>> Warn rather than stopping with an error in those cases.
> 
> This line in the patch commentary, and the title, refers to the part of the patch submitted [1], which is already applied as commit a0e9b53238c3033222c53b1654da535c0743ab6e.
> 
> I separated that out because of the discussion starting at [2] ("Remove SO_NAME_PATH_SIZE instead"...)
> 
> [1] https://sourceware.org/pipermail/gdb-patches/2020-January/164695.html
> [2] https://sourceware.org/pipermail/gdb-patches/2016-January/130435.html
> 

Curiously, after upstreaming your _sigbe unwinder recently, I looked at upstreaming this patch (the version of the 2016 patch 
in downstream cygwin gdb), but then this same thought of removing the limit hit me.  (at least I'm consistent over the
years, eh.)

I then realized that Simon is working on a series that switches the solib path storage to a std::string, which will
let us easily not use SO_NAME_MAX_PATH_SIZE at all in the Windows code.  So I just dropped that patch from
my upstreaming queue...

Pedro Alves
  

On Thu, Mar 21, 2024 at 6:13 PM Pedro Alves <pedro@palves.net> wrote:
>
> On 2024-03-21 14:45, Jon Turney wrote:
> > On 21/03/2024 06:53, Orgad Shaneh wrote:
> >> From: Jon Turney <jon.turney@dronecode.org.uk>
> >
> > Not sure where this is coming from, but this doesn't seem to be my latest version of this patch.
> >
> >> Fix a memory leak which would occur in the case when the result of realpath() is
> >> greater than or equal to SO_NAME_MAX_PATH_SIZE.
> >>
> >> Distinguish between realpath() failing (returning NULL), and returning a path
> >> longer than SO_NAME_MAX_PATH_SIZE
> >>
> >> Warn rather than stopping with an error in those cases.
> >
> > This line in the patch commentary, and the title, refers to the part of the patch submitted [1], which is already applied as commit a0e9b53238c3033222c53b1654da535c0743ab6e.
> >
> > I separated that out because of the discussion starting at [2] ("Remove SO_NAME_PATH_SIZE instead"...)
> >
> > [1] https://sourceware.org/pipermail/gdb-patches/2020-January/164695.html
> > [2] https://sourceware.org/pipermail/gdb-patches/2016-January/130435.html

I took it from MSYS2 patches, which were taken from cygwin patches,
but apparently diverged (possibly my fault, in
https://github.com/msys2/MSYS2-packages/pull/2475. Not sure why :))

Thanks for the references. I see that some of the work is already done
(so_name, so_original_name and so->name are all std::strings), so it
looks like there's not much left.

> Curiously, after upstreaming your _sigbe unwinder recently, I looked at upstreaming this patch (the version of the 2016 patch
> in downstream cygwin gdb), but then this same thought of removing the limit hit me.  (at least I'm consistent over the
> years, eh.)
>
> I then realized that Simon is working on a series that switches the solib path storage to a std::string, which will
> let us easily not use SO_NAME_MAX_PATH_SIZE at all in the Windows code.  So I just dropped that patch from
> my upstreaming queue...

Great! So I'll drop this patch.

Thank you.

- Orgad
  

On 2024-03-21 16:31, Orgad Shaneh wrote:
> On Thu, Mar 21, 2024 at 6:13 PM Pedro Alves <pedro@palves.net> wrote:
>>
>> On 2024-03-21 14:45, Jon Turney wrote:
>>> On 21/03/2024 06:53, Orgad Shaneh wrote:
>>>> From: Jon Turney <jon.turney@dronecode.org.uk>
>>>
>>> Not sure where this is coming from, but this doesn't seem to be my latest version of this patch.
>>>
>>>> Fix a memory leak which would occur in the case when the result of realpath() is
>>>> greater than or equal to SO_NAME_MAX_PATH_SIZE.
>>>>
>>>> Distinguish between realpath() failing (returning NULL), and returning a path
>>>> longer than SO_NAME_MAX_PATH_SIZE
>>>>
>>>> Warn rather than stopping with an error in those cases.
>>>
>>> This line in the patch commentary, and the title, refers to the part of the patch submitted [1], which is already applied as commit a0e9b53238c3033222c53b1654da535c0743ab6e.
>>>
>>> I separated that out because of the discussion starting at [2] ("Remove SO_NAME_PATH_SIZE instead"...)
>>>
>>> [1] https://sourceware.org/pipermail/gdb-patches/2020-January/164695.html
>>> [2] https://sourceware.org/pipermail/gdb-patches/2016-January/130435.html
> 
> I took it from MSYS2 patches, which were taken from cygwin patches,
> but apparently diverged (possibly my fault, in
> https://github.com/msys2/MSYS2-packages/pull/2475. Not sure why :))
> 
> Thanks for the references. I see that some of the work is already done
> (so_name, so_original_name and so->name are all std::strings), so it
> looks like there's not much left.

Indeed.  I actually thought the std::string conversion hadn't been merged yet.  Should
have checked.  :-)

So we can simplify this code already.  I sent a series to clean up things:

https://inbox.sourceware.org/gdb-patches/20240322190424.1231540-1-pedro@palves.net/T/

[PATCH 0/4] Down with SO_NAME_MAX_PATH_SIZE and windows_make_so spring cleaning
  [PATCH 1/4] Remove SO_NAME_MAX_PATH_SIZE limit from core solib code
  [PATCH 2/4] Simplify windows-nat.c:windows_make_so #ifdefery
  [PATCH 3/4] windows-nat: Remove SO_NAME_MAX_PATH_SIZE limit
  [PATCH 4/4] windows-nat: Use gdb_realpath

Thanks,
Pedro Alves

> 
>> Curiously, after upstreaming your _sigbe unwinder recently, I looked at upstreaming this patch (the version of the 2016 patch
>> in downstream cygwin gdb), but then this same thought of removing the limit hit me.  (at least I'm consistent over the
>> years, eh.)
>>
>> I then realized that Simon is working on a series that switches the solib path storage to a std::string, which will
>> let us easily not use SO_NAME_MAX_PATH_SIZE at all in the Windows code.  So I just dropped that patch from
>> my upstreaming queue...
> 
> Great! So I'll drop this patch.
>