[Toybox] awk (was: strlower() bug)

[enh]

On Wed, Jun 12, 2024 at 4:57 PM Rob Landley <rob at landley.net> wrote:
>
> On 6/11/24 16:56, Ray Gardner wrote:
> > Elliot, thanks for the positive feedback on the docs, but I really
> > wish you and Rob would try the program. I waited a while to see what
> > Rob would have to say. He doesn't seem the sort to be at a loss for
> > words, but ... nothing. Any idea why he's had nothing to say about an
> > awk for toybox?
>
> Why are you asking Elliott? He's in California, I'm currently in Minneapolis, we
> haven't spoken in person since before the pandemic. (I mean, he has my cell
> phone number and can text me, but it doesn't come up much?)

yeah, i make a lot of noise, but i don't have any real power :-)

> Remember the "poke me a week later if I forget"? I consider myself poked,
> somewhat passive-aggressively. :)
>
> I have the tab open, the reason I haven't looked at it yet is A) it's 4500
> lines, B) in a thing I have WAY insufficient existing domain expertise in (but
> multiple bookmarked tutorials and an entire book on somewhere).

(fwiw, there's a second edition just come out. from one-true-awk it
seems like csv support is the main new feature. amusingly one of the
errata for the new edition on https://awk.dev/ is a behavioral
difference between one-true-awk and gawk.)

> I'm currently re-familiarizing myself with the toysh redirection code to fix a
> nasty bug there, which has been a todo item I haven't finished for 3 weeks, and
> in the past week I installed a new laptop with a non-EOL version of devuan, and
> setting that up I found multiple things that the world moving on without me broke.
>
> Sigh, my blog's way behind, just updated it to the 21st, but here's blog
> spoilers which probably do not render properly as html yet and doesn't have
> links to things I mention (the patch mentioned at the end is
> https://landley.net/bin/mkroot/latest/linux-patches/0008-elfcrap.patch and no I
> haven't fixed it yet):
>
> June 7
>
> <p>Stuff's a bit chopped up since I'm straddling two laptops. Still blogging
> from the old one, and the old one has the reasonable battery (I should order
> another battery) so I can't take the new one out to random coffee shop
> yet but only use it plugged in at the desk. So I'm blogging about what I did
> based on a notes.txt file I scp'd over to the old machine.</p>
>
> <p>Package dependencies remain out of control: for some reason "apt-get install
> git" wanted "libperl-error" which is just sad. I'm vaguely annoyed that
> build-essential installed fakeroot and three *-perl packages and so on,
> but that's the cost of using a meta-package somebody else curates.
> (Saying "the following additional packages will be
> installed" and then "the following NEW packages will be installed" with
> the only difference being the second one incldues the package I requested...
> that seems non-optimal, especially when the list is 37 packages long).
>
> <p>The new debain toolchain is hallucinating a warning when I build toybox
> with it, <b>toys/posix/grep.c:211:24: warning: 'regexec0' accessing 8 bytes
> in a region of size 4 [-Wstringop-overflow=]</b>
> and futher <b>note: referencing argument 5 of type 'regmatch_t[0]'</b>.
> This warning is wrong in multiple ways.</p>
>
> <p>First the code's been run under ASAN
> a lot without complaint, and no other toolchain produces this warning: not llvm,
> not gcc, and musl-cross-make has been building the same gcc 12.0 version which
> does NOT produce the warning. Something debian locally patched into its
> "gcc 12.0-14" is producing a warning that vanilla gcc does not produce.
> That makes it a bit suspicious to begin with.</p>
>
> <p>I inspected the code anyway, and argument 5 of the call to regexec0() in
> do_grep() is an 8 byte pointer to a 16 byte structure. There's no "region
> of size 4" to be found. The argument <b>&shoe->m</b> is a pointer
> to an entry of type regmatch_t, and that struct contains two entries of
> regoff_t which is ssize_t which is long, thus 16 bytes on a 64-bit system.
> Even on a 32 bit system, the two of them would still add up to 8 bytes.
> The structure is allocated to its full size. There's nothing wrong with the
> code that I've been able to spot.</p>
>
> <p>I _think_ what might be happening is shoe->m lives in "shoe" which is
> most recently assigned to in the enclosing for() loop via
> <b>shoe = (void *)TT.reg;</b> and TT.reg in the GLOBALS() block is declared as
> <b>struct double_list *reg;</b> because at that level we only care that it's
> a doubly linked list, not what members each list entry has in
> the command-local "struct reg". Except even THAT theory is funky because
> double_list has three pointers: next, prev, and data, each of which is 8 bytes,
> where is it getting size 4? If it was comparing sizeof(*TT.reg) with
> sizeof(*shoe) then shoe->m starts off the end of the smaller struct.
> If the compiler can't keep the types straight then it's not a size 4 issue,
> it's an out of bounds access.</p>
>
> <p>The type of the "shoe" pointer is "struct reg", which has 5 members.
> The argument it's complaining about is a pointer to the 5th member, which
> is indeed a regmatch_t. (And the error is SAYING it's a regmatch_t, which
> is neither 4 nor 8 bytes long, it's 16. Neither the pointer, not the struct,
> nor any member OF that struct, match the constraint it's insisting was
> violated.)</p>
>
> <p>The only place there's a member of size 4 is "int rc", the third member
> of struct reg. And struct double_list only HAS 3 members, and "m" is
> the last member struct reg, so maybe somehow the compiler is confusing
> (struct reg *)shoe->m with (struct reg *)shoe->rc because
> (struct double_list *)TT.reg only has 3 members? The last member of struct reg
> is the 5th member, the last member of struct double_list is the 3rd member,
> and the 3rd member of reg is 4 bytes. (Of course the typecast multiple lines
> previously saying "this is not actually a pointer to that other type, they
> have nothing to do with each other". It would have to bounce off an irrelevant
> historical type AND specially care about "last member" to get this WRONG.)</p>
>
> <p>Dunno. It really seems like a broken warning. I dunno how to squelch it.
> There is no region of size 4 involved in any way with the 5th argument.
> (shoe->rc isn't used as an argument, the return value is assigned to it,
> no pointers involved there). Maybe if I change the prototype of regexec0()
> in lib/lib.h so its 5th argument says regmatch_t *pmatch instead of regmatch_t
> pmatch[] it'll shut up? (It's the same thing! Magic tweak to avoid triggering
> someone else's bug, and that's IF it works. I'm at the wrong laptop to
> check...)</p>
>
> <p>The new debian toolchain also broke gcc/glibc ASAN, complaining (at runtime)
> "ASan runtime does not come first in initial library list; you should
> either link runtime to your application or manually preload it with
> LD_PRELOAD." which is that <a href=27-04-2024>library ordering</a>
> nonsense back to rear its ugly head again and I refuse to humor
> these INSANE ASSHOLES. If LLVM/bionic works without this, then it's
> NOT REQUIED, they're just really bad at it. Notice how the error message
> doesn't don't say which library to LD_PRELOAD if I _did_ want to fix it,
> it just refuses to work where the previous version worked. A clear regression.
> Which I'm late enough in reporting it's a fait accompli, and I'm in the wrong
> for not noticing their fuck-up in a timely manner. Far too late to start making
> a fuss about it now...</p>
>
> <p>(Is a required library not installed? I used "build-essential" instead
> of manually installing gcc and make precisely so it would scoop up that kind
> of nonsense... And it's complaining about library ORDERING, which is not
> supposed to be a thing when dynamic linking.)</p>
>
> <p>And THEN, of course, the 6.10-rc2 kernel broke my libelf removal patch
> (really it's a patch to allow x86-64 to use the frame pointer unwinder
> like EVERY OTHER ARCHITECTURE CAN). But now kconfig is
> saying there's a circular dependency with HAVE_OBJTOOL
> being selected. I do not understand what it's complaining about, but this
> is an error not a warning so the build refuses to build.</p>
>
> <p>Red queen's race. Running to stay in place...</p>