[Toybox] [PATCH] Fix devmem build with clang.

[enh]

On Fri, Nov 8, 2024 at 5:56 PM Rob Landley <rob at landley.net> wrote:

> On 11/8/24 08:10, enh wrote:
> > hmm... thinking about this more we don't actually need this condition
> >
> >        else if (sizeof(long)==8 && bytes==8) data = *(unsigned long *)p;
> >
> > because we already check that we don't let you set bytes to 8 if you're
> on
> > ILP32 (which is good, because _that_ would be a bug).
> >
> > so we can just write
> >
> >        else data = *(unsigned long *)p;
>
> It was a compile time optimization to let dead code elimination yank a
> tiny amount of code because sizeof(long) is a compile type constant. I
> mostly don't do that level of micromanagement anymore because busybox
> exists and compilers sometimes get smarter. Well, they change their
> optimizations around a lot. Somewhat randomly. And get lucky sometimes.
>
> > here (and in the other "copy" of this if above) and make this go away
> with
> > less code? if we do that, we can remove the QUIET from data completely
> > (because at least with `make defconfig` gcc 13 doesn't complain about
> this
> > code at all).
>
> I already applied the patch.
>
> And I should try the "commit to a branch then delete the branch" trick
> to close https://github.com/landley/toybox/issues/520 now I've lost web
> login access.
>
> I hadn't gotten around to that until you poked me because last I checked
> LLVM was setting __gnuc__ so this didn't bite in NDK testing, and I
> thought they were talking about qnx or something. Which I should deal
> with before cutting a release but isn't otherwise time critical because
> I don't have a test environment for it so never know when I break it (or
> fix it)...
>
> I'm guessing one of the updates finally yanked that define?
>

no, clang has defined __GNUC__ since before Android switched from gcc. i
think this is just the first time that clang has agreed with gcc about the
questionability of a piece of code. (there are very few QUIETs in the tree
still.)


> > and if we do _that_, then i wonder whether we should keep clang on the
> > "no-op QUIET" path until/unless we really do need it, to force us to
> > continue to look at and think about these things? there are very few uses
> > of QUIET, and it does seem like something we should avoid...
>
> It would be nice if gcc would just stop producing unsilenceable false
> positives because it can't understand the relationship between two
> variables. Alas, 7 year time horizon also means building with older gcc,
> and a LOT of cross compilers aren't the latest and greatest because
> updating just causes pain when upstream doesn't know you exist and isn't
> happy to hear from you...
>

to be fair, this one is pretty complicated --- you have to understand a
string literal amongst other things. (which is probably why this is the
first one where even clang says "i dunno about this...".)


> Anyway, commit 542f62759a779
>

thanks! i've updated AOSP and the tests are passing.


> Rob
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20241112/cf1f7a80/attachment.htm>