On Mon, 15 Jul 2024, Jakub Jelinek wrote:

> On Mon, Jul 15, 2024 at 09:16:29AM +0200, Richard Biener wrote:
> > > Nick has implemented a new .base64 directive in gas (to be shipped in
> > > the upcoming binutils 2.43; big thanks for that).
> > > See https://sourceware.org/bugzilla/show_bug.cgi?id=31964
> > > 
> > > The following patch adjusts default_elf_asm_output_ascii (i.e.
> > > ASM_OUTPUT_ASCII elfos.h implementation) to use it if it detects binary
> > > data and gas supports it.
> > > 
> > > Without this patch, we emit stuff like:
> > >         .string "\177ELF\002\001\001\003"
> > >         .string ""
> > >         .string ""
> > >         .string ""
> > >         .string ""
> > >         .string ""
> > >         .string ""
> > >         .string ""
> > >         .string "\002"
> > >         .string ">"
> > > ...
> > >         .string "\324\001\236 
> > > 0FS\202\002E\n0@\203\004\005&\202\021\337)\021\203C\020A\300\220I\004\t\b\206(\234\0132l\004b\300\bK\006\220$0\303\020P$\233\211\002D\f"
> > > etc., with this patch more compact
> > >         .base64 
> > > "f0VMRgIBAQMAAAAAAAAAAAIAPgABAAAAABf3AAAAAABAAAAAAAAAAACneB0AAAAAAAAAAEAAOAAOAEAALAArAAYAAAAEAAAAQAAAAAAAAABAAEAAAAAAAEAAQAAAAAAAEAMAAAAAAAAQAwAAAAAAAAgAAAAAAAAAAwAAAAQAAABQAwAAAAAAAFADQAAAAAAAUANAAAAAAAAcAAAAAAAAABwAAAAAAAAAAQAAAAAAAAABAAAABAAAAAAAAAAAAAAAAABAAAAAAAAAAEAAAAAAADBwOQAAAAAAMHA5AAAAAAAAEAAAAAAAAAEAAAAFAAAAAIA5AAAAAAAAgHkAAAAA"
> > >         .base64 
> > > "AACAeQAAAAAAxSSgAgAAAADFJKACAAAAAAAQAAAAAAAAAQAAAAQAAAAAsNkCAAAAAACwGQMAAAAAALAZAwAAAADMtc0AAAAAAMy1zQAAAAAAABAAAAAAAAABAAAABgAAAGhmpwMAAAAAaHbnAwAAAABoducDAAAAAOAMAQAAAAAA4MEeAAAAAAAAEAAAAAAAAAIAAAAGAAAAkH2nAwAAAACQjecDAAAAAJCN5wMAAAAAQAIAAAAAAABAAgAAAAAAAAgAAAAAAAAABAAAAAQAAABwAwAAAAAAAHADQAAAAAAAcANAAAAAAABAAAAAAAAAAEAAAAAAAAAACAAAAAAA"
> > >         .base64 
> > > "AAAEAAAABAAAALADAAAAAAAAsANAAAAAAACwA0AAAAAAACAAAAAAAAAAIAAAAAAAAAAEAAAAAAAAAAcAAAAEAAAAaGanAwAAAABoducDAAAAAGh25wMAAAAAAAAAAAAAAAAQAAAAAAAAAAgAAAAAAAAAU+V0ZAQAAABwAwAAAAAAAHADQAAAAAAAcANAAAAAAABAAAAAAAAAAEAAAAAAAAAACAAAAAAAAABQ5XRkBAAAAAw/WAMAAAAADD+YAwAAAAAMP5gDAAAAAPy7CgAAAAAA/LsKAAAAAAAEAAAAAAAAAFHldGQGAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
> > >         .base64 
> > > "AAAAAAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAUuV0ZAQAAABoZqcDAAAAAGh25wMAAAAAaHbnAwAAAACYGQAAAAAAAJgZAAAAAAAAAQAAAAAAAAAvbGliNjQvbGQtbGludXgteDg2LTY0LnNvLjIAAAAAAAQAAAAwAAAABQAAAEdOVQACgADABAAAAAEAAAAAAAAAAQABwAQAAAAJAAAAAAAAAAIAAcAEAAAAAwAAAAAAAAAEAAAAEAAAAAEAAABHTlUAAAAAAAMAAAACAAAAAAAAAAOAAACsqAAAgS0AAOJWAAAjNwAAXjAAAAAAAAAAAAAAF1gAAHsxAABBBwAA"
> > >         .base64 
> > > "G0kAALGmAACwoAAAAAAAAAAAAACQhAAAAAAAAOw1AACNYgAAAAAAAFQoAAAAAAAAx3UAALZAAAAAAAAAiIUAALGeAABBlAAAWEsAAPmRAACmOgAAAAAAADh3AAAAAAAAlCAAAAAAAABymgAAaosAAMIjAAAKMQAAMkIAADU0AAAAAAAA5ZwAAAAAAAAAAAAAAAAAAFIdAAAIGQAAAAAAAMFbAAAoTQAAGDcAAIRgAAA6HgAAlxwAAAAAAADOlgAAAAAAAEhPAAARiwAAMGgAAOVtAADMFgAAAAAAAAAAAACrjgAAYl4AACZVAAA/HgAAAAAAAAAAAABqPwAAAAAA"
> > > The patch attempts to juggle between readability and compactness, so
> > > if it detects some hunk of the initializer that would be shorter to be
> > > emitted as .string/.ascii directive, it does so, but if it previously
> > > used .base64 directive it switches mode only if there is a 16+ char
> > > ASCII-ish string.
> > > 
> > > On my #embed testcase from yesterday
> > > unsigned char a[] = {
> > > #embed "cc1plus"
> > > };
> > > without this patch it emits 2.4GB of assembly, while with this
> > > patch 649M.
> > > Compile times (trunk, so yes,rtl,extra checking) are:
> > > time ./xgcc -B ./ -S -std=c23 -O2 embed-11.c                              
> > >                                                                           
> > >                                   
> > >                                                                           
> > >                                                                           
> > >                                   
> > > real    0m13.647s                                                         
> > >                                                                           
> > >                                   
> > > user    0m7.157s                                                          
> > >                                                                           
> > >                                   
> > > sys     0m2.597s                                                          
> > >                                                                           
> > >                                   
> > > time ./xgcc -B ./ -c -std=c23 -O2 embed-11.c                              
> > >                                                                           
> > >                                   
> > >                                                                           
> > >                                                                           
> > >                                   
> > > real    0m28.649s                                                         
> > >                                                                           
> > >                                   
> > > user    0m26.653s                                                         
> > >                                                                           
> > >                                   
> > > sys     0m1.958s                                                          
> > >                                                                           
> > >                                   
> > > without the patch and
> > > time ./xgcc -B ./ -S -std=c23 -O2 embed-11.c
> > > 
> > > real      0m4.283s
> > > user      0m2.288s
> > > sys       0m0.859s
> > > time ./xgcc -B ./ -c -std=c23 -O2 embed-11.c
> > > 
> > > real      0m6.888s
> > > user      0m5.876s
> > > sys       0m1.002s
> > > with the patch, so that feels like significant improvement.
> > > The resulting embed-11.o is identical between the two ways of expressing
> > > the mostly binary data in the assembly.  But note that there are portions
> > > like:
> > >         .base64 
> > > "nAAAAAAAAAAvZRcAIgAOAFAzMwEAAAAABgAAAAAAAACEQBgAEgAOAFBHcAIAAAAA7AAAAAAAAAAAX19nbXB6X2dldF9zaQBtcGZyX3NldF9zaV8yZXhwAG1wZnJfY29zaABtcGZyX3RhbmgAbXBmcl9zZXRfbmFuAG1wZnJfc3ViAG1wZnJfdGFuAG1wZnJfc3RydG9mcgBfX2dtcHpfc3ViX3VpAF9fZ21wX2dldF9tZW1vcnlfZnVuY3Rpb25zAF9fZ21wel9zZXRfdWkAbXBmcl9wb3cAX19nbXB6X3N1YgBfX2dtcHpfZml0c19zbG9uZ19wAG1wZnJfYXRh"
> > >         .base64 
> > > "bjIAX19nbXB6X2RpdmV4YWN0AG1wZnJfc2V0X2VtaW4AX19nbXB6X3NldABfX2dtcHpfbXVsAG1wZnJfY2xlYXIAbXBmcl9sb2cAbXBmcl9hdGFuaABfX2dtcHpfc3dhcABtcGZyX2FzaW5oAG1wZnJfYXNpbgBtcGZyX2NsZWFycwBfX2dtcHpfbXVsXzJleHAAX19nbXB6X2FkZG11bABtcGZyX3NpbmgAX19nbXB6X2FkZF91aQBfX2dtcHFfY2xlYXIAX19nbW9uX3N0YXJ0X18AbXBmcl9hY29zAG1wZnJfc2V0X2VtYXgAbXBmcl9jb3MAbXBmcl9zaW4A"
> > >         .string "__gmpz_ui_pow_ui"
> > >         .string "mpfr_get_str"
> > >         .string "mpfr_acosh"
> > >         .string "mpfr_sub_ui"
> > >         .string "__gmpq_set_ui"
> > >         .string "mpfr_set_inf"
> > > ...
> > >         .string "GLIBC_2.14"
> > >         .string "GLIBC_2.11"
> > >         .base64 
> > > "AAABAAIAAQADAAMAAwADAAMAAwAEAAUABgADAAEAAQADAAMABwABAAEAAwADAAMAAwAIAAEAAwADAAEAAwABAAMAAwABAAMAAQADAAMAAwADAAMAAwADAAYAAwADAAEAAQAIAAMAAwADAAMAAwABAAMAAQADAAMAAQABAAEAAwAIAAEAAwADAAEAAwABAAMAAQADAAEABgADAAMAAQAHAAMAAwADAAMAAwABAAMAAQABAAMAAwADAAkAAQABAAEAAwAKAAEAAwADAAMAAQABAAMAAwALAAEAAwADAAEAAQADAAMAAwABAAMAAwABAAEAAwADAAMABwABAAMAAwAB"
> > >         .base64 
> > > "AAEAAwADAAEAAwABAAMAAQADAAMAAwADAAEAAQABAAEAAwADAAMAAQABAAEAAQABAAEAAQADAAMAAwADAAMAAQABAAwAAwADAA0AAwADAAMAAwADAAEAAQADAAMAAQABAAMAAwADAAEAAwADAAEAAwAIAAMAAwADAAMABgABAA4ACwAGAAEAAQADAAEAAQADAAEAAwABAAMAAwABAAEAAwABAAMAAwABAAEAAwADAAMAAwABAAMAAQABAAEAAQABAAMADwABAAMAAQADAAMAAwABAAEAAQAIAAEADAADAAMAAQABAAMAAwADAAEAAQABAAEAAQADAAEAAwADAAEA"
> > >         .base64 
> > > "AwABAAMAAQADAAMAAQABAAEAAwADAAMAAwADAAMAAQADAAMACAAQAA8AAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQABAAEAAQA="
> > > so it isn't all just totally unreadable stuff.
> > > 
> > > Tested on top of the #embed patchset (just to get the above
> > > data) and then with full bootstrap/regtest on top of vanilla trunk,
> > > on x86_64-linux against trunk binutils prepended into $PATH 
> > > (HAVE_GAS_BASE64
> > > is true) and i686-linux (against older binutils, HAVE_GAS_BASE64 not
> > > defined), ok for trunk?
> > 
> > OK.
> > 
> > Do we want to have a command-line switch controlling the use of
> > .base64?  It might be the 16+ char limit is too large for the
> > readability purpose and/or with wide-strings, etc. .base64 is
> > unwanted to some people at least when debugging?  (does gas
> > support sth like .wstring or UTF-8/16/32 input?)  I don't have
> > a good suggestion for the flag name, a quick scan for assembler
> > related options doesn't reveal a nice "pattern";  -f[no-]base64-in-asm?
> 
> There is no directive to emit wide etc. strings, so UCS-2/UTF-16/UTF-32
> are definitely barely readable even now.
> const char8_t a[] = u8"Žluťoučký";
> const char16_t b[] = u"Žluťoučký";
> const char32_t c[] = U"Žluťoučký";
> The first one is
>       .string "\305\275lu\305\245ou\304\215k\303\275"
> so one can with a magnifying glass find the ASCII chars in there,
> the second one
>         .string "}\001l"
>         .string "u"
>         .string "e\001o"
>         .string "u"
>         .string "\r\001k"
>         .string "\375"
>         .string ""
>         .string ""
> on little endian and third
>         .string "}\001"
>         .string ""
>         .string "l"
>         .string ""
>         .string ""
>         .string "u"
>         .string ""
>         .string ""
>         .string "e\001"
>         .string ""
>         .string "o"
>         .string ""
>         .string ""
>         .string "u"
>         .string ""
>         .string ""
>         .string "\r\001"
>         .string ""
>         .string "k"
>         .string ""
>         .string ""
>         .string "\375"
>         .string ""
>         .string ""
>         .string ""
>         .string ""
>         .string ""
>         .string ""
> I think that is simply binary rubbish.

