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

Reply via email to