[PATCH 0/4] Down with SO_NAME_MAX_PATH_SIZE and windows_make_so spring cleaning

[PATCH 0/4] Down with SO_NAME_MAX_PATH_SIZE and windows_make_so spring cleaning

[Pedro Alves]

The commit subjects below say it all.

Pedro Alves (4):
  Remove SO_NAME_MAX_PATH_SIZE limit from core solib code
  Simplify windows-nat.c:windows_make_so #ifdefery
  windows-nat: Remove SO_NAME_MAX_PATH_SIZE limit
  windows-nat: Use gdb_realpath

 gdb/solib.c       |  2 --
 gdb/windows-nat.c | 42 ++++++++++++++++++++++++------------------
 2 files changed, 24 insertions(+), 20 deletions(-)


base-commit: 319719bb2921e978738acd408e6b16dabf0e7f5e
-- 
2.43.2

[PATCH 1/4] Remove SO_NAME_MAX_PATH_SIZE limit from core solib code

[Pedro Alves]

solib_map_sections errors out if the library file name is longer than
SO_NAME_MAX_PATH_SIZE.

solib::so_name and solib::so_original_name used to be arrays of
SO_NAME_MAX_PATH_SIZE size, so that check made sense then.

However, since commit 98107b0b17ac ("gdb: make
so_list::{so_original_name,so_name} std::strings") those fields are of
std::string type, so there's really no need for the limit.

This commit simply removes the length limit check.

Change-Id: I2ec676b231cd18ae900c61c5caea461f47e989e6
---
 gdb/solib.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/gdb/solib.c b/gdb/solib.c
index 952897c37fc..9497f5d3099 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -591,8 +591,6 @@ solib_map_sections (solib &so)
      the library's host-side path.  If we let the target dictate
      that objfile's path, and the target is different from the host,
      GDB/MI will not provide the correct host-side path.  */
-  if (strlen (bfd_get_filename (so.abfd.get ())) >= SO_NAME_MAX_PATH_SIZE)
-    error (_ ("Shared library file name is too long."));
 
   so.so_name = bfd_get_filename (so.abfd.get ());
   so.sections = build_section_table (so.abfd.get ());
-- 
2.43.2

[PATCH 2/4] Simplify windows-nat.c:windows_make_so #ifdefery

[Pedro Alves]

There are two separate #ifndef __CYGWIN__/#else/#endif sections in the
windows_make_so function with 3 lines of shared code separating them.
I find this makes the code harder to understand than necessary.
AFAICS, there is no reason those three shared lines need to be after
the first #ifdef block.  There is no early return, nor are 'load_addr'
nor 'name' modified.

This commit moves that shared code to the top of the function, and
then combines the two #ifndef sections.

Change-Id: If2678b52836b1c3134a5e9f9fdaee74448d8b7bc
---
 gdb/windows-nat.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index a90388922e2..a01011248c1 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -826,6 +826,10 @@ windows_nat_target::store_registers (struct regcache *regcache, int r)
 static windows_solib *
 windows_make_so (const char *name, LPVOID load_addr)
 {
+  windows_solib *so = &windows_process.solibs.emplace_back ();
+  so->load_addr = load_addr;
+  so->original_name = name;
+
 #ifndef __CYGWIN__
   char *p;
   char buf[__PMAX];
@@ -854,6 +858,8 @@ windows_make_so (const char *name, LPVOID load_addr)
       GetSystemDirectory (buf, sizeof (buf));
       strcat (buf, "\\ntdll.dll");
     }
+
+  so->name = buf;
 #else
   wchar_t buf[__PMAX];
 
@@ -866,13 +872,6 @@ windows_make_so (const char *name, LPVOID load_addr)
 	  wcscat (buf, L"\\ntdll.dll");
 	}
     }
-#endif
-  windows_solib *so = &windows_process.solibs.emplace_back ();
-  so->load_addr = load_addr;
-  so->original_name = name;
-#ifndef __CYGWIN__
-  so->name = buf;
-#else
   if (buf[0])
     {
       char cname[SO_NAME_MAX_PATH_SIZE];
-- 
2.43.2

[PATCH 3/4] windows-nat: Remove SO_NAME_MAX_PATH_SIZE limit

[Pedro Alves]

There is no need to limit shared library path sizes to
SO_NAME_MAX_PATH_SIZE nowadays.  windows_solib::name and
windows_solib::original_name are std::strings nowadays, and so are
solib::so_name and solib::so_original_name in the core solib code.

This commit reworks the code to remove that limit.  This also fixes a
leak where we were not releasing 'rname' in the realpath branch if the
'rname' string was larger than SO_NAME_MAX_PATH_SIZE.

Note: I tested the cygwin_conv_path with a manual hack to force that
path, and then stepping through the code.  You only get to that path
if Windows doesn't report an absolute path for ntdll.dll, and on my
machine (running Windows 10), it always does.

Change-Id: I79e9862d5a7646eebfef7ab5b05b96318a7ca0c5
---
 gdb/windows-nat.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index a01011248c1..278bfb0e1f1 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -874,22 +874,32 @@ windows_make_so (const char *name, LPVOID load_addr)
     }
   if (buf[0])
     {
-      char cname[SO_NAME_MAX_PATH_SIZE];
-      cygwin_conv_path (CCP_WIN_W_TO_POSIX, buf, cname,
-			SO_NAME_MAX_PATH_SIZE);
-      so->name = cname;
+      bool ok = false;
+
+      /* Check how big the output buffer has to be.  */
+      ssize_t size = cygwin_conv_path (CCP_WIN_W_TO_POSIX, buf, nullptr, 0);
+      if (size > 0)
+	{
+	  /* SIZE includes the null terminator.  */
+	  so->name.resize (size - 1);
+	  if (cygwin_conv_path (CCP_WIN_W_TO_POSIX, buf, so->name.data (),
+				size) == 0)
+	    ok = true;
+	}
+      if (!ok)
+	so->name = so->original_name;
     }
   else
     {
       char *rname = realpath (name, NULL);
-      if (rname && strlen (rname) < SO_NAME_MAX_PATH_SIZE)
+      if (rname != nullptr)
 	{
 	  so->name = rname;
 	  free (rname);
 	}
       else
 	{
-	  warning (_("dll path for \"%s\" too long or inaccessible"), name);
+	  warning (_("dll path for \"%s\" inaccessible"), name);
 	  so->name = so->original_name;
 	}
     }
-- 
2.43.2

[PATCH 4/4] windows-nat: Use gdb_realpath

[Pedro Alves]

Use gdb_realpath instead of realpath in windows-nat.c:windows_make_so,
so that we don't have to manually call free.

Change-Id: Id3cda7e177ac984c9a5f7c23f354e72bd561edff
---
 gdb/windows-nat.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 278bfb0e1f1..ee38b985efa 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -891,12 +891,9 @@ windows_make_so (const char *name, LPVOID load_addr)
     }
   else
     {
-      char *rname = realpath (name, NULL);
+      gdb::unique_xmalloc_ptr<char> rname = gdb_realpath (name);
       if (rname != nullptr)
-	{
-	  so->name = rname;
-	  free (rname);
-	}
+	so->name = rname.get ();
       else
 	{
 	  warning (_("dll path for \"%s\" inaccessible"), name);
-- 
2.43.2

Re: [PATCH 0/4] Down with SO_NAME_MAX_PATH_SIZE and windows_make_so spring cleaning

[John Baldwin]

On 3/22/24 12:04 PM, Pedro Alves wrote:
> The commit subjects below say it all.
> 
> Pedro Alves (4):
>    Remove SO_NAME_MAX_PATH_SIZE limit from core solib code
>    Simplify windows-nat.c:windows_make_so #ifdefery
>    windows-nat: Remove SO_NAME_MAX_PATH_SIZE limit
>    windows-nat: Use gdb_realpath
> 
>   gdb/solib.c       |  2 --
>   gdb/windows-nat.c | 42 ++++++++++++++++++++++++------------------
>   2 files changed, 24 insertions(+), 20 deletions(-)
> 
> 
> base-commit: 319719bb2921e978738acd408e6b16dabf0e7f5e

These all look good to me.

Approved-By: John Baldwin <jhb@FreeBSD.org>

-- 
John Baldwin

Re: [PATCH 0/4] Down with SO_NAME_MAX_PATH_SIZE and windows_make_so spring cleaning

[Pedro Alves]

On 2024-03-22 19:37, John Baldwin wrote:
> On 3/22/24 12:04 PM, Pedro Alves wrote:
>> The commit subjects below say it all.
>>
>> Pedro Alves (4):
>>    Remove SO_NAME_MAX_PATH_SIZE limit from core solib code
>>    Simplify windows-nat.c:windows_make_so #ifdefery
>>    windows-nat: Remove SO_NAME_MAX_PATH_SIZE limit
>>    windows-nat: Use gdb_realpath
>>
>>   gdb/solib.c       |  2 --
>>   gdb/windows-nat.c | 42 ++++++++++++++++++++++++------------------
>>   2 files changed, 24 insertions(+), 20 deletions(-)
>>
>>
>> base-commit: 319719bb2921e978738acd408e6b16dabf0e7f5e
> 
> These all look good to me.
> 
> Approved-By: John Baldwin <jhb@FreeBSD.org>
> 

Thanks John.  I added your tag and merged this.