[PATCH] Fix minor portability issues including ISO C 23 prototypes.
Hi, I noticed some compiler warnings that might be worth fixing. I
wasn't sure if patches should have ChangeLog entries so I left it
alone and tried to make it easy to copy paste for you. Feel free to
use or ignore changes as you see fit.
Thanks,
Collin
* lib/malloc/malloc.c (botch): Add correct prototype so declaration is
compatible with C23.
* lib/malloc/table.h (mregister_describe_mem): Add correct prototype
so declaration is compatible with C23.
* lib/sh/getenv.c (getenv, putenv, setenv, unsetenv): Don't assume
that NULL is equivalent to 0 and just use the macro itself.
* lib/sh/strlcpy.c (strlcpy): Remove duplicate const qualifier.
diff from commit fbc7d97de6c6f3dedb34f49f89a628a99ef6ddf5 on devel:
---
lib/malloc/malloc.c | 3 ++-
lib/malloc/table.h | 2 +-
lib/sh/getenv.c | 10 +-
lib/sh/strlcpy.c| 2 +-
4 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/lib/malloc/malloc.c b/lib/malloc/malloc.c
index f829d395..9674f4d0 100644
--- a/lib/malloc/malloc.c
+++ b/lib/malloc/malloc.c
@@ -322,7 +322,8 @@ static PTR_T internal_valloc (size_t, const char *, int,
int);
static PTR_T internal_remap (PTR_T, size_t, int, int);
#if defined (botch)
-extern void botch ();
+/* XXX - set to `programming_error' by Makefile. */
+extern void botch (const char *, ...);
#else
static void botch (const char *, const char *, int);
#endif
diff --git a/lib/malloc/table.h b/lib/malloc/table.h
index e7803762..f8f74639 100644
--- a/lib/malloc/table.h
+++ b/lib/malloc/table.h
@@ -60,7 +60,7 @@ typedef struct mr_table {
extern mr_table_t *mr_table_entry (PTR_T);
extern void mregister_alloc (const char *, PTR_T, size_t, const char *, int);
extern void mregister_free (PTR_T, int, const char *, int);
-extern void mregister_describe_mem ();
+extern void mregister_describe_mem (PTR_T, FILE *);
extern void mregister_dump_table (void);
extern void mregister_table_init (void);
diff --git a/lib/sh/getenv.c b/lib/sh/getenv.c
index 6917f075..f19d2c69 100644
--- a/lib/sh/getenv.c
+++ b/lib/sh/getenv.c
@@ -51,7 +51,7 @@ getenv (const char *name)
{
SHELL_VAR *var;
- if (name == 0 || *name == '\0')
+ if (name == NULL || *name == '\0')
return ((char *)NULL);
var = find_tempenv_variable ((char *)name);
@@ -103,7 +103,7 @@ putenv (char *str)
char *name, *value;
int offset;
- if (str == 0 || *str == '\0')
+ if (str == NULL || *str == '\0')
{
errno = EINVAL;
return -1;
@@ -148,13 +148,13 @@ setenv (const char *name, const char *value, int rewrite)
SHELL_VAR *var;
char *v;
- if (name == 0 || *name == '\0' || strchr (name, '=') != 0)
+ if (name == NULL || *name == '\0' || strchr (name, '=') != NULL)
{
errno = EINVAL;
return -1;
}
- var = 0;
+ var = NULL;
v = (char *)value; /* some compilers need explicit cast */
/* XXX - should we worry about readonly here? */
if (rewrite == 0)
@@ -186,7 +186,7 @@ _setenv (const char *name, const char *value, int rewrite)
int
unsetenv (const char *name)
{
- if (name == 0 || *name == '\0' || strchr (name, '=') != 0)
+ if (name == NULL || *name == '\0' || strchr (name, '=') != NULL)
{
errno = EINVAL;
return (-1);
diff --git a/lib/sh/strlcpy.c b/lib/sh/strlcpy.c
index 787e4c36..ae30c2ee 100644
--- a/lib/sh/strlcpy.c
+++ b/lib/sh/strlcpy.c
@@ -23,7 +23,7 @@
#include
size_t
-strlcpy(char *dest, const const char *src, size_t size)
+strlcpy (char *dest, const char *src, size_t size)
{
size_t ret;
--
2.39.2
Re: [PATCH] Fix minor portability issues including ISO C 23 prototypes.
On 2/14/24 11:56 AM, Chet Ramey wrote: > This isn't necessary in general; ISO C guarantees that a constant > expression with value 0 is a null pointer constant just like NULL, > and the compiler will make them equivalent even if the internal > representation of a null pointer isn't all zeroes. Oops, You're right. That's my bad. I think I saw a compiler for the first comparison to zero in setenv () which is right above a NULL return. I think the NULL looks more clear but that is probably just personal preference. :) Collin
Re: build failure without system extension macros
Hi Chet, On 5/16/24 7:25 AM, Chet Ramey wrote: > Yes. This was inspired by > > https://lists.gnu.org/archive/html/bug-bash/2024-04/msg00158.html I'm not sure if you have dealt with this particular one yet, but I would just get rid of the u_char, u_short, u_long, types. They are old BSD stuff that is less portable than the standard C types. I did the same in GNU Inetutils recently. Collin
Incorrect string length check
Hi Chet,
In this commit in the devel branch:
commit 03c8c43b79177fa678714893e9f05b1c517592c0
Author: Chet Ramey
Date: Fri Apr 5 09:03:52 2024 -0400
man page typesetting updates for compatibilityand layout issues
I think there was a typo in execute_cmd.c:
diff --git a/execute_cmd.c b/execute_cmd.c
[...]
void
coproc_setvars (struct coproc *cp)
{
@@ -6072,14 +6073,14 @@ shell_execve (char *command, char **args, char **env)
interp = getinterp (sample, sample_len, (int *)NULL);
ilen = strlen (interp);
errno = i;
- if (interp[ilen - 1] == '\r')
+ if (interp > 0 && interp[ilen - 1] == '\r')
{
interp = xrealloc (interp, ilen + 2);
interp[ilen - 1] = '^';
interp[ilen] = 'M';
interp[ilen + 1] = '\0';
}
Shouldn't that condition be something like this:
if (ilen > 0 && interp[ilen - 1] == '\r')
{
/* Rest of code. */
}
Since you want to protect against an '#!' without an interpreter
following it. I'm thinking it was just a typo but feel free to correct
me if I am missing something.
Thanks,
Collin
Re: printf %m$ and %*m$ unimplemented in Bash, but implementated in C compilers
porterleete writes: > Fix: > Either update the documentation of what printf in bash actually does. > If printf is using a standard for printf other than "what the biggest > C compilers currently do", document which standard it's using or > update it to the newest standard used by gcc and clang. If it really > is just this one feature that's missing, add it in or document its > absence. The 'printf' in shells is different than the 'printf' specified by ISO C/POSIX. The documentation for the bash built-in can be found here: $ info '(bash)Bash Builtins' The 'printf' provided in your C library should be found in man page section 3. Your system probably comes with a program (not a shell built-in) too. That can be found in man page section 1. # C library. $ man -s 3 printf # System program. $ man -s 1 printf Collin
[PATCH] malloc: fix out-of-bounds read
Hi,
In lib/malloc/malloc.c there is a read that occurs 1 or 2 indexes before
the first element in the buffer. The issue is this macro:
/* Use this when we want to be sure that NB is in bucket NU. */
#define RIGHT_BUCKET(nb, nu) \
(((nb) > binsizes[(nu)-1]) && ((nb) <= binsizes[(nu)]))
Where 'binsizes' is an array like this:
static const unsigned long binsizes[NBUCKETS] = {
32UL, 64UL, 128UL, 256UL, 512UL, 1024UL, 2048UL, 4096UL,
... };
The out-of-bounds read occurs in a line like this:
/* If ok, use the same block, just marking its size as changed. */
if (RIGHT_BUCKET(nbytes, nunits) || RIGHT_BUCKET(nbytes, nunits-1))
{
...
}
Where 'nunits' isn't properly checked. This can easily be seen by
-fsanitize=undefined when running make check:
< malloc.c:1205:7: runtime error: index -1 out of bounds for type 'long
unsigned int [28]'
< malloc.c:1205:39: runtime error: index -2 out of bounds for type 'long
unsigned int [28]'
< malloc.c:1205:39: runtime error: index -1 out of bounds for type 'long
unsigned int [28]'
I've attached a patch that silences ubsan atleast. I didn't look into
the surrounding code much so a double check would be nice. :)
Collin
>From 4863afd5260e11f05f69adc64c496f6d8bace627 Mon Sep 17 00:00:00 2001
From: Collin Funk
Date: Thu, 18 Jul 2024 21:45:51 -0700
Subject: [PATCH] malloc: fix out-of-bounds read
* lib/malloc/malloc.c (internal_realloc): Check value of nunits before
using RIGHT_BUCKET.
---
lib/malloc/malloc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/malloc/malloc.c b/lib/malloc/malloc.c
index 7b2c3f25..07487fa8 100644
--- a/lib/malloc/malloc.c
+++ b/lib/malloc/malloc.c
@@ -1202,7 +1202,8 @@ internal_realloc (PTR_T mem, size_t n, const char *file, int line, int flags)
nbytes = ALLOCATED_BYTES(n);
/* If ok, use the same block, just marking its size as changed. */
- if (RIGHT_BUCKET(nbytes, nunits) || RIGHT_BUCKET(nbytes, nunits-1))
+ if ((1 <= nunits && RIGHT_BUCKET (nbytes, nunits))
+ || (2 <= nunits && RIGHT_BUCKET (nbytes, nunits - 1)))
{
/* Compensate for increment above. */
m -= 4;
--
2.45.2
Re: [PATCH] malloc: fix out-of-bounds read
Hi Chet, Chet Ramey writes: >> /* Use this when we want to be sure that NB is in bucket NU. */ >> #define RIGHT_BUCKET(nb, nu) \ >> (((nb) > binsizes[(nu)-1]) && ((nb) <= binsizes[(nu)])) > > The right fix here is two-fold: fix the first test here to evaluate to 0 > if nu == 0, and change the call in internal_realloc similarly to how your > patch changes it for the nunits - 1 case. Ah, okay I see what you mean. Thanks. Did you want a revised patch or do you have it under control? Collin
Out of bounds read in parse.y.
Hi,
When compiling with undefined behavior sanitizer and then running:
$ ./bash
parse.y:1000:93: runtime error: index -1 out of bounds for type 'int [257]'
The offending section of code:
case_command: CASE WORD newline_list IN newline_list ESAC
{
$$ = make_case_command ($2, (PATTERN_LIST *)NULL,
word_lineno[word_top]);
if (word_top >= 0) word_top--;
}
| CASE WORD newline_list IN case_clause_sequence newline_list ESAC
{
/* Access of word_lineno[word_top] causes bad read.
*/
$$ = make_case_command ($2, $5,
word_lineno[word_top]);
if (word_top >= 0) word_top--;
}
And the definition of word top and word_lineno:
#define MAX_COMPOUND_NEST 256
static int word_lineno[MAX_COMPOUND_NEST+1];
static int word_top = -1;
The value of word_top appears to only be set in 'set_word_top':
static inline int
set_word_top (int t)
{
switch (t)
{
case CASE:
case SELECT:
case FOR:
case IF:
case WHILE:
case UNTIL:
if (word_top < MAX_COMPOUND_NEST)
word_top++;
word_lineno[word_top] = line_number;
break;
default:
break;
}
return word_top;
}
Shouldn't all the decrements of word_top be protected by:
if (word_top > 0) word_top--;
instead of:
if (word_top >= 0) word_top--;
Or is there something more complicated that I am missing here?
Collin
Re: Out of bounds read in parse.y.
Hi Chet,
Chet Ramey writes:
> Which version?
This was from bash devel branch, commit hash
2e01122fe78eb5a42c9b9f3ca46b91f895959675.
Built with:
./configure CFLAGS='-fsanitize=undefined'
> Why? 0 is a valid index. set_word_top increments word_top before assigning
> to word_lineno[word_top].
Ah, okay. I see what you mean.
> I suspect there is a decrement that isn't matched by a call to
> set_word_top(). But a reproducer would help, otherwise we're all just
> guessing.
Sure, the bad read was happening while reading my .profile and .bashrc
file. I've narrowed it down to a bash completion file installed by my
system packages. I've attached it to this message.
Running:
./bash --norc ovs-vsctl-bashcomp.bash
triggers the out of bounds read.
SAVE_IFS=$IFS
IFS="
"
_OVSDB_SERVER_LOCATION=""
# Run ovs-vsctl and make sure that ovs-vsctl is always called with
# the correct --db argument.
_ovs_vsctl () {
local _db
if [ -n "$_OVSDB_SERVER_LOCATION" ]; then
_db="--db=$_OVSDB_SERVER_LOCATION"
fi
ovs-vsctl ${_db} "$@"
}
# ovs-vsctl --commands outputs in this format:
#
# main = ,,
# localopts = ([] )*
# localopt = --[^]]*
# name = [^,]*
# arguments = ((!argument|?argument|*argument|+argument) )*
# argument = ([^ ]*|argument\|argument)
#
# The [] characters in local options are just delimiters. The
# argument prefixes mean:
# !argument :: The argument is required
# ?argument :: The argument is optional
# *argument :: The argument may appear any number (0 or more) times
# +argument :: The argument may appear one or more times
# A bar (|) character in an argument means thing before bar OR thing
# after bar; for example, del-port can take a port or an interface.
_OVS_VSCTL_COMMANDS="$(_ovs_vsctl --commands)"
# This doesn't complete on short arguments, so it filters them out.
_OVS_VSCTL_OPTIONS="$(_ovs_vsctl --options | awk '/^--/ { print $0 }' \
| sed -e 's/\(.*\)=ARG/\1=/')"
IFS=$SAVE_IFS
declare -A _OVS_VSCTL_PARSED_ARGS
declare -A _OVS_VSCTL_NEW_RECORDS
# This is a convenience function to make sure that user input is
# looked at as a fixed string when being compared to something. $1 is
# the input; this behaves like 'grep "^$1"' but deals with regex
# metacharacters in $1.
_ovs_vsctl_check_startswith_string () {
awk 'thearg == "" || index($0, thearg)==1' thearg="$1"
}
# $1 = word to complete on.
# Complete on global options.
_ovs_vsctl_bashcomp_globalopt () {
local options result
options=""
result=$(printf "%s\n" "${_OVS_VSCTL_OPTIONS}" \
| _ovs_vsctl_check_startswith_string "${1%=*}")
if [[ $result =~ "=" ]]; then
options="NOSPACE"
fi
printf -- "${options}\nEO\n${result}"
}
# $1 = word to complete on.
# Complete on local options.
_ovs_vsctl_bashcomp_localopt () {
local options result possible_opts
possible_opts=$(printf "%s\n" "${_OVS_VSCTL_COMMANDS}" | cut -f1 -d',')
# This finds all options that could go together with the
# already-seen ones
for prefix_arg in $1; do
possible_opts=$(printf "%s\n" "$possible_opts" \
| grep -- "\[${prefix_arg%%=*}=\?\]")
done
result=$(printf "%s\n" "${possible_opts}" \
| tr ' ' '\n' | tr -s '\n' | sort | uniq)
# This removes the already-seen options from the list so that
# users aren't completed for the same option twice.
for prefix_arg in $1; do
result=$(printf "%s\n" "${result}" \
| grep -v -- "\[${prefix_arg%%=*}=\?\]")
done
result=$(printf "%s\n" "${result}" | sed -ne 's/\[\(.*\)\]/\1/p' \
| _ovs_vsctl_check_startswith_string "$2")
if [[ $result =~ "=" ]]; then
options="NOSPACE"
fi
printf -- "${options}\nEO\n${result}"
}
# $1 = given local options.
# $2 = word to complete on.
# Complete on command that could contain the given local options.
_ovs_vsctl_bashcomp_command () {
local result possible_cmds
possible_cmds=$(printf "%s\n" "${_OVS_VSCTL_COMMANDS}")
for prefix_arg in $1; do
possible_cmds=$(printf "%s\n" "$possible_cmds" \
| grep -- "\[$prefix_arg=\?\]")
done
result=$(printf "%s\n" "${possible_cmds}" \
| cut -f2 -d',' \
| _ovs_vsctl_check_startswith_string "$2")
printf -- "${result}"
}
# $1 = completion result to check.
# Return 0 if the completion result is non-empty, otherwise return 1.
_ovs_vsctl_detect_nonzero_completions () {
local tmp newarg
newarg=${1#*EO}
readarray tmp <<< "$newarg"
if [ "${#tmp[@]}" -eq 1 ] && [ "${#newarg}" -eq 0 ]; then
return 1
fi
return 0
}
# $1 = argument format to expand.
# Expand '+ARGUMENT' in argument format to '!ARGUMENT *ARGUMENT'.
_ovs_vsctl_expand_command () {
result=$(printf "%s\n" "${_OVS_VSCTL_COMMANDS}" \
| grep -- ",$1," | cut -f3 -d',' | tr ' ' '\n' \
| awk '/\+.*/ { name=substr($0,2);
Re: Out of bounds read in parse.y.
Chet Ramey writes:
> Thanks. Here's the simple reproducer:
>
> x()
> {
> case y in
> z)
> if (! false); then
> foo=bar
> fi
> ;;
> esac
> }
>
>
> It was what I suspected.
Ah, nice! Thank you for the help.
Collin
Re: Is this a bug?
Hi,
George R Goffe writes:
> I've been trying to build bash from a repository
> "https://git.savannah.gnu.org/git/bash.git"; and a having the devil's own time
> in the process.
>
> Did I just catch the repository in the middle of a rework? I have a
> full log of the build process if it's needed. The failure appears with
> the nonexperimental GCC as well as a version I built a few days ago
> from their repository. gcc-15.0.1-0.9.fc42.x86_64 or gcc (GCC) 15.0.1
> 20250320 (experimental).
>
> I'm seeing a TON of messages like these, am I doing something wrong?
Recent versions of GCC and Clang have gotten strict about function
prototypes since C23 made the following:
int foo ();
equal to:
int foo (void);
In previous versions of C this was not true, 'foo' was just declared as
a function without any information about number of arguments or their
type.
C23 also got rid of K&R declarations like:
int
main (argc, argv)
int argc;
char *argv;
{
...
}
This is why you now see the warnings after upgrading to Fedora 42 with a
new C compiler.
Try building from the 'devel' branch where prototypes have been fixed.
Collin
[PATCH] Fix link error on GNU/Hurd.
Hi Chet, Building bash from the devel branch fails the link on GNU/Hurd. Here is the error: gcc -L./builtins -L./lib/readline -L./lib/readline -L./lib/glob -L./lib/tilde -L./lib/malloc -L./lib/sh -L./lib/termcap -g -O2 -o bash shell.o eval.o y.tab.o general.o make_cmd.o print_cmd.o dispose_cmd.o execute_cmd.o variables.o copy_cmd.o error.o expr.o flags.o jobs.o subst.o hashcmd.o hashlib.o mailcheck.o trap.o input.o unwind_prot.o pathexp.o sig.o test.o version.o alias.o array.o arrayfunc.o assoc.o braces.o bracecomp.o bashhist.o bashline.o list.o stringlib.o locale.o findcmd.o redir.o pcomplete.o pcomplib.o syntax.o xmalloc.o -lbuiltins -lglob -lsh -lreadline -lhistory ./lib/termcap/libtermcap.a -ltilde -lmalloc-ldl /usr/bin/ld: ./lib/termcap/libtermcap.a(termcap.o):/home/collin/bash-5.3-rc1/lib/termcap/termcap.c:283: multiple definition of `PC'; ./lib/readline/libreadline.a(terminal.o):/home/collin/bash-5.3-rc1/lib/readline/terminal.c:109: first defined here /usr/bin/ld: ./lib/termcap/libtermcap.a(tparam.o):/home/collin/bash-5.3-rc1/lib/termcap/tparam.c:115: multiple definition of `BC'; ./lib/readline/libreadline.a(terminal.o):/home/collin/bash-5.3-rc1/lib/readline/terminal.c:109: first defined here /usr/bin/ld: ./lib/termcap/libtermcap.a(tparam.o):/home/collin/bash-5.3-rc1/lib/termcap/tparam.c:116: multiple definition of `UP'; ./lib/readline/libreadline.a(terminal.o):/home/collin/bash-5.3-rc1/lib/readline/terminal.c:109: first defined here I have attached a patch which fixes it. The issue is very simple, just a missing '#if defined'. Collin >From 253559a51dda6c15afbae3174b7b887dede7d9e3 Mon Sep 17 00:00:00 2001 From: Collin Funk Date: Thu, 8 May 2025 22:21:15 -0700 Subject: [PATCH] Fix link error on GNU/Hurd. * lib/readline/terminal.c (PC, BC, UP) [__gnu_hurd__]: Remove duplicate declarations. --- lib/readline/terminal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/readline/terminal.c b/lib/readline/terminal.c index 2c70553d..99f63f9c 100644 --- a/lib/readline/terminal.c +++ b/lib/readline/terminal.c @@ -102,7 +102,7 @@ static char *term_string_buffer = (char *)NULL; static int tcap_initialized; -#if !defined (__linux__) && !defined (NCURSES_VERSION) +#if !(defined (__linux__) || defined (__gnu_hurd__)) && !defined (NCURSES_VERSION) # if defined (__EMX__) || defined (NEED_EXTERN_PC) extern # endif /* __EMX__ || NEED_EXTERN_PC */ -- 2.49.0
Re: [PATCH] Fix link error on GNU/Hurd.
Hi Chet, Chet Ramey writes: > Thanks for the report. I'm interested in why you're not using ncurses. Is > it just not installed on your build system? I occasionally test Gnulib in a Hurd VM (fresh each time since my image doesn't like to reboot, unfortunately). I had assumed that libncurses-dev was installed as a transitive dependency, but I guess not. After running 'apt install libncurses-dev', I can confirm that bash builds on the devel branch. Thanks! Collin
