[Toybox] [landley/toybox] weak symbols hack broke the build (Issue #554)

[Rob Landley]

On 7/2/25 07:22, enh-google wrote:
> enh-google created an issue (landley/toybox#554)
> 
> (was "clang build still broken" on the mailing list, but my mails to the list [and direct to rob] are being bounced.)

Sigh. I submitted _another_ removal request to check.spamhaus.org, and 
also sent a long explanatory message through their contact-center webform.

I should have saved a copy of that message so I can send it to the NEXT 
one. Shipping compiler binaries means malware authors use them, virus 
writers decide that gcc leaking the compiler's build paths into the 
resulting binaries gives them sufficiently unique strings to check for, 
and then it's just:

https://www.akamai.com/blog/security-research/new-rce-botnet-spreads-mirai-via-zero-days

Over and over again...

> bounced mail first:
> 
> On Tue, Jul 1, 2025 at 12:41 PM Rob Landley <[rob at landley.net](mailto:rob at landley.net)> wrote:
>>
>> On 6/30/25 14:46, enh wrote:
>>> sending as a separate thread, so it's obvious that there's new
>>> information here... in particular, note that clang isn't complaining
>>> about _attribute_ ordering, it's complaining that the _definition_ it
>>> already saw doesn't match the redeclaration:
>>
>> It built for me with the android-ndk-r27. I'm confused.
> 
> the ndk is generally behind the current platform clang.

There still isn't a self-contained NDK build I can git pull and run on 
my laptop, is there?

>>> external/toybox/lib/hash.c:309:16: error: attribute declaration must
>>> precede definition [-Werror,-Wignored-attributes]
>>>     309 | __attribute__((__weak__)) void hash_by_name(int fd, char
>>> *name, char *result)
>>>         |                ^
>>
>> Define "precede". The attribute declaration is now the first thing on
>> the line. (That's what commit 17d77a264ab6 changed.)
> 
> note that it explicitly says "declaration" must precede "definition".

It says attribute declaration, not function declaration.

The attribute doesn't go on the prototype. Otherwise you couldn't tell 
WHICH symbol was weak.

>> void blah(int, char *, char *)
>> void blah(int, char *, char *)
>>
>> What differs other than one being weak? I'm not spotting it.
> 
> i think that's exactly the complaint. if you're going to say weak, a
> weak declaration has to come before anything else.

Do you have example syntax I could use? Because I have no idea what the 
compiler wants.

>> Sigh, I can go back to #ifdefs if clang is that buggy,
> 
> tbh, i didn't understand why you made this change in the first place.
> given the two variants, there's always going to be a need to build and
> test both. so is "always building both [and trusting that we throw the
> right one away later]" a big improvement?

I prefer dead code elimination to #ifdefs because with the first I 
regression testing everything each build, and with the second there are 
codepaths I haven't personally tried in who knows how long which may or 
may not still compile.

> ---------------
> 
> but "new shit has come to light". turns out there's at least one version of gcc that can't build this too:
> 
> ```
> % make android_defconfig && make
> ..
> lib/hash.c: At top level:
> lib/hash.c:309:28: error: redefinition of 'hash_by_name'
>    309 | __attribute__((weak)) void hash_by_name(int fd, char *name,
> char *result)
>        |                            ^~~~~~~~~~~~
> lib/hash.c:22:6: note: previous definition of 'hash_by_name' with type
> 'void(int,  char *, char *)'
>     22 | void hash_by_name(int fd, char *name, char *result)
>        |      ^~~~~~~~~~~~
> make: *** [Makefile:18: toybox] Error 1
> % cc --version
> cc (Debian 14.2.0-19+build4) 14.2.0
> Copyright (C) 2024 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> ```

So it cares that they're in two different compilation units. It works 
fine as long as they're NOT in the same file. That seems deeply silly, 
and clearly _is_ a regression from what both gcc and clang were both 
doing this time last year. I still think this is a compiler bug (weak 
means there can be more than one, that's literally ALL it means), but 
that doesn't mean I don't have to work around it.

No wonder musl is using:

extern __typeof(old) new __attribute__((__weak__, __alias__(#old)))

(Yelling "toro!" as the problem charges the red cape and zooms past.)

Except I was trying to do that for crypt and hit the "is old prototyped 
or not" problem in the __typeof(). The alias depends on the prototype of 
variant you're switching to existing, you can't #ifdef or 
__has_include() for prototypes. Here I can probably switch to that, but 
crypt is only _sometimes_ in the headers posix says it's in, so I was 
trying to make the other way work.

 > https://godbolt.org/z/qG5jxbaax

Which compiler version is this? I want to be able to reproduce it so I 
can tell if I've fixed it. That site pops up a "privacy policy" and when 
that's dismissed the URL redirects to just gotbolt.org with no info on 
what the query was, and the tabs say "x86-64 gcc (trunk)" which is... 
current gcc git maybe?