Aurelien Jarno | 16 Jul 2012 10:56

[PATCH] include/libbb.h: declare messages with ALIGN1

Some messages strings are defined with ALIGN1 in libbb/messages.c
to make sure strings are not aligned and thus to save some bytes. The
corresponding declaration in include/libbb.h should also use ALIGN1,
otherwise the compiler may assume they are aligned and generate wrong
code to access them. This is the case on at least s390x.

Signed-off-by: Aurelien Jarno <aurelien <at> aurel32.net>
---
 include/libbb.h |   36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/include/libbb.h b/include/libbb.h
index 322a28c..f22e58e 100644
--- a/include/libbb.h
+++ b/include/libbb.h
 <at>  <at>  -1612,8 +1612,8  <at>  <at>  unsigned get_cpu_count(void) FAST_FUNC;
 char *percent_decode_in_place(char *str, int strict) FAST_FUNC;

 
-extern const char bb_uuenc_tbl_base64[];
-extern const char bb_uuenc_tbl_std[];
+extern const char bb_uuenc_tbl_base64[] ALIGN1;
+extern const char bb_uuenc_tbl_std[] ALIGN1;
 void bb_uuencode(char *store, const void *s, int length, const char *tbl) FAST_FUNC;
 enum {
 	BASE64_FLAG_UU_STOP = 0x100,
 <at>  <at>  -1694,24 +1694,24  <at>  <at>  extern const char *applet_name;
  * Therefore now we use #defines.
  */
 /* "BusyBox vN.N.N (timestamp or extra_version)" */
(Continue reading)

Michael Tokarev | 16 Jul 2012 16:54
Picon

Re: [PATCH] include/libbb.h: declare messages with ALIGN1

On 16.07.2012 12:56, Aurelien Jarno wrote:
> Some messages strings are defined with ALIGN1 in libbb/messages.c
> to make sure strings are not aligned and thus to save some bytes. The
> corresponding declaration in include/libbb.h should also use ALIGN1,
> otherwise the compiler may assume they are aligned and generate wrong
> code to access them. This is the case on at least s390x.
> 
> Signed-off-by: Aurelien Jarno <aurelien <at> aurel32.net>
> ---
>  include/libbb.h |   36 ++++++++++++++++++------------------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/include/libbb.h b/include/libbb.h
> index 322a28c..f22e58e 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
>  <at>  <at>  -1612,8 +1612,8  <at>  <at>  unsigned get_cpu_count(void) FAST_FUNC;
>  char *percent_decode_in_place(char *str, int strict) FAST_FUNC;
>  
>  
> -extern const char bb_uuenc_tbl_base64[];
> -extern const char bb_uuenc_tbl_std[];
> +extern const char bb_uuenc_tbl_base64[] ALIGN1;
> +extern const char bb_uuenc_tbl_std[] ALIGN1;

I wonder whenever changing all these from arrays to
pointers is a good idea?  Compiler should not assume
any alignments about pointer to char, unlike an array,
I think.

(Continue reading)

Aurelien Jarno | 17 Jul 2012 22:40

Re: [PATCH] include/libbb.h: declare messages with ALIGN1

On Mon, Jul 16, 2012 at 06:54:38PM +0400, Michael Tokarev wrote:
> On 16.07.2012 12:56, Aurelien Jarno wrote:
> > Some messages strings are defined with ALIGN1 in libbb/messages.c
> > to make sure strings are not aligned and thus to save some bytes. The
> > corresponding declaration in include/libbb.h should also use ALIGN1,
> > otherwise the compiler may assume they are aligned and generate wrong
> > code to access them. This is the case on at least s390x.
> > 
> > Signed-off-by: Aurelien Jarno <aurelien <at> aurel32.net>
> > ---
> >  include/libbb.h |   36 ++++++++++++++++++------------------
> >  1 file changed, 18 insertions(+), 18 deletions(-)
> > 
> > diff --git a/include/libbb.h b/include/libbb.h
> > index 322a28c..f22e58e 100644
> > --- a/include/libbb.h
> > +++ b/include/libbb.h
> >  <at>  <at>  -1612,8 +1612,8  <at>  <at>  unsigned get_cpu_count(void) FAST_FUNC;
> >  char *percent_decode_in_place(char *str, int strict) FAST_FUNC;
> >  
> >  
> > -extern const char bb_uuenc_tbl_base64[];
> > -extern const char bb_uuenc_tbl_std[];
> > +extern const char bb_uuenc_tbl_base64[] ALIGN1;
> > +extern const char bb_uuenc_tbl_std[] ALIGN1;
> 
> I wonder whenever changing all these from arrays to
> pointers is a good idea?  Compiler should not assume
> any alignments about pointer to char, unlike an array,
> I think.
(Continue reading)

Denys Vlasenko | 24 Jul 2012 16:30

Re: [PATCH] include/libbb.h: declare messages with ALIGN1

On Mon, Jul 16, 2012 at 10:56 AM, Aurelien Jarno <aurelien <at> aurel32.net> wrote:
> Some messages strings are defined with ALIGN1 in libbb/messages.c
> to make sure strings are not aligned and thus to save some bytes. The
> corresponding declaration in include/libbb.h should also use ALIGN1,
> otherwise the compiler may assume they are aligned and generate wrong
> code to access them. This is the case on at least s390x.

Applied, thanks!

What is the bloatcheck result for this change on s390?

IOW: does ALIGN1 make code bigger on s390?

--

-- 
vda
Aurelien Jarno | 31 Jul 2012 19:32

Re: [PATCH] include/libbb.h: declare messages with ALIGN1

On Tue, Jul 24, 2012 at 04:30:27PM +0200, Denys Vlasenko wrote:
> On Mon, Jul 16, 2012 at 10:56 AM, Aurelien Jarno <aurelien <at> aurel32.net> wrote:
> > Some messages strings are defined with ALIGN1 in libbb/messages.c
> > to make sure strings are not aligned and thus to save some bytes. The
> > corresponding declaration in include/libbb.h should also use ALIGN1,
> > otherwise the compiler may assume they are aligned and generate wrong
> > code to access them. This is the case on at least s390x.
> 
> Applied, thanks!
> 
> What is the bloatcheck result for this change on s390?
> 
> IOW: does ALIGN1 make code bigger on s390?
> 

Yes it does, here are the corresponding bloatcheck results on s390x:

 function                                             old     new   delta
.rodata                                           102702  103334    +632
xrealloc                                              56      68     +12
xmalloc                                               56      68     +12
write_wtmp                                           152     164     +12
validate_tm_time                                      56      68     +12
check_and_close                                      188     200     +12
xstrdup                                               56      62      +6
xsetenv                                               46      52      +6
xputenv                                               42      48      +6
xfdopen_helper                                        78      84      +6
xasprintf                                            116     122      +6
vfork_compressor                                     332     338      +6
(Continue reading)


Gmane