-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
archutil: refactor generation and validation #4607
Conversation
util/archutil/amd64_binary.go
Outdated
// Do not edit manually. | ||
|
||
const Binaryamd64 = "\x1f\x8b\x08\x00\x00\x00\x00\x00\x02\xff\xec\x98\x3f\x8b\x13\x41\x18\xc6\x9f\xd9\x6c\xee\x0f\x08\xb9\xe0\x81\xc2\x59\xa8\x58\xd8\xb8\xe1\x54\x38\x05\x95\x55\x50\x07\xf1\xec\xce\xd2\x65\x93\x5d\xce\x80\x97\x5b\xb2\xb3\xe7\x0a\x07\x77\x47\xbe\x83\x58\x07\x14\x0b\x4b\xc1\x94\xa9\x5c\x2c\x6d\xac\x4d\x21\x04\xad\xec\x0c\x28\xca\x6e\xde\x49\xb2\x63\xa2\x82\xdd\x31\x3f\x08\xcf\xbc\xcf\xbe\x33\xef\x90\xa4\xd9\x67\xef\xe6\xdd\x5b\x06\x63\x90\x18\xb8\x86\xac\x5a\xb2\xb3\xda\x26\xff\x63\x79\xd4\x02\x1b\x97\x50\x80\x8d\x39\x14\xb3\x5e\x13\x93\xd8\x39\x3d\x42\x47\x4b\xc5\xd2\x50\xd2\xb2\x38\x51\xcb\x79\x52\x03\xb2\xa5\xca\x3e\x93\x3e\x7d\xb2\xfb\x34\x47\xea\x19\xf2\xa5\x9a\x13\xba\x0c\xa0\x00\xe0\xf6\xbd\x0d\xf8\xcf\xdf\x5c\xfe\xf0\xf2\xd3\xfd\xf5\x1f\x4f\x8f\x7e\x7f\xf1\x79\xef\xed\x85\xf7\xaf\xa1\xd1\x68\x34\x1a\x8d\x46\xa3\xd1\x68\x34\x1a\xcd\x21\x85\xaf\x76\x4b\x6d\xde\xfa\x36\xbf\x7b\x87\x77\x00\xec\xa7\x66\xa9\x7d\x95\x01\xfb\xbb\x6b\x3c\xe9\xb2\xac\xe9\x5d\xa9\xcd\x0f\x7a\xec\xf4\x33\xf0\x83\x41\x2a\xd1\x0a\xef\x30\x6a\x1f\x3e\x6e\xf5\x18\x6f\x0d\x58\x64\x7c\x59\xe4\x49\x72\x1d\x40\xb6\xb8\x91\x2e\xc0\x93\xee\x95\xf4\xe0\xe2\x9f\xee\x52\x00\x1b\xbd\xc7\xe7\x7d\x63\x9c\x0f\x60\x9c\x1f\x98\xf8\xfa\x53\xed\x2d\x53\x8a\xb1\xa1\xf4\x2f\x93\xff\x40\xf1\x8f\x91\xbf\xa9\xf8\xa7\xb2\xc8\xe1\xf7\xb9\x27\xa4\x7f\x32\xef\x9f\x9d\xe1\x57\x66\xf8\xa8\x88\xad\xa0\x52\xab\xc5\xb5\xb5\xc7\xcd\x75\x6b\x1b\x3b\xab\xd8\x39\x0f\x3f\xae\x0b\x38\x4e\x35\x0c\x9d\x50\xb8\x4d\x01\xc7\xf7\x5c\xe1\xc2\xf1\x1b\x1e\x60\x85\x4f\xb6\x84\x5b\x85\x15\x8a\xe6\x50\x1f\xca\x55\x63\x5b\xf8\xd6\x66\x23\xb2\xaa\x51\xfd\x91\x77\xae\xee\xc1\x12\x7e\x2c\xfe\xfb\xff\xb1\x02\x60\x3e\xfb\x86\xd4\xbc\x25\x9f\xb3\x40\xc9\x5b\x24\x16\xfd\x56\x73\xa3\x1c\xc7\xce\xe5\x39\x81\xd2\xcf\xa6\xd4\xc6\x94\x7b\x05\xb4\x7f\x81\x8d\xe7\xa6\xf7\x5c\xa0\xe7\xc7\x49\x17\x29\xf3\x51\x89\x29\xcf\xba\xf8\x97\xf9\xe5\x19\xfb\x5f\xfd\xe3\xfe\x5f\x01\x00\x00\xff\xff\xa5\x58\xb1\x16\x60\x13\x00\x00" | ||
const Binaryamd64 = "\x1f\x8b\x08\x00\x00\x00\x00\x00\x02\xff\xec\x98\x3f\x8b\x13\x41\x18\xc6\x9f\xd9\x6c\xee\x0f\x08\xb9\xe0\x81\xc2\x59\xa8\x58\xd8\xb8\xe1\x4e\x41\x05\x95\x55\x38\x1d\x45\x6d\xe4\x2c\x5d\x36\xd9\xe5\x5c\xf0\x72\xcb\xed\xec\x11\xe1\xe0\x2e\xe4\x3b\x88\x75\x40\xb1\xb0\x14\x4c\x99\xca\xc5\xd2\xc6\xda\x14\x42\xd0\xca\xce\x80\xa2\xec\xe6\x9d\x24\x3b\x26\x2a\xd8\xc9\xfc\x20\x3c\xf3\x3e\xfb\xce\xbc\x43\x92\x66\x9f\xfd\xf5\xdb\xd7\x0d\xc6\x20\x31\x70\x05\x59\xb5\x64\x67\xb5\x4d\xfe\x87\xf2\xa8\x05\x36\x2e\xa0\x00\x1b\x73\x28\x66\xbd\x26\x26\xb1\x73\x7a\x88\x8e\x96\x8a\xa5\xa1\xa4\x65\x71\xa2\x96\xf3\xa4\x86\x64\x4b\x95\x7d\x26\x7d\xfa\x64\xf7\x69\x8e\xd4\x53\xe4\x4b\x35\x27\x74\x19\x40\x01\xc0\x8d\xbb\x1b\xf0\x9f\xbd\xbe\xf8\xfe\xc5\xc7\xfb\x77\xbe\x3f\x39\xfc\xed\xf9\xa7\xfd\x37\x67\xdf\xbd\x82\x46\xa3\xd1\x68\x34\x1a\x8d\x46\xa3\xd1\x68\x34\xff\x29\x7c\xb5\x5b\x6a\xf3\xd6\xd7\xf9\xbd\x5b\xbc\x03\xe0\x20\x35\x4b\xed\xcb\x0c\x38\xd8\x3b\xcf\x93\x2e\xcb\x9a\xde\x96\xda\xbc\xd9\x63\x27\x9f\x82\x37\x07\xa9\xc4\x2b\xbc\xc3\xa8\x7d\xf8\xb8\xd5\x63\xbc\x35\x60\xb1\xf1\x79\x91\x27\xc9\x55\x00\xd9\xe2\x5a\xba\x00\x4f\xba\x97\xd2\x83\x8b\xbf\xbb\x4b\x01\x6c\xf4\x1e\x9f\xf7\x8d\x71\x3e\x80\x71\x7e\x60\xe2\xcb\x0f\xb5\xb7\x4c\x29\xc6\x86\xd2\xbf\x4c\xfe\x03\xc5\x3f\x42\xfe\xa6\xe2\x9f\xc8\x22\x87\x5f\xe7\x1e\x93\xfe\xf1\xbc\x7f\x7a\x86\x5f\x99\xe1\xa3\x22\xb6\xc2\x4a\xad\xb6\xb6\x1e\xd4\xee\xdd\xb4\xb6\xb1\xbb\x8a\xdd\x35\xf8\x8d\x40\xc0\x71\xaa\x51\xe4\x44\xc2\xdd\x11\x70\x7c\xcf\x15\x2e\x1c\xbf\xee\x01\x56\xf4\x78\x4b\xb8\x55\x58\x91\xd8\x19\xea\x43\xb9\xaa\x6f\x0b\xdf\xda\xac\xc7\x56\x35\x0e\x1e\x79\x67\x02\x0f\x96\xf0\x1b\xe2\x9f\xff\x1f\x2b\x00\xe6\xb3\x6f\x48\xcd\x5b\xf2\x39\x0b\x94\xbc\x45\x62\xd1\x6f\x35\x37\xca\x71\xec\x5c\x9e\x13\x2a\xfd\x6c\x4a\x6d\x4c\xb9\x57\x48\xfb\x17\xd8\x78\x6e\x7a\xcf\x05\x7a\x7e\x94\x74\x91\x32\x1f\x95\x06\xe5\x59\xe7\xfe\x30\xbf\x3c\x63\xff\xcb\xbf\xdc\xff\x33\x00\x00\xff\xff\xad\x50\x19\xd5\x60\x13\x00\x00" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This diff looks odd when generating bins locally: https://github.com/moby/buildkit/actions/runs/7740278596/job/21104904747#step:4:682
Might be linked to current microarchitecture level on my pc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok when reverting this change I still got a diff on ci: https://github.com/moby/buildkit/actions/runs/7740278596/job/21104904747#step:4:682
Something is flaky, I will add an exception in the meantime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an exception in validation: aa14573
ef6c6ec
to
046e17c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
hack/dockerfiles/archutil.Dockerfile
Outdated
FROM base AS exit-amd64 | ||
COPY fixtures/exit.amd64.S . | ||
COPY util/archutil/fixtures/exit.amd64.S . | ||
RUN x86_64-linux-gnu-gcc -static -nostdlib -o exit exit.amd64.S |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it missing --noexecstack
that is causing the validation issue?
Seems to contain a temp filename.
» diff d1.dis d2.dis !4185
2c2
< f1.bin: file format elf64-x86-64
---
> f2.bin: file format elf64-x86-64
25c25
< 400107: 7f c6 jg 0x4000cf </tmp/cc2EicSI.o+0x4000cf>
---
> 400107: 7f c6 jg 0x4000cf </tmp/ccxc7wrM.o+0x4000cf>
146,148c146,151
< 6: 63 63 32 movslq 50(%rbx), %esp
< 9: 45 69 63 53 49 2e 6f 00 imull $7286345, 83(%r11), %r12d # imm = 0x6F2E49
< 11: 76 31 jbe 0x44 <.symtab+0x44>
---
> 6: 63 63 78 movslq 120(%rbx), %esp
> 9: 63 37 movslq (%rdi), %esi
> b: 77 72 ja 0x7f <.symtab+0x7f>
> d: 4d 2e cs
> f: 6f outsl (%rsi), %dx
> 10: 00 76 31 addb %dh, 49(%rsi)
» diff f1.xxd f2.xxd !4186
141c141
< 00001180: 3245 6963 5349 2e6f 0076 3100 7632 0065 7869 7400 5f5f 6273 735f 7374 6172 7400 2EicSI.o.v1.v2.exit.__bss_start.
---
> 00001180: 7863 3777 724d 2e6f 0076 3100 7632 0065 7869 7400 5f5f 6273 735f 7374 6172 7400 xc7wrM.o.v1.v2.exit.__bss_start.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it missing
--noexecstack
that is causing the validation issue?
Doesn't look like it: https://github.com/moby/buildkit/actions/runs/7752978105/job/21143440422#step:4:724
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't see how the compiler has interpreted and potentially modified the original assembly instructions with temp files like jg 0x4000cf </tmp/cc2EicSI.o+0x4000cf>
, even with debug output -S -v
and -fverbose-asm
And striping debug info and ident directives doesn't seem to solve this. I think optimization is already disabled with gcc by default so should not be this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the the difference now in binary? There is still /tmp/_.o
in the binary, even with no execstack and strip?
231d91a
to
4a12942
Compare
47a7a0d
to
cae021a
Compare
No relevant diff between local and ci: local objdump
ci objdump
Diff
With an hex dump:
objdump with all headers
|
e85224d
to
13de276
Compare
Was comparing builds on ci ubuntu 20.04 vs ubuntu 22.04: https://github.com/moby/buildkit/actions/runs/7845795579?pr=4607
|
fc60834
to
b49628b
Compare
Ok using |
b49628b
to
4d14911
Compare
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
4d14911
to
9d4de4d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand why the binary size only changed for amd64 when adding strip.
Moves archutil generation and validation to main bake definition and makefile. This will trigger
validate-archutil
on ci as well through bakevalidate
targets group:buildkit/.github/workflows/validate.yml
Line 35 in 1981eb1