On Thu, Nov 30, 2023 at 2:29 PM Rob Landley <[email protected]> wrote:
>
> On 11/30/23 14:29, enh wrote:
> > On Thu, Nov 30, 2023 at 11:49 AM Rob Landley <[email protected]> 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.
now you've pushed, i've just run the ToT tests on debian "testing",
and all the tests you had problems with pass (including this one, so i
guess they switched at some point?).
there _is_ still a failure, but it's a real "they did what?!" one:
```
FAIL: readelf -s
echo -ne '' | readelf -s
/tmp/toybox/tests/files/elf/ndk-elf-note-short | sed s/@.*//
--- expected 2023-11-30 14:51:19.769696679 -0800
+++ actual 2023-11-30 14:51:19.773698857 -0800
@@ -3,8 +3,8 @@
Num: Value Size Type Bind Vis Ndx Name
0: 00000000 0 NOTYPE LOCAL DEFAULT UND
1: 00000000 0 FUNC GLOBAL DEFAULT UND __libc_init
- 2: 00000000 0 FUNC GLOBAL DEFAULT UND __stack_chk_fail
- 3: 00000000 0 OBJECT GLOBAL DEFAULT UND __stack_chk_guard
+ 2: 00000000 0 FUNC GLOBAL DEFAULT UND __stack[...]
+ 3: 00000000 0 OBJECT GLOBAL DEFAULT UND __stack[...]
4: 00000000 0 FUNC GLOBAL DEFAULT UND memset
5: 000010d8 12 FUNC GLOBAL DEFAULT 13 __aeabi_memclr
6: 000010d8 12 FUNC GLOBAL DEFAULT 13 __aeabi_memclr4
```
i mean, they had to do _extra work_ to get _worse_ results! there
really is a GNU version of https://www.youtube.com/watch?v=6LHGY33cFiE
waiting to be written...
i'm pretty tempted to toyonly that test with a comment along the lines of
```
# GNU binutils 2.41 deliberately mangles __stack_chk_fail and
# __stack_chk_guard to __stack[...] for both for some reason,
# which is a bug we shouldn't duplicate, so we filter that out.
```
just like we've toyonly-ed a few of the other tests where binutils
actively _lies_ about what's in the file, which is really the last
thing i want my elf dumping tool to do.
alternatively, adding `| grep -v __stack` before the existing sed (and
deleting the two lines from the expected output) lets us keep running
this on the host too, for what that's worth.
> 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
_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net