[Toybox] [PATCH] in seq.c rename itoa to toybox_itoa, to avoid clashes

[enh]

On Sat, Feb 19, 2022 at 2:59 AM Rob Landley <rob at landley.net> wrote:

> On 2/17/22 12:05 PM, enh via Toybox wrote:
> > On Thu, Feb 17, 2022 at 5:23 AM John wrote:
> > > This is a simple patch for seq.c to rename itoa to toybox_itoa, to
> avoid
> > > clashes with functions in the toolchain.
> >
> > > As Piraty mentioned on IIRC there are 2 different implementations of
> itoa in
> > > musl alone, so this should avoid any clashes people have with
> > > implementations in their toolchain.
> >
> > the two musl itoa functions are both static, so there's no possibility
> of a
> > clash (otherwise musl itself would have a problem internally!).
> >
> > what was the actual issue you were seeing?
>
> I've been waiting for the reply to Elliott's question myself. I'm happy to
> apply
> a fix for an actual bug, but not a purely theoretical "maybe someday" bug.
>
> But even if it did happen (which seems especially unlikely in musl since
> Rich is
> scrupulous about namespace pollution), in theory a static function declared
> earlier in the same compilation unit is going to bind before anything
> outside
> even if some library does have its own version of it. Which means all I'd
> really
> have to worry about is header namespace polution (#including a header with
> the
> conflicting declaration):
>
> $ cat boing.c
> #include <stdio.h>
>
> int exit(char *str)
> {
>   return printf("string was %s\n", str);
> }
>
> int main(int argc, char *argv[])
> {
>   int x = exit("potato");
>
>   printf("and it returned %d\n", x);
>
>   return 0;
> }
> $ gcc boing.c
> boing.c:3:5: warning: conflicting types for built-in function ‘exit’
> [-Wbuiltin-declaration-mismatch]
>  int exit(char *str)
>      ^~~~
> $ ./a.out
> string was potato
> and it returned 18
>
> In practice modern compilers are polluting their own namespaces all over
> the
> place, but the option to tell it to stop is "-fno-builtin". and even
> without
> adding that the gcc version works fine.
>
> Of course the Android NDK version calls my builtin exit() twice and then
> segfaults:
>
> $ ./a.out
> string was potato
> and it returned 18
> string was (null)
> error: TLS segment alignment in " is not a power of 2: 1
>
> libc: error: TLS segment alignment in " is not a power of 2: 1
>
> Aborted
>
> But I'm guessing that's because bionic's post-main() code that gets
> returned to
> tries to call exit() out of libc and apparently binds to mine if I
> redefine it.
> (Seriously, I break everything. And I point out it wasn't MY code
> deviating from
> expected behavior, it was android's...


define "expected". this is mostly WAI.

"WAI" because interposing random libc functions is definitely something we
support (for asan etc), but only "mostly" because i'm not sure whether we
have a sanitizer that actually needs to interpose exit() or not.

long ago, we -Bsymbolic'ed everything, but that breaks interposing.

in practice, i find "connect" is the function that confuses the most
people. i've yet to see anyone _want_ to call their own function "exit" :-)

this might matter more on other systems, but on Android where all apps on
user's devices are run by forking from the zygote anyway, you can't be
bitten by what i call the "executable always wins" rule of dynamic linking
because app_process is your exectuable, and we can just not make that
mistake there (and/or deliberately interpose; things like ndk_translation's
tricks to run Android arm binaries on glibc x86 systems, say, or ART's
trickery with chaining signal handlers).


> I want to say crtend.a but they don't
> seem to have one? Only .so versions. Eh, it's successfully linking and
> running
> as a static binary (which I still have to do because I haven't installed
> bionic
> on the host) and I'm not running it under strace to try to find out where
> it's
> pulling this stuff from...)
>

a static binary avoids these kinds of questions.


> However, if I make my new exit() static and feed the compiler
> -fno-builtin, then
> the NDK behaves the same way gcc did. (And oddly enough for a _static_
> function
> clang both works properly and doesn't warn even without -fno-builtin,
> while gcc
> still warns about the static function not matching its expectations. Go
> figure.)
>
> Anyway, the point is that even in this contrived case (where I'm fighting
> compiler builtins, which itoa() is not likely to become any time soon), the
> static function defined before the other function should work fine. Yes
> even in
> the presence of a library that has its own function with the same name I
> use in
> my code.
>
> This is part of the reason the compiler puts -lc at the end. I had to get
> that
> right back when I had a wrapper to rewrite the gcc command line to start
> with
> --nostdinc --notstdlib and then add everything _back_ to link against
> uClibc
> portability in a compiler that you could extract into an arbitrary
> directory and
> add to the $PATH from there, defeating gcc's horrible "all my resources
> live at
> an absolute path determined at compile time" logic:
>
> https://github.com/landley/aboriginal/blob/master/sources/toys/ccwrap.c
>
> The -lc goes after all the .o files to ensure local references are
> satisfied
> first, but stuff resolved in the same compilation unit should be fine
> either way.
>
> Rob
>
> P.S. I was extremely annoyed as a teenager when I upgraded my turbo c++
> version
> and suddenly the function throw() in the bbs software I'd written stopped
> working because the language committee had decided to make that a keyword.
> Since
> then I maintained the uClibc toolchain using the above wrapper for a
> decade or
> so, maintained my own tinycc fork for 3 years, helped get dynamic linking
> working on a new Linux port (qualcomm heaxagon: they had static linking
> working
> already but building x11 with static linking was just painful: yes you CAN
> stick
> debug printfs into uClibc's dynamic loader before it's relocated itself,
> if by
> "printf" you mean a macro that uses a char array on the stack as the
> buffer it's
> passing to an open coded write() system call), learned all SORTS of fun
> nommu
> weirdness about ... These days toybox has a header that #includes all the
> other
> standard headers out of posix and and common bsd ancestry, and then if some
> build environment gets weird I hit it with a rock via #ifdefs and #defines
> in
> portability.h. Anything CAN break, at any time. The way to deal with sharp
> edges
> is to find them and file them off.
>
> P.P.S. "theoretically correct" and "works properly" are two different
> things,
> which sometimes require years of repeatedly explaining at maintainers in
> extensive detail:
>
>
> https://lore.kernel.org/qemu-devel/4B73F8CC.6010406@suse.de/t/#md050a8a3df649c375e0ebcd74273f72ff2bf9653
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20220223/80e648d3/attachment-0001.htm>