[Toybox] [PATCH] readelf: Fix the section flags handling.

[Rob Landley]

On 11/30/23 14:29, enh wrote:
> On Thu, Nov 30, 2023 at 11:49 AM Rob Landley <rob at landley.net> wrote:
>>
>> On 11/30/23 13:18, enh wrote:
>> >> Design-wise calling for an exact match for a very large block of english text is
>> >> brittle. If spacing changes, the test breaks.
>> >
>> > we already have the `NOSPACE=1` "ignore whitespace differences"
>> > functionality in the readelf tests.
>> >
>> > what's the actual difference in the failure you're seeing?
>>
>> Version skew in the host readelf:
>>
>> echo -ne '' | readelf -SW
>> /home/landley/toybox/toybox/tests/files/elf/ndk-elf-note-shflags | head -32
>> --- expected    2023-11-30 13:44:43.199143754 -0600
>> +++ actual      2023-11-30 13:44:43.203143754 -0600
>> @@ -1,7 +1,7 @@
>>  There are 28 section headers, starting at offset 0xc74:
>>
>>  Section Headers:
>> -  [Nr] Name              Type            Address  Off    Size   ES Flg Lk Inf Al
>> +  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
> 
> okay, that looks like the kind of nonsense you're complaining about :-(
> 
> but why doesn't the existing test on line 34 fail this too? (or do you
> really have a binutils older than any i've used while working on
> toybox readelf?)

I haven't made "test host" pass all the tests yet (there's been version skew
from upgrades that _broke_ some), and I was holding off on that janitorial step
until I upgrade my laptop to current debian.

I applied the patch. Doesn't make the situation _worse_. But in general the
approach of matching a large block of text exactly, some of which is clearly a
matter of opinion, makes me nervous and I'd like to clean it up in future.

Which is design work, I think I'm missing some test plumbing. I could just pipe
to the output through sed/grep again, but there should be an easier syntax.

>>    [ 0]                   NULL            00000000 000000 000000 00      0   0  0
>>    [ 1] .interp           PROGBITS        000001b4 0001b4 000013 00   A  0   0  1
>>    [ 2] .note.android.ident NOTE            000001c8 0001c8 000018 00   A  0   0  4
>> @@ -11,8 +11,8 @@
>>    [ 6] .gnu.version_r    VERNEED         0000026c 00026c 000020 00   A  8   1  4
>>    [ 7] .gnu.hash         GNU_HASH        0000028c 00028c 000018 00   A  4   0  4
>>    [ 8] .dynstr           STRTAB          000002a4 0002a4 000064 00   A  0   0  1
>> -  [ 9] .rel.dyn          ANDROID_REL     00000308 000308 00000d 01   A  4   0  4
>> -  [10] .relr.dyn         RELR            00000318 000318 00000c 04   A  0   0  4
>> +  [ 9] .rel.dyn          LOOS+0x1        00000308 000308 00000d 01   A  4   0  4
>> +  [10] .relr.dyn         00000013: <unknown> 00000318 000318 00000c 04   A  0
> 
> (yeah, that's probably just a binutils too old to recognize RELR.)

Matching new ones instead of old ones is good, but it has a history of changing
enough that new binutils versions are likely to break this exact match _again_
in future, on a similar timescale.

>> 0  4
>>    [11] .ARM.exidx        ARM_EXIDX       00000324 000324 000028 00  AL 14   0  4
>>    [12] .rel.plt          REL             0000034c 00034c 000020 08  AI  4  19  4
>>    [13] .rodata           PROGBITS        0000036c 00036c 000015 01 AMS  0   0  1
>> @@ -24,7 +24,7 @@
>>    [19] .got.plt          PROGBITS        000026a0 0006a0 00001c 00  WA  0   0  4
>>    [20] .data             PROGBITS        000036bc 0006bc 00000c 00  WA  0   0  4
>>    [21] .comment          PROGBITS        00000000 0006c8 0000cc 01  MS  0   0  1
>> -  [22] .ARM.attributes   ATTRIBUTES      00000000 000794 000042 00      0   0  1
>> +  [22] .ARM.attributes   ARM_ATTRIBUTES  00000000 000794 000042 00      0   0  1
> 
> this one's my fault from a recent change --- i left it alone because
> _really_ when you're decoding stuff in that space you need to _also_
> check the architecture, and we don't have anything like that atm, and
> it didn't seem worth it yet.

Exact match is hard. And brittle.

>>    [23] .debug_frame      PROGBITS        00000000 0007d8 00007a 00   C  0   0  4
>>    [24] .symtab           SYMTAB          00000000 000854 000220 10     26  27  4
>>    [25] .shstrtab         STRTAB          00000000 000a74 00010e 00      0   0  1
>> make: *** [.singlemake:809: test_readelf] Error 1
>>
>>
>> (I still haven't switched from devuan bridezilla to devuan deadlock.)
> 
> that might be the difference here, yes.
I may have had a bit of tabsplosion the past few months. Gotta close a zillion
open windows if I don't want the reboot to lose state. I was going to do more of
that this morning, but I'm working on grep instead.

> the other differences you can make go away by just not using an arm32
> ELF file (or possibly even just "not built by the NDK").

The ELF file was submitted as part of the test, I don't chose it, but I don't
mind having that be the test file. My objection is basically test granularity:
we're asking it to recite the declaration of independence exactly word for word
when we just want to grab "life, liberty, and the purfuit of happineff" and make
sure the s's look like f's.

https://www.youtube.com/watch?v=iOOQfGWt8Hc#t=2m20s

> (the ARM_ATTRIBUTES one we might just want to change and admit that
> our decoding of machine-specific SHT_ values is a bit dodgy.)

I don't currently have the domain expertise to evaluate that, and am not
following the tangent just now. I trust your judgement on what's right to do there.

I can adjust the test myself, lemme get grep checked in first. And I think I
finally figured out how to redo the shell backslash newline processing on the
plane and need to finish that. And I need to close all the various open terminal
tabs to the various todo.txt files and close them all so I can reboot the laptop
(and swap in the 16 gig ram sticks and reinstall devuan, might want to get a new
ssd while I'm at it because it's been a few years and they do wear out). And
finish adding the or1k target (and, sigh, maybe riscv64) to mcm/buildall.sh and
mkroot so I can build a root filesystem for my orange pi that I'm willing to
expose to the internet so that can do nightly builds and automated regression
testing. And I really need to cut the now month-overdue toybox release...

There's already a "make all TEST_HOST pass" todo item, this can get revisited
there...

Rob