Building loadables depends on main build

[Christian Weisgerber]

Building the loadable modules depends on files created during the
main build.  However, the Makefile doesn't record any such dependency.
Running for instance "make -j10 all loadables" will fail due to a
lack of enforced sequencing.

A straightforward fix would be to make the "loadables" target depend
on ".made":

--- Makefile.in.orig
+++ Makefile.in
@@ -803,7 +803,7 @@ $(srcdir)/configure:$(srcdir)/configure.ac 
$(srcdir)/
 reconfig: force
sh $(srcdir)/configure -C
 
-loadables:
+loadables: .made
cd $(LOADABLES_DIR) && $(MAKE) $(MFLAGS) all
 
 #newversion:   mkversion
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

loadables/finfo: fix time_t printing

[Christian Weisgerber]

loadables/finfo.c uses the %ld format string to print time_t values.
This is wrong on OpenBSD, where time_t is long long on all platforms.

I suggest %lld and a cast to long long.
Alternatively, %jd and intmax_t could be used.

--- examples/loadables/finfo.c.orig Mon Jun 29 16:56:32 2020
+++ examples/loadables/finfo.c  Thu Dec  8 16:24:34 2022
@@ -328,17 +328,17 @@
if (flags & OPT_ASCII)
printf("%s", ctime(>st_atime));
else
-   printf("%ld\n", st->st_atime);
+   printf("%lld\n", (long long)st->st_atime);
} else if (flags & OPT_MTIME) {
if (flags & OPT_ASCII)
printf("%s", ctime(>st_mtime));
else
-   printf("%ld\n", st->st_mtime);
+   printf("%lld\n", (long long)st->st_mtime);
} else if (flags & OPT_CTIME) {
if (flags & OPT_ASCII)
printf("%s", ctime(>st_ctime));
else
-   printf("%ld\n", st->st_ctime);
+   printf("%lld\n", (long long)st->st_ctime);
} else if (flags & OPT_DEV)
printf("%lu\n", (unsigned long)st->st_dev);
else if (flags & OPT_INO)
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Fix module loading for OpenBSD

[Christian Weisgerber]

Dynamic loading of modules is broken on OpenBSD:

bash-5.2$ enable finfo
bash:/usr/local/lib/bash/finfo: undefined symbol 'sh_optind'
bash:/usr/local/lib/bash/finfo: undefined symbol 'sh_optarg'
bash: enable: finfo: not a shell builtin

This is trivially fixed, configure simply needs to add -rdynamic to
the build flags.

The FreeBSD entry is also bizarrely obsolete.  a.out hasn't been
a thing forever; freebsdelf must have been experimental if it ever
even existed; freebsd[3-9] fails to capture freebsd10 and above--
the oldest supported FreeBSD branch is 12.x right now.  All this
also leaves bash without functioning module loading when built on
a recent FreeBSD.  A simple "freebsd*) LOCAL_LDFLAGS=-rdynamic ;;"
entry would make more sense.


--- configure.ac.orig   Thu Dec  8 16:56:15 2022
+++ configure.acThu Dec  8 16:56:38 2022
@@ -1197,16 +1197,17 @@
 dnl FreeBSD-3.x can have either a.out or ELF
 case "${host_os}" in
 freebsd[[3-9]]*)
if test -x /usr/bin/objformat && test "`/usr/bin/objformat`" = 
"elf" ; then
LOCAL_LDFLAGS=-rdynamic # allow dynamic loading
fi ;;
 freebsdelf*)   LOCAL_LDFLAGS=-rdynamic ;;  # allow dynamic loading
 dragonfly*)LOCAL_LDFLAGS=-rdynamic ;;  # allow dynamic loading
+openbsd*)  LOCAL_LDFLAGS=-rdynamic ;;  # allow dynamic loading
 midnightbsd*)  LOCAL_LDFLAGS=-rdynamic ;;  # allow dynamic loading
 esac
 
 case "$host_cpu" in
 *cray*)LOCAL_CFLAGS="-DCRAY" ;; # shell var so config.h can 