OK, so the "fix" for this would be to have .w8string .w16string and
.w32string (or similar), or even allow .string U"Žluťoučký" directly.
It's unrelated to .base64 which I agree doesn't make the above worse.

> The way the patch is written is that if it sees a limited string (i.e. zero
> terminated) string and it is in itself shorter than base64 encoding, then it
> emits it as such, so
>       .string "foobar"
> should be emitted like that, or even
>       .string "barbaz\305\275foobarbar"
> but once if is not cheaper, it chooses base64 in the ~ 256 char chunks
> including the '\0's too.  The 16 char limit is there just to avoid
>       .base64 "somethingveryloooooooooooooooooooooooooooooooong"
>       .string "I"
>       .base64 "somethingverylongagain"
> which did happen but only with short readable strings that appear at the
> ~ 256 char boundaries, others in the middle of base64 were still emitted
> unreadably.  And it happens only after a previous .base64.

So the overhead is at most '\t.string ""' which is 11 chars, you
save the \0.  If you make the limit 8 chars does the size get any
worse when encoding cc1plus?  How many extra strings get visible
that way and how many of those are gibberish?

> I think best way to read partially binary data is objdump or similar,
> but if people insist otherwise, -fno-base64-data-asm is possible.

We'll see if it pops up as a request.  I do find it quite nice to
have the actual strings better visually separated from binary data
which is worse in the current way with only .string

Richard.

Reply via email to