dc hitting a compiler bug, or undefined behavior
Hi, I'm seeing busybox dc acting funny when compiled with some versions of gcc. This is on busybox git. The x86 binary busybox_unstripped and config are attached. gcc 4.2.2 - ok gcc 4.7.2: nc 10 1 add p 2.738e+93 So either bb is hitting a compiler bug, or undefined behavior somewhere with new gcc's more aggressive optimizations. - Lauri -- http://www.fastmail.fm - Email service worth paying for. Try it for free busybox_unstripped.gz Description: GNU Zip compressed data bb-config.gz Description: GNU Zip compressed data ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: dc hitting a compiler bug, or undefined behavior
Lauri Kasanen wrote: I'm seeing busybox dc acting funny when compiled with some versions of gcc. This is on busybox git. The x86 binary busybox_unstripped and config are attached. gcc 4.2.2 - ok gcc 4.7.2: nc 10 1 add p 2.738e+93 So either bb is hitting a compiler bug, or undefined behavior somewhere with new gcc's more aggressive optimizations. I have 4.7.2 on x86 and I get 11 as output. You could add debug output to push and pop to see what happens. Whether the output 11 is to be expected is another question: $ dc --version dc (GNU bc 1.06.95) 1.3.95 $ dc --help Usage: dc [OPTION] [file ...] -e, --expression=EXPRevaluate expression -f, --file=FILE evaluate contents of file -h, --help display this help and exit -V, --versionoutput version information and exit $ dc 10 1 add p dc: Could not open file 10 dc: Could not open file 1 dc: Could not open file add dc: Could not open file p $ dc -e '10 1 add p' $ dc -e '10 1 + p' 11 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: dc hitting a compiler bug, or undefined behavior
On Sun, Mar 30, 2014, at 15:05, Ralf Friedl wrote: I'm seeing busybox dc acting funny when compiled with some versions of gcc. This is on busybox git. The x86 binary busybox_unstripped and config are attached. gcc 4.2.2 - ok gcc 4.7.2: nc 10 1 add p 2.738e+93 So either bb is hitting a compiler bug, or undefined behavior somewhere with new gcc's more aggressive optimizations. I have 4.7.2 on x86 and I get 11 as output. You could add debug output to push and pop to see what happens. I think it's related to http://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html which hit the radeon driver as well http://lists.freedesktop.org/archives/dri-devel/2013-August/044122.html but changing the stack[1] to stack[0], stack[], or even stack[10] did not fix the issue. What's even worse is that adding any output to push(), even a puts(hi) that does not print the argument or any of the stack vars, fixes it. So something magic is going on inside the GCC optimization, I'm afraid this is above my pay grade. FWIW printf calls in pop() or any other functions did not affect the calculation. push() is being given the correct argument after strtod. - Lauri -- http://www.fastmail.fm - The way an email service should be ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: dc hitting a compiler bug, or undefined behavior
Lauri Kasanen schrieb: On Sun, Mar 30, 2014, at 15:05, Ralf Friedl wrote: I'm seeing busybox dc acting funny when compiled with some versions of gcc. This is on busybox git. The x86 binary busybox_unstripped and config are attached. gcc 4.2.2 - ok gcc 4.7.2: nc 10 1 add p 2.738e+93 So either bb is hitting a compiler bug, or undefined behavior somewhere with new gcc's more aggressive optimizations. I have 4.7.2 on x86 and I get 11 as output. You could add debug output to push and pop to see what happens. I think it's related to http://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html which hit the radeon driver as well http://lists.freedesktop.org/archives/dri-devel/2013-August/044122.html but changing the stack[1] to stack[0], stack[], or even stack[10] did not fix the issue. If the change didn't fix it, then maybe it's not related to that. What's even worse is that adding any output to push(), even a puts(hi) that does not print the argument or any of the stack vars, fixes it. So something magic is going on inside the GCC optimization, I'm afraid this is above my pay grade. Could you send the file miscutils/dc.o that is created with and without this puts(hi) in push()? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: dc hitting a compiler bug, or undefined behavior
On Sun, Mar 30, 2014, at 18:26, Ralf Friedl wrote: What's even worse is that adding any output to push(), even a puts(hi) that does not print the argument or any of the stack vars, fixes it. So something magic is going on inside the GCC optimization, I'm afraid this is above my pay grade. Could you send the file miscutils/dc.o that is created with and without this puts(hi) in push()? Attached. - Lauri -- http://www.fastmail.fm - Email service worth paying for. Try it for free fail-dc.o.gz Description: GNU Zip compressed data success-dc.o.gz Description: GNU Zip compressed data ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH v1] Size reduction, cleanup, fixes, improvements for swapon/swapoff
Hi to all,
this patch started as a personal fun and learning project on swapon/swapoff
after
Matt Whitlock's patches. I was curios if there was a way to remove all the
#ifdefs
from within the code as for my personal taste this unindented stuff in indented
code makes the code harder to read.
So I tried hard by using the preprocessor and compilers dead code elimination
and in the end I was able to achieve my goal: a ifdef free code.
During the process as side effects I spotted a few differences between
our swapon/swapoff inplementation and the one on my debian box
so I added this fixes/improvements to the code:
1) real swapon/swapoff handles also devices on the commandline with -a;
2) xstat(device) in swap_enable_disable aborts on error when cycling through
fstab so some devices are not handled;
3) duplicated code for ENABLE_FEATURE_SWAPON_DISCARD and
ENABLE_FEATURE_SWAPON_PRI was moved to functions.
4) silence some error messages with -a;
5) minor cleanups and code refactoring reduced the size as per bloat-check:
./scripts/bloat-o-meter busybox_unstripped.old busybox_unstripped
function old new delta
set_discard_flag - 106+106
set_priority_flag - 75 +75
swap_enable_disable 149 219 +70
.rodata 141270 141260 -10
swap_on_off_main 638 325-313
--
(add/remove: 2/0 grow/shrink: 1/2 up/down: 251/-323) Total: -72 bytes
6) I also added support for /proc/swaps handling to swapoff:
When the -a flag is given, swapping is disabled on all known swap devices
and files (as found in /proc/swaps or /etc/fstab).
So now swapoff first cycles through /proc/swaps and then through fstab
to swapoff all devices.
With this additional feature bloat-check is:
./scripts/bloat-o-meter busybox_unstripped.old busybox_unstripped
function old new delta
set_discard_flag - 106+106
set_priority_flag - 75 +75
swap_enable_disable 149 219 +70
.rodata 141270 141272 +2
swap_on_off_main 638 416-222
--
(add/remove: 2/0 grow/shrink: 2/1 up/down: 253/-222) Total: 31 bytes
so it comes at almost no extra size cost.
I've decided not to add an extra config option for that, but it could
be easily added if needed.
7) Something I'm forgettin...;
Sadly I cannot provide a patch series for the different changes but
only a allinone patch, therefore I also attach a drop-in replacement
file for swaponoff.c for review and testing by the list members.
I've tested it a little on my box with a few test cases I could think of,
but more testing is needed as there is a lot of code changed and bugs
can creep in.
Hint, critics and improvements are welcome.
Ciao,
Tito
Swapon/swapoff fixes, enhacements and size reduction:
1) real swapon/swapoff handles also devices on the commandline with -a;
2) xstat(device) in swap_enable_disable aborts on error when cycling through
fstab so some devices are not handled;
3) duplicated code for ENABLE_FEATURE_SWAPON_DISCARD and
ENABLE_FEATURE_SWAPON_PRI was moved to functions.
4) silence some error messages with -a;
5) minor cleanups and code refactoring to reduce binary size.
6) added support for /proc/swaps handling to swapoff.
Signed-off by Tito Ragusa farmat...@tiscali.it
--- util-linux/swaponoff.c.orig 2014-03-26 07:46:29.263718690 +0100
+++ util-linux/swaponoff.c 2014-03-30 21:49:11.642492640 +0200
@@ -63,90 +63,142 @@
} FIX_ALIASING;
#define G (*(struct globals*)bb_common_bufsiz1)
#define g_flags (G.flags)
+#define save_g_flags() int save_g_flags = g_flags
+#define restore_g_flags() g_flags = save_g_flags
#else
#define g_flags 0
+#define save_g_flags()((void)0)
+#define restore_g_flags() ((void)0)
#endif
#define INIT_G() do { } while (0)
+#define do_swapoff (applet_name[5] == 'f')
+
+/* Command line options */
+enum {
+ OPTBIT_a, /* -a all */
+ IF_FEATURE_SWAPON_DISCARD( OPTBIT_d ,) /* -d discard */
+ IF_FEATURE_SWAPON_PRI( OPTBIT_p ,) /* -p priority */
+ OPT_a = 1 OPTBIT_a,
+ OPT_d = IF_FEATURE_SWAPON_DISCARD((1 OPTBIT_d)) + 0,
+ OPT_p = IF_FEATURE_SWAPON_PRI((1 OPTBIT_p)) + 0,
+};
+
+#define OPT_ALL (option_mask32 OPT_a)
+#define OPT_DISCARD (option_mask32 OPT_d)
+#define OPT_PRIO(option_mask32 OPT_p)
+
static int swap_enable_disable(char *device)
[PATCH] Avoid double argument evaluation in MIN and MAX macros
Commit c3a27b0b fixes a double argument evaluation by modifying the
macro invocation. What about preventing it on macro definition level?
Signed-off-by: Bartosz Golaszewski bartekg...@gmail.com
---
include/libbb.h| 22 +++---
util-linux/swaponoff.c | 5 ++---
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/include/libbb.h b/include/libbb.h
index 1cbe2c8..db75641 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -265,13 +265,21 @@ struct BUG_off_t_size_is_misdetected {
#define SKIP ((int) 2)
/* Macros for min/max. */
-#ifndef MIN
-#define MIN(a,b) (((a)(b))?(a):(b))
-#endif
-
-#ifndef MAX
-#define MAX(a,b) (((a)(b))?(a):(b))
-#endif
+#undef MIN
+#define MIN(a,b) \
+ ({ \
+ __typeof__(a) _a = (a); \
+ __typeof__(b) _b = (b); \
+ _a _b ? _a : _b; \
+ })
+
+#undef MAX
+#define MAX(a,b) \
+ ({ \
+ __typeof__(a) _a = (a); \
+ __typeof__(b) _b = (b); \
+ _a _b ? _a : _b; \
+ })
/* buffer allocation schemes */
#if ENABLE_FEATURE_BUFFERS_GO_ON_STACK
diff --git a/util-linux/swaponoff.c b/util-linux/swaponoff.c
index a7ad6db..b3d265f 100644
--- a/util-linux/swaponoff.c
+++ b/util-linux/swaponoff.c
@@ -137,11 +137,10 @@ static int do_em_all(void)
p = hasmntopt(m, pri);
if (p) {
/* Max allowed 32767 (==
SWAP_FLAG_PRIO_MASK) */
- unsigned prio = bb_strtou(p + 4, NULL,
10);
+ unsigned prio = MIN(bb_strtou(p + 4,
NULL, 10), SWAP_FLAG_PRIO_MASK);
/* We want to allow ,foo, thus
errno == EINVAL is allowed too */
if (errno != ERANGE) {
- g_flags = (g_flags
~SWAP_FLAG_PRIO_MASK) | SWAP_FLAG_PREFER |
- MIN(prio,
SWAP_FLAG_PRIO_MASK);
+ g_flags = (g_flags
~SWAP_FLAG_PRIO_MASK) | SWAP_FLAG_PREFER | prio;
}
}
#endif
--
1.8.4.5
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: dc hitting a compiler bug, or undefined behavior
Lauri Kasanen wrote: On Sun, Mar 30, 2014, at 18:26, Ralf Friedl wrote: What's even worse is that adding any output to push(), even a puts(hi) that does not print the argument or any of the stack vars, fixes it. So something magic is going on inside the GCC optimization, I'm afraid this is above my pay grade. Could you send the file miscutils/dc.o that is created with and without this puts(hi) in push()? Attached. Are you using some special compiler options, especially regarding parameter passing in registers and stack alignment? The relevant part of fail-dc.o is this: push: 0: dd 07 fldl (%edi) The function expects the value to push at the address pointed to by %edi. But the functions that call push pass the value at the top of the CPU stack (not to be confused with the stack dc implements). The relevant part ofsuccess-dc.o is this: push: 0: 57 push %edi 1: 8d 7c 24 08 lea0x8(%esp),%edi 5: 83 e4 f0and$0xfff0,%esp 8: ff 77 fcpushl -0x4(%edi) b: 55 push %ebp c: 89 e5 mov%esp,%ebp e: 57 push %edi f: 83 ec 14sub$0x14,%esp 12: dd 07 fldl (%edi) These lines set up an aligned stack. 0: save %edi 1: put address of top of stack at the time the function was called in %edi. This is the address of the parameter. 5: align stack to 0x10 boundary 8: push return address of the function b, c: normal frame setup e: save %edi for later use f: make space for a double and align to 0x10 12: load parameter, %edi still points to the address of the parameter. The instruction at 12 loads the double from address %edi after %edi has been set to point to the parameter area. The instruction at 0 in the failed case is exactly the same, except that %edi has not been setup before. So I would consider this a compiler bug. I wrote that the instruction at f makes space for an aligned double. This is itself is strange because later on the double that is loaded from %edi is saved on the CPU stack and later loaded from the CPU stack and saved in the dc stack, which is unnecessary. Also the double is always loaded to the FPU stack and then removed if bb_error_msg_and_die is called, instead of loading it only after it is clear that it will be used. So there is also opportunity for further optimization of the compiler. This stack alignment makes your code bigger, and the additional instructions also have to be executed, which also takes time. I'm not sure whether the aligned stack saves enough time to offset this. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