use it
 esac
 
 case "$host_cpu-$host_os" in
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

5.0: CPPFLAGS doesn't propagate to loadables

[Christian Weisgerber]

There is a small omission in bash 5.0's build infrastructure:
The CPPFLAGS variable doesn't propagate to the actual compiler flags
used to build the loadables.  In practice, this means that the seq
loadable will fail to build on operating systems that have libintl
outside the default paths (e.g. OpenBSD with GNU gettext in /usr/local):

cc -fPIC -DHAVE_CONFIG_H -DSHELL -DDEV_FD_STAT_BROKEN -O2 -pipe 
-Wno-parentheses -Wno-format-security -I. -I.. -I../.. -I../../lib 
-I../../builtins -I.  -I../../include -I/usr/obj/bash-5.0.2/bash-5.0 
-I/usr/obj/bash-5.0.2/bash-5.0/lib  -I/usr/obj/bash-5.0.2/bash-5.0/builtins  -c 
-o seq.o seq.c
In file included from seq.c:32:
In file included from ../../bashintl.h:30:
../../include/gettext.h:27:11: fatal error: 'libintl.h' file not found
# include 
  ^~~
1 error generated.

Trivial fix:

Index: examples/loadables/Makefile.in
--- examples/loadables/Makefile.in.orig
+++ examples/loadables/Makefile.in
@@ -76,7 +76,7 @@ INTL_BUILDDIR = ${LIBBUILD}/intl
 INTL_INC = @INTL_INC@
 LIBINTL_H = @LIBINTL_H@
 
-CCFLAGS = $(DEFS) $(LOCAL_DEFS) $(LOCAL_CFLAGS) $(CFLAGS)
+CCFLAGS = $(DEFS) $(LOCAL_DEFS) $(LOCAL_CFLAGS) $(CPPFLAGS) $(CFLAGS)
 
 #
 # These values are generated for configure by ${topdir}/support/shobj-conf.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

bash 5.0 ships with very old autoconf macros

[Christian Weisgerber]

The bash 5.0 release still ships with very old autoconf macros to
detect gettext.  In aclocal.m4, the copy of gettext.m4 and the
supporting lib-link.m4 are from gettext-0.12 dating from 2003.

In particular, the included version of AC_LIB_LINKFLAGS_BODY cannot
detect shared libraries on OpenBSD.  (It checks for *.so; OpenBSD
only has fully numbered libraries: *.so.0.0, etc.)

These macros should be updated to newer versions from a recent
release of gettext.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: examples/loadables/finfo.c type problems

[Christian Weisgerber]

Chet Ramey:

> > printf("%ld\n", (long) st->st_size);
> > 
> > which potentially truncate values.
> 
> Pretty much all the systems bash runs on these days have 64-bit longs.
> How big a file do you have? But the fix is the same as above.

32-bit platforms (IA-32, ARMv7) are still around.  And BSD has had
64-bit off_t on 32-bit architectures for about a quarter century.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

5.0alpha: tests/test1.sub is unportable

[Christian Weisgerber]

I'm not sure what the new tests/test1.sub in bash 5.0alpha is
intended to test, but it fails on OpenBSD because /dev/fd/* are
actual character devices there, so test -p /dev/fd/6 will always
be unsuccessful.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

examples/loadables/finfo.c type problems

[Christian Weisgerber]

Compiling examples/loadables/finfo.c (bash 4.4.23, 5.0-alpha) on
OpenBSD produces various warnings about ill-matched types:

--->
finfo.c:325:20: warning: format specifies type 'long' but the argument has type 
'time_t' (aka 'long long') [-Wformat]
printf("%ld\n", st->st_atime);
~~~ ^~~~
%lld
finfo.c:330:20: warning: format specifies type 'long' but the argument has type 
'time_t' (aka 'long long') [-Wformat]
printf("%ld\n", st->st_mtime);
~~~ ^~~~
%lld
finfo.c:335:20: warning: format specifies type 'long' but the argument has type 
'time_t' (aka 'long long') [-Wformat]
printf("%ld\n", st->st_ctime);
~~~ ^~~~
%lld
finfo.c:339:18: warning: format specifies type 'int' but the argument has type 
'ino_t' (aka 'unsigned long long') [-Wformat]
printf("%d\n", st->st_ino);
~~ ^~
%llu
finfo.c:341:34: warning: format specifies type 'long' but the argument has type 
'ino_t' (aka 'unsigned long long') [-Wformat]
printf("%d:%ld\n", st->st_dev, st->st_ino);
   ~~~ ^~
   %llu
5 warnings generated.
<---

I was thinking about how to fix those, but then I noticed existing bad
casts in the code, e.g.,

printf("%ld\n", (long) st->st_size);

which potentially truncate values.

I don't know if the example loadables are considered to be more
than, well, rough examples, so I'm uncertain if this should even
be considered a problem.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

The loadables are built during install

[Christian Weisgerber]

There is an issue in the build framework of bash 4.4.23 (and 5.0-alpha):
"make all" does not build examples/loadables.
"make install" however recurses into examples/loadables and, since
the loadable modules aren't there, proceeds to build them before
installation.

Shouldn't the ".made" target have "loadables" as a prerequisite?

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: Bash patches format

[Christian Weisgerber]

Marty E. Plummer:

> Maintainers, I'd really like to hear your thoughts on this matter. If
> the diffs are produced as -p1 unified diffs, then downstreams who do
> convert from -p0 context won't have to, and distros who work around it
> won't either.

Speaking in my capacity as the OpenBSD packager for bash, either
way is fine.  We use the upstream patches as provided ("distribution
patches").  These are applied with -p0 by default, but it's utterly
trivial to specify -p1 if necessary.  The choice of context vs.
unified diffs is immaterial; patch(1) handles both formats.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Wrong AC_TRY_COMPILE idiom

[Christian Weisgerber]

I'm forwarding this bug report by Robert Nagy ,
which also concerns bash 4.4:

>
Unbreak autoconf checks with clang by not using nested functions
in the checks.

Someone clearly did not read the autoconf documentation because
using the following functions with a function declaration inside
the body will end up declaring a function inside a function.

- AC_TRY_COMPILE( [], [ int main() { return 0; } ],
- AC_LANG_PROGRAM([[]], [[int main (void) { return 0; }]])],
- AC_TRY_LINK([], [int main (void) { return 0; }],

Result:

int
main ()
{
int main (void) { return 0; }
  ;
  return 0;
}

nested functions is a gcc extension which is not supported by
clang.

test.c:4:17: error: function definition is not allowed here
int main (void) { return 0; }
^
1 error generated.

This causes tests to fail in the configure scripts resulting in
missing compile and link time flags from the builds.

This resulted in weird behaviour of several software, like gnome
hanging completely due to gtk+3 not being built properly.

This change intrudces the following fixes:

- remove int main() declaration from AC_TRY_COMPILE, AC_LANG_PROGRAM, 
AC_TRY_LINK
  as it comes with a declaration already, and people misused them

- change to use AC_LANG_SOURCE when needed in case a complete source block is 
specified

<

Here's the trivial patch for bash 4.4:

--- configure.ac.orig   Wed Sep  7 22:56:28 2016
+++ configure.acMon Sep 25 19:03:03 2017
@@ -808,7 +808,7 @@
 AC_CACHE_VAL(bash_cv_strtold_broken,
[AC_TRY_COMPILE(
[#include ],
-   [int main() { long double r; char *foo, bar; r = strtold(foo, 
);}],
+   [long double r; char *foo, bar; r = strtold(foo, );],
bash_cv_strtold_broken=no, bash_cv_strtold_broken=yes,
[AC_MSG_WARN(cannot check for broken strtold if cross-compiling, 
defaulting to no)])
]

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: 4.4: crash in redir10 test; use after free?

[Christian Weisgerber]

Chet Ramey:

> > Running the bash 4.4 regression test suite on OpenBSD/amd64, I noticed
> > a crash in the redir tests.  Specifically, running redir10.sub with
> > bash 4.4 causes it to die with a bus error most of the time.
> 
> Thanks for the report.  I can't reproduce this,

Here's the backtrace:

#0  0x0d78f3634009 in find_pipeline (pid=11813, alive_only=1,   
jobp=0x7f7f61b4) at jobs.c:1481 
#1  0x0d78f36340f5 in find_process (pid=11813, alive_only=1,
jobp=0x7f7f61b4) at jobs.c:1506
#2  0x0d78f3637c53 in waitchld (wpid=-1, block=0) at jobs.c:3531
#3  0x0d78f363795c in sigchld_handler (sig=20) at jobs.c:3411
#4  
#5  0x0d7b431cc78b in ofree (argpool=0xd7be5448350, p=0xd7b4d731360)
at /usr/src/lib/libc/stdlib/malloc.c:1085
#6  0x0d7b431ccc8b in free (ptr=0xd7b0d3aa3e0)
at /usr/src/lib/libc/stdlib/malloc.c:1416
#7  0x0d78f3633a97 in discard_pipeline (chain=0xd7b0d3aa3e0) at jobs.c:1232
#8  0x0d78f364a3c5 in process_substitute (string=0xd7afbf56490 "echo x",
open_for_read_in_child=0) at subst.c:5812

* In process_substitute(), discard_pipeline(last_procsub_child)
  is called.
* discard_pipeline() frees last_procsub_child.
* free() is interrupted by a signal.
* The signal handler eventually calls find_pipeline(), which accesses
  the just-freed memory last_procsub_child points to.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

4.4: crash in redir10 test; use after free?

[Christian Weisgerber]

Running the bash 4.4 regression test suite on OpenBSD/amd64, I noticed
a crash in the redir tests.  Specifically, running redir10.sub with
bash 4.4 causes it to die with a bus error most of the time.

Program terminated with signal 10, Bus error.

#0  0x1c9ad0634009 in find_pipeline (pid=97028, alive_only=1,
jobp=0x7f7ea514) at jobs.c:1481
1481  if (p->pid == pid && ((alive_only == 0 && PRECYCLED(p) == 0) 
|| PALIVE(p)))
(gdb) p last_procsub_child
$1 = (PROCESS *) 0x1c9d2b698ca0
(gdb) p *last_procsub_child
$2 = {next = 0xdfdfdfdfdfdfdfdf, pid = -538976289, status = -538976289,
  running = -538976289,
  command = 0xdfdfdfdfdfdfdfdf }
(gdb) p /x last_procsub_child->pid 
$3 = 0xdfdfdfdf

This looks like a use after free() since OpenBSD's malloc fills
some of the freed memory with 0xdf.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: Help output has bad indentation

[Christian Weisgerber]

Christian Weisgerber:

 The output of help builtin suffers from various indentation problems.

PS:
I ran *.def through expand(1), which is one way to fix the problem,
but this also reveals that some help texts run over the 80-column
limit when indented by four characters: mapfile, read, test, ...

-- 
Christian naddy Weisgerber  na...@mips.inka.de

Help output has bad indentation

[Christian Weisgerber]

The output of help builtin suffers from various indentation problems.
E.g., an excerpt from help history:

...
  -aappend history lines from this session to the history file
  -nread all history lines not already read from the history file
  -rread the history file and append the contents to the history
list
  -wwrite the current history to the history file
and append them to the history list

  -pperform history expansion on each ARG and display the result
without storing it in the history list
  -sappend the ARGs to the history list as a single entry
...

The problem is that the documentation in builtins/*.def is written
starting in column 1 and includes tab characters, but the help
output is then indented by BASE_INDENT (4) characters.

Affects at least 4.2.x, 4.3.x.
-- 
Christian naddy Weisgerber  na...@mips.inka.de

Re: y.tab.c inclusion within the source tree

[Christian Weisgerber]

Mark Goldfinch:

 Can someone clarify to me why y.tab.c is included within the bash source
 tree if it is generated from parse.y?
 
 If one looks in the FreeBSD ports tree, they're deliberately taking the
 initiative to touch parse.y to ensure that y.tab.c is always rebuilt.

They also have a dependency on the bison port, because parse.y does
not build correctly with FreeBSD's yacc(1).  You end up with a bash
that has broken $(...) parsing.  Same issue on OpenBSD, where the
port doesn't touch parse.y because there is no need to.

 If y.tab.c's timestamp ends up being newer than parse.y,

Why would this happen?

 a patch which (correctly) only patches parse.y,

... will cause parse.y to have a newer timestamp.

-- 
Christian naddy Weisgerber  na...@mips.inka.de

Re: Patch file bash42-049 is broken

[Christian Weisgerber]

Deron Meranda:

 I was wondering if anybody was going to address the problem with 4.2 patch
 49 ?
 
 It is still corrupted on the FTP server.  There are a few lines that appear
 to have been deleted out of the middle of the patch file.

Indeed.

 Not only is there a critical line of code missing, but the the 'patch'
 command will also fail when used with the --fuzz=0 option -- which is
 something that rpmbuild (Fedora, etc) uses.

That's GNU patch.  OpenBSD's patch just fails with it.

-- 
Christian naddy Weisgerber  na...@mips.inka.de

Small markup error in bash.1

[Christian Weisgerber]

This turned up when I compared the output of groff(1) and mandoc(1)
(http://mdocml.bsd.lv/).

--- doc/bash.1.orig Sat Aug  4 21:34:54 2012
+++ doc/bash.1  Sat Aug  4 21:35:13 2012
@@ -2271,7 +2271,7 @@ The value of \fIp\fP determines whether or not the fra
 included.
 .IP
 If this variable is not set, \fBbash\fP acts as if it had the
-value \fB$\(aq\enreal\et%3lR\enuser\et%3lU\ensys\t%3lS\(aq\fP.
+value \fB$\(aq\enreal\et%3lR\enuser\et%3lU\ensys\et%3lS\(aq\fP.
 If the value is null, no timing information is displayed.
 A trailing newline is added when the format string is displayed.
 .PD 0
-- 
Christian naddy Weisgerber  na...@mips.inka.de

Re: $() parsing still broken

[Christian Weisgerber]

Chet Ramey:

 I suppose the only real variable is the revision of bison:

2.4.1 and 2.3 on my FreeBSD and OpenBSD box, respectively.

-- 
Christian naddy Weisgerber  na...@mips.inka.de

Re: $() parsing still broken

[Christian Weisgerber]

Chet Ramey:

  I suppose the only real variable is the revision of bison:
  
  2.4.1 and 2.3 on my FreeBSD and OpenBSD box, respectively.
 
 Try 1.875 and see if the problems go away.

Red herring.  I found the problem, it is embarrassingly stupid, and
Andreas was right.  The fault lies with the official FreeBSD and
OpenBSD ports.

Since bash 4's $() parsing is broken if you build the parser with
BSD yacc, the ports now specify the use of bison by passing YACC=bison
to the configure script.  Running bison -d parse.y creates
parse.tab.c rather than y.tab.c, so y.tab.c is never regenerated,
and the parser fix from patch 001 is effectively not applied.

We just need to use YACC=bison -y.

-- 
Christian naddy Weisgerber  na...@mips.inka.de

4.0 patch 25 breaks with gcc 3.3

[Christian Weisgerber]

Bash-4.0 official patch 25 adds a section that looks to the compiler
like a nested C comment.  Obviously somebody recognized this and
added #if 0 ... #endif around the whole comment.

Alas, GCC 3.3 still errors out:

cc -c  -DHAVE_CONFIG_H -DSHELL -I/usr/local/include -I. -I../.. -I../..
-I../../include -I../../lib   -O2 -pipe glob.c
glob.c:1023:69: missing terminating ' character

Apparently GCC 3.4 and later versions handle this as intended.

[I'm not subscribed to bug-bash.]
-- 
Christian naddy Weisgerber  na...@mips.inka.de