Discussion:
[BUG] du applet prints wrong output for 2GB+ files
(too old to reply)
darken
2016-09-29 18:07:22 UTC
Permalink
The du applet prints a ridiculously large size if the total size exceeds
2GB.

Reproduced on a Nexus5 with Android 6.0, but I don't think this is Android
specific.
It doesn't matter if it's multiple files or a single one.
Could reproduce it by dd'ing a 2GB file full of zeros.

***@hammerhead:/cache # busybox du -hs /sdcard/2GBTestFile
2.5G /sdcard/2GBTestFile
***@hammerhead:/cache # ./toybox-armv6l du -s /sdcard/2GBTestFile
18446744073707954404 /sdcard/2GBTestFile
***@hammerhead:/cache # ./toybox-armv6l du -hs /sdcard/2GBTestFile
16E /sdcard/2GBTestFile
This ticket has more console logs showing the issue.
https://github.com/landley/toybox/issues/51
enh
2016-09-29 18:17:44 UTC
Permalink
works for me on Nexus 9. looks like du.c's TT.total is a signed long,
so on LP32...
Post by darken
The du applet prints a ridiculously large size if the total size exceeds
2GB.
Reproduced on a Nexus5 with Android 6.0, but I don't think this is Android
specific.
It doesn't matter if it's multiple files or a single one.
Could reproduce it by dd'ing a 2GB file full of zeros.
Post by darken
2.5G /sdcard/2GBTestFile
18446744073707954404 /sdcard/2GBTestFile
16E /sdcard/2GBTestFile
This ticket has more console logs showing the issue.
https://github.com/landley/toybox/issues/51
_______________________________________________
Toybox mailing list
http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
enh
2016-09-29 18:22:56 UTC
Permalink
(looks like the real problem is dirtree::extra is long.)
Post by enh
works for me on Nexus 9. looks like du.c's TT.total is a signed long,
so on LP32...
Post by darken
The du applet prints a ridiculously large size if the total size exceeds
2GB.
Reproduced on a Nexus5 with Android 6.0, but I don't think this is Android
specific.
It doesn't matter if it's multiple files or a single one.
Could reproduce it by dd'ing a 2GB file full of zeros.
Post by darken
2.5G /sdcard/2GBTestFile
18446744073707954404 /sdcard/2GBTestFile
16E /sdcard/2GBTestFile
This ticket has more console logs showing the issue.
https://github.com/landley/toybox/issues/51
_______________________________________________
Toybox mailing list
http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
darken
2016-09-29 18:37:10 UTC
Permalink
Why isn't `TT.total` the issue?

I couldn't find `dirtree::extra`, could you link me to that?

A patch would consist of changing one or more `long` into `long long`?
Post by enh
(looks like the real problem is dirtree::extra is long.)
Post by enh
works for me on Nexus 9. looks like du.c's TT.total is a signed long,
so on LP32...
Post by darken
The du applet prints a ridiculously large size if the total size exceeds
2GB.
Reproduced on a Nexus5 with Android 6.0, but I don't think this is
Android
Post by enh
Post by darken
specific.
It doesn't matter if it's multiple files or a single one.
Could reproduce it by dd'ing a 2GB file full of zeros.
Post by darken
2.5G /sdcard/2GBTestFile
18446744073707954404 /sdcard/2GBTestFile
16E /sdcard/2GBTestFile
This ticket has more console logs showing the issue.
https://github.com/landley/toybox/issues/51
_______________________________________________
Toybox mailing list
http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a
reviewer.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
enh
2016-09-29 18:44:18 UTC
Permalink
the attached patch fixes LP32 for me, without obviously breaking
anything. a quick grep didn't show any obvious case where making
dirtree::extra larger would break anything, but rob will know
better...
Post by darken
Why isn't `TT.total` the issue?
I couldn't find `dirtree::extra`, could you link me to that?
A patch would consist of changing one or more `long` into `long long`?
Post by enh
(looks like the real problem is dirtree::extra is long.)
Post by enh
works for me on Nexus 9. looks like du.c's TT.total is a signed long,
so on LP32...
Post by darken
The du applet prints a ridiculously large size if the total size exceeds
2GB.
Reproduced on a Nexus5 with Android 6.0, but I don't think this is Android
specific.
It doesn't matter if it's multiple files or a single one.
Could reproduce it by dd'ing a 2GB file full of zeros.
Post by darken
2.5G /sdcard/2GBTestFile
18446744073707954404 /sdcard/2GBTestFile
16E /sdcard/2GBTestFile
This ticket has more console logs showing the issue.
https://github.com/landley/toybox/issues/51
_______________________________________________
Toybox mailing list
http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
darken
2016-09-30 08:39:59 UTC
Permalink
I can confirm that the patch works.
With the patch there are a few warnings during compilation though, safe to
ignore?

See: https://gist.github.com/d4rken/710cd1ab6265e062aaa2374e7be75452
Post by enh
the attached patch fixes LP32 for me, without obviously breaking
anything. a quick grep didn't show any obvious case where making
dirtree::extra larger would break anything, but rob will know
better...
Post by darken
Why isn't `TT.total` the issue?
I couldn't find `dirtree::extra`, could you link me to that?
A patch would consist of changing one or more `long` into `long long`?
Post by enh
(looks like the real problem is dirtree::extra is long.)
Post by enh
works for me on Nexus 9. looks like du.c's TT.total is a signed long,
so on LP32...
Post by darken
The du applet prints a ridiculously large size if the total size exceeds
2GB.
Reproduced on a Nexus5 with Android 6.0, but I don't think this is Android
specific.
It doesn't matter if it's multiple files or a single one.
Could reproduce it by dd'ing a 2GB file full of zeros.
Post by darken
2.5G /sdcard/2GBTestFile
18446744073707954404 /sdcard/2GBTestFile
16E /sdcard/2GBTestFile
This ticket has more console logs showing the issue.
https://github.com/landley/toybox/issues/51
_______________________________________________
Toybox mailing list
http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a
reviewer.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
enh
2016-09-30 16:31:49 UTC
Permalink
those particular ones are, though it would be better to explicitly
cast to uintptr_t (or use a data/ptr union in struct dirtree) to make
it clear we're doing this on purpose. i assumed rob would have a
strong opinion one way or the other though...
Post by darken
I can confirm that the patch works.
With the patch there are a few warnings during compilation though, safe to
ignore?
See: https://gist.github.com/d4rken/710cd1ab6265e062aaa2374e7be75452
Post by enh
the attached patch fixes LP32 for me, without obviously breaking
anything. a quick grep didn't show any obvious case where making
dirtree::extra larger would break anything, but rob will know
better...
Post by darken
Why isn't `TT.total` the issue?
I couldn't find `dirtree::extra`, could you link me to that?
A patch would consist of changing one or more `long` into `long long`?
Post by enh
(looks like the real problem is dirtree::extra is long.)
Post by enh
works for me on Nexus 9. looks like du.c's TT.total is a signed long,
so on LP32...
Post by darken
The du applet prints a ridiculously large size if the total size exceeds
2GB.
Reproduced on a Nexus5 with Android 6.0, but I don't think this is Android
specific.
It doesn't matter if it's multiple files or a single one.
Could reproduce it by dd'ing a 2GB file full of zeros.
Post by darken
2.5G /sdcard/2GBTestFile
18446744073707954404 /sdcard/2GBTestFile
16E /sdcard/2GBTestFile
This ticket has more console logs showing the issue.
https://github.com/landley/toybox/issues/51
_______________________________________________
Toybox mailing list
http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
Rob Landley
2016-10-01 23:36:34 UTC
Permalink
I do have an opinion, but I was out of comission for a couple days with
eyestrain. (I fell asleep with my glasses on and rolled over them,
bending them all out of shape. Then yesterday I accidentally sat on them
and it bent them _back_. Yay? I should get a spare pair...)

I don't like the warnings. I don't like adding lots of typecasts for the
warnings either. And I don't like intptr_t because the only place it's
NOT long is windows. (It's long on linux, bsd, macos...)

This 32/64 bit problem is basically the same problem with command line
option parsing writing # fields into a long. The field needs to store
pointer and integer data, and the size that does that without gcc
flipping out and spewing warnings everywhere is a long. (I'm also
slightly concerned that LP64 defines sizeof char, short, int, and long,
but only gives a MINIMUM size for long long. In theory that could be 128
bits.)

The original design idea was that if you need more data than fits in
->extra, you malloc more data and store a pointer to it in ->extra. The
problem is, this is a corner case. For 64 bits, long is already the
right size, so any code I add is _just_ added for 32 bit platforms,
which makes me uncomfortable.

But in this case, I didn't mean for du to max out at 2 gigs on 32 bit
systems, I meant for it to max out at 2 _terabytes_ on 32 bit systems,
because 1<<32 blocks * 512 is 2 terabytes. (I did think of this, but got
it wrong. The math needs some typecasts.)

While we're at it, this should be unsigned math given that modern
optimizers are INSANE and think that signed integer math should somehow
give you a different bit pattern than unsigned integer when it
overflows. (Which is 100% wrong.)

Anybody else notice that when C++ developers got ahold of compiler
development and started rewriting C compilers in C++, they've been
trying to turn C into C++? C is a portable assembly language with
minimal complexity between the developer and what the hardware is
actually doing, and C++ is a minefield of insane corner cases that make
no sense and have to be memorized. They're using the optimizers to try
to take away C's advantage of C++ (which is hilarious because that's
just the tip of the iceberg). They are wrong and the fix is to unbreak
the darn compiler, either with -fno-being-stupid or in extreme cases -O0.

(And no I'm not switching dirtree->extra to unsigned because then
storing filehandles in it gets funky.)

Rob
Post by enh
those particular ones are, though it would be better to explicitly
cast to uintptr_t (or use a data/ptr union in struct dirtree) to make
it clear we're doing this on purpose. i assumed rob would have a
strong opinion one way or the other though...
Post by darken
I can confirm that the patch works.
With the patch there are a few warnings during compilation though, safe to
ignore?
See: https://gist.github.com/d4rken/710cd1ab6265e062aaa2374e7be75452
Post by enh
the attached patch fixes LP32 for me, without obviously breaking
anything. a quick grep didn't show any obvious case where making
dirtree::extra larger would break anything, but rob will know
better...
Post by darken
Why isn't `TT.total` the issue?
I couldn't find `dirtree::extra`, could you link me to that?
A patch would consist of changing one or more `long` into `long long`?
Post by enh
(looks like the real problem is dirtree::extra is long.)
Post by enh
works for me on Nexus 9. looks like du.c's TT.total is a signed long,
so on LP32...
Post by darken
The du applet prints a ridiculously large size if the total size exceeds
2GB.
Reproduced on a Nexus5 with Android 6.0, but I don't think this is Android
specific.
It doesn't matter if it's multiple files or a single one.
Could reproduce it by dd'ing a 2GB file full of zeros.
Post by darken
2.5G /sdcard/2GBTestFile
18446744073707954404 /sdcard/2GBTestFile
16E /sdcard/2GBTestFile
This ticket has more console logs showing the issue.
https://github.com/landley/toybox/issues/51
_______________________________________________
Toybox mailing list
http://lists.landley.net/listinfo.cgi/toybox-landley.net
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
--
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
Loading...