From: ZenitDS Subject: Re: Patching libgnat with strlcpy and strlcat to remove warnings To: Theo de Raadt Cc: Stuart Henderson , "ports@openbsd.org" Date: Sun, 22 Mar 2026 08:43:12 +0000 Something like this? More could be done for those memcpy's but that would make the patches more complex. Index: 15/patches/patch-gcc_ada_adaint_c =================================================================== RCS file: /cvs/ports/lang/gcc/15/patches/patch-gcc_ada_adaint_c,v diff -u -p -r1.1.1.1 patch-gcc_ada_adaint_c --- 15/patches/patch-gcc_ada_adaint_c 18 Aug 2025 19:49:19 -0000 1.1.1.1 +++ 15/patches/patch-gcc_ada_adaint_c 22 Mar 2026 08:41:41 -0000 @@ -1,16 +1,131 @@ Index: gcc/ada/adaint.c --- gcc/ada/adaint.c.orig +++ gcc/ada/adaint.c -@@ -3714,6 +3714,12 @@ void __gnat_killprocesstree (int pid, int sig_num) - */ +@@ -156,6 +156,8 @@ + #include + #endif + ++#include ++ + #ifdef __cplusplus + extern "C" { + #endif +@@ -576,9 +578,12 @@ __gnat_try_lock (char *dir, char *file) + GNAT_STRUCT_STAT stat_result; + int fd; + +- sprintf (full_path, "%s%c%s", dir, DIR_SEPARATOR, file); +- sprintf (temp_file, "%s%cTMP-%ld-%ld", +- dir, DIR_SEPARATOR, (long)getpid(), (long)getppid ()); ++ assert((unsigned int) snprintf(full_path, sizeof(full_path), ++ "%s%c%s", ++ dir, DIR_SEPARATOR, file) < sizeof(full_path)); ++ assert((unsigned int) snprintf(temp_file, sizeof(temp_file), ++ "%s%cTMP-%ld-%ld", dir, DIR_SEPARATOR, ++ (long)getpid(), (long)getppid ()) < sizeof(temp_file)); + + /* Create the temporary file and write the process number. */ + fd = open (temp_file, O_CREAT | O_WRONLY, 0600); +@@ -756,8 +761,8 @@ __gnat_os_filename (char *filename ATTRIBUTE_UNUSED, + strcpy (encoding, "encoding=utf8"); + *e_length = strlen (encoding); + #else +- strcpy (os_name, filename); + *o_length = strlen (filename); ++ memcpy (os_name, filename, *o_length + 1); + *e_length = 0; + #endif } +@@ -1088,7 +1093,7 @@ __gnat_open_new_temp (char *path, int fmode) + int fd; + int o_fmode = O_BINARY; + +- strcpy (path, "GNAT-XXXXXX"); ++ memcpy (path, "GNAT-XXXXXX", strlen("GNAT-XXXXXX")); + + #if (defined (__FreeBSD__) || defined (__NetBSD__) || defined (__OpenBSD__) \ + || defined (__linux__) || defined (__GLIBC__) || defined (__ANDROID__) \ +@@ -1275,10 +1280,12 @@ __gnat_tmp_name (char *tmp_filename) + #ifdef __ANDROID__ + strcpy (tmp_filename, "/cache/gnat-XXXXXX"); + #else +- strcpy (tmp_filename, "/tmp/gnat-XXXXXX"); ++ assert(strlcpy (tmp_filename, "/tmp/gnat-XXXXXX", ++ __gnat_max_path_len) < (unsigned int) __gnat_max_path_len); + #endif + else +- sprintf (tmp_filename, "%s/gnat-XXXXXX", tmpdir); ++ assert(snprintf (tmp_filename, __gnat_max_path_len, ++ "%s/gnat-XXXXXX", tmpdir) < __gnat_max_path_len); + + close (mkstemp(tmp_filename)); + #elif defined (__vxworks) && !defined (VTHREADS) +@@ -1373,8 +1380,8 @@ __gnat_readdir (DIR *dirp, char *buffer, int *len) + + if (dirent != NULL) + { +- strcpy (buffer, dirent->d_name); +- *len = strlen (buffer); ++ *len = strlen (dirent->d_name); ++ memcpy (buffer, dirent->d_name, *len + 1); + return buffer; + } + else +@@ -2936,7 +2943,8 @@ __gnat_locate_file_with_predicate (char *file_name, ch + int (*predicate)(char *)) + { + char *ptr; +- char *file_path = (char *) alloca (strlen (file_name) + 1); ++ const unsigned int file_name_len = strlen(file_name); ++ char *file_path = (char *) alloca (file_name_len + 1); + int absolute; + + /* Return immediately if file_name is empty */ +@@ -2950,7 +2958,7 @@ __gnat_locate_file_with_predicate (char *file_name, ch + if (*ptr == '"') + ptr++; + +- strcpy (file_path, ptr); ++ assert(strlcpy (file_path, ptr, file_name_len + 1) < file_name_len + 1); + + ptr = file_path + strlen (file_path) - 1; +@@ -3015,7 +3023,7 @@ __gnat_locate_file_with_predicate (char *file_name, ch + if (!IS_DIRECTORY_SEPARATOR(*ptr)) + *++ptr = DIR_SEPARATOR; + +- strcpy (++ptr, file_name); ++ memcpy (++ptr, file_name, strlen(file_name) + 1); + + if (predicate (file_path)) + return xstrdup (file_path); +@@ -3112,13 +3120,14 @@ __gnat_locate_exec_on_path (char *exec_name, + + #else + const char *path_val = getenv ("PATH"); ++ const unsigned int path_val_len = strlen(path_val); + + /* If PATH is not defined, proceed with __gnat_locate_exec anyway, so we can + find files that contain directory names. */ + + if (path_val == NULL) path_val = ""; +- apath_val = (char *) alloca (strlen (path_val) + 1); +- strcpy (apath_val, path_val); ++ apath_val = (char *) alloca (path_val_len + 1); ++ assert(strlcpy (apath_val, path_val, path_val_len + 1) < path_val_len + 1); + #endif + + return __gnat_locate_exec (exec_name, apath_val); +@@ -3712,6 +3721,12 @@ void __gnat_killprocesstree (int pid, int sig_num) + The 5th and 6th words are the pid and the 7th and 8th the ppid. + See: /usr/include/sys/procfs.h (struct pstatus). + */ ++} ++ +const char * +fname_as_string(int pretty_p __attribute__((__unused__))) +{ + return NULL; -+} -+ - #ifdef __cplusplus } - #endif + + #ifdef __cplusplus Index: 15/patches/patch-gcc_ada_cstreams_c =================================================================== RCS file: /cvs/ports/lang/gcc/15/patches/patch-gcc_ada_cstreams_c,v diff -u -p -r1.1.1.1 patch-gcc_ada_cstreams_c --- 15/patches/patch-gcc_ada_cstreams_c 18 Aug 2025 19:49:19 -0000 1.1.1.1 +++ 15/patches/patch-gcc_ada_cstreams_c 22 Mar 2026 08:41:41 -0000 @@ -1,7 +1,15 @@ Index: gcc/ada/cstreams.c --- gcc/ada/cstreams.c.orig +++ gcc/ada/cstreams.c -@@ -186,7 +186,7 @@ __gnat_full_name (char *nam, char *buffer) +@@ -47,6 +47,7 @@ + #include + #include + #include ++#include + + #ifdef _AIX + /* needed to avoid conflicting declarations */ +@@ -186,7 +187,7 @@ __gnat_full_name (char *nam, char *buffer) *p = '\\'; } @@ -10,3 +18,22 @@ Index: gcc/ada/cstreams.c /* Use realpath function which resolves links and references to . and .. on those Unix systems that support it. Note that GNU/Linux provides it but +@@ -228,12 +229,15 @@ __gnat_full_name (char *nam, char *buffer) + /* If the name returned is an absolute path, it is safe to append '/' + to the path and concatenate the name of the file. */ + if (buffer[0] == '/') +- strcat (buffer, "/"); ++ assert(strlcat(buffer, "/", ++ __gnat_max_path_len) < (unsigned int )__gnat_max_path_len); + +- strcat (buffer, nam); ++ assert(strlcat(buffer, nam, ++ __gnat_max_path_len) < (unsigned int) __gnat_max_path_len); + } + else +- strcpy (buffer, nam); ++ assert(strlcpy(buffer, nam, ++ __gnat_max_path_len) < (unsigned int) __gnat_max_path_len); + #endif + + return buffer; Index: 15/patches/patch-gcc_ada_env_c =================================================================== RCS file: 15/patches/patch-gcc_ada_env_c diff -N 15/patches/patch-gcc_ada_env_c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ 15/patches/patch-gcc_ada_env_c 22 Mar 2026 08:41:41 -0000 @@ -0,0 +1,22 @@ +Index: gcc/ada/env.c +--- gcc/ada/env.c.orig ++++ gcc/ada/env.c +@@ -84,6 +84,8 @@ extern "C" { + + #include "env.h" + ++#include ++ + void + __gnat_getenv (char *name, int *len, char **value) + { +@@ -109,7 +111,8 @@ __gnat_setenv (char *name, char *value) + + expression = (char *) xmalloc (size * sizeof (char)); + +- sprintf (expression, "%s=%s", name, value); ++ assert((unsigned int) snprintf (expression, size, ++ "%s=%s", name, value) == size - 1); + putenv (expression); + #if defined (__MINGW32__) || defined (__vxworks) + /* putenv for Windows and VxWorks 6 kernel modules makes a copy of the On Sunday, March 22nd, 2026 at 9:00 AM, ZenitDS wrote: > The original code in theory is correct, they could be surrounded by assert(). > > On Saturday, March 21st, 2026 at 11:20 PM, Theo de Raadt wrote: > > > Why are you not error checking, and accepting truncation? > > > > ZenitDS wrote: > > > > > The patch to libphobos/configure got there by mistake. > > > I did not touch it though. > > > > > > On Saturday, March 21st, 2026 at 9:18 PM, ZenitDS wrote: > > > > Hi again, > > > > > > > > I have started with the patching and this is what I have got for now. > > > > > > > > Some strcpy's could be changed to their strlcpy variant, but some just > > > > depend on the size of the original string. For those cases I just > > > > substituted by memcpy(). Maybe it is just better to do memcpy() at > > > > every instance since that would preserve completely the semantics of > > > > the strcpy(). > > > > > > > > I am having difficulty generating the library again after having built > > > > it for the first time. I can do gmake gnatlib inside build-amd64 but > > > > this is very hacky, plus I then cant figure out how to rebuild the package > > > > in the fake-amd64 directory. Is there any documentation on this matter? > > > > > > > > Index: 15/patches/patch-gcc_ada_adaint_c > > > > =================================================================== > > > > RCS file: /cvs/ports/lang/gcc/15/patches/patch-gcc_ada_adaint_c,v > > > > diff -u -p -r1.1.1.1 patch-gcc_ada_adaint_c > > > > --- 15/patches/patch-gcc_ada_adaint_c 18 Aug 2025 19:49:19 -0000 1.1.1.1 > > > > +++ 15/patches/patch-gcc_ada_adaint_c 21 Mar 2026 19:15:52 -0000 > > > > @@ -1,16 +1,102 @@ > > > > Index: gcc/ada/adaint.c > > > > --- gcc/ada/adaint.c.orig > > > > +++ gcc/ada/adaint.c > > > > -@@ -3714,6 +3714,12 @@ void __gnat_killprocesstree (int pid, int sig_num) > > > > - */ > > > > +@@ -576,8 +576,8 @@ __gnat_try_lock (char *dir, char *file) > > > > + GNAT_STRUCT_STAT stat_result; > > > > + int fd; > > > > + > > > > +- sprintf (full_path, "%s%c%s", dir, DIR_SEPARATOR, file); > > > > +- sprintf (temp_file, "%s%cTMP-%ld-%ld", > > > > ++ snprintf (full_path, sizeof(full_path), "%s%c%s", dir, DIR_SEPARATOR, file); > > > > ++ snprintf (temp_file, sizeof(temp_file), "%s%cTMP-%ld-%ld", > > > > + dir, DIR_SEPARATOR, (long)getpid(), (long)getppid ()); > > > > + > > > > + /* Create the temporary file and write the process number. */ > > > > +@@ -756,8 +756,8 @@ __gnat_os_filename (char *filename ATTRIBUTE_UNUSED, > > > > + strcpy (encoding, "encoding=utf8"); > > > > + *e_length = strlen (encoding); > > > > + #else > > > > +- strcpy (os_name, filename); > > > > + *o_length = strlen (filename); > > > > ++ memcpy (os_name, filename, *o_length + 1); > > > > + *e_length = 0; > > > > + #endif > > > > } > > > > +@@ -1278,7 +1278,7 @@ __gnat_tmp_name (char *tmp_filename) > > > > + strcpy (tmp_filename, "/tmp/gnat-XXXXXX"); > > > > + #endif > > > > + else > > > > +- sprintf (tmp_filename, "%s/gnat-XXXXXX", tmpdir); > > > > ++ snprintf (tmp_filename, __gnat_max_path_len, "%s/gnat-XXXXXX", tmpdir); > > > > + > > > > + close (mkstemp(tmp_filename)); > > > > + #elif defined (__vxworks) && !defined (VTHREADS) > > > > +@@ -1373,8 +1373,8 @@ __gnat_readdir (DIR *dirp, char *buffer, int *len) > > > > + > > > > + if (dirent != NULL) > > > > + { > > > > +- strcpy (buffer, dirent->d_name); > > > > +- *len = strlen (buffer); > > > > ++ *len = strlen (dirent->d_name); > > > > ++ memcpy (buffer, dirent->d_name, *len + 1); > > > > + return buffer; > > > > + } > > > > + else > > > > +@@ -2936,7 +2936,8 @@ __gnat_locate_file_with_predicate (char *file_name, ch > > > > + int (*predicate)(char *)) > > > > + { > > > > + char *ptr; > > > > +- char *file_path = (char *) alloca (strlen (file_name) + 1); > > > > ++ int file_name_len = strlen(file_name); > > > > ++ char *file_path = (char *) alloca (file_name_len + 1); > > > > + int absolute; > > > > + > > > > + /* Return immediately if file_name is empty */ > > > > +@@ -2950,7 +2951,7 @@ __gnat_locate_file_with_predicate (char *file_name, ch > > > > + if (*ptr == '"') > > > > + ptr++; > > > > + > > > > +- strcpy (file_path, ptr); > > > > ++ strlcpy (file_path, ptr, file_name_len + 1); > > > > + > > > > + ptr = file_path + strlen (file_path) - 1; > > > > + > > > > +@@ -3015,7 +3016,7 @@ __gnat_locate_file_with_predicate (char *file_name, ch > > > > + if (!IS_DIRECTORY_SEPARATOR(*ptr)) > > > > + *++ptr = DIR_SEPARATOR; > > > > + > > > > +- strcpy (++ptr, file_name); > > > > ++ memcpy (++ptr, file_name, strlen(file_name) + 1); > > > > + > > > > + if (predicate (file_path)) > > > > + return xstrdup (file_path); > > > > +@@ -3112,13 +3113,14 @@ __gnat_locate_exec_on_path (char *exec_name, > > > > > > > > + #else > > > > + const char *path_val = getenv ("PATH"); > > > > ++ int path_val_len = strlen(path_val); > > > > + > > > > + /* If PATH is not defined, proceed with __gnat_locate_exec anyway, so we can > > > > + find files that contain directory names. */ > > > > + > > > > + if (path_val == NULL) path_val = ""; > > > > +- apath_val = (char *) alloca (strlen (path_val) + 1); > > > > +- strcpy (apath_val, path_val); > > > > ++ apath_val = (char *) alloca (path_val_len + 1); > > > > ++ strlcpy (apath_val, path_val, path_val_len + 1); > > > > + #endif > > > > + > > > > + return __gnat_locate_exec (exec_name, apath_val); > > > > +@@ -3712,6 +3714,12 @@ void __gnat_killprocesstree (int pid, int sig_num) > > > > + The 5th and 6th words are the pid and the 7th and 8th the ppid. > > > > + See: /usr/include/sys/procfs.h (struct pstatus). > > > > + */ > > > > ++} > > > > ++ > > > > +const char * > > > > +fname_as_string(int pretty_p __attribute__((__unused__))) > > > > +{ > > > > + return NULL; > > > > -+} > > > > -+ > > > > - #ifdef __cplusplus > > > > } > > > > - #endif > > > > + > > > > + #ifdef __cplusplus > > > > Index: 15/patches/patch-gcc_ada_cstreams_c > > > > =================================================================== > > > > RCS file: /cvs/ports/lang/gcc/15/patches/patch-gcc_ada_cstreams_c,v > > > > diff -u -p -r1.1.1.1 patch-gcc_ada_cstreams_c > > > > --- 15/patches/patch-gcc_ada_cstreams_c 18 Aug 2025 19:49:19 -0000 1.1.1.1 > > > > +++ 15/patches/patch-gcc_ada_cstreams_c 21 Mar 2026 19:15:52 -0000 > > > > @@ -10,3 +10,19 @@ Index: gcc/ada/cstreams.c > > > > > > > > /* Use realpath function which resolves links and references to . and .. > > > > on those Unix systems that support it. Note that GNU/Linux provides it but > > > > +@@ -228,12 +228,12 @@ __gnat_full_name (char *nam, char *buffer) > > > > + /* If the name returned is an absolute path, it is safe to append '/' > > > > + to the path and concatenate the name of the file. */ > > > > + if (buffer[0] == '/') > > > > +- strcat (buffer, "/"); > > > > ++ strlcat (buffer, "/", __gnat_max_path_len); > > > > + > > > > +- strcat (buffer, nam); > > > > ++ strlcat (buffer, nam, __gnat_max_path_len); > > > > + } > > > > + else > > > > +- strcpy (buffer, nam); > > > > ++ strlcpy (buffer, nam, __gnat_max_path_len); > > > > + #endif > > > > + > > > > + return buffer; > > > > Index: 15/patches/patch-gcc_ada_env_c > > > > =================================================================== > > > > RCS file: 15/patches/patch-gcc_ada_env_c > > > > diff -N 15/patches/patch-gcc_ada_env_c > > > > --- /dev/null 1 Jan 1970 00:00:00 -0000 > > > > +++ 15/patches/patch-gcc_ada_env_c 21 Mar 2026 19:15:52 -0000 > > > > @@ -0,0 +1,12 @@ > > > > +Index: gcc/ada/env.c > > > > +--- gcc/ada/env.c.orig > > > > ++++ gcc/ada/env.c > > > > +@@ -109,7 +109,7 @@ __gnat_setenv (char *name, char *value) > > > > + > > > > + expression = (char *) xmalloc (size * sizeof (char)); > > > > + > > > > +- sprintf (expression, "%s=%s", name, value); > > > > ++ snprintf (expression, size, "%s=%s", name, value); > > > > + putenv (expression); > > > > + #if defined (__MINGW32__) || defined (__vxworks) > > > > + /* putenv for Windows and VxWorks 6 kernel modules makes a copy of the > > > > Index: 15/patches/patch-libphobos_configure > > > > =================================================================== > > > > RCS file: /cvs/ports/lang/gcc/15/patches/patch-libphobos_configure,v > > > > diff -u -p -r1.2 patch-libphobos_configure > > > > --- 15/patches/patch-libphobos_configure 7 Mar 2026 21:43:19 -0000 1.2 > > > > +++ 15/patches/patch-libphobos_configure 21 Mar 2026 19:15:52 -0000 > > > > @@ -48,14 +48,14 @@ Index: libphobos/configure > > > > druntime_fiber_asm_external=yes > > > > ;; > > > > esac > > > > -@@ -16067,6 +16081,10 @@ if test -z "${DRUNTIME_CPU_POWERPC_TRUE}" && test -z " > > > > - as_fn_error $? "conditional \"DRUNTIME_CPU_POWERPC\" was never defined. > > > > - Usually this means the macro was only invoked conditionally." "$LINENO" 5 > > > > +@@ -16065,6 +16079,10 @@ Usually this means the macro was only invoked conditio > > > > fi > > > > -+if test -z "${DRUNTIME_CPU_SPARC64_TRUE}" && test -z "${DRUNTIME_CPU_SPARC64_FALSE}"; then > > > > -+ as_fn_error $? "conditional \"DRUNTIME_CPU_SPARC64\" was never defined. > > > > + if test -z "${DRUNTIME_CPU_POWERPC_TRUE}" && test -z "${DRUNTIME_CPU_POWERPC_FALSE}"; then > > > > + as_fn_error $? "conditional \"DRUNTIME_CPU_POWERPC\" was never defined. > > > > +Usually this means the macro was only invoked conditionally." "$LINENO" 5 > > > > +fi > > > > - if test -z "${DRUNTIME_CPU_X86_TRUE}" && test -z "${DRUNTIME_CPU_X86_FALSE}"; then > > > > - as_fn_error $? "conditional \"DRUNTIME_CPU_X86\" was never defined. > > > > ++if test -z "${DRUNTIME_CPU_SPARC64_TRUE}" && test -z "${DRUNTIME_CPU_SPARC64_FALSE}"; then > > > > ++ as_fn_error $? "conditional \"DRUNTIME_CPU_SPARC64\" was never defined. > > > > Usually this means the macro was only invoked conditionally." "$LINENO" 5 > > > > + fi > > > > + if test -z "${DRUNTIME_CPU_X86_TRUE}" && test -z "${DRUNTIME_CPU_X86_FALSE}"; then > > > > > > > > On Friday, March 20th, 2026 at 10:39 AM, Stuart Henderson wrote: > > > > > On 2026/03/19 23:33, ZenitDS wrote: > > > > > > Hello, > > > > > > > > > > > > for gcc ada toolchain, libgnat uses several strcpy and strcat that spawn > > > > > > the annoying warnings of strcpy and strcat on each compilation. I think > > > > > > it may be a good idea to add some patches to fix this situation with > > > > > > strlcpy. I am willing to do it in case it is ok and could be merged. > > > > > > > > > > > > Cheers > > > > > > > > > > > > > > > > As long as the patches aren't too intrusive I think this makes sense. > > > > > > > > > >